Re: [PATCH v3 0/5] Introduce "non-pixel" sub node within iris video node

Bryan O'Donoghue
2025-10-09T00:10:50Z

On 08/10/2025 19:03, Charan Teja Kalla wrote:
>>>> Couldn't we list the entire set of iommus - then detach - subsequently
>>>> re-attaching in our platform code with FUNCTION_IDs we keep listed in
>>>> our drivers ?
>>>>
> TMK, there is no api exist to detach a device once it is attached to
> smmu. We used to have one but removed[1], not sure how well it will be
> received to introduce it again.
>
> There is other problem exist attaching the entire set of iommus in the
> first place: Usually writes to SMR registers are protected through
> emulation by hyp. Thus adding the sids of protected/non-protected
> usecases in the same iommu set will not allowed by the
> hypervisors(eg:gunyah), as all will end up in using the same context
> bank, thus there can be failure to attach to smmu in the first place.
>
>
> [1]
> https://lore.kernel.org/all/20230110025408.667767-1-
> baolu.lu@linux.intel.com/
>
>>>> That way the DT is complete and correct, we have a compliant upstream DT
>>>> but we also find a way to make the FUNCTION_ID specific setup we need.
>>> i.e. you can keep the FUNCTION_ID "metadata" in the driver and
>>> associate specific iommu indexes with the FUNCTION_ID you want in there.
>>>
>>> That way you could have multiple FUNCTION_ID smmu entries in the DT
>>> and just associate the DT indexes locally in drivers/platform/qcom/
>>> iris_metadata_goes_here.c
>>>
>>> ---
>>> bod
>> Actually why can't we specify FUNCTION_ID in the iommus = <entries>
>>
>> Surely we could do
>>
>>     #iommu-cells = <4>;
>>     iommus = <&apps_smmu 0x420 0x2 FUNCTION_ID>;
>>
>> and encode the real data we need directly in the iommus list...
>>
> Since it is the smmu device property , this suggestion expects all the
> devices, not just video, to define additional argument. Does this look
> valid?

If it is legitimate meta-data for the SMMU setup then why _shouldn't_ it
go into the DT ?

We've basically identified that the smmu entries - for qcom platforms
should encode the FUNCTION_ID. Rather than shy away from fixing the DT
we should work back from the principle "DT should represent the
hardware" and then if necessary update the upstream descriptions to capture.

Surely then we can teach the mapping routines to consume the
FUNCTION_ID. Rob suggested an implied FUNCTION_ID based on index.

I think we need to have something like:

#1
iommus = <&apps_smmu 0x420 0x2 FUNCTION_ID0>,
<&apps_smmu 0x424 0x2 FUNCTION_ID0>,
<&apps_smmu 0x428 0x2 FUNCTION_ID1>;

or with implied indexes..

#2
/* implied FUNCTION_ID0 */
iommus = <&apps_smmu 0x420 0x2>,
<&apps_smmu 0x424 0x2>;

/* implied FUNCTION_ID1 */
iommus = <&apps_smmu 0x428 0x2>;

Either way the DT should not contain fake devices or fake sub-nodes and
IMO should contain either explicitly with another field or implicitly
with an index the FUNCTION_IDs in the DT itself - the FUNCTION_ID.

For devices that don't currently require the FUNCTION_ID parameter the
FUNCTION_ID is the APPS so you could infer that by the absence of
FUNCTION_ID for older platforms and require FUNCTION_ID for new
platforms #1 or again infer it via an implied index if you have multiple
iommus = <> per #2.

---
bod