Re: [PATCH 1/4] media: qcom: camss: clamp VFE clock rates to the highest level

[Gjorgji Rosikopulos (Consultant)]

On 10/23/2026 10:07 PM, Gjorgji Rosikopulos (Consultant) wrote:
> Hi Atanas, Loic, All,
> 
> On 10/23/2026 6:50 PM, Atanas Filipov wrote:
>> On 10/23/2026 6:45 PM, Loic Poulain wrote:
>>> On Fri, Oct 23, 2026 at 5:29 PM Atanas Filipov
>>> <atanas.filipov@xxxxxxxxxxxxxxxx> wrote:
>>>>
>>>> The VFE fails to power on when the pixel rate exceeds the highest rate
>>>> listed for any of its clocks. The rate tables of the VFE clocks are
>>>> independent and need not have the same number of levels, so a clock
>>>> with a lower top rate than the VFE core clock, such as the SC7280
>>>> camnoc_axi clock, makes the VFE reject pixel rates the core clock
>>>> still supports.
>>>>
>>>> Clamp each clock to its highest listed rate instead, and warn that the
>>>> pixel rate exceeds it when the clock lists more than one rate. Take the
>>>> clamp into account when another line powers on an already running VFE,
>>>> so that a clock running at its highest rate does not reject the
>>>> request.
>>>
>>> IMO, this function has become a bit messy. We should split the core
>>> clock selection into a dedicated helper, as it is directly tied to the
>>> pixel rate. The AXI clock scaling/selection should be handled
>>> separately, potentially through the interconnect (ICC) framework (when
>>> available).
>>>
>>> Regards,
>>> Loic
>>>
>>
>> I completely agree. I just don't want to mix the two issues, as 
>> requested by the reviewers. IMO, if we can reach an agreement on the ICC 
>> migration, that would be great.
>>
>> Regards,
>> Atanas
>>
> 
> There have been several discussions about this topic. 
> I would like to summarize the background here and reopen the discussion
> about the direction proposed by Atanas.
> 
> The issue initially came up with the JPEG encoder driver, which is
> represented as a separate device. Dmitry identified that changing the
> camnoc_ax clock from the JPEG driver conflicts with CAMSS.
> I will not revisit the discussion about why JPEG is not part of CAMSS, 
> as this has already been discussed extensively.
> 
> Currently, CAMSS is represented as a single device and drives the front-end
> blocks that write data to memory, such as VFE, IFE, and TFE.
> 
> Depending on the hardware architecture, CAMSS may contain different number
> of front-end blocks. Some camera subsystems also contain offline engines
> like JPEG and OPE.
> All these engines write data to memory through the internal CAMNOC and
> share the camnoc_axi clock.
> 
> The relevant topology can be summarized as follows:
> 
> CAMSS device: Front end -> CAMNOC -> MNOC
> OPE device: OPE -> CAMNOC -> MNOC
> JPEG device: JPEG -> CAMNOC -> MNOC
> 
> An ICC path is already available for MNOC, and each device can vote on it.
> 
> CAMSS currently has two issues on this topic that need to be addressed.
> 
> 1. Clock aggregation for concurrent streaming, As described in the cover
> letter, CAMSS currently does not aggregate the clock requirements when
> multiple front-end engines are streaming concurrently.
> 
> 2. ICC bandwidth voting
> The currently available MNOC ICC voting is implemented using max bandwidth
> values. This can increase power consumption in single-camera use cases.
> This is not included in this patchset.
> 
> Addressing the first issue depends, to some extent, on the direction we
> choose for CAMNOC ICC support. Atanas has proposed two possible approaches.
> 
> 1. [PATCH v2 0/3] Add Qualcomm CAMNOC ICC provider
> 
> The patchset adds an initial CAMNOC ICC provider with multiple ports, where
> each port represents an engine client of the CAMNOC.
> 
> This allows a device such as CAMSS, which is represented as a single device,
> to vote independently through different CAMNOC ports. The CAMNOC ICC provider
> can then aggregate the votes and calculate the appropriate camnoc_axi clock
> rate.
> 
> 1. [PATCH v3 0/3] Use icc-clk for the SM8250 camera NoC AXI clock
> 
> The ICC framework, together with CAMCC support, can represent a clock as an
> ICC provider. The implementation is relatively simple because it reuses the
> existing infrastructure, the camnoc_axi clock only needs to be exposed as an
> ICC provider.
> 
> The limitation of this approach is that a device such as CAMSS cannot vote for
> each engine independently.

After carefully reviewing the ICC code and discussing it with Georgi Dzakov, 
I believe this assumption is incorrect. ICC aggregation is performed per client,
so each VFE sub-device can call icc_get() for the same endpoint and achieve the 
desired aggregate bandwidth. So i think is better to continue with this approach
and see how the things will look like with camss and jpeg migrated to this provider.

~Gjorgji