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

[Atanas Filipov]

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

> > 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