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

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

[Hangxiang Ma]

On hardware architectures where a single CAMNOC module is split into
two, one for each of the real time (RT) and non real time (NRT) modules
within camera sub system, processing VFE output over the AXI bus
requires enabling and setting the appropriate clock rate for the RT
CAMNOC. This change lays the groundwork for supporting such
configurations.

Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
This change lays the groundwork for supporting configurations for
hardware architectures that split a single CAMNOC module into real time
(RT) and non real time (NRT).
---
 drivers/media/platform/qcom/camss/camss-vfe.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index ee08dbbddf88..09b29ba383f1 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -914,7 +914,8 @@ static int vfe_match_clock_names(struct vfe_device *vfe,
 	return (!strcmp(clock->name, vfe_name) ||
 		!strcmp(clock->name, vfe_lite_name) ||
 		!strcmp(clock->name, "vfe_lite") ||
-		!strcmp(clock->name, "camnoc_axi"));
+		!strcmp(clock->name, "camnoc_axi") ||
+		!strcmp(clock->name, "camnoc_rt_axi"));
 }
 
 /*

---
base-commit: 69a67cb382f428c6dd8ba63e44cd2c59cb84f736
change-id: 20251012-add-new-clock-in-vfe-matching-list-25fb1e378c49

Best regards,
-- 
Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>

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

[Bryan O'Donoghue]

On 15/10/2025 03:43, Hangxiang Ma wrote:
> On hardware architectures where a single CAMNOC module is split into
> two, one for each of the real time (RT) and non real time (NRT) modules
> within camera sub system, processing VFE output over the AXI bus
> requires enabling and setting the appropriate clock rate for the RT
> CAMNOC. This change lays the groundwork for supporting such
> configurations.
> 
> Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
> ---
> This change lays the groundwork for supporting configurations for
> hardware architectures that split a single CAMNOC module into real time
> (RT) and non real time (NRT).
> ---
>   drivers/media/platform/qcom/camss/camss-vfe.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index ee08dbbddf88..09b29ba383f1 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -914,7 +914,8 @@ static int vfe_match_clock_names(struct vfe_device *vfe,
>   	return (!strcmp(clock->name, vfe_name) ||
>   		!strcmp(clock->name, vfe_lite_name) ||
>   		!strcmp(clock->name, "vfe_lite") ||
> -		!strcmp(clock->name, "camnoc_axi"));
> +		!strcmp(clock->name, "camnoc_axi") ||
> +		!strcmp(clock->name, "camnoc_rt_axi"));
>   }
>   
>   /*
> 
> ---
> base-commit: 69a67cb382f428c6dd8ba63e44cd2c59cb84f736
> change-id: 20251012-add-new-clock-in-vfe-matching-list-25fb1e378c49
> 
> Best regards,
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

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

[Krzysztof Kozlowski]

On 15/10/2025 04:43, Hangxiang Ma wrote:
> On hardware architectures where a single CAMNOC module is split into
> two, one for each of the real time (RT) and non real time (NRT) modules
> within camera sub system, processing VFE output over the AXI bus
> requires enabling and setting the appropriate clock rate for the RT
> CAMNOC. This change lays the groundwork for supporting such
> configurations.
> 
> Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
> ---
> This change lays the groundwork for supporting configurations for
> hardware architectures that split a single CAMNOC module into real time
> (RT) and non real time (NRT).
> ---
>  drivers/media/platform/qcom/camss/camss-vfe.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index ee08dbbddf88..09b29ba383f1 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -914,7 +914,8 @@ static int vfe_match_clock_names(struct vfe_device *vfe,
>  	return (!strcmp(clock->name, vfe_name) ||
>  		!strcmp(clock->name, vfe_lite_name) ||
>  		!strcmp(clock->name, "vfe_lite") ||
> -		!strcmp(clock->name, "camnoc_axi"));
> +		!strcmp(clock->name, "camnoc_axi") ||
> +		!strcmp(clock->name, "camnoc_rt_axi"));

Just use camnoc_axi for both. Look at your bindings - why do you keep
different names for same signal?

Best regards,
Krzysztof

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

[Konrad Dybcio]

On 10/16/25 7:54 AM, Krzysztof Kozlowski wrote:
> On 15/10/2025 04:43, Hangxiang Ma wrote:
>> On hardware architectures where a single CAMNOC module is split into
>> two, one for each of the real time (RT) and non real time (NRT) modules
>> within camera sub system, processing VFE output over the AXI bus
>> requires enabling and setting the appropriate clock rate for the RT
>> CAMNOC. This change lays the groundwork for supporting such
>> configurations.
>>
>> Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
>> ---
>> This change lays the groundwork for supporting configurations for
>> hardware architectures that split a single CAMNOC module into real time
>> (RT) and non real time (NRT).
>> ---
>>  drivers/media/platform/qcom/camss/camss-vfe.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
>> index ee08dbbddf88..09b29ba383f1 100644
>> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
>> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
>> @@ -914,7 +914,8 @@ static int vfe_match_clock_names(struct vfe_device *vfe,
>>  	return (!strcmp(clock->name, vfe_name) ||
>>  		!strcmp(clock->name, vfe_lite_name) ||
>>  		!strcmp(clock->name, "vfe_lite") ||
>> -		!strcmp(clock->name, "camnoc_axi"));
>> +		!strcmp(clock->name, "camnoc_axi") ||
>> +		!strcmp(clock->name, "camnoc_rt_axi"));
> 
> Just use camnoc_axi for both. Look at your bindings - why do you keep
> different names for same signal?

I think the correct question to ask is:

Is camnoc_axi going to represent the other (NRT) clock in this
setting?

Konrad

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

[Bryan O'Donoghue]

On 16/10/2025 12:30, Konrad Dybcio wrote:
> On 10/16/25 7:54 AM, Krzysztof Kozlowski wrote:
>> On 15/10/2025 04:43, Hangxiang Ma wrote:
>>> On hardware architectures where a single CAMNOC module is split into
>>> two, one for each of the real time (RT) and non real time (NRT) modules
>>> within camera sub system, processing VFE output over the AXI bus
>>> requires enabling and setting the appropriate clock rate for the RT
>>> CAMNOC. This change lays the groundwork for supporting such
>>> configurations.
>>>
>>> Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
>>> ---
>>> This change lays the groundwork for supporting configurations for
>>> hardware architectures that split a single CAMNOC module into real time
>>> (RT) and non real time (NRT).
>>> ---
>>>   drivers/media/platform/qcom/camss/camss-vfe.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
>>> index ee08dbbddf88..09b29ba383f1 100644
>>> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
>>> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
>>> @@ -914,7 +914,8 @@ static int vfe_match_clock_names(struct vfe_device *vfe,
>>>   	return (!strcmp(clock->name, vfe_name) ||
>>>   		!strcmp(clock->name, vfe_lite_name) ||
>>>   		!strcmp(clock->name, "vfe_lite") ||
>>> -		!strcmp(clock->name, "camnoc_axi"));
>>> +		!strcmp(clock->name, "camnoc_axi") ||
>>> +		!strcmp(clock->name, "camnoc_rt_axi"));
>>
>> Just use camnoc_axi for both. Look at your bindings - why do you keep
>> different names for same signal?
> 
> I think the correct question to ask is:
> 
> Is camnoc_axi going to represent the other (NRT) clock in this
> setting?
> 
> Konrad

I'm - perhaps naively - assuming this clock really is required ... and 
that both will be needed concurrently.

---
bod

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

[Loic Poulain]

On Thu, Oct 16, 2025 at 1:50 PM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
> >>>
> >>> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> >>> index ee08dbbddf88..09b29ba383f1 100644
> >>> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> >>> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> >>> @@ -914,7 +914,8 @@ static int vfe_match_clock_names(struct vfe_device *vfe,
> >>>     return (!strcmp(clock->name, vfe_name) ||
> >>>             !strcmp(clock->name, vfe_lite_name) ||
> >>>             !strcmp(clock->name, "vfe_lite") ||
> >>> -           !strcmp(clock->name, "camnoc_axi"));
> >>> +           !strcmp(clock->name, "camnoc_axi") ||
> >>> +           !strcmp(clock->name, "camnoc_rt_axi"));
> >>
> >> Just use camnoc_axi for both. Look at your bindings - why do you keep
> >> different names for same signal?
> >
> > I think the correct question to ask is:
> >
> > Is camnoc_axi going to represent the other (NRT) clock in this
> > setting?
> >
> > Konrad
>
> 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.

Regards,
Loic

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

[Vladimir Zapolskiy]

On 10/16/25 15:22, Loic Poulain wrote:
> On Thu, Oct 16, 2025 at 1:50 PM Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>>>>
>>>>> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
>>>>> index ee08dbbddf88..09b29ba383f1 100644
>>>>> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
>>>>> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
>>>>> @@ -914,7 +914,8 @@ static int vfe_match_clock_names(struct vfe_device *vfe,
>>>>>      return (!strcmp(clock->name, vfe_name) ||
>>>>>              !strcmp(clock->name, vfe_lite_name) ||
>>>>>              !strcmp(clock->name, "vfe_lite") ||
>>>>> -           !strcmp(clock->name, "camnoc_axi"));
>>>>> +           !strcmp(clock->name, "camnoc_axi") ||
>>>>> +           !strcmp(clock->name, "camnoc_rt_axi"));
>>>>
>>>> Just use camnoc_axi for both. Look at your bindings - why do you keep
>>>> different names for same signal?
>>>
>>> I think the correct question to ask is:
>>>
>>> Is camnoc_axi going to represent the other (NRT) clock in this
>>> setting?
>>>
>>> Konrad
>>
>> 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.
> 

Does it mean the clock handling should be removed from QCM2290 or
X1E80100 VFEx resources? Has it been tested/verified?

-- 
Best wishes,
Vladimir

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

[Bryan O'Donoghue]

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

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

[Vijay Kumar Tumati]

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.

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

[Bryan O'Donoghue]

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

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

[Hangxiang Ma]

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

We now prefer to add 'camnoc_rt_axi' (Right?). Maybe its better to add 
comment lines to remove the ambiguity whether 'camnoc_axi' denotes to RT 
or NRT. Please advise and correct me. Willing to receive feedback and 
suggestions. Thanks you for all.

---
Hangxiang

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

[Loic Poulain]

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

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

[Vladimir Zapolskiy]

Hi Hangxiang.

On 10/20/25 06:23, Hangxiang Ma 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

Thank you so much for performing the tests.

I would want to add that I've made right the same tests for SM8650 CAMSS,
which also has two 'camnoc_rt_axi' and 'camnoc_nrt_axi' clocks, and due
to my tests the latter one is not needed for the raw image producing, you
may notice that I've excluded it from the v3 series sent for review:

https://lore.kernel.org/linux-media/20251017031131.2232687-2-vladimir.zapolskiy@linaro.org

> We now prefer to add 'camnoc_rt_axi' (Right?). Maybe its better to add
> comment lines to remove the ambiguity whether 'camnoc_axi' denotes to RT
> or NRT. Please advise and correct me. Willing to receive feedback and
> suggestions. Thanks you for all.

-- 
Best wishes,
Vladimir

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

[Vijay Kumar Tumati]

On 10/20/2025 6:35 AM, Vladimir Zapolskiy wrote:
> Hi Hangxiang.
>
> On 10/20/25 06:23, Hangxiang Ma 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
>
> Thank you so much for performing the tests.
>
> I would want to add that I've made right the same tests for SM8650 CAMSS,
> which also has two 'camnoc_rt_axi' and 'camnoc_nrt_axi' clocks, and due
> to my tests the latter one is not needed for the raw image producing, you
> may notice that I've excluded it from the v3 series sent for review:
I agree. The NRT AXI clock shouldn't be required even for Kaanapali for 
RT blocks. @Hangxiang, can we please try to understand this better? 
Either way, I think the NRT clock part is not connected to this patch 
series I guess? Just as Bryan advised, we confirm that the 
'camnoc_axi_clk' is not required for Kaanapali to close out the comments 
on this series. Perhaps, we can continue the discussion on the NRT AXI 
clock in the Kaanapali patch series? Please advise.
>
> https://lore.kernel.org/linux-media/20251017031131.2232687-2-vladimir.zapolskiy@linaro.org 
>
>
>> We now prefer to add 'camnoc_rt_axi' (Right?). Maybe its better to add
>> comment lines to remove the ambiguity whether 'camnoc_axi' denotes to RT
>> or NRT. Please advise and correct me. Willing to receive feedback and
>> suggestions. Thanks you for all.
>

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

[Vijay Kumar Tumati]

On 10/20/2025 6:46 AM, Vijay Kumar Tumati wrote:
>
> On 10/20/2025 6:35 AM, Vladimir Zapolskiy wrote:
>> Hi Hangxiang.
>>
>> On 10/20/25 06:23, Hangxiang Ma 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
>>
>> Thank you so much for performing the tests.
>>
>> I would want to add that I've made right the same tests for SM8650 
>> CAMSS,
>> which also has two 'camnoc_rt_axi' and 'camnoc_nrt_axi' clocks, and due
>> to my tests the latter one is not needed for the raw image producing, 
>> you
>> may notice that I've excluded it from the v3 series sent for review:
> I agree. The NRT AXI clock shouldn't be required even for Kaanapali 
> for RT blocks. @Hangxiang, can we please try to understand this 
> better? Either way, I think the NRT clock part is not connected to 
> this patch series I guess? Just as Bryan advised, we confirm that the 
> 'camnoc_axi_clk' is not required for Kaanapali to close out the 
> comments on this series. Perhaps, we can continue the discussion on 
> the NRT AXI clock in the Kaanapali patch series? Please advise.

Some more clarification on this . Starting Kaanapali, we have PDX NOC 
after RT / NRT CAMNOCs and before MMNOC for domain crossing. Our HW team 
has confirmed that, for the PDX NOC to ensure that there is no traffic 
from either RT or NRT at the beginning of a session (it's called 
qchannel handshake) and start functioning, we need both the RT AXI and 
NRT AXI clocks. So two things,

1. Like I said, this patch is required regardless as it is about RT_AXI 
clock, which is required for Kaanapali.

2. In the other Kaanapali patch series, we will keep the NRT AXI clock 
in the VFE resources.

Hope this clarifies. Please let us know if you have any further 
questions. Thank you very much.

>>
>> https://lore.kernel.org/linux-media/20251017031131.2232687-2-vladimir.zapolskiy@linaro.org 
>>
>>
>>> We now prefer to add 'camnoc_rt_axi' (Right?). Maybe its better to add
>>> comment lines to remove the ambiguity whether 'camnoc_axi' denotes 
>>> to RT
>>> or NRT. Please advise and correct me. Willing to receive feedback and
>>> suggestions. Thanks you for all.
>>

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

[Bryan O'Donoghue]

On 21/10/2025 20:19, Vijay Kumar Tumati wrote:
> Hope this clarifies. Please let us know if you have any further
> questions. Thank you very much.

Eh.

So can I take this statement as Review-by: from you ?

That's basically all I really need here, RB or NAK.

---
bod

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

[Vijay Kumar Tumati]

On 10/22/2025 9:00 AM, Bryan O'Donoghue wrote:
> On 21/10/2025 20:19, Vijay Kumar Tumati wrote:
>> Hope this clarifies. Please let us know if you have any further
>> questions. Thank you very much.
>
> Eh.
>
> So can I take this statement as Review-by: from you ?
>
> That's basically all I really need here, RB or NAK.
>
> ---
> bod
Sorry, yes.
Reviewed-by: Vijay Kumar Tumati <vijay.tumati@oss.qualcomm.com>