Re: [PATCH v2 05/17] media: v4l2-subdev: Allocate frame descriptors based on the need

[Frank Li <Frank.li@nxp.com>]

On Tue, May 19, 2026 at 05:53:57PM -0400, Frank Li wrote:
> On Mon, May 18, 2026 at 07:43:05PM +0300, Sakari Ailus wrote:
> > Frame descriptors entries require a small amount of memory per entry (20
> > bytes), but if the number of entries in a frame descriptor is large, an
> > unreasonably large amount of memory would need to be allocated in the
> > stack. Therefore the number of entries has been limited to 8.
> >
> > Support larger frame descriptors by allocating as much memory as required.
> > The get_frame_desc() op can now set the num_entries to a number larger
> > than V4L2_FRAME_BUS_ENTRY_PREALLOC and return -ENOSPC. The caller,
> > v4l2_subdev_get_frame_desc(), will then allocate memory for that amount of
> > memory and call the get_frame_desc() op again.
> >
> > The caller is also responsible for releasing the allocated memory by
> > calling v4l2_subdev_free_frame_desc().
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> ...
> >  /**
> >   * enum v4l2_mbus_frame_desc_type - media bus frame description type
> > @@ -393,13 +395,17 @@ enum v4l2_mbus_frame_desc_type {
> >  /**
> >   * struct v4l2_mbus_frame_desc - media bus data frame description
> >   * @type: type of the bus (enum v4l2_mbus_frame_desc_type)
> > - * @entry: frame descriptors array
> > - * @num_entries: number of entries in @entry array
> > + * @entry_mem: memory for the frame descriptors (@entry)
> > + * @entry: pointer to the frame descriptors
> > + * @num_entries: number of entries in @entry
> > + * @len_entries: number of entries allocated for @entry
> >   */
> >  struct v4l2_mbus_frame_desc {
> >  	enum v4l2_mbus_frame_desc_type type;
> > -	struct v4l2_mbus_frame_desc_entry entry[V4L2_FRAME_DESC_ENTRY_MAX];
> > +	struct v4l2_mbus_frame_desc_entry entry_mem[V4L2_FRAME_DESC_ENTRY_PREALLOC];
> > +	struct v4l2_mbus_frame_desc_entry *entry;
> >  	unsigned short num_entries;
> > +	unsigned short len_entries;
>
> name is not direct reflect means and quite easy to confuse with num_entries.
> Is it num_dym_entries or other names little better?

Please ignore this comments. I understand what's means.

Reviewed-by: Frank Li <Frank.Li@nxp.com>

Frank
>
> Frank
> >  };
> >
> >  /**
> > @@ -781,7 +787,14 @@ struct v4l2_subdev_state {
> >   * @link_validate: used by the media controller code to check if the links
> >   *		   that belongs to a pipeline can be used for stream.
> >   *
> > - * @get_frame_desc: get the current low level media bus frame parameters.
> > + * @get_frame_desc: get the current low level media bus frame parameters. The
> > + *		    callback is required to update the num_entries field to the
> > + *		    total number of entries in the frame descriptor. The
> > + *		    callback shall fill the first entries array up to
> > + *		    len_entries, which signifies the number of entries
> > + *		    allocated. If num_entries exceeds len_entries, the callback
> > + *		    shall return -ENOSPC. Never call this directly in drivers,
> > + *		    use v4l2_subdev_get_frame_desc() instead.
> >   *
> >   * @set_frame_desc: set the low level media bus frame parameters, @fd array
> >   *                  may be adjusted by the subdev driver to device capabilities.
> > @@ -1794,8 +1807,9 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
> >   *
> >   * The caller is required to set @desc->type to the expected bus type.
> >   *
> > - * The caller is required to release the memory of the frame descriptor entries
> > - * for each frame descriptor obtained by calling this function using
> > + * The entries in the frame descriptor are allocated based on the need. The
> > + * caller is required to release the memory of the frame descriptor entries for
> > + * each frame descriptor obtained by calling this function using
> >   * v4l2_subdev_free_frame_desc().
> >   *
> >   * Use __free() to release the frame descriptor automatically::
> > @@ -1813,7 +1827,8 @@ v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> >   * v4l2_subdev_free_frame_desc() - Release the memory of a frame descriptor
> >   * @desc: A pointer to a frame descriptor
> >   *
> > - * Release the frame descriptor.
> > + * Release the frame descriptor entries in a frame descriptor as well as the
> > + * frame descriptor itself.
> >   */
> >  void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc);
> >
> > --
> > 2.47.3
> >