[PATCH v5 6/8] drm/msm/dsi: Add check for slice_width in dsi_timing_setup

[Dmitry Baryshkov]

On 12/04/2023 22:09, Jessica Zhang wrote:
> Add a check for valid dsc->slice_width value in dsi_timing_setup.
> 
> Signed-off-by: Jessica Zhang <quic_jesszhan at quicinc.com>
> ---
>   drivers/gpu/drm/msm/dsi/dsi_host.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 508577c596ff..6a6218a9655f 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -937,6 +937,12 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>   			return;
>   		}
>   
> +		if (!dsc->slice_width || (mode->hdisplay < dsc->slice_width)) {

This is an erroneous condition, correct. Can we move it to a better 
place, where we can return an error instead of ignoring it?

I'd say that we should validate dsc->slice_width at the 
dsi_host_attach(). It well might be a good idea to add a helper that 
validates required dsc properties (e.g. version, bpp/bpc, slice_width, 
slice_height, slice_count).

As for the mode->hdisplay, we have the following code in 
msm_dsi_host_check_dsc() (where pic_width = mode->hdisplay):

if (pic_width % dsc->slice_width) {...}

This way the only way how mode->hdisplay can be less than 
dsc->slice_width is if mode->hdisplay is 0 (which is forbidden if I 
remember correctly). So the second part of the check is useless.

> +			pr_err("DSI: invalid slice width %d (pic_width: %d)\n",
> +			       dsc->slice_width, mode->hdisplay);
> +			return;
> +		}
> +
>   		dsc->pic_width = mode->hdisplay;
>   		dsc->pic_height = mode->vdisplay;
>   		DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height);
> 

-- 
With best wishes
Dmitry