Re: [PATCH v3 2/2] media: nxp: imx8-isi: Prioritize pending buffers over discard buffers

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Wed, May 20, 2026 at 07:10:27PM +0200, Laurent Pinchart wrote:
> Hello Guoniu,
> 
> Thank you for the patch.
> 
> On Fri, Mar 20, 2026 at 02:42:02PM +0800, Guoniu Zhou wrote:
> > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> > 
> > The number of times to use the discard buffer is determined by the
> > out_pending list size:
> > 
> >   discard = list_empty(&video->out_pending) ? 2
> >             : list_is_singular(&video->out_pending) ? 1
> > 	    : 0;
> > 
> > In the current buffer selection logic, when both discard and pending
> > buffers are available, the driver fills hardware slots with discard
> > buffers first which results in an unnecessary frame drop even though
> > a user buffer was queued and ready.
> > 
> > Change the buffer selection logic to use pending buffers first (up to
> > the number available), and only use discard buffers to fill remaining
> > slots when insufficient pending buffers are queued.
> > 
> > This improves behavior by:
> > - Reducing discarded frames at stream start when user buffers are ready
> > - Decreasing latency in delivering captured frames to user-space
> > - Ensuring user buffers are utilized as soon as they are queued
> > - Improving overall buffer utilization efficiency
> 
> There's a bit of repeat here, but that's OK.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > ---
> > Changes in v3:
> > - Expanded commit message to explain the problem in current driver and the
> >   benefits gained from this change
> > - No code changes
> > 
> > Changes in v2:
> > - Replace "This ensures" with "ensure"
> > - Put example from commit message to comment in driver suggested by Frank
> >   https://lore.kernel.org/linux-media/20260311-isi_min_buffers-v1-0-c9299d6e8ae6@nxp.com/T/#m2774912ed31553ef1fdcc840bd6eae53a03ecccd
> > ---
> >  drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> > index 1be3a728f32f..77ebff03323a 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> > @@ -792,7 +792,14 @@ static void mxc_isi_video_queue_first_buffers(struct mxc_isi_video *video)
> >  		struct mxc_isi_buffer *buf;
> >  		struct list_head *list;
> >  
> > -		list = i < discard ? &video->out_discard : &video->out_pending;
> > +		/*
> > +		 * Queue buffers: prioritize pending buffers, then discard buffers
> > +		 * For example:
> > +		 * - 2 pending buffers: both slots use pending buffers
> > +		 * - 1 pending buffer: first slot uses pending, second uses discard
> > +		 * - 0 pending buffers: both slots use discard buffers
> > +		 */

Having a second look, I think this is a bit too verbose. Unless you
object, I'll write

		 * Queue buffers: prioritize pending buffers, then discard
		 * buffers.

No need to submit a new version.

> > +		list = (i < 2 - discard) ? &video->out_pending : &video->out_discard;
> >  		buf = list_first_entry(list, struct mxc_isi_buffer, list);
> >  
> >  		mxc_isi_channel_set_outbuf(video->pipe, buf->dma_addrs, buf_id);

-- 
Regards,

Laurent Pinchart