Re: [PATCH v4 1/6] media: qcom: camss: factor out the VFE minimum clock rate

[Loic Poulain]

On Wed, Oct 28, 2026 at 2:24 PM Atanas Filipov
<atanas.filipov@xxxxxxxxxxxxxxxx> wrote:
>
> vfe_set_clock_rates() and vfe_check_clock_rates() both compute the
> minimum clock rate needed by the VFE lines from their pixel rates,
> inside the loop over the VFE clocks. The result is the same for every
> clock.
>
> Move the computation into vfe_get_min_clock_rate() and call it once
> per VFE, so that the clock loops only select and check rates. No
> functional change.
>
> Signed-off-by: Atanas Filipov <atanas.filipov@xxxxxxxxxxxxxxxx>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>

Reviewed-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>

> ---
>  drivers/media/platform/qcom/camss/camss-vfe.c | 108 ++++++++----------
>  1 file changed, 46 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index 2680a9914765..59be37f83df3 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -935,52 +935,66 @@ static bool vfe_check_clock_levels(struct camss_clock *clock)
>  }
>
>  /*
> - * vfe_set_clock_rates - Calculate and set clock rates on VFE module
> + * vfe_get_min_clock_rate - Get the minimum VFE clock rate for its lines
>   * @vfe: VFE device
>   *
> - * Return 0 on success or a negative error code otherwise
> + * Return the highest rate needed by the VFE lines for their pixel rates,
> + * including the clock margin, or 0 if no pixel rate is available
>   */
> -static int vfe_set_clock_rates(struct vfe_device *vfe)
> +static u64 vfe_get_min_clock_rate(struct vfe_device *vfe)
>  {
> -       struct device *dev = vfe->camss->dev;
>         u64 pixel_clock[VFE_LINE_NUM_MAX];
> -       int i, j;
> -       int ret;
> +       u64 min_rate = 0;
> +       int i;
>
>         for (i = VFE_LINE_RDI0; i < vfe->res->line_num; i++) {
> -               ret = camss_get_pixel_clock(&vfe->line[i].subdev.entity,
> -                                           &pixel_clock[i]);
> -               if (ret)
> +               if (camss_get_pixel_clock(&vfe->line[i].subdev.entity,
> +                                         &pixel_clock[i]))
>                         pixel_clock[i] = 0;
>         }
>
> -       for (i = 0; i < vfe->nclocks; i++) {
> -               struct camss_clock *clock = &vfe->clock[i];
> +       for (i = VFE_LINE_RDI0; i < vfe->res->line_num; i++) {
> +               u32 tmp;
> +               u8 bpp;
>
> -               if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
> -                       u64 min_rate = 0;
> -                       long rate;
> +               if (i == VFE_LINE_PIX) {
> +                       tmp = pixel_clock[i];
> +               } else {
> +                       struct vfe_line *l = &vfe->line[i];
>
> -                       for (j = VFE_LINE_RDI0; j < vfe->res->line_num; j++) {
> -                               u32 tmp;
> -                               u8 bpp;
> +                       bpp = camss_format_get_bpp(l->formats,
> +                                                  l->nformats,
> +                                                  l->fmt[MSM_VFE_PAD_SINK].code);
> +                       tmp = pixel_clock[i] * bpp / 64;
> +               }
>
> -                               if (j == VFE_LINE_PIX) {
> -                                       tmp = pixel_clock[j];
> -                               } else {
> -                                       struct vfe_line *l = &vfe->line[j];
> +               if (min_rate < tmp)
> +                       min_rate = tmp;
> +       }
>
> -                                       bpp = camss_format_get_bpp(l->formats,
> -                                                                  l->nformats,
> -                                                                  l->fmt[MSM_VFE_PAD_SINK].code);
> -                                       tmp = pixel_clock[j] * bpp / 64;
> -                               }
> +       camss_add_clock_margin(&min_rate);
>
> -                               if (min_rate < tmp)
> -                                       min_rate = tmp;
> -                       }
> +       return min_rate;
> +}
>
> -                       camss_add_clock_margin(&min_rate);
> +/*
> + * vfe_set_clock_rates - Calculate and set clock rates on VFE module
> + * @vfe: VFE device
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int vfe_set_clock_rates(struct vfe_device *vfe)
> +{
> +       struct device *dev = vfe->camss->dev;
> +       u64 min_rate = vfe_get_min_clock_rate(vfe);
> +       int i, j;
> +       int ret;
> +
> +       for (i = 0; i < vfe->nclocks; i++) {
> +               struct camss_clock *clock = &vfe->clock[i];
> +
> +               if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
> +                       long rate;
>
>                         for (j = 0; j < clock->nfreqs; j++)
>                                 if (min_rate < clock->freq[j])
> @@ -1024,45 +1038,15 @@ static int vfe_set_clock_rates(struct vfe_device *vfe)
>   */
>  static int vfe_check_clock_rates(struct vfe_device *vfe)
>  {
> -       u64 pixel_clock[VFE_LINE_NUM_MAX];
> -       int i, j;
> -       int ret;
> -
> -       for (i = VFE_LINE_RDI0; i < vfe->res->line_num; i++) {
> -               ret = camss_get_pixel_clock(&vfe->line[i].subdev.entity,
> -                                           &pixel_clock[i]);
> -               if (ret)
> -                       pixel_clock[i] = 0;
> -       }
> +       u64 min_rate = vfe_get_min_clock_rate(vfe);
> +       int i;
>
>         for (i = 0; i < vfe->nclocks; i++) {
>                 struct camss_clock *clock = &vfe->clock[i];
>
>                 if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
> -                       u64 min_rate = 0;
>                         unsigned long rate;
>
> -                       for (j = VFE_LINE_RDI0; j < vfe->res->line_num; j++) {
> -                               u32 tmp;
> -                               u8 bpp;
> -
> -                               if (j == VFE_LINE_PIX) {
> -                                       tmp = pixel_clock[j];
> -                               } else {
> -                                       struct vfe_line *l = &vfe->line[j];
> -
> -                                       bpp = camss_format_get_bpp(l->formats,
> -                                                                  l->nformats,
> -                                                                  l->fmt[MSM_VFE_PAD_SINK].code);
> -                                       tmp = pixel_clock[j] * bpp / 64;
> -                               }
> -
> -                               if (min_rate < tmp)
> -                                       min_rate = tmp;
> -                       }
> -
> -                       camss_add_clock_margin(&min_rate);
> -
>                         rate = clk_get_rate(clock->clk);
>                         if (rate < min_rate)
>                                 return -EBUSY;
> --
> 2.34.1
>