Re: [PATCH] iommu/arm-smmu: add actlr settings for mdss and fastrpc

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

On 11/27/25 12:14 PM, Bibek Kumar Patro wrote:
> 
> 
> On 11/12/2025 7:32 PM, Konrad Dybcio wrote:
>> On 11/11/25 3:02 PM, Charan Teja Kalla wrote:
>>>
>>>
>>> On 11/5/2025 2:44 PM, Konrad Dybcio wrote:
>>>>> +    { .compatible = "qcom,fastrpc-compute-cb",
>>>>> +            .data = (const void *) (PREFETCH_DEEP | CPRE | CMTLB) },
>>>> This will be globally applied to all smmu-v2 targets with fastrpc,
>>>> starting from MSM8996 ending at Kaanapali and everything inbetween
>>>>
>>>> Are these settings always valid?
>>> Oops, you are correct...this settings are not always applicable it seems.
>>>
>>> Example: lpass compute and cdsp compute node uses the
>>> "qcom,fastrpc-compute-cb", but it is for the later that ACTLR settings
>>> are valid.
>>>
>>> Then for these cases, should we be extending the actlr match array with
>>> additional compatible string and then add them in all the device nodes
>>> that require actlr setting? example:
>>>
>>> @@ -3119,7 +3119,8 @@ fastrpc {
>>>        compute-cb@1 {
>>> -         compatible = "qcom,fastrpc-compute-cb";
>>> +         compatible = "qcom,fastrpc-compute-cb",
>>> +                      "qcom,fastrpc-compute-cb-actlr";
>>
>> dt-bindings (and especially compatible strings) must not be altered solely
>> to work around a driver being suboptimal
>>
>> But because you reported that these settings can change both between
>> platforms and instances of the same devices on these platforms, we could
>> possibly reconsider adding ACTRL settings to the consumer device nodes
>> where they stray away from the otherwise seemingly reasonable baseline
>> we have in the driver so far..
>>
> 
> During initial discussion of ACTLR design phase, It was concluded that
> prefetch alike QoS settings are not hardware definitions, and device
> tree would not be the right place to store such configurable/tunable
> settings [1]. So it might be contradicting to add it in consumer nodes.
> 
> For fastrpc client, there are 2 hiccups:
> 1. For a particular SoC, Different SIDs of compute client are using the
>    same compatible  "qcom,fastrpc-compute-cb" but can have different
>    prefetch settings <refer table below> e.g. CDSP and ADSP have same
>    compat string "qcom,fastrpc-compute-cb" with different labels.
> 
> e.g.
> fastrpc {
>         compatible = "qcom,fastrpc";
>         qcom,glink-channels = "fastrpcglink-apps-dsp";
>         label = "adsp"; /*---different label: adsp---*/
>         compute-cb@3 {
>                 compatible = "qcom,fastrpc-compute-cb";
>                 reg = <3>;
>         /*---Prefetch Default---*/
>                 iommus = <&apps_smmu 0x1003 0x80>,
>                          <&apps_smmu 0x1063 0x0>;
>                 dma-coherent;
> fastrpc {
>         compatible = "qcom,fastrpc";
>         qcom,glink-channels = "fastrpcglink-apps-dsp";
>         label = "cdsp"; /*---Different Label: cdsp---*/
>         qcom,non-secure-domain;
>         #address-cells = <1>;
>         #size-cells = <0>;
>         compute-cb@1 {
>                 compatible = "qcom,fastrpc-compute-cb";
>                 reg = <1>;
>         /*---Prefetch DEEP----*/
>                 iommus = <&apps_smmu 0x1961 0x0>,
>                          <&apps_smmu 0x0c01 0x20>,
>                          <&apps_smmu 0x19c1 0x10>;
> 
> 2. Different SoCs having the same compatible string and same label but
>    different prefetch settings.
>    e.g:
>    in below table sm6550, sm8250 and sm8550
>    compat string = "qcom,fastrpc-compute-cb" and label = "cdsp"
>    but prefetch settings are different
> 
> As per my idea both the above cases could be resolved by compat string addition,
> case 1 : compatible = "qcom,fastrpc-compute-cb-cdsp";
> case 2 : compatible = "qcom,sm8550-fastrpc-compute-cb-cdsp"
> 
> This discrpeancy is not applicable for other clients, except fastrpc.
> One way both the above cases could be resolved by additional fastrpc
> compat string with below format:
> compatible = "qcom,<soc_name>-fastrpc-compute-cb-<label_name>"
> 
> e.g: "qcom,sm8550-fastrpc-compute-cb-cdsp", "qcom,sm6550-fastrpc-compute-cb-adsp"
> 
> This should handle both case 1 & case 2.
> 
> But as you mentioned previously this might need change alteration of DT bindings addition which might not be allowed or favoured always.
> 
> [1]: https://lore.kernel.org/all/a01e7e60-6ead-4a9e-ba90-22a8a6bbd03f@quicinc.com/
> 
>> Or we could introduce some more bespoke matching logic.
>>
>> I would like to know more about the scope of this issue, i.e. the matrix
>> of (soc, device, actlr_val) that needs special handling.
>>
> 
> +---------+-----------------------------------------------+----------+
> | SoC     | Description                                   | Prefetch |
> +---------+-----------------------------------------------+----------+
> | sc7180  | qcom,fastrpc-compute-cb | label = cdsp        | DEEP     |
> |         | qcom,fastrpc-compute-cb | label = adsp        | DEFAULT  |
> +---------+-----------------------------------------------+----------+
> | sc7280  | qcom,fastrpc-compute-cb | label = cdsp        | DEEP     |
> |         | qcom,fastrpc-compute-cb | label = adsp        | DEFAULT  |
> +---------+-----------------------------------------------+----------+
> | sm6115  | qcom,fastrpc-compute-cb | label = cdsp        | DEFAULT  |
> |         | qcom,fastrpc-compute-cb | label = adsp        | DEFAULT  |
> +---------+-----------------------------------------------+----------+
> | sm6125  | qcom,fastrpc-compute-cb (NA upstream)         |          |
> +---------+-----------------------------------------------+----------+
> | sm6350  | qcom,fastrpc-compute-cb | label = cdsp        | SHALLOW  |
> |         | qcom,fastrpc-compute-cb | label = adsp        | SHALLOW  |
> +---------+-----------------------------------------------+----------+
> | sm8250  | qcom,fastrpc-compute-cb | label = cdsp        | DEFAULT  |
> |         | qcom,fastrpc-compute-cb | label = adsp        | DEFAULT  |
> |         | qcom,fastrpc-compute-cb | label = sdsp        | DEFAULT  |
> +---------+-----------------------------------------------+----------+
> | sm8350  | qcom,fastrpc-compute-cb | label = cdsp        | DEEP     |
> |         | qcom,fastrpc-compute-cb | label = adsp        | DEFAULT  |
> |         | qcom,fastrpc-compute-cb | label = sdsp        | DEFAULT  |
> +---------+-----------------------------------------------+----------+
> | sm8450  | qcom,fastrpc-compute-cb | label = cdsp        | DEEP     |
> |         | qcom,fastrpc-compute-cb | label = adsp        | DEFAULT  |
> |         | qcom,fastrpc-compute-cb | label = sdsp        | DEFAULT  |
> +---------+-----------------------------------------------+----------+
> | sm8550  | qcom,fastrpc-compute-cb | label = cdsp        | DEEP     |
> |         | qcom,fastrpc-compute-cb | label = adsp        | DEFAULT  |
> +---------+-----------------------------------------------+----------+
> | sm8650  | qcom,fastrpc-compute-cb | label = cdsp        | DEEP     |
> |         | qcom,fastrpc-compute-cb | label = adsp        | DEFAULT  |
> +---------+-----------------------------------------------+----------+

Thanks for the detailed explanation. It feel quite impossible to keep
track of all this in the current model.

On the other hand, the approach where a third OF cell is introduced
isn't my favorite thing in the world, and that seems to be the general
sentiment.

FWIW downstream solves this by carrying the list of <sid mask val> under
the SMMU node. Perhaps that could be reconsidered, either as a storage
for all the values, or as storage for non-default overrides.. Krzysztof,
Bjorn, opinions?

Konrad