Le vendredi 20 mai 2022 à 01:25 +0000, Ming Qian a écrit : > > 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. Fair point, but it should be bug_on or at least an error trace. regards, Nicolas > > 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) >