Re: [PATCH v4 0/6] media: qcom: camss: share VFE clock sources by the highest rate

["Gjorgji Rosikopulos (Consultant)" <gjorgji.rosikopulos@oss.qualcomm.com>]

Hi Atanas,

The whole series were verified on RB8 board (SA8775P).

Tested-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>

~Gjorgji

On 10/28/2026 3:23 PM, Atanas Filipov wrote:
> On SM8250 several VFE clocks are fed by one clock source: the AREG
> clocks of the two full VFEs share the fast AHB source, the two VFE Lite
> share one core clock, and all four VFEs share the camera NoC AXI clock.
> Each VFE sets such a clock to its own rate, so the last VFE to power on
> wins and can lower the rate under a VFE that is still running. The CSID
> also sets the VFE core clocks to their lowest level when it powers on
> after the VFE, and nothing sets the camera NoC AXI rate, so the camera
> NoC runs from XO and two concurrent streams stall.
> 
> Two examples on RB5 (QRB5165):
> 
> - VFE0 streams the CSID test pattern and VFE1 then starts an imx577
>   stream. Nothing sets the camera NoC AXI rate, so the camera NoC stays
>   at XO (19.2 MHz) and one of the two streams stalls. With the series
>   the VFEs set it to 300 or 400 MHz and both streams complete.
> 
> - VFE Lite 0 streams the CSID test pattern at 480 MHz and VFE Lite 1
>   then starts an imx577 stream that needs 400 MHz. The two share one
>   core clock, so VFE Lite 1 lowers it to 400 MHz under the running VFE
>   Lite 0. With the series the clock stays at 480 MHz until VFE Lite 0
>   stops.
> 
> This series lets the VFE own its clock rates:
> 
> - Patch 1 moves the minimum rate computation out of the VFE clock
>   loops, with no functional change.
> - Patch 2 keeps failing when the pixel rate exceeds the VFE core
>   clock, and clamps the other VFE clocks to their highest listed rate,
>   as the VFE clock rate tables need not have the same number of
>   levels.
> - Patch 3 stops the CSID from setting the VFE core clock rates.
> - Patch 4 sets the VFE clock rates as minimums with
>   clk_set_rate_range(), so that a shared clock runs at the highest
>   minimum of its running VFEs. This applies to all SoCs.
> - Patch 5 lets the VFE set the AREG clock at a fixed 400 MHz.
> - Patch 6 drops the unused camera NoC AXI RCG from the VFE clocks and
>   lets the VFE set the camera NoC AXI rate.
> 
> The camera NoC AXI clock is also used by drivers outside CAMSS, and how
> to aggregate their requests with the CAMSS one is still being discussed
> on the CAMNOC interconnect series. Patch 6 only covers the CAMSS VFEs
> and fixes the concurrent stall today.
> 
> Tested on RB5 (QRB5165) with imx577 and ov9282 sensors and the CSID test
> pattern generator, on single, concurrent test pattern plus sensor, and
> concurrent dual sensor RDI captures on all four VFEs: all streams
> complete, no shared clock is lowered under a running VFE, and the
> camera NoC AXI clock runs at 300 or 400 MHz while streaming.
> 
> Also tested on the Lemans EVK (SA8775P) with imx577 and the CAMSS TPG,
> on single and concurrent test pattern plus sensor RDI captures on all
> seven IFEs: all streams complete and no shared clock is lowered under
> a running IFE.
> 
> Changes in v4:
> - Keep -EINVAL for the VFE core clock and clamp only the other VFE
>   clocks, without a warning (Bryan O'Donoghue, Loic Poulain)
> - Use clk_set_rate_range() on all SoCs and drop the vfe_clock_ranges
>   flag (Loic Poulain, Bryan O'Donoghue)
> - Add Suggested-by to patch 4 (Bryan O'Donoghue)
> - Drop Reviewed-by from patch 2 due to changes
> 
> Changes in v3:
> - Replace the CAMSS clock domains with clk_set_rate_range() and fold
>   the rate ranges into patch 4 (Bryan O'Donoghue)
> - Fix the VFE AREG clock at 400 MHz (Bryan O'Donoghue)
> - Collect Reviewed-by tags
> 
> Changes in v2:
> - Split the minimum rate computation out of the clock loops into its
>   own patch (Loic Poulain)
> - Add the Fixes tag to the CSID patch (Bryan O'Donoghue)
> - Split the clock grouping from the AREG clock change and add an
>   example to the grouping commit message (Bryan O'Donoghue)
> - Rename the per-clock entries of a clock domain from user to vote
>   (Bryan O'Donoghue)
> 
> Atanas Filipov (6):
>   media: qcom: camss: factor out the VFE minimum clock rate
>   media: qcom: camss: clamp VFE clock rates to the highest level
>   media: qcom: camss: sm8250: do not set VFE core clock rates from CSID
>   media: qcom: camss: share VFE clock sources by the highest rate
>   media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE
>   media: qcom: camss: sm8250: set the camera NoC AXI rate from the VFE
> 
>  drivers/media/platform/qcom/camss/camss-vfe.c | 182 +++++++++++-------
>  drivers/media/platform/qcom/camss/camss.c     |  44 ++---
>  2 files changed, 136 insertions(+), 90 deletions(-)
> 
> 
> base-commit: bd1ecbb61afefc6a8b876ccbc31e9228b44e9eab