Re: [PATCH 3/6] media: v4l2-isp: Rename v4l2_isp_params_buffer_size

[Jacopo Mondi]

Hello Antoine

On Wed, May 06, 2026 at 10:11:31AM +0200, Antoine Bouyer wrote:
> On 5/5/26 4:12 PM, Jacopo Mondi wrote:
> >
> >
> > Rename v4l2_isp_params_buffer_size() to v4l2_isp_buffer_size() to
> > support statistics.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> > ---
> >   .../media/platform/arm/mali-c55/mali-c55-params.c  | 12 ++++++------
>
> Hi Jacopo
>
> Is it ok to meld driver and include changes and remove all occurrences of
> v4l2_isp_params_buffer_size ? Of course it prevents build errors, but what
> if pending changes are already using this macro ?

This is an internal header, we don't keep compatibility for internal
kAPI symbols.

If you're referring to your in-review patches which use the now
renamed 'v4l2_isp_params_buffer_size' symbol, I'm sorry but we'll
have to synchronize here, there's no way around it.

>  >   include/media/v4l2-isp.h                           | 22
> +++++++++++++---------
> >   2 files changed, 19 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> > index de0e9d898db7..dc483f0322d6 100644
> > --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> > @@ -487,7 +487,7 @@ static int mali_c55_params_g_fmt_meta_out(struct file *file, void *fh,
> >   {
> >          static const struct v4l2_meta_format mfmt = {
> >                  .dataformat = V4L2_META_FMT_MALI_C55_PARAMS,
> > -               .buffersize = v4l2_isp_params_buffer_size(MALI_C55_PARAMS_MAX_SIZE),
> > +               .buffersize = v4l2_isp_buffer_size(MALI_C55_PARAMS_MAX_SIZE),
> >          };
> >
> >          f->fmt.meta = mfmt;
> > @@ -540,13 +540,13 @@ mali_c55_params_queue_setup(struct vb2_queue *q, unsigned int *num_buffers,
> >          if (*num_planes && *num_planes > 1)
> >                  return -EINVAL;
> >
> > -       if (sizes[0] && sizes[0] < v4l2_isp_params_buffer_size(MALI_C55_PARAMS_MAX_SIZE))
> > +       if (sizes[0] && sizes[0] < v4l2_isp_buffer_size(MALI_C55_PARAMS_MAX_SIZE))
> >                  return -EINVAL;
> >
> >          *num_planes = 1;
> >
> >          if (!sizes[0])
> > -               sizes[0] = v4l2_isp_params_buffer_size(MALI_C55_PARAMS_MAX_SIZE);
> > +               sizes[0] = v4l2_isp_buffer_size(MALI_C55_PARAMS_MAX_SIZE);
> >
> >          return 0;
> >   }
> > @@ -556,7 +556,7 @@ static int mali_c55_params_buf_init(struct vb2_buffer *vb)
> >          struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> >          struct mali_c55_params_buf *buf = to_mali_c55_params_buf(vbuf);
> >
> > -       buf->config = kvmalloc(v4l2_isp_params_buffer_size(MALI_C55_PARAMS_MAX_SIZE),
> > +       buf->config = kvmalloc(v4l2_isp_buffer_size(MALI_C55_PARAMS_MAX_SIZE),
> >                                 GFP_KERNEL);
> >          if (!buf->config)
> >                  return -ENOMEM;
> > @@ -583,7 +583,7 @@ static int mali_c55_params_buf_prepare(struct vb2_buffer *vb)
> >          int ret;
> >
> >          ret = v4l2_isp_params_validate_buffer_size(mali_c55->dev, vb,
> > -                       v4l2_isp_params_buffer_size(MALI_C55_PARAMS_MAX_SIZE));
> > +                       v4l2_isp_buffer_size(MALI_C55_PARAMS_MAX_SIZE));
> >          if (ret)
> >                  return ret;
> >
> > @@ -593,7 +593,7 @@ static int mali_c55_params_buf_prepare(struct vb2_buffer *vb)
> >           * changed to the buffer content whilst the driver processes it.
> >           */
> >
> > -       memcpy(buf->config, config, v4l2_isp_params_buffer_size(MALI_C55_PARAMS_MAX_SIZE));
> > +       memcpy(buf->config, config, v4l2_isp_buffer_size(MALI_C55_PARAMS_MAX_SIZE));
> >
> >          return v4l2_isp_params_validate_buffer(mali_c55->dev, vb, buf->config,
> >                                                 mali_c55_params_block_types_info,
> > diff --git a/include/media/v4l2-isp.h b/include/media/v4l2-isp.h
> > index f3a6d0edcb24..d70ed6b431e7 100644
> > --- a/include/media/v4l2-isp.h
> > +++ b/include/media/v4l2-isp.h
> > @@ -15,17 +15,21 @@ struct device;
> >   struct vb2_buffer;
> >
> >   /**
> > - * v4l2_isp_params_buffer_size - Calculate size of v4l2_isp_params_buffer
> > - * @max_params_size: The total size of the ISP configuration blocks
> > + * v4l2_isp_buffer_size - Calculate size of v4l2_isp_buffer
> > + * @max_size: The total size of the ISP configuration or statistics blocks
> > + *
> > + * Users of v4l2-isp will have differing sized data arrays for parameters and
> > + * statistics, depending on their specific blocks. Drivers need to be able to
> > + * calculate the appropriate size of the buffer to accommodate all ISP blocks
> > + * supported by the platform. This macro provides a convenient tool for the
> > + * calculation.
> > + *
> > + * The intended users of this function are drivers initializing the size
> > + * of their metadata (parameters and statistics) buffers.
> >    *
> > - * Users of the v4l2 extensible parameters will have differing sized data arrays
> > - * depending on their specific parameter buffers. Drivers and userspace will
> > - * need to be able to calculate the appropriate size of the struct to
> > - * accommodate all ISP configuration blocks provided by the platform.
> > - * This macro provides a convenient tool for the calculation.
> >    */
> > -#define v4l2_isp_params_buffer_size(max_params_size) \
> > -       (offsetof(struct v4l2_isp_params_buffer, data) + (max_params_size))
> > +#define v4l2_isp_buffer_size(max_size)                 \
> > +       (offsetof(struct v4l2_isp_buffer, data) + (max_size))
>
> Shouldn't we keep v4l2_isp_params_buffer_size for compatibility ?

I don't think so, no. This is internal stuff, not exposed to
userspace. We should port all existing users to the new symbol, don't
we ?

>
> BR
> Antoine

Thanks!

>
> >
> >   /**
> >    * v4l2_isp_params_validate_buffer_size - Validate a V4L2 ISP buffer sizes
> >
> > --
> > 2.53.0
> >
>
>