On 13/04/2023 01:40, Abhinav Kumar wrote:
On 4/12/2023 12:24 PM, Dmitry Baryshkov wrote:
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@xxxxxxxxxxx>
---
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.
Lets drop this from this series and come up with a better approach to
validate dsc params. We will take it up once dsc over dsi and dp lands.
Sure, why not.
+ 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
