Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <20452e233a9a4b39b58139081d818d3b1454105a.camel@ndufresne.ca>
Subject: Re: [PATCH 1/2] [RESEND] media: v4l2-mem2mem: allow device run
 without buf
From:   Nicolas Dufresne <nicolas@ndufresne.ca>
To:     Hsia-Jun Li <randy.li@synaptics.com>, linux-media@vger.kernel.org
Cc:     ayaka@soulik.info, hans.verkuil@cisco.com, tfiga@chromium.org,
        mchehab@kernel.org, laurent.pinchart@ideasonboard.com,
        hiroh@chromium.org, hverkuil@xs4all.nl,
        linux-kernel@vger.kernel.org,
        Sebastian Fricke <sebastian.fricke@collabora.com>
Date:   Fri, 07 Jul 2023 15:14:23 -0400
In-Reply-To: <20230704040044.681850-2-randy.li@synaptics.com>
References: <20230704040044.681850-1-randy.li@synaptics.com>
         <20230704040044.681850-2-randy.li@synaptics.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.3 (3.48.3-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

Hi Randy,

Le mardi 04 juillet 2023 =C3=A0 12:00 +0800, Hsia-Jun Li a =C3=A9crit=C2=A0=
:
> From: Randy Li <ayaka@soulik.info>
>=20
> 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.
>=20
> We need to make the device run or we can't get those
> metadata.
>=20
> Signed-off-by: Randy Li <ayaka@soulik.info>
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>=20
> 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,
> =20
>  	dprintk("Trying to schedule a job for m2m_ctx: %p\n", m2m_ctx);
> =20
> -	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/ac59eafd5076c4deb3=
bfe1fb85b3b776586ef3eb
https://gitlab.collabora.com/chipsnmedia/kernel/-/commit/5de4fbe0abb20b8e8d=
862b654f93e3efeb1ef251

Sebastien and I authored this without giving it much thought, but we believ=
e
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 poten=
tial
side effects in drivers that aren't prepared to that. We didn't want to tie=
d 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, w=
e
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;
>  	}