| Message ID | 20241209053654.52-6-jackson.lee@chipsnmedia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Improve decoder performance and fix critical bugs | expand |
Le lundi 09 décembre 2024 à 14:36 +0900, Jackson.lee a écrit : > 521C Wave5 variant does not support 10 bit. When 10 bit support for 515 > variant was added, the code which returns an error was removed. While > testing 10bit hevc fluster on the 521C hw, timeout happened. > > Fixes: 143e7ab4d9a0 ("media: chips-media: wave5: support decoding HEVC Main10 profile") > > Signed-off-by: Jackson.lee <jackson.lee@chipsnmedia.com> > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> > --- > .../platform/chips-media/wave5/wave5-vpu-dec.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > index ce3fc47dc9d8..28462e01ca27 100644 > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > @@ -1406,10 +1406,24 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count > if (ret) > goto free_bitstream_vbuf; > } else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { > + struct dec_initial_info *initial_info = > + &inst->codec_info->dec_info.initial_info; > + > if (inst->state == VPU_INST_STATE_STOP) > ret = switch_state(inst, VPU_INST_STATE_INIT_SEQ); > if (ret) > goto return_buffers; > + > + if (inst->state == VPU_INST_STATE_INIT_SEQ && > + inst->dev->product_code == WAVE521C_CODE) { > + if (initial_info->luma_bitdepth != 8) { > + dev_info(inst->dev->dev, "%s: no support for %d bit depth", > + __func__, initial_info->luma_bitdepth); > + ret = -EINVAL; > + goto return_buffers; > + } > + } Let's take that fix for now, but consider filling some capabilities flag in the long term so as more variant get enable we don't endup with tones of produce_code == branch all over the place. Also, please send that one with 1/5 and 2/5 seperatly (perhaps 4/5 too if applicable). Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> p.s. its better to split the obvious fixes that are guarantied to go quickly from very complex changes. Specially that these are important bug fix, where the other change is for performance improvement. This makes the maintenance of your driver a lot more responsive to major issues. > + > } > pm_runtime_mark_last_busy(inst->dev->dev); > pm_runtime_put_autosuspend(inst->dev->dev);
Hi Nicolas Thanks for your review. I will prepare 1/5, 2/5, 4/5 and 5/5 separately patches for upstream. Jackson > -----Original Message----- > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > Sent: Saturday, December 14, 2024 4:56 AM > To: jackson.lee <jackson.lee@chipsnmedia.com>; mchehab@kernel.org; > hverkuil-cisco@xs4all.nl; sebastian.fricke@collabora.com; > bob.beckett@collabora.com; dafna.hirschfeld@collabora.com > Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; lafley.kim > <lafley.kim@chipsnmedia.com>; b-brnich@ti.com; hverkuil@xs4all.nl; Nas > Chung <nas.chung@chipsnmedia.com> > Subject: Re: [PATCH v1 5/5] media: chips-media: wave5: Fix timeout while > testing 10bit hevc fluster > > Le lundi 09 décembre 2024 à 14:36 +0900, Jackson.lee a écrit : > > 521C Wave5 variant does not support 10 bit. When 10 bit support for > > 515 variant was added, the code which returns an error was removed. > > While testing 10bit hevc fluster on the 521C hw, timeout happened. > > > > Fixes: 143e7ab4d9a0 ("media: chips-media: wave5: support decoding HEVC > > Main10 profile") > > > > Signed-off-by: Jackson.lee <jackson.lee@chipsnmedia.com> > > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> > > --- > > .../platform/chips-media/wave5/wave5-vpu-dec.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > > b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > > index ce3fc47dc9d8..28462e01ca27 100644 > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > > @@ -1406,10 +1406,24 @@ static int wave5_vpu_dec_start_streaming(struct > vb2_queue *q, unsigned int count > > if (ret) > > goto free_bitstream_vbuf; > > } else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { > > + struct dec_initial_info *initial_info = > > + &inst->codec_info->dec_info.initial_info; > > + > > if (inst->state == VPU_INST_STATE_STOP) > > ret = switch_state(inst, VPU_INST_STATE_INIT_SEQ); > > if (ret) > > goto return_buffers; > > + > > + if (inst->state == VPU_INST_STATE_INIT_SEQ && > > + inst->dev->product_code == WAVE521C_CODE) { > > + if (initial_info->luma_bitdepth != 8) { > > + dev_info(inst->dev->dev, "%s: no support for %d > bit depth", > > + __func__, initial_info->luma_bitdepth); > > + ret = -EINVAL; > > + goto return_buffers; > > + } > > + } > > Let's take that fix for now, but consider filling some capabilities flag > in the long term so as more variant get enable we don't endup with tones > of produce_code == branch all over the place. Also, please send that one > with 1/5 and 2/5 seperatly (perhaps 4/5 too if applicable). > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > p.s. its better to split the obvious fixes that are guarantied to go > quickly from very complex changes. Specially that these are important bug > fix, where the other change is for performance improvement. This makes the > maintenance of your driver a lot more responsive to major issues. > > > + > > } > > pm_runtime_mark_last_busy(inst->dev->dev); > > pm_runtime_put_autosuspend(inst->dev->dev);
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c index ce3fc47dc9d8..28462e01ca27 100644 --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c @@ -1406,10 +1406,24 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count if (ret) goto free_bitstream_vbuf; } else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { + struct dec_initial_info *initial_info = + &inst->codec_info->dec_info.initial_info; + if (inst->state == VPU_INST_STATE_STOP) ret = switch_state(inst, VPU_INST_STATE_INIT_SEQ); if (ret) goto return_buffers; + + if (inst->state == VPU_INST_STATE_INIT_SEQ && + inst->dev->product_code == WAVE521C_CODE) { + if (initial_info->luma_bitdepth != 8) { + dev_info(inst->dev->dev, "%s: no support for %d bit depth", + __func__, initial_info->luma_bitdepth); + ret = -EINVAL; + goto return_buffers; + } + } + } pm_runtime_mark_last_busy(inst->dev->dev); pm_runtime_put_autosuspend(inst->dev->dev);
521C Wave5 variant does not support 10 bit. When 10 bit support for 515 variant was added, the code which returns an error was removed. While testing 10bit hevc fluster on the 521C hw, timeout happened. Fixes: 143e7ab4d9a0 ("media: chips-media: wave5: support decoding HEVC Main10 profile") Signed-off-by: Jackson.lee <jackson.lee@chipsnmedia.com> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> --- .../platform/chips-media/wave5/wave5-vpu-dec.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)