lkml.org 
[lkml]   [2022]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [EXT] Re: [PATCH] media: amphion: return error if format is unsupported by vpu
Date
> From: Nicolas Dufresne [mailto:nicolas@ndufresne.ca]
> Sent: Saturday, May 21, 2022 2:09 AM
> To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org;
> hverkuil-cisco@xs4all.nl
> Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; linux-media@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [EXT] Re: [PATCH] media: amphion: return error if format is
> unsupported by vpu
>
> Caution: EXT Email
>
> Le vendredi 20 mai 2022 à 01:25 +0000, Ming Qian a écrit :
> > > From: Nicolas Dufresne [mailto:nicolas@ndufresne.ca]
> > > Sent: Friday, May 20, 2022 2:13 AM
> > > To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org;
> > > hverkuil-cisco@xs4all.nl
> > > Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> > > <linux-imx@nxp.com>; linux-media@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > > Subject: [EXT] Re: [PATCH] media: amphion: return error if format is
> > > unsupported by vpu
> > >
> > > Caution: EXT Email
> > >
> > > Hi Ming Qian,
> > >
> > > Le jeudi 19 mai 2022 à 15:28 +0800, Ming Qian a écrit :
> > > > return error if format is unsupported by vpu, otherwise the vpu
> > > > will be stalled at decoding
> > >
> > > I have a reasonable doubt about this patch. I don't think such a
> > > case should be reachable by users. Normally, calls to S_FMT should
> > > ensure the driver format state is valid on both ends but modifying
> > > the relevant structures. As an example, for decoders, setting the
> > > CODEC (OUTPUT queue) format, may change the raw format (CAPTURE
> > > queue) implicitly to prevent this situation.
> > > Are we certain this change isn't papering around some missing format
> > > propagation ?
> > >
> > > regards,
> > > Nicolas
> > >
> >
> > Hi Nicolas,
> > You're right, it's not reachable currently.
> > And there are some formats supported by VPU, but I didn't add
> > support in driver, as they are not defined in kernel yet.
> > So if someone wants to enable them in future, and if he only adds a
> > format into vdec_formats[] without modifying the vpu_malone part ,
> > then he can enum_fmt and set_fmt successfully, but meet vpu hang
> > without any error message.
> > I think driver should report an error in case of the new format
> > is not implemented fully.
>
> Fair point, but it should be bug_on or at least an error trace.
>
> regards,
> Nicolas
>

I'll add some error trace in v2

> >
> > Ming
> >
> > > >
> > > > Signed-off-by: Ming Qian <ming.qian@nxp.com>
> > > > ---
> > > > drivers/media/platform/amphion/vpu_malone.c | 2 ++
> > > > drivers/media/platform/amphion/vpu_v4l2.c | 4 ++--
> > > > 2 files changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/amphion/vpu_malone.c
> > > > b/drivers/media/platform/amphion/vpu_malone.c
> > > > index f29c223eefce..0930b6ba8c42 100644
> > > > --- a/drivers/media/platform/amphion/vpu_malone.c
> > > > +++ b/drivers/media/platform/amphion/vpu_malone.c
> > > > @@ -610,6 +610,8 @@ static int vpu_malone_set_params(struct
> > > vpu_shared_addr *shared,
> > > > enum vpu_malone_format malone_format;
> > > >
> > > > malone_format =
> > > vpu_malone_format_remap(params->codec_format);
> > > > + if (malone_format == MALONE_FMT_NULL)
> > > > + return -EINVAL;
> > > > iface->udata_buffer[instance].base = params->udata.base;
> > > > iface->udata_buffer[instance].slot_size =
> > > > params->udata.size;
> > > >
> > > > diff --git a/drivers/media/platform/amphion/vpu_v4l2.c
> > > > b/drivers/media/platform/amphion/vpu_v4l2.c
> > > > index 446f07d09d0b..89b88e063e45 100644
> > > > --- a/drivers/media/platform/amphion/vpu_v4l2.c
> > > > +++ b/drivers/media/platform/amphion/vpu_v4l2.c
> > > > @@ -500,10 +500,10 @@ static int vpu_vb2_start_streaming(struct
> > > vb2_queue *q, unsigned int count)
> > > > fmt->sizeimage[1], fmt->bytesperline[1],
> > > > fmt->sizeimage[2], fmt->bytesperline[2],
> > > > q->num_buffers);
> > > > - call_void_vop(inst, start, q->type);
> > > > + ret = call_vop(inst, start, q->type);
> > > > vb2_clear_last_buffer_dequeued(q);
> > > >
> > > > - return 0;
> > > > + return ret;
> > > > }
> > > >
> > > > static void vpu_vb2_stop_streaming(struct vb2_queue *q)
> >

\
 
 \ /
  Last update: 2022-05-23 03:42    [W:0.062 / U:0.864 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site