Re: [PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

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

On 6/4/25 2:05 PM, Renjiang Han wrote:
> 
> On 6/3/2025 9:21 PM, Dmitry Baryshkov wrote:
>> On Thu, May 29, 2025 at 10:29:46AM +0800, Renjiang Han wrote:
>>> On 5/28/2025 7:04 PM, Dmitry Baryshkov wrote:
>>>> On Wed, May 28, 2025 at 05:13:06PM +0800, Renjiang Han wrote:
>>>>> On 5/27/2025 9:57 PM, Konrad Dybcio wrote:
>>>>>> On 5/27/25 5:32 AM, Renjiang Han wrote:
>>>>>>> Add the venus node to the devicetree for the qcs615 platform to enable
>>>>>>> video functionality. The qcs615 platform currently lacks video
>>>>>>> functionality due to the absence of the venus node. Fallback to sc7180 due
>>>>>>> to the same video core.
>>>>>>>
>>>>>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>>>>>>> ---
>>>>>> [...]
>>>>>>
>>>>>>> +            interconnect-names = "video-mem",
>>>>>>> +                         "cpu-cfg";
>>>>>>> +
>>>>>>> +            iommus = <&apps_smmu 0xe40 0x20>;
>>>>>> fwiw docs mention 0xe60 0x20 (which result in the exact same resulting sid)
>>>>> OK. Will update it with next version.
>>>> How would you update this?
>>> Thanks for your comments. I'll update it like this.
>>> iommus = <&apps_smmu 0xe60 0x20>;
>>>
>>> This 0xe40 SID was based on a previous project. However, after rechecking
>>> the documentation yesterday and confirming with colleagues, the correct
>>> SID value should be 0xe60. I’ve also validated it on local device, it
>>> works as expected. The reason 0xe40 seemed to work earlier is due to the
>>> mask value being 0x20, which causes the effective SID derived from 0xe40
>>> to be the same as 0xe60.
>> Using 0xe60 would be counterintuitive, as we have a non-zero masked bits
>> in the base value. It should be either <0xe60 0x0> or <0xe40 0x20>.
> 
> Hi Dmitry
> 
> Thank you for your comment.
> 
> I’ve followed up on this sid with a colleague from the kernel team,
> and based on our discussion, it seems that the sid in this case should
> be the result sid. The actual sid is 0xe60, and with a mask of 0x20,
> the resulting sid would be 0xe40. Therefore, it should be <0xe40 0x20>.
> 
> @Konrad, I’d appreciate any thoughts or suggestions you might have on it.

What our docs describe as 'result sid' is literally 'base ~& mask', so if
we used that, setting the mask would be useless..

Now, some old NHLOS builds are known to cause issues if the values aren't
exactly what they expect (some whitelisting must be going on there).

I don't think this should be an issue on this platform, but let's just
use 0xe60 0x20 here to reflect the real values

Konrad