Commit ac47870fd795 ("drm/msm/dsi: fix hdisplay calculation when
programming dsi registers") makes a false and ungrounded statement that
"Since CMD mode does not use this, we can remove !(msm_host->mode_flags
& MIPI_DSI_MODE_VIDEO) safely." which isn't the case at all.
dsi_timing_setup() affects both command mode and video mode panels, and
by no longer having any path that sets bits_per_pclk = 48 (contrary to
the updated code-comment) all DSI DSC panels on SM8350 and above (i.e.
with widebus support) regress thanks to this patch.

The entire reason that video mode was originally omitted from this code
path is because it was never tested before; any change that enables
widebus for video mode panels should not regress the command mode path.

Thus add back the path allows 6 bytes or 48 bits to be sent per pclk
on command mode DSI panels with widebus, restoring the panel on devices
like the Sony Xperia 1 III and upwards.

Fixes: ac47870fd795 ("drm/msm/dsi: fix hdisplay calculation when programming 
dsi registers")
Signed-off-by: Marijn Suijten <[email protected]>
---
In addition I can't say I understand the original commit message
at all; it mentions a BPC=10 mode however the highest format that
mipi_dsi_pixel_format_to_bpp supports is RGB888 thus it won't
ever return anything above 24, which is the original amount the
non-command-mode path defaulted to (regardless of widebus)...  Was that
patch doing anything for video mode at all?

It feels like the conditional introduced here is only making things more
confusing, but I don't have enough input to confirm what the video-mode
path should be doing in widebus mode (multiply BPC by the number of
components and by 2 in case of widebus?).
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index e8e83ee61eb0..168e37e38fc7 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1029,10 +1029,14 @@ static void dsi_timing_setup(struct msm_dsi_host 
*msm_host, bool is_bonded_dsi)
                 * unused anyway.
                 */
                h_total -= hdisplay;
-               if (wide_bus_enabled)
-                       bits_per_pclk = 
mipi_dsi_pixel_format_to_bpp(msm_host->format);
-               else
+               if (wide_bus_enabled) {
+                       if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO)
+                               bits_per_pclk = 
mipi_dsi_pixel_format_to_bpp(msm_host->format);
+                       else
+                               bits_per_pclk = 48;
+               } else {
                        bits_per_pclk = 24;
+               }
 
                hdisplay = 
DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc) * 8, bits_per_pclk);
 

---
base-commit: 3fa5e5702a82d259897bd7e209469bc06368bf31
change-id: 20260311-dsi-dsc-regresses-again-4be27dfc4001

Best regards,
-- 
Marijn Suijten <[email protected]>

Reply via email to