Re: [PATCH v18 0/5] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 11/12/25 9:45 AM, Bibek Kumar Patro wrote:
> 
> 
> On 11/7/2025 2:58 PM, Enric Balletbo i Serra wrote:
>> On Thu, Dec 12, 2024 at 4:14 PM Bibek Kumar Patro
>> <quic_bibekkum@quicinc.com> wrote:
>>>
>>> This patch series consist of six parts and covers the following:
>>>
>>> 1. Provide option to re-enable context caching to retain prefetcher
>>>     settings during reset and runtime suspend.
>>>
>>> 2. Remove cfg inside qcom_smmu structure and replace it with single
>>>     pointer to qcom_smmu_match_data avoiding replication of multiple
>>>     members from same.
>>>
>>> 3. Add support for ACTLR PRR bit setup via adreno-smmu-priv interface.
>>>
>>> 4. Introduce intital set of driver changes to implement ACTLR register
>>>     for custom prefetcher settings in Qualcomm SoCs.
>>>
>>> 5. Add ACTLR data and support for qcom_smmu_500.
>>>
>>> Changes in v18 from v17_RESEND:
>>>   - 1/5 : No changes
>>>   - 2/5 : No changes - reviewed-by tags collected
>>>   - 3/5 : Addition of
>>>     pm_runtime_resume_and_get()/pm_runtime_put_autosuspend()
>>>     around register access of PRR related private interfaces
>>>     as discussed in v17_RESEND.
>>>   - 4/5, 5/5 : No changes - reviewed-by tags collected
>>>   Link to v17_RESEND:
>>>   https://lore.kernel.org/all/20241114160721.1527934-1-quic_bibekkum@quicinc.com/
>>>
>>> Resend of v17:
>>>   Addition of minor fix of the build warning reported by kernel test robot [1] by powerpc_random config [2].
>>>   [1]:https://lore.kernel.org/all/202411140748.6mcFdJdO-lkp@intel.com/#t
>>>   [2]:https://download.01.org/0day-ci/archive/20241114/202411140748.6mcFdJdO-lkp@intel.com/config
>>>
>>> Changes in v17 from v16:
>>>   Tags provided earlier not collected yet on patch 1/5, 3/5, 4/5, 5/5
>>>   due to the following revisions:
>>>   - 1/5 : Move the CPRE workaround out of qualcomm specific logic and gate with config
>>>           , update the silicon-errata.rst file
>>>   - 2/5 : No changes - reviewed-by tags collected
>>>   - 3/5 : Move the compatible check before assignment of callback as suggested.
>>>   - 4/5 : Add the actlr setting for *adreno variant* of MMU-500 as well.
>>>   - 5/5 : Due to changes in 1/5, minor refactoring had to be done before adding table.
>>>   Link to v16:
>>>   https://lore.kernel.org/all/20241008125410.3422512-1-quic_bibekkum@quicinc.com/
>>>
>>> Changes in v16 from v15:
>>>   - Incorporate Dimitry's suggestion on patch 4/5 to use dev_dbg instead of dev_notice.
>>>   - Fix kernel test robot warning [1] coming for 32bit architecture configuration.
>>>   - Updatingthe tags
>>>   [1]: https://lore.kernel.org/all/202409230343.Q8KnYl2w-lkp@intel.com/
>>>   Link to v15:
>>>   https://lore.kernel.org/all/20240920155813.3434021-1-quic_bibekkum@quicinc.com/
>>>
>>> Changes in v15 from v14:
>>>   - As discussed with Robin and Dmitry modify the actlr table and logic to use
>>>     compatible string instead of sid, mask for device matching which is
>>>     similar to qcom_smmu_client_of_match mechanism.
>>>   - Expand the comment in qcom_smmu500_reset to document reason why CPRE bit is re-enabled again
>>>     after arm_mmu500_reset resets the bit.
>>>   - Rearrange the series in order to keep prefetch setting patches in the end.
>>>   Link to v14:
>>>   https://lore.kernel.org/all/20240816174259.2056829-1-quic_bibekkum@quicinc.com/
>>>
>>
>> Hi all,
>>
>> I've been looking at the evolution of the Qualcomm SMMU driver
>> (arm-smmu-qcom.c) and had a question about the design change for
>> configuring the SMMU_CB_ACTLR register.
>>
>> It appears there are two different approaches:
>>
>> 1. The "SID" Approach (Older): In some versions, a driver would use a
>> large, platform-specific struct (e.g., sa8775p_apps_actlr_cfg) that
>> maps individual Stream IDs (SIDs) to specific ACTLR values. This
>> allowed for very granular, per-SID tuning. For example, a device with
>> 10 SIDs could have a specific setting for just one (0x2141), while the
>> others used a default.
>>
>> 2. The "Compatible String" Approach (Newer/Upstream): The current
>> upstream driver appears to use a client_match array (an of_device_id
>> list) in qcom_smmu_init_context. This maps a device's compatible
>> string to a single ACTLR value, which is then written to the device's
>> assigned Context Bank.
>>
>> I want to make sure I understand the implications of this new, simpler
>> approach, as it seems to introduce a trade-off where granularity is
>> lost:
>>
>> Multiple SIDs, One Device: A single device that uses many SIDs in the
>> device tree (e.g., the video codec, which might list 0x2141, 0x25C1,
>> 0x2161, etc.) will now have one ACTLR setting applied to the Context
>> Bank that serves all of those SIDs, even if the old SID-based list
>> only specified a setting for one of them.
>>
>> Am I correct in this analysis? I'm assuming this was an intentional
>> design choice to simplify the driver, based on the assumption that all
>> SIDs for a single device (and all devices with the same compatible)
>> can share the same ACTLR tuning.
>>
> 
> Hi Eric,
> 
> If a device has multiple SIDs, all serving the same functionality and grouped under the same "iommus" field, for example:
> 
> iommus = <&apps_smmu, 0x2141, 0x0>,
>           <&apps_smmu, 0x25c1, 0x0>,
>           <&apps_smmu, 0x2161, 0x0>;
> 
> In this case, all the SIDs will be associated with the same context bank. Even if the three SIDs have different ACTLR settings, since SMMU_CB_ACTLR is per CB setting, all SIDs attached to that bank will share the same ACTLR configuration. This is why we designed it to be "per device / per compatible".

Does that suggest the settings may be slightly suboptimal?

There's some work being done to allow more granular association of
the passed SIDs:

https://lore.kernel.org/linux-arm-msm/20250928171718.436440-1-charan.kalla@oss.qualcomm.com/

Konrad