linux-kernel - Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2d5edf11-2d13-bcc7-93a9-e0a223bd6eb8@quicinc.com>
Date: Thu, 17 Jul 2025 17:32:01 +0530
From: Vikash Garodia <quic_vgarodia@...cinc.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Dikshita Agarwal
	<quic_dikshita@...cinc.com>,
        Abhinav Kumar <abhinav.kumar@...ux.dev>,
        "Bryan
 O'Donoghue" <bryan.odonoghue@...aro.org>,
        Mauro Carvalho Chehab
	<mchehab@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Philipp Zabel
	<p.zabel@...gutronix.de>
CC: <linux-media@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)


On 7/17/2025 4:24 PM, Krzysztof Kozlowski wrote:
> On 17/07/2025 12:50, Dikshita Agarwal wrote:
>>>>>>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>>>>>>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>>>>>>> +					 val, val & 0x400000, 2000, 20000);
>>>>>>> +		if (ret)
>>>>>>> +			goto disable_power;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>>>>>>> +				 val, val & BIT(0), 200, 2000);
>>>>>> what are you polling here for?
>>>>>
>>>>>
>>>>> This is not different than existing code. I don't understand why you are
>>>>> commenting on something which is already there.
>>>>
>>>> Which code are you referring to?
>>>
>>> To the existing vpu33 which had Reviewed-by: Vikash Garodia
>>> <quic_vgarodia@...cinc.com>
>>>
>>> You understand that everything here is the same, everything is a copy
>>> while adding just few more things?
>>>
>>> My patch is not doing in this respect anything different that what you
>>> reviewed.
>>>
>>
>> It seems to have been missed in vpu33 power off sequence as well and should
>> be fixed.
>>
>> Still, as mentioned earlier as well, your reference should be
>> HPG/downstream driver of SM8750 not the previous generation (SM8650).
> 
> Yes and partially no, because we write upstream code matching or
> extending existing upstream driver. As you said earlier, downstream is
> not the truth always:
> 
> "That shouldn’t be the case. The downstream design is different, which
> is why the driver requires the above code to move the GDSC"
> 
> so here I built on top of SM8650 and re-iterate whatever mistakes are
> there. The best if someone fixes VPU33 and then I rebase on top,
> re-using fixed code as my base.

You have mixed different comments made earlier.

1. Downstream GDSCs are still in HW_CTRL mode, while upstream GDSCs are migrated
to HW_CTRL_TRIGGER. This does not need a fix in SM8650, but in the
"iris_vpu35_power_on_hw" which you have added in this patch for SM8750.

2. Register write "AON_WRAPPER_MVP_NOC_LPI_CONTROL" with 0x1 is needed on both
SM8650 and SM8750, before polling AON_WRAPPER_MVP_NOC_LPI_STATUS in
"iris_vpu35_power_off_hw" function.

I can soon submit a patch to fix SM8650 with the missing register write, but i
do not see a need to wait for it to continue your development on SM8750.

Regards,
Vikash