Re: [PATCH v1 5/5] media: chips-media: wave5: Fix timeout while testing 10bit hevc fluster

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

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);