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

[PATCH RFC] 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>
---
This patch changes the data_width calculation to match downstream,
and avoids needing to change the divisor for timing->width to /24.

However I am not sure if this is a correct approach, but at least
with this I get the same values for data_width as downstream now:

data_width w/ no widebus
before: 288
after: 360

data_with w/ widebus
before: 144
after: 180

This was tested with the BOE BF068MWM-TD0 on the Nothing Phone (3a),
it would be greatly appreciated if someone from QCOM could weigh in
on if this is a correct approach.

Tested with kmscube and getting 120FPS, also note that this was
tested with a few other changes:

(drm/msm/dsi: fix hdisplay calculation when programming dsi registers)
(drm/msm/dsi: fix pclk rate calculation for bonded dsi)
(drm/msm/dsi: fix bits_per_pclk)
(drm/msm/dsi: fix hdisplay calculation for CMD mode panel)
---
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c        | 23 +++++++++++++++++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h        |  1 +
 3 files changed, 21 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..de6b5b0f70c4 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,26 @@ 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.
 	 */
-	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 RFC] drm/msm/dpu: calculate data_width like downstream

[Dmitry Baryshkov]

On Sun, Mar 22, 2026 at 02:31:13PM +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>
> ---
> This patch changes the data_width calculation to match downstream,
> and avoids needing to change the divisor for timing->width to /24.
> 
> However I am not sure if this is a correct approach, but at least
> with this I get the same values for data_width as downstream now:
> 
> data_width w/ no widebus
> before: 288
> after: 360
> 
> data_with w/ widebus
> before: 144
> after: 180
> 
> This was tested with the BOE BF068MWM-TD0 on the Nothing Phone (3a),
> it would be greatly appreciated if someone from QCOM could weigh in
> on if this is a correct approach.
> 
> Tested with kmscube and getting 120FPS, also note that this was
> tested with a few other changes:
> 
> (drm/msm/dsi: fix hdisplay calculation when programming dsi registers)

This is a part of msm-fixes

> (drm/msm/dsi: fix pclk rate calculation for bonded dsi)

This is a part of msm-fixes

> (drm/msm/dsi: fix bits_per_pclk)
> (drm/msm/dsi: fix hdisplay calculation for CMD mode panel)

Okay, let's pick these two. I assume we've had enough of testing. I hope
Marijn can also r-b those.

> ---
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  2 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c        | 23 +++++++++++++++++-----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h        |  1 +
>  3 files changed, 21 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..de6b5b0f70c4 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,26 @@ 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()

Please keep the DP comment. We need to fix the assymmetry between DSI
and DP, but that needs DP DSC support first.


With that in place, the patch LGTM.

> +	 * 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.
>  	 */
> -	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);

I was concerned about DPU < 5.0, but data_width is only used for
HCTL-related timings, which means DPU 5.0+. I'll look at restructuring
the code.

> +	} 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>
> 
> 

-- 
With best wishes
Dmitry

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: 2
Reviewed: 2026-03-25T08:16:31.541245

---

This is a single RFC patch that changes how `data_width` is calculated for the INTF timing engine when DSC compression is enabled on DSI. Instead of deriving `data_width` from the already-compressed `p->width` (which was divided by the compression ratio), it uses `dce_bytes_per_line` (= `slice_count * slice_chunk_size`) divided by 3 or 6 depending on widebus.

The approach is reasonable and aligns with downstream, but the patch has some issues worth noting.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

**Concept is sound but needs refinement:**

The core idea — using `dce_bytes_per_line` to compute `data_width` independently from the compressed `timing->width` — is a valid approach. The existing code relies on `p->width` which was already integer-divided by the compression ratio (potentially losing precision), then optionally halved for widebus. Using the byte-level calculation with `DIV_ROUND_UP` avoids that compound rounding error.

**Issue 1: Missing `dsc` pointer in `drm_mode_to_intf_timing_params`**

The patch adds:
```c
timing->dce_bytes_per_line = msm_dsc_get_bytes_per_line(dsc);
```

But looking at the existing code at `dpu_encoder_phys_vid.c:130-131`, the `dsc` pointer is a local variable obtained via `dpu_encoder_get_dsc_config()` inside the `if (compression_en)` block. The patch places the new line at line 139 (after `timing->xres = timing->width`), which is within scope, so this is fine. However, the diff context shows it placed correctly inside the existing `if` block.

**Issue 2: Magic divisors 3 and 6 need explanation**

```c
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);
```

The divisor `3` represents 3 bytes per pixel (24bpp), and `6` is `3 * 2` (widebus doubles the data rate). These magic numbers should have a brief comment or use named constants. Compare with `dsi_host.c:1033` which uses `bytes_per_pclk = 3` / `bytes_per_pclk = 6` — the same pattern but at least assigned to a named variable.

**Issue 3: Interaction with the existing `timing->width` compression adjustment**

The existing code at `dpu_encoder_phys_vid.c:136-138` still compresses `timing->width` using integer division:
```c
timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
                (dsc->bits_per_component * 3);
```

This compressed `p->width` is still used to initialize `data_width` at `dpu_hw_intf.c:173`:
```c
data_width = p->width;
```

...before being overridden by the new `dce_bytes_per_line` calculation. This works but is a bit misleading — the initial assignment is immediately dead for the compression case. A comment noting this would help readability.

**Issue 4: DP case handling inconsistency**

The new code checks `!dp_intf` for the compression case:
```c
if (p->compression_en && !dp_intf) {
```

This is consistent with the existing comment `/* TODO: handle DSC+DP case */` and matches the guard on the existing `wide_bus_en` check. Good.

**Issue 5: The `dce_bytes_per_line` field is only set for video mode**

The field is set in `drm_mode_to_intf_timing_params()` in `dpu_encoder_phys_vid.c`, but command mode uses a different path (`dpu_encoder_phys_cmd.c`). If command mode ever needs this calculation, the field won't be populated. This is acceptable for an RFC since the existing compression path is also only in the vid encoder, but worth noting.

**Issue 6: Values sanity check**

From the cover letter: for a panel with `dce_bytes_per_line`, the values shift from 288→360 (no widebus) and 144→180 (widebus). The ratio 360/288 = 1.25 and 180/144 = 1.25. This means the old integer-division formula was under-counting by 20%, which is a significant error that could cause display artifacts or underflow. The fix seems directionally correct.

**Minor: RFC status**

The author explicitly marks this as RFC and asks for Qualcomm feedback, which is appropriate given the hardware-specific nature of the timing calculation. The patch depends on other unmerged DSI fixes listed in the cover letter, which should be noted as dependencies.

---
Generated by Claude Code Patch Reviewer