Claude review: drm/i915/dp: Decode DSC max delta bpp from sink DPCD

Claude review: drm/i915/dp: Decode DSC max delta bpp from sink DPCD

[Claude Code Review Bot]

Patch Review

This adds `intel_dp_dsc_max_delta_bppx16()` and inserts it as a new fallback level in `intel_dp_dsc_max_sink_compressed_bppx16()` between the eDP absolute DPCD value and the spec-table defaults.

**Bug: Reserved 444 delta values produce wrong output.** The 444 mask (`0x1f`) yields a 5-bit field with range 0–31. The valid range check only covers 1–21:

```c
max_bpp_delta = dsc_dpcd[DP_DSC_MAX_BPP_DELTA - DP_DSC_SUPPORT] &
    DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK;
if (max_bpp_delta >= 1 && max_bpp_delta <= 21)
    max_bpp_delta =  max_bpp_delta + MIN_DSC_BPP_DELTA_444 - 1;
```

If a sink reports a reserved value (22–31), the raw encoded value passes through without decoding, and `max_bpp_delta << 4` returns a nonsensical bppx16 value (e.g., value 25 would give 400, meaning 25 bpp — which is a wrong decode). Reserved values should return 0 (unsupported) to fall through to the bpc-based defaults:

```c
if (max_bpp_delta >= 1 && max_bpp_delta <= 21)
    max_bpp_delta = max_bpp_delta + MIN_DSC_BPP_DELTA_444 - 1;
else
    max_bpp_delta = 0;
```

The 420 case doesn't have this issue since the 3-bit field (0xe0 >> 5) maxes at 7, and the check `>= 1 && <= 7` covers all non-zero values.

**Misleading function name.** `intel_dp_dsc_max_delta_bppx16` says "delta" but the function returns the *absolute* maximum compressed bpp (in x16 format), not a delta. The delta is an encoding detail internal to the function. Consider `intel_dp_dsc_sink_max_bppx16_from_delta` or similar.

**Variable type.** `max_bpp_delta` is declared as `int` but only holds non-negative values derived from masked byte reads. Using `u8` would be more precise and self-documenting.

**Missing `else` for default case value.** When the `default:` case in the switch is hit, the function does `MISSING_CASE()` and `return 0`, but if neither the RGB/444 nor 420 cases match and `output_format` is some unexpected value, the flow is correct. This is fine.

**Integration logic is correct.** The insertion into `intel_dp_dsc_max_sink_compressed_bppx16` creates a clean three-tier fallback:
1. eDP absolute DPCD (0x067/0x068)
2. Delta-based DPCD (0x06E) — new
3. Spec table defaults based on bpc — existing

This ordering is logical. The duplicated `if (max_bppx16) return max_bppx16;` pattern is slightly repetitive but functionally correct.

---
Generated by Claude Code Patch Reviewer

[PATCH 0/4] DSC max delta bpp support

[Nemesa Garg]

 Some sinks exposes DSC max bpp through delta based
DPCD fields. To support those sinks, add DP DPCD field
field and logic to decode the delta value in bppx16
format.

Nemesa Garg (4):
  drm/dp: Add DP_DSC_MAX_BPP_DELTA register
  drm/dp: Rename YCbCr420 bpp delta mask to native
  drm/dp: Add max bpp delta computation constants
  drm/i915/dp: Decode dsc max delta bpp from sink dpcd

 drivers/gpu/drm/i915/display/intel_dp.c | 41 +++++++++++++++++++++++--
 include/drm/display/drm_dp.h            | 12 ++++++--
 2 files changed, 49 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH 1/4] drm/dp: Add DP_DSC_MAX_BPP_DELTA register

[Nemesa Garg]

The dsc max bpp delta masks were incorrectly placed
under the DP_DSC_BITS_PER_PIXEL_INC(0x06F) register.
Move these under correct DP_DSC_MAX_BPP_DELTA(0x06E)
register.

v2: Separate patch for correcting register. [Ankit]

Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 include/drm/display/drm_dp.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index 829e4d98d61c..e65aafcccf99 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -354,9 +354,11 @@
 # define DP_DSC_20_PER_DP_DSC_SINK          (1 << 1)
 # define DP_DSC_24_PER_DP_DSC_SINK          (1 << 2)
 
-#define DP_DSC_BITS_PER_PIXEL_INC           0x06F
+#define DP_DSC_MAX_BPP_DELTA		    0x06E
 # define DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK 0x1f
 # define DP_DSC_RGB_YCbCr420_MAX_BPP_DELTA_MASK 0xe0
+
+#define DP_DSC_BITS_PER_PIXEL_INC           0x06F
 # define DP_DSC_BITS_PER_PIXEL_1_16         0x0
 # define DP_DSC_BITS_PER_PIXEL_1_8          0x1
 # define DP_DSC_BITS_PER_PIXEL_1_4          0x2
-- 
2.25.1

[PATCH 2/4] drm/dp: Rename YCbCr420 bpp delta mask to native

[Nemesa Garg]

Rename DP_DSC_RGB_YCbCr420_MAX_BPP_DELTA_MASK to
DP_DSC_NATIVE_YCbCr420_MAX_BPP_DELTA_MASK to align with
the DP DSC specification, where the field represents the
native YCbCr 4:2:0 format.

Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
---
 include/drm/display/drm_dp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index e65aafcccf99..dce290acf735 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -356,7 +356,7 @@
 
 #define DP_DSC_MAX_BPP_DELTA		    0x06E
 # define DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK 0x1f
-# define DP_DSC_RGB_YCbCr420_MAX_BPP_DELTA_MASK 0xe0
+# define DP_DSC_NATIVE_YCbCr420_MAX_BPP_DELTA_MASK 0xe0
 
 #define DP_DSC_BITS_PER_PIXEL_INC           0x06F
 # define DP_DSC_BITS_PER_PIXEL_1_16         0x0
-- 
2.25.1

[PATCH 3/4] drm/dp: Add max bpp delta computation constants

[Nemesa Garg]

Define macros used for decoding DSC max bpp delta values
from the sink DPCD. This includes per-format masks for
RGB/YCbCr444 and YCbCr420, as well as definitions for delta
scaling and the YCbCr420 bit shift. Also add version_1 as
suffix to MAX_DELTA_BPP.

v2: Move constants under 0x6E register. [Ankit]
    Add mask for Native 422 also. [Ankit]
v3: Rename _DSC_NATIVE4222 to _DSC_NATIVE_YCbCr422. [Ankit]

Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
---
 include/drm/display/drm_dp.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index dce290acf735..a905aa49d1d0 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -354,10 +354,14 @@
 # define DP_DSC_20_PER_DP_DSC_SINK          (1 << 1)
 # define DP_DSC_24_PER_DP_DSC_SINK          (1 << 2)
 
-#define DP_DSC_MAX_BPP_DELTA		    0x06E
+#define DP_DSC_MAX_BPP_DELTA_VERSION_1		0x06E
 # define DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK 0x1f
 # define DP_DSC_NATIVE_YCbCr420_MAX_BPP_DELTA_MASK 0xe0
 
+# define DP_DSC_BPP_DELTA_444			16
+# define DP_DSC_BPP_DELTA_420			12
+# define DP_DSC_BPP_DELTA_SHIFT_420		5
+
 #define DP_DSC_BITS_PER_PIXEL_INC           0x06F
 # define DP_DSC_BITS_PER_PIXEL_1_16         0x0
 # define DP_DSC_BITS_PER_PIXEL_1_8          0x1
@@ -365,6 +369,8 @@
 # define DP_DSC_BITS_PER_PIXEL_1_2          0x3
 # define DP_DSC_BITS_PER_PIXEL_1_1          0x4
 # define DP_DSC_BITS_PER_PIXEL_MASK         0x7
+# define DP_DSC_NATIVE_YCbCr422_MAX_BPP_DELTA_MASK  0x78
+# define DP_DSC_BPP_DELTA_NATIVE_422		16
 
 #define DP_PSR_SUPPORT                      0x070   /* XXX 1.2? */
 # define DP_PSR_IS_SUPPORTED                1
-- 
2.25.1

[PATCH 4/4] drm/i915/dp: Decode dsc max delta bpp from sink dpcd

[Nemesa Garg]

Add intel_dp_dsc_max_delta_bppx16() to parse sink dsc max
delta bpp from dpcd when DP_DSC_MAX_BPP_DELTA_AVAILABILITY
is set. This helper decodes the delta range for both RGB/YCbCr444
and YCbCr420 formats from DP_DSC_MAX_BPP_DELTA.

With this addition, the flow becomes:
1. First, check for a format-specific range and use it to calculate
max compressed bpp.
2. If not, check for sink supported max compressed bpp and
use that
3. If this is also not there go with mandatory
max range supported bpp.

v2: Reorder the check flow for max_bpp. [Ankit]
v3: Put RGB and YCbCr444 mask assignment in the same line. [Ankit]

Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 41 +++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 3f9123a53244..3b18cb4a64af 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2203,17 +2203,54 @@ static int dsc_compute_link_config(struct intel_dp *intel_dp,
 	return -EINVAL;
 }
 
+static u16 intel_dp_dsc_max_delta_bppx16(const struct intel_connector *connector,
+					 enum intel_output_format output_format)
+{
+	const u8 *dsc_dpcd = connector->dp.dsc_dpcd;
+	u8 max_bpp_delta_v1 = dsc_dpcd[DP_DSC_MAX_BPP_DELTA_VERSION_1 - DP_DSC_SUPPORT];
+	int max_bpp;
+
+	if (!(dsc_dpcd[DP_DSC_MAX_BITS_PER_PIXEL_HI - DP_DSC_SUPPORT] &
+	    DP_DSC_MAX_BPP_DELTA_AVAILABILITY))
+		return 0;
+
+	switch (output_format) {
+	case INTEL_OUTPUT_FORMAT_RGB:
+	case INTEL_OUTPUT_FORMAT_YCBCR444:
+		max_bpp =  max_bpp_delta_v1 & DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK;
+		if (max_bpp >= 1 && max_bpp <= 21)
+			max_bpp =  max_bpp + DP_DSC_BPP_DELTA_444 - 1;
+		break;
+	case INTEL_OUTPUT_FORMAT_YCBCR420:
+		max_bpp = (max_bpp_delta_v1 & DP_DSC_NATIVE_YCbCr420_MAX_BPP_DELTA_MASK) >>
+			  DP_DSC_BPP_DELTA_SHIFT_420;
+		if (max_bpp >= 1 && max_bpp <= 7)
+			max_bpp = max_bpp + DP_DSC_BPP_DELTA_420 - 1;
+		break;
+	default:
+		MISSING_CASE(output_format);
+		return 0;
+	}
+
+	return max_bpp << 4;
+}
+
 static
 u16 intel_dp_dsc_max_sink_compressed_bppx16(const struct intel_connector *connector,
 					    enum intel_output_format output_format,
 					    int bpc)
 {
-	u16 max_bppx16 = drm_edp_dsc_sink_output_bpp(connector->dp.dsc_dpcd);
+	u16 max_bppx16 = intel_dp_dsc_max_delta_bppx16(connector, output_format);
+
+	if (max_bppx16)
+		return max_bppx16;
+
+	max_bppx16 = drm_edp_dsc_sink_output_bpp(connector->dp.dsc_dpcd);
 
 	if (max_bppx16)
 		return max_bppx16;
 	/*
-	 * If support not given in DPCD 67h, 68h use the Maximum Allowed bit rate
+	 * If support not given in DPCD 67h, 68h, 6Eh, 6Fh use the Maximum Allowed bit rate
 	 * values as given in spec Table 2-157 DP v2.0
 	 */
 	switch (output_format) {
-- 
2.25.1

Claude review: DSC max delta bpp support

[Claude Code Review Bot]

Overall Series Review

Subject: DSC max delta bpp support
Author: Nemesa Garg <nemesa.garg@intel.com>
Patches: 5
Reviewed: 2026-05-28T12:24:07.251231

---

This 4-patch series adds support for decoding DSC maximum bpp from the delta-based DPCD field (register 0x06E) for DP sinks that expose this capability. The series is logically structured: fix the register placement in drm_dp.h, rename for spec alignment, add computation constants, then use them in intel_dp.c. The concept is sound and the patches are a reasonable size.

However, there are several issues worth flagging: the YCbCr422 mask is placed under the wrong register, there's a potential logic bug where an out-of-range delta value silently returns an uninitialized-looking result, and the naming of the new delta constants is inconsistent.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/dp: Add DP_DSC_MAX_BPP_DELTA register

[Claude Code Review Bot]

Patch Review

**Status: Looks correct**

This patch moves the `DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK` and `DP_DSC_RGB_YCbCr420_MAX_BPP_DELTA_MASK` definitions from under `DP_DSC_BITS_PER_PIXEL_INC` (0x06F) to a new `DP_DSC_MAX_BPP_DELTA` (0x06E) register heading. This is a straightforward and correct fix -- these masks apply to register 0x06E, not 0x06F.

No concerns.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/dp: Rename YCbCr420 bpp delta mask to native

[Claude Code Review Bot]

Patch Review

**Status: Looks correct**

Renames `DP_DSC_RGB_YCbCr420_MAX_BPP_DELTA_MASK` to `DP_DSC_NATIVE_YCbCr420_MAX_BPP_DELTA_MASK`. The old name `RGB_YCbCr420` was indeed misleading -- the 420 field describes native YCbCr 4:2:0, not RGB. No users of the old name remain in the tree.

No concerns.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/dp: Add max bpp delta computation constants

[Claude Code Review Bot]

Patch Review

**Issues found:**

1. **YCbCr422 mask placed under wrong register.** The commit message and version notes say the 422 mask was added "under 0x6E register," but the code places it under `DP_DSC_BITS_PER_PIXEL_INC` (0x06F):

   ```c
   #define DP_DSC_BITS_PER_PIXEL_INC           0x06F
   ...
   # define DP_DSC_NATIVE_YCbCr422_MAX_BPP_DELTA_MASK  0x78
   # define DP_DSC_BPP_DELTA_NATIVE_422		16
   ```

   If this mask really belongs to register 0x06E (as the commit message implies), it should be grouped with the other masks under `DP_DSC_MAX_BPP_DELTA_VERSION_1`. If it genuinely belongs to 0x06F (bits [6:3] of the BPP increment register), then the commit message is wrong. Either way, something needs to be reconciled with the DP spec.

2. **Inconsistent naming convention.** The 444/420 constants are named `DP_DSC_BPP_DELTA_444` and `DP_DSC_BPP_DELTA_420`, but the 422 constant is `DP_DSC_BPP_DELTA_NATIVE_422`. The naming should be consistent -- either all include `NATIVE` or none do.

3. **Unused definitions.** `DP_DSC_NATIVE_YCbCr422_MAX_BPP_DELTA_MASK` and `DP_DSC_BPP_DELTA_NATIVE_422` are defined but not used anywhere in the series. If YCbCr422 support isn't being added yet, it may be better to defer these definitions to when they're actually needed, or at minimum note in the commit message that they're being added for future use.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/i915/dp: Decode dsc max delta bpp from sink dpcd

[Claude Code Review Bot]

Patch Review

**Issues found:**

1. **Out-of-range delta value produces garbage.** When the delta value falls outside the valid range, `max_bpp` retains its raw DPCD field value (unscaled), which then gets shifted left by 4 and returned:

   ```c
   case INTEL_OUTPUT_FORMAT_RGB:
   case INTEL_OUTPUT_FORMAT_YCBCR444:
       max_bpp =  max_bpp_delta_v1 & DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK;
       if (max_bpp >= 1 && max_bpp <= 21)
           max_bpp =  max_bpp + DP_DSC_BPP_DELTA_444 - 1;
       break;
   ```

   If `max_bpp` is 0, the function returns 0 (fine, interpreted as "not available"). But if `max_bpp` is > 21 (values 22-31 for the 5-bit field), the raw field value is returned as a bpp, which is nonsensical. The same applies to the 420 case when `max_bpp` > 7. The function should either return 0 for reserved/invalid values, or clamp. Returning the raw DPCD field value as a bpp is almost certainly wrong.

2. **Superfluous double spaces.** Minor style nit: there are double spaces in two places:

   ```c
   max_bpp =  max_bpp_delta_v1 & DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK;
   max_bpp =  max_bpp + DP_DSC_BPP_DELTA_444 - 1;
   ```

3. **No handling for YCBCR422.** The function handles RGB, YCbCr444, and YCbCr420 but not YCbCr422, even though patch 3 added the 422 mask and delta constant. The `default: MISSING_CASE()` path will fire if `INTEL_OUTPUT_FORMAT_YCBCR422` is ever passed. This is arguably fine for now (if 422 isn't used in this code path), but is worth noting since the header constants were clearly added with 422 in mind.

4. **The delta-to-bpp conversion semantics should be documented.** The formula `max_bpp = raw_delta + base - 1` is non-obvious. A brief comment explaining what the delta values represent per the DP spec (e.g., "delta of N maps to max bpp of N + 15 for 444") would help future readers understand the magic numbers 16 and 12.

5. **Commit message mentions `DP_DSC_MAX_BPP_DELTA` but the code uses `DP_DSC_MAX_BPP_DELTA_VERSION_1`.** The commit message references the old unversioned name. Minor, but should be consistent.

---

**Summary:** The main issues are the out-of-range delta value bug in patch 4 (which can return a nonsensical bpp value), and the misplacement of the YCbCr422 mask in patch 3 (placed under the wrong register). Both should be fixed before merging.

---
Generated by Claude Code Patch Reviewer