> From: Nicolas Dufresne [mailto:nicolas@xxxxxxxxxxxx] > Sent: Friday, May 20, 2022 2:13 AM > To: Ming Qian <ming.qian@xxxxxxx>; mchehab@xxxxxxxxxx; > hverkuil-cisco@xxxxxxxxx > Cc: shawnguo@xxxxxxxxxx; robh+dt@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; > kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; dl-linux-imx > <linux-imx@xxxxxxx>; linux-media@xxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > 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. Ming > > > > Signed-off-by: Ming Qian <ming.qian@xxxxxxx> > > --- > > 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)
