Re: [PATCH 1/2] [RESEND] media: v4l2-mem2mem: allow device run without buf

[Nicolas Dufresne]

Hi Randy,

Le mardi 04 juillet 2023 à 12:00 +0800, Hsia-Jun Li a écrit :
> From: Randy Li <ayaka@xxxxxxxxxxx>
> 
> For the decoder supports Dynamic Resolution Change,
> we don't need to allocate any CAPTURE or graphics buffer
> for them at inital CAPTURE setup step.
> 
> We need to make the device run or we can't get those
> metadata.
> 
> Signed-off-by: Randy Li <ayaka@xxxxxxxxxxx>
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 0cc30397fbad..c771aba42015 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -301,8 +301,9 @@ static void __v4l2_m2m_try_queue(struct v4l2_m2m_dev *m2m_dev,
>  
>  	dprintk("Trying to schedule a job for m2m_ctx: %p\n", m2m_ctx);
>  
> -	if (!m2m_ctx->out_q_ctx.q.streaming
> -	    || !m2m_ctx->cap_q_ctx.q.streaming) {
> +	if (!(m2m_ctx->out_q_ctx.q.streaming || m2m_ctx->out_q_ctx.buffered)
> +	    || !(m2m_ctx->cap_q_ctx.q.streaming
> +		 || m2m_ctx->cap_q_ctx.buffered)) {

I have a two atches with similar goals in my wave5 tree. It will be easier to
upstream with an actual user, though, I'm probably a month or two away from
submitting this driver again.

https://gitlab.collabora.com/chipsnmedia/kernel/-/commit/ac59eafd5076c4deb3bfe1fb85b3b776586ef3eb
https://gitlab.collabora.com/chipsnmedia/kernel/-/commit/5de4fbe0abb20b8e8d862b654f93e3efeb1ef251

Sebastien and I authored this without giving it much thought, but we believe
this massively simplify our handling of DRC (dynamic resolution change).

The main difference, is that we added ignore_streaming to the ctx, so that
drivers can opt-in the mode of operation. Thinking it would avoid any potential
side effects in drivers that aren't prepared to that. We didn't want to tied it
up to buffered, this is open to discussion of course, we do use buffered on both
queues and use a slightly more advance job_ready function, that take into
account our driver state.

In short, Sebastien and I agree this small change is the right direction, we
simply have a different implementation. I can send it as RFC if one believe its
would be useful now (even without a user).

>  		dprintk("Streaming needs to be on for both queues\n");
>  		return;
>  	}