Re: [PATCH v2 10/14] dt-bindings: media: qcom: Add CAMSS Offline Processing Engine (OPE)

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

On 5/5/26 3:16 PM, Loic Poulain wrote:
> On Tue, Apr 28, 2026 at 12:16 PM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 4/27/26 10:33 PM, Loic Poulain wrote:
>>> On Mon, Apr 27, 2026 at 4:22 PM Konrad Dybcio
>>> <konrad.dybcio@oss.qualcomm.com> wrote:
>>>>
>>>> On 4/27/26 2:43 PM, Loic Poulain wrote:
>>>>> Add Devicetree binding documentation for the Qualcomm Camera Subsystem
>>>>> Offline Processing Engine (OPE) found on platforms such as Agatti.
>>>>> The OPE is a memory-to-memory image processing block which operates
>>>>> on frames read from and written back to system memory.
>>>>>
>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>> +        clocks = <&gcc GCC_CAMSS_OPE_CLK>,
>>>>> +                 <&gcc GCC_CAMSS_OPE_AHB_CLK>,
>>>>> +                 <&gcc GCC_CAMSS_NRT_AXI_CLK>;
>>>>
>>>> Should the two AXI clocks be aggregated by camss-top instead?
>>>>
>>>> Otherwise we run the risk of the OPE driver setting a rate of A
>>>> and another sub-device setting a rate of B
>>>
>>> On qcm2290, OPE appears to be the only consumer of the NRT AXI clock,
>>> while the capture path (VFE/TFE) relies on the RT AXI clock. That
>>> said, this may not always be the case and these clocks (AXI / NRT‑AXI
>>> / RT‑AXI) seem like they could reasonably be managed at the
>>> camss-bus/top level.
>>>
>>> The open question is how the NRT AXI clock should be enabled when
>>> required? enabling them unconditionally (similar to other camss PM
>>> clocks), introducing a dedicated CAMSS top‑level interface for voting,
>>> or leveraging an existing framework to handle this?
>>
>> So, interconnect, or some internal, smaller version of it?
> 
> Downstream, there is a CPAS driver that handles these clocks in
> conjunction with the internal CAMNOC block. Dmitry also mentioned the
> existing icc_clock mechanism, but we likely need to investigate this
> further to support proper dynamic scaling of the required clocks.
> However, I don’t plan to address this as part of the current series,
> as it would significantly increase its scope.
>
> I believe the current approach is acceptable for now because:
> - This NRT clock is required by this specific sub-block, but not by
> all CAMSS sub-blocks (unlike, for example, camss-ahb), so referencing
> it makes sense here.
> - At the moment, the OPE only enables this clock without setting its
> rate (i.e., it uses the default), so this should not conflict with
> introducing a more complete scaling framework later.
> 
> Does this sound good?

Depends how you're going to want to tackle this debt in the future.
If you go through the icc framework, you'll end up needing to alter
the DT

If you bake in something internal to the camss family, say
camss_foo_bar_request_bandwidth(u32 val), you may get it done
without that. But I don't know what practical challenges that will
entail

Konrad