Re: [PATCH] media: qcom: camss: Enable setting the rate to camnoc_rt_axi clock

Loic Poulain
2025-10-20T08:13:27Z

On Mon, Oct 20, 2025 at 5:23 AM Hangxiang Ma
<hangxiang.ma@oss.qualcomm.com> wrote:
>
> On 10/17/2025 7:41 PM, Bryan O'Donoghue wrote:
> > On 16/10/2025 21:53, Vijay Kumar Tumati wrote:
> >>
> >> On 10/16/2025 8:31 AM, Bryan O'Donoghue wrote:
> >>> On 16/10/2025 13:22, Loic Poulain wrote:
> >>>>> I'm - perhaps naively - assuming this clock really is required ... and
> >>>>> that both will be needed concurrently.
> >>>> AFAIU, the NRT clock is not in use for the capture part, and only
> >>>> required for the offline processing engine (IPE, OPE), which will
> >>>> likely be described as a separated node.
> >>>
> >>> Maybe yeah though we already have bindings.
> >>>
> >>> @Hangxiang I thought we had discussed this clock was required for your
> >>> setup.
> >>>
> >>> Can you confirm with a test and then
> >>>
> >>> 1. Repost with my RB - I assume you included this on purpose
> >>> 2. Respond that you can live without it.
> >>>
> >>> ---
> >>> bod
> >>>
> >> @Bryan and others, sorry, I am just trying to understand the exact ask
> >> here. Just to add a bit more detail here, On certain architectures,
> >> there is one CAMNOC module that connects all of the camera modules (RT
> >> and NRT) to MMNOC. In these, there is one 'camnoc_axi' clock that needs
> >> to be enabled for it's operation. However, on the newer architectures,
> >> this single CAMNOC is split into two, one for RT modules (TFEs and IFE
> >> Lites) and the other for NRT (IPE and OFE). So, on a given architecture,
> >> we either require 'camnoc_axi' or 'camnoc_rt_axi' for RT operation, not
> >> both. And yes, one of them is a must. As you know, adding the support
> >> for the newer clock in "vfe_match_clock_names" will only enable the
> >> newer chip sets to define this in it's resource information and set the
> >> rate to it based on the pixel clock. In kaanapali vfe resources, we do
> >> not give the 'camnoc_axi_clk'. Hopefully we are all on the same page
> >> now, is it the suggestion to use 'camnoc_axi_clk' name for
> >> CAM_CC_CAMNOC_RT_AXI_CLK ? We thought it would be clearer to use the
> >> name the matches the exact clock. Please advise and thank you.
> >
> > The ask is to make sure this clock is needed @ the same time as the
> > other camnoc clock.
> >
> > If so then update the commit log on v2 to address the concerns given
> > that it may not be necessary.
> >
> > If not then just pining back to this patch "we checked and its not
> > needed" will do.
> >
> > ---
> > bod
>
> @Bryan, I test two scenarios individually that also consider @Vladimir's
> concern. I confirm this clock rate setting is necessary.
> 1. Remove 'camnoc_rt_axi' from the vfe clock matching function.
> 2. Remove 'camnoc_nrt_axi' from the vfe clock resources in camss.c.
> Both of them block the image buffer write operation. More clearly, we
> will stuck at the stage when all buffers acquired but CAMSS takes no action.
>
> I agree with @Vijay to keep 'camnoc_rt_axi' to distinguish between the
> new one and 'camnoc_axi'. The disagreement concerns how to standardize
> the camnoc clock name or how to differentiate between RT and NRT clock
> names if a new RT clock name is introduced. Other chips like sm8550,
> sm8775p depend on 'camnoc_axi'. Meanwhile, 'camnoc_rt_axi' and
> 'camnoc_nrt_axi' are both necessary for QCM2290 and X1E80100. But chips
> like QCM2290 and X1E80100 may not need to set the clock rate but
> Kaanapali needs. @Vladimir

Actually NRT clock does not seem necessary for QCM2290 as capture only
involves RealTime elements.
I tried without that clock and the capture is working, so I think of
making it optional in the QCM2290 bindings.
Also, I thought these clocks could be indirectly scaled via their
respective interconnects?

Regards,
Loic