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

[Loic Poulain]

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

>
> Signed-off-by: Atanas Filipov <atanas.filipov@xxxxxxxxxxxxxxxx>
> ---
>  drivers/media/platform/qcom/camss/camss-vfe.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index 2680a9914765..87096095b1eb 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -982,15 +982,13 @@ static int vfe_set_clock_rates(struct vfe_device *vfe)
>
>                         camss_add_clock_margin(&min_rate);
>
> -                       for (j = 0; j < clock->nfreqs; j++)
> +                       for (j = 0; j < clock->nfreqs - 1; j++)
>                                 if (min_rate < clock->freq[j])
>                                         break;
>
> -                       if (j == clock->nfreqs) {
> -                               dev_err(dev,
> -                                       "Pixel clock is too high for VFE");
> -                               return -EINVAL;
> -                       }
> +                       if (clock->nfreqs > 1 && min_rate >= clock->freq[j])
> +                               dev_warn(dev, "Pixel clock is too high for VFE clock %s\n",
> +                                        clock->name);
>
>                         /* if sensor pixel clock is not available */
>                         /* set highest possible VFE clock rate */
> @@ -1041,6 +1039,7 @@ static int vfe_check_clock_rates(struct vfe_device *vfe)
>                 if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
>                         u64 min_rate = 0;
>                         unsigned long rate;
> +                       long max_rate;
>
>                         for (j = VFE_LINE_RDI0; j < vfe->res->line_num; j++) {
>                                 u32 tmp;
> @@ -1063,6 +1062,13 @@ static int vfe_check_clock_rates(struct vfe_device *vfe)
>
>                         camss_add_clock_margin(&min_rate);
>
> +                       /* A clock already at its highest rate cannot go higher */
> +                       max_rate = clk_round_rate(clock->clk, clock->freq[clock->nfreqs - 1]);
> +                       if (max_rate < 0)
> +                               return max_rate;
> +
> +                       min_rate = min_t(u64, min_rate, max_rate);
> +
>                         rate = clk_get_rate(clock->clk);
>                         if (rate < min_rate)
>                                 return -EBUSY;
> --
> 2.34.1
>