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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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.

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





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux