On 2/11/2026 5:28 PM, Imre Deak wrote:
The user's request for a maximum BPC - via the max-bpc connector
property - determines the pipe BPP selected by the encoder, which is in
turn used either as the uncompressed output BPP or as the input BPP for
the DSC engine. This user-requested BPC->BPP can be outside of the
source/sink's supported valid min/max pipe BPP range and atm such an
out-of-bound request will be rejected by the encoder's state
computation.

As opposed to the above, the semantic for the max-bpc connector property
- which the user may reasonably expect - is not to fail the modeset in
case of an out-of-bound max BPC request, rather to adjust the request
clamping it to the valid BPP range.

Based on the above, calculate the baseline (i.e. the non-DP specific
platform/EDID) _maximum_ pipe BPP, storing it in
intel_crtc_state::max_pipe_bpp, separately from the baseline _target_
pipe BPP (which is the lower BPP of the baseline maximum and requested
BPP, stored in intel_crtc_state::pipe_bpp). This allows the encoder
state computation to use the baseline maximum pipe BPP as a hard limit
for the selected pipe BPP, while also letting it use the baseline target
pipe BPP only as a preference, clamping this target BPP to the valid
DP pipe BPP range.

Signed-off-by: Imre Deak <[email protected]>
---
  drivers/gpu/drm/i915/display/intel_display.c  | 12 ++++++++++
  .../drm/i915/display/intel_display_types.h    |  1 +
  drivers/gpu/drm/i915/display/intel_dp.c       | 24 +++++++++++++++++--
  3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index ab4b59916d2e7..dae7a7d11cb84 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4374,12 +4374,24 @@ compute_sink_pipe_bpp(const struct drm_connector_state 
*conn_state,
        struct intel_display *display = to_intel_display(crtc_state);
        struct drm_connector *connector = conn_state->connector;
        const struct drm_display_info *info = &connector->display_info;
+       int edid_bpc = info->bpc ? : 8;
        int target_pipe_bpp;
+       int max_edid_bpp;
+
+       max_edid_bpp = bpc_to_bpp(edid_bpc);
+       if (max_edid_bpp < 0)
+               return max_edid_bpp;
target_pipe_bpp = bpc_to_bpp(conn_state->max_bpc);
        if (target_pipe_bpp < 0)
                return target_pipe_bpp;
+ /*
+        * The maximum pipe BPP is the minimum of the max platform BPP and
+        * the max EDID BPP.
+        */
+       crtc_state->max_pipe_bpp = min(crtc_state->pipe_bpp, max_edid_bpp);

The function compute_sink_pipe_bpp() is actually just limiting the pipe_bpp to sink max limits. Instead of filling the crtc_state->max_pipe_bpp in this function itself, can we have a separate function only to set the crtc_state->max_pipe_bpp from the edid max:
compute_max_pipe_bpp(const struct drm_connector_state *conn_state,
                     struct intel_crtc_state *crtc_state)

This can be called separately from compute_baseline_pipe_bpp:

ret = compute_max_pipe_bpp(connector_state, crtc_state);
if (ret)
    return ret;

ret = compute_sink_pipe_bpp(connector_state, crtc_state);
...

This way the compute_sink_pipe_bpp will only do one thing(adjusting the pipe_bpp to sink limits).

˚oO(Perhaps we should name it to adjust_pipe_bpp_for_sink()).


+
        if (target_pipe_bpp < crtc_state->pipe_bpp) {
                drm_dbg_kms(display->drm,
                            "[CONNECTOR:%d:%s] Limiting target display pipe bpp to 
%d "
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index e6298279dc892..e8e4af03a6a6c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1163,6 +1163,7 @@ struct intel_crtc_state {
        } dsi_pll;
int max_link_bpp_x16; /* in 1/16 bpp units */
+       int max_pipe_bpp;       /* in 1 bpp units */
        int pipe_bpp;           /* in 1 bpp units */
        int min_hblank;
        struct intel_link_m_n dp_m_n;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 48845899298e4..4018b0122e8e0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1769,7 +1769,7 @@ static int intel_dp_max_bpp(struct intel_dp *intel_dp,
        struct intel_connector *connector = intel_dp->attached_connector;
        int bpp, bpc;
- bpc = crtc_state->pipe_bpp / 3;
+       bpc = crtc_state->max_pipe_bpp / 3;
if (intel_dp->dfp.max_bpc)
                bpc = min_t(int, bpc, intel_dp->dfp.max_bpc);
@@ -2726,7 +2726,7 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp,
                 * previously. This hack should be removed once we have the
                 * proper retry logic in place.
                 */
-               limits->pipe.max_bpp = min(crtc_state->pipe_bpp, 24);
+               limits->pipe.max_bpp = min(crtc_state->max_pipe_bpp, 24);
        } else {
                limits->pipe.max_bpp = intel_dp_max_bpp(intel_dp, crtc_state,
                                                        
respect_downstream_limits);
@@ -2757,6 +2757,26 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp,
        if (dsc && !intel_dp_dsc_compute_pipe_bpp_limits(connector, limits))
                return false;
+ /*
+        * crtc_state->pipe_bpp is the non-DP specific baseline (platform/EDID)
+        * maximum pipe BPP limited by the max-BPC connector property request.
+        * Since by now pipe.max_bpp is <= the above baseline BPP, the only

Hmm I think I am missing something. Till now we have set pipe.max_bpp using crtc_state->max_pipe_bpp.

This is set using min of max platform Bpp and the max edid bpp (and DP dfp considerations, DSC sink input bpp cap).

So the relationship between pipe.max_bpp and the crtc_state->max_pipe_bpp is not yet clear.

I do agree with the rest of the below: we need to clamp the limits.pipe.max_bpp between crtc_state->pipe_bpp  and limit.pipe->min_bpp.


Regards,

Ankit

+        * remaining reason for adjusting pipe.max_bpp is the max-BPC connector
+        * property request. Adjust pipe.max_bpp to this request within the
+        * current valid pipe.min_bpp .. pipe.max_bpp range.
+        */
+       limits->pipe.max_bpp = clamp(crtc_state->pipe_bpp, limits->pipe.min_bpp,
+                                    limits->pipe.max_bpp);
+       if (dsc)
+               limits->pipe.max_bpp = align_max_sink_dsc_input_bpp(connector,
+                                                                   
limits->pipe.max_bpp);
+
+       if (limits->pipe.max_bpp != crtc_state->pipe_bpp)
+               drm_dbg_kms(display->drm,
+                           "[CONNECTOR:%d:%s] Adjusting requested max pipe bpp %d 
-> %d\n",
+                           connector->base.base.id, connector->base.name,
+                           crtc_state->pipe_bpp, limits->pipe.max_bpp);
+
        if (is_mst || intel_dp->use_max_params) {
                /*
                 * For MST we always configure max link bw - the spec doesn't

Reply via email to