[PATCH v2] drm/msm/dpu: calculate data_width like downstream

[PATCH v2] drm/msm/dpu: calculate data_width like downstream

[Alexander Koskovich]

Derive INTF data_width from dce_bytes_per_line rather than
timing->width when DSC is enabled. Use DIV_ROUND_UP to avoid
rounding errors.

Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
Changes in v2:
- Added back comment about DSC & DP
- Link to v1: https://lore.kernel.org/r/20260322-fix-data-width-calc-v1-1-128880f5a067@pm.me
---
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c        | 26 +++++++++++++++++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h        |  1 +
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 0ba777bda253..ba810f26ea30 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -10,6 +10,7 @@
 #include "dpu_formats.h"
 #include "dpu_trace.h"
 #include "disp/msm_disp_snapshot.h"
+#include "msm_dsc_helper.h"
 
 #include <drm/display/drm_dsc_helper.h>
 #include <drm/drm_managed.h>
@@ -136,6 +137,7 @@ static void drm_mode_to_intf_timing_params(
 		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
 				(dsc->bits_per_component * 3);
 		timing->xres = timing->width;
+		timing->dce_bytes_per_line = msm_dsc_get_bytes_per_line(dsc);
 	}
 }
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 7e620f590984..ac82b69aedf6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -173,13 +173,29 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *intf,
 	data_width = p->width;
 
 	/*
-	 * If widebus is enabled, data is valid for only half the active window
-	 * since the data rate is doubled in this mode. But for the compression
-	 * mode in DP case, the p->width is already adjusted in
-	 * drm_mode_to_intf_timing_params()
+	 * If widebus is disabled:
+	 * For uncompressed stream, the data is valid for the entire active
+	 * window period.
+	 * For compressed stream, data is valid for a shorter time period
+	 * inside the active window depending on the compression ratio.
+	 *
+	 * If widebus is enabled:
+	 * For uncompressed stream, data is valid for only half the active
+	 * window, since the data rate is doubled in this mode.
+	 * For compressed stream, data validity window needs to be adjusted for
+	 * compression ratio and then further halved.
+	 *
+	 * For the compression mode in DP case, the p->width is already
+	 * adjusted in drm_mode_to_intf_timing_params().
 	 */
-	if (p->wide_bus_en && !dp_intf)
+	if (p->compression_en && !dp_intf) {
+		if (p->wide_bus_en)
+			data_width = DIV_ROUND_UP(p->dce_bytes_per_line, 6);
+		else
+			data_width = DIV_ROUND_UP(p->dce_bytes_per_line, 3);
+	} else if (p->wide_bus_en && !dp_intf) {
 		data_width = p->width >> 1;
+	}
 
 	/* TODO: handle DSC+DP case, we only handle DSC+DSI case so far */
 	if (p->compression_en && !dp_intf &&
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
index f6ef2c21b66d..badd26305fc9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -35,6 +35,7 @@ struct dpu_hw_intf_timing_params {
 
 	bool wide_bus_en;
 	bool compression_en;
+	u32 dce_bytes_per_line;
 };
 
 struct dpu_hw_intf_prog_fetch {

---
base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
change-id: 20260322-fix-data-width-calc-c44287df08b8

Best regards,
-- 
Alexander Koskovich <akoskovich@pm.me>

Re: [PATCH v2] drm/msm/dpu: calculate data_width like downstream

[Dmitry Baryshkov]

On Sun, Mar 22, 2026 at 06:48:09PM +0000, Alexander Koskovich wrote:
> Derive INTF data_width from dce_bytes_per_line rather than
> timing->width when DSC is enabled. Use DIV_ROUND_UP to avoid
> rounding errors.
> 
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
> Changes in v2:
> - Added back comment about DSC & DP
> - Link to v1: https://lore.kernel.org/r/20260322-fix-data-width-calc-v1-1-128880f5a067@pm.me
> ---
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  2 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c        | 26 +++++++++++++++++-----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h        |  1 +
>  3 files changed, 24 insertions(+), 5 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/msm/dpu: calculate data_width like downstream

[Pengyu Luo]

On Mon, Mar 23, 2026 at 2:48 AM Alexander Koskovich <akoskovich@pm.me> wrote:
>
> Derive INTF data_width from dce_bytes_per_line rather than
> timing->width when DSC is enabled. Use DIV_ROUND_UP to avoid
> rounding errors.
>
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---

Tested-by: Pengyu Luo <mitltlatltl@gmail.com> # Lenovo Legion Y700 Gen4 (SM8750)

Best wishes,
Pengyu

Claude review: drm/msm/dpu: calculate data_width like downstream

[Claude Code Review Bot]

Overall Series Review

Subject: drm/msm/dpu: calculate data_width like downstream
Author: Alexander Koskovich <akoskovich@pm.me>
Patches: 3
Reviewed: 2026-03-25T08:12:58.528087

---

This is a single-patch fix to how the DPU INTF timing engine calculates the `data_width` (the data validity window) when DSC compression is enabled on DSI interfaces. The current upstream code derives `data_width` from `p->width`, which was already adjusted by a simplified compression ratio formula (`width * bpp_int / (bpc * 3)`). The patch replaces this with a calculation based on `msm_dsc_get_bytes_per_line()` (i.e., `slice_count * slice_chunk_size`), matching downstream/vendor behavior. It uses `DIV_ROUND_UP` to avoid truncation errors.

The approach is reasonable: using the actual DSC slice geometry (`slice_count * slice_chunk_size`) is more accurate than the existing approximation, which uses `drm_dsc_get_bpp_int()` and loses the fractional BPP. The pattern also mirrors how `dsi_host.c` already computes `hdisplay` for DSC.

**Overall: The patch looks correct in concept but has one notable concern.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm/dpu: calculate data_width like downstream

[Claude Code Review Bot]

Patch Review

**Positive aspects:**
- Uses the established `msm_dsc_get_bytes_per_line()` helper already used in `dsi_host.c`, maintaining consistency.
- `DIV_ROUND_UP` is the right choice to avoid off-by-one truncation in the data width.
- The `dce_bytes_per_line` field is properly zero-initialized (struct uses `= { 0 }`) and only read when `compression_en` is set.
- The expanded comment block is a welcome improvement over the previous terse comment.

**Concern — widebus video mode divisor:**

In `dsi_host.c:1028-1035`, the widebus divisor for DSC depends on the mode:
```c
if (wide_bus_enabled && !(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO))
    bytes_per_pclk = 6;
else
    bytes_per_pclk = 3;
```

For **video mode** with widebus, DSI host uses `bytes_per_pclk = 3` (not 6). But this patch (which is in `dpu_encoder_phys_vid.c`, i.e., video mode only) divides by 6 when widebus is enabled:

```c
if (p->wide_bus_en)
    data_width = DIV_ROUND_UP(p->dce_bytes_per_line, 6);
```

These are admittedly computing different things (DSI packet timing vs. INTF data validity window), so this may be intentional and correct for the INTF hardware — but the reviewer should verify that the INTF timing engine truly expects the `data_width` to be halved for widebus even in video mode with DSC. It would be worth having a comment explaining why the INTF widebus behavior differs from the DSI host here, or citing the downstream source/hardware documentation that confirms the divide-by-6.

**Minor — commit message could be more specific:**

The commit message says "calculate data_width like downstream" but doesn't explain *what was wrong* with the existing calculation. A sentence like "The existing formula using `drm_dsc_get_bpp_int()` loses fractional BPP precision and doesn't account for actual slice geometry" would help reviewers understand the motivation.

**Nit — the "TODO: handle DSC+DP" comment is retained but could be updated:**

The comment at line 184 (`/* TODO: handle DSC+DP case... */`) still refers to the old behavior. Since this patch explicitly skips DP (`!dp_intf`) in the new compression branch, the TODO is still valid, but it might be worth noting in the commit message that DSC+DP remains unhandled.

**Summary:** The patch is a logical improvement, aligning the INTF data width calculation with the downstream approach using actual DSC byte counts. The main point to verify is whether the divide-by-6 for widebus video mode is correct for the INTF timing engine, given that the DSI host uses divide-by-3 for the same scenario.

---
Generated by Claude Code Patch Reviewer