lkml.org 
[lkml]   [2018]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v11 0/4] iommu/arm-smmu: Add runtime pm/sleep support
    Hi Robin,


    On Mon, May 21, 2018 at 7:12 PM, Robin Murphy <robin.murphy@arm.com> wrote:
    > On 22/03/18 10:22, Vivek Gautam wrote:
    >>
    >> This series provides the support for turning on the arm-smmu's
    >> clocks/power domains using runtime pm. This is done using the
    >> recently introduced device links patches, which lets the smmu's
    >> runtime to follow the master's runtime pm, so the smmu remains
    >> powered only when the masters use it.
    >> As not all implementations support clock/power gating, we are checking
    >> for a valid 'smmu->dev's pm_domain' to conditionally enable the runtime
    >> power management for such smmu implementations that can support it.
    >>
    >> This series also adds support for Qcom's arm-smmu-v2 variant that
    >> has different clocks and power requirements.
    >>
    >> Took some reference from the exynos runtime patches [1].
    >>
    >> With conditional runtime pm now, we avoid touching dev->power.lock
    >> in fastpaths for smmu implementations that don't need to do anything
    >> useful with pm_runtime.
    >> This lets us to use the much-argued pm_runtime_get_sync/put_sync()
    >> calls in map/unmap callbacks so that the clients do not have to
    >> worry about handling any of the arm-smmu's power.
    >>
    >> Previous version of this patch series is @ [5].
    >>
    >> [v11]
    >> * Some more cleanups for device link. We don't need an explicit
    >> delete for device link from the driver, but just set the flag
    >> DL_FLAG_AUTOREMOVE.
    >> device_link_add() API description says -
    >> "If the DL_FLAG_AUTOREMOVE is set, the link will be removed
    >> automatically when the consumer device driver unbinds."
    >
    >
    > But then if the consumer subsequently rebinds, there's no opportunity to
    > recreate the link, and PM will just quietly stop functioning properly. Given
    > that it's a lot more reasonable for users to unload and reload modules (or
    > otherwise switch drivers) for a consumer device than it is to unbind the
    > SMMU driver or expect actual device hotplug, I'd rather do nothing and
    > theoretically leak links than autoremove them incorrectly.
    >
    > AFAICS the previous discussion on this got as far as Lukas' suggestion of
    > adding an equivalent flag for supplier-managed links (which FWIW sounds good
    > to me), but seems to have stalled there.

    Thanks for the review. I have posted a small change [1] for this.
    Kindly take a look.

    Best regards
    Vivek

    [1] https://patchwork.kernel.org/patch/10438109/

    >
    > Robin.
    >
    >
    >> * Addressed the comments for 'smmu' in arm_smmu_map/unmap().
    >> * Dropped the patch [10] that introduced device_link_del_dev() API.
    >>
    >> [v10]
    >> * Introduce device_link_del_dev() API to delete the link between
    >> given consumer and supplier devices. The users of device link
    >> do not need to store link pointer to delete the link later.
    >> They can straightaway use this API by passing consumer and
    >> supplier devices.
    >> * Made corresponding changes to arm-smmu driver patch handling the
    >> device links.
    >> * Dropped the patch [9] that was adding device_link_find() API to
    >> device core layer. device_link_del_dev() serves the purpose to
    >> directly delete the link between two given devices.
    >>
    >> [v9]
    >> * Removed 'rpm_supported' flag, instead checking on pm_domain
    >> to enable runtime pm.
    >> * Creating device link only when the runtime pm is enabled, as we
    >> don't need a device link besides managing the power dependency
    >> between supplier and consumer devices.
    >> * Introducing a patch to add device_link_find() API that finds
    >> and existing link between supplier and consumer devices.
    >> Also, made necessary change to device_link_add() to use this API.
    >> * arm_smmu_remove_device() now uses this device_link_find() to find
    >> the device link between smmu device and the master device, and then
    >> delete this link.
    >> * Dropped the destroy_domain_context() fix [8] as it was rather,
    >> introducing catastrophically bad problem by destroying
    >> 'good dev's domain context.
    >> * Added 'Reviwed-by' tag for Tomasz's review.
    >>
    >> [v8]
    >> * Major change -
    >> - Added a flag 'rpm_supported' which each platform that supports
    >> runtime pm, can enable, and we enable runtime_pm over arm-smmu
    >> only when this flag is set.
    >> - Adding the conditional pm_runtime_get/put() calls to .map, .unmap
    >> and .attach_dev ops.
    >> - Dropped the patch [6] that exported pm_runtim_get/put_suupliers(),
    >> and also dropped the user driver patch [7] for these APIs.
    >>
    >> * Clock code further cleanup
    >> - doing only clk_bulk_enable() and clk_bulk_disable() in runtime pm
    >> callbacks. We shouldn't be taking a slow path
    >> (clk_prepare/unprepare())
    >> from these runtime pm callbacks. Thereby, moved clk_bulk_prepare()
    >> to
    >> arm_smmu_device_probe(), and clk_bulk_unprepare() to
    >> arm_smmu_device_remove().
    >> - clk data filling to a common method arm_smmu_fill_clk_data() that
    >> fills the clock ids and number of clocks.
    >>
    >> * Addressed other nits and comments
    >> - device_link_add() error path fixed.
    >> - Fix for checking negative error value from pm_runtime_get_sync().
    >> - Documentation redo.
    >>
    >> * Added another patch fixing the error path in arm_smmu_attach_dev()
    >> to destroy allocated domain context.
    >>
    >> [v7]
    >> * Addressed review comments given by Robin Murphy -
    >> - Added device_link_del() in .remove_device path.
    >> - Error path cleanup in arm_smmu_add_device().
    >> - Added pm_runtime_get/put_sync() in .remove path, and replaced
    >> pm_runtime_force_suspend() with pm_runtime_disable().
    >> - clk_names cleanup in arm_smmu_init_clks()
    >> * Added 'Reviewed-by' given by Rob H.
    >>
    >> [V6]
    >> * Added Ack given by Rafael to first patch in the series.
    >> * Addressed Rob Herring's comment for adding soc specific compatible
    >> string as well besides 'qcom,smmu-v2'.
    >>
    >> [V5]
    >> * Dropped runtime pm calls from "arm_smmu_unmap" op as discussed over
    >> the list [3] for the last patch series.
    >> * Added a patch to export pm_runtime_get/put_suppliers() APIs to the
    >> series as agreed with Rafael [4].
    >> * Added the related patch for msm drm iommu layer to use
    >> pm_runtime_get/put_suppliers() APIs in msm_mmu_funcs.
    >> * Dropped arm-mmu500 clock patch since that would break existing
    >> platforms.
    >> * Changed compatible 'qcom,msm8996-smmu-v2' to 'qcom,smmu-v2' to
    >> reflect
    >> the IP version rather than the platform on which it is used.
    >> The same IP is used across multiple platforms including msm8996,
    >> and sdm845 etc.
    >> * Using clock bulk APIs to handle the clocks available to the IP as
    >> suggested by Stephen Boyd.
    >> * The first patch in v4 version of the patch-series:
    >> ("iommu/arm-smmu: Fix the error path in arm_smmu_add_device") has
    >> already made it to mainline.
    >>
    >> [V4]
    >> * Reworked the clock handling part. We now take clock names as data
    >> in the driver for supported compatible versions, and loop over them
    >> to get, enable, and disable the clocks.
    >> * Using qcom,msm8996 based compatibles for bindings instead of a
    >> generic
    >> qcom compatible.
    >> * Refactor MMU500 patch to just add the necessary clock names data and
    >> corresponding bindings.
    >> * Added the pm_runtime_get/put() calls in .unmap iommu op (fix added
    >> by
    >> Stanimir on top of previous patch version.
    >> * Added a patch to fix error path in arm_smmu_add_device()
    >> * Removed patch 3/5 of V3 patch series that added qcom,smmu-v2
    >> bindings.
    >>
    >> [V3]
    >> * Reworked the patches to keep the clocks init/enabling function
    >> separately for each compatible.
    >>
    >> * Added clocks bindings for MMU40x/500.
    >>
    >> * Added a new compatible for qcom,smmu-v2 implementation and
    >> the clock bindings for the same.
    >>
    >> * Rebased on top of 4.11-rc1
    >>
    >> [V2]
    >> * Split the patches little differently.
    >>
    >> * Addressed comments.
    >>
    >> * Removed the patch #4 [2] from previous post
    >> for arm-smmu context save restore. Planning to
    >> post this separately after reworking/addressing Robin's
    >> feedback.
    >>
    >> * Reversed the sequence to disable clocks than enabling.
    >> This was required for those cases where the
    >> clocks are populated in a dependent order from DT.
    >>
    >> [1] https://lkml.org/lkml/2016/10/20/70
    >> [2] https://patchwork.kernel.org/patch/9389717/
    >> [3] https://patchwork.kernel.org/patch/10204925/
    >> [4] https://patchwork.kernel.org/patch/10102445/
    >> [5] https://lkml.org/lkml/2018/3/14/127
    >> [6] https://patchwork.kernel.org/patch/10204945/
    >> [7] https://patchwork.kernel.org/patch/10204925/
    >> [8] https://patchwork.kernel.org/patch/10254105/
    >> [9] https://patchwork.kernel.org/patch/10277975/
    >> [10] https://patchwork.kernel.org/patch/10281613/
    >>
    >> Sricharan R (3):
    >> iommu/arm-smmu: Add pm_runtime/sleep ops
    >> iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
    >> iommu/arm-smmu: Add the device_link between masters and smmu
    >>
    >> Vivek Gautam (1):
    >> iommu/arm-smmu: Add support for qcom,smmu-v2 variant
    >>
    >> .../devicetree/bindings/iommu/arm,smmu.txt | 42 +++++
    >> drivers/iommu/arm-smmu.c | 178
    >> +++++++++++++++++++--
    >> 2 files changed, 210 insertions(+), 10 deletions(-)
    >>
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html



    --
    QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
    of Code Aurora Forum, hosted by The Linux Foundation

    \
     
     \ /
      Last update: 2018-05-30 12:03    [W:5.596 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site