linux-kernel - Re: [PATCH 8/8] media: iris: enable support for SC7280 platform

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: <0a0afe32-7814-4901-bfbb-6694cd846257@oss.qualcomm.com>
Date: Thu, 9 Oct 2025 17:49:45 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Vikash Garodia <vikash.garodia@....qualcomm.com>,
        Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>,
        Abhinav Kumar <abhinav.kumar@...ux.dev>,
        Bryan O'Donoghue <bod@...nel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 8/8] media: iris: enable support for SC7280 platform

On 10/9/25 5:00 PM, Dmitry Baryshkov wrote:
> On Thu, Oct 09, 2025 at 11:19:31AM +0200, Konrad Dybcio wrote:
>> On 10/8/25 9:25 PM, Dmitry Baryshkov wrote:
>>> On Wed, Oct 08, 2025 at 10:26:02AM +0200, Konrad Dybcio wrote:
>>>> On 10/8/25 6:33 AM, Dmitry Baryshkov wrote:
>>>>> As a part of migrating code from the old Venus driver to the new Iris
>>>>> one, add support for the SC7280 platform. It is very similar to SM8250,
>>>>> but it (currently) uses no reset controls (there is an optional
>>>>> GCC-generated reset, it will be added later) and no AON registers
>>>>> region. The Venus driver names this platform "IRIS2_1", so the ops in
>>>>
>>>> Which we've learnt in the past is "IRIS2, 1-pipe"
>>>
>>> Well, I'm open for better suggestions. iris_vpu2_no_aon_ops?
>>
>> [...]
>>
>>>>> +	writel(CTL_AXI_CLK_HALT | CTL_CLK_HALT,
>>>>> +	       core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG);
>>>>> +	writel(RESET_HIGH, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET);
>>>>> +	writel(0x0, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET);
>>>>> +	writel(0x0, core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG);
>>>>> +
>>>>> +disable_power:
>>>>> +	iris_disable_unprepare_clock(core, IRIS_AHB_CLK);
>>>>
>>>> ..for this line
>>>
>>> Not only. You missed the absense of AON_WRAPPER_MVP_NOC_LPI_CONTROL /
>>> AON_WRAPPER_MVP_NOC_LPI_STATUS. Which in theory can become a flag in
>>> iris_platform_data.
>>>
>>>>
>>>> but this could be added to that one instead, since both clk APIs and the
>>>> Iris wrappers around it are happy to consume a null pointer (funnily
>>>> enough this one returns !void and is never checked)
>>>>
>>>> similar story for other func additions
>>>
>>> In fact, initially I had them merged, but then I couldn't find an
>>> elegant way to handle AON regs. I can squash them back, if that's the
>>> consensus. Any idea regarding AON regs?
>>
>> Digging in techpack/video, I found:
>>
>> commit c543f70aca8d40c593b8ad342d42e913a422c552
>> Author: Priyanka Gujjula <pgujjula@...eaurora.org>
>> Date:   Fri Feb 14 13:38:31 2020 +0530
>>
>>     msm: vidc: Skip AON register programming for lagoon
>>     
>>     AON register programming is used to set NOC to low
>>     power mode during IRIS2 power off sequence. However
>>     AON register memory map is not applicable and hence
>>     skipping AON register programming for lagoon.
>>     
>>     Change-Id: Ib63248d118ed9fecfa5fa87925e8f69625dc1ba8
>>     Signed-off-by: Priyanka Gujjula <pgujjula@...eaurora.org>
>>
>>
>> lagoon being a downstream codename of the aforementioned sm6350
>>
>> Meaning yeah it's bus topology.. so I think an if-statement within
>> a common flow would be what we want here..
>>
>> perhaps
>>
>> if (core->iris_platform_data->num_vpp_pipe == 1)
>>
>> just like venus and downstream do for the most part, and kick the
>> can down the road.. In an unlikely event someone decides to implement
>> IRIS2_1 on a brand new SoC, we can delay our worries..
> 
> But this function is being used for VPU3 devices too, if I'm not
> mistaken. So it becomes a bit ugly... Also I'm not sure if this is
> really related to a num of VPP pipes or the CVP.

Oh.. hm.. maybe we can add a .aon_shutdown op? I'm not sure how
to proceed either

Konrad