On Thu, Feb 12, 2026 at 09:44:15AM +0530, Nautiyal, Ankit K wrote:
>
> On 2/11/2026 10:42 PM, Imre Deak wrote:
> > On Wed, Feb 11, 2026 at 09:36:55PM +0530, Nautiyal, Ankit K wrote:
> > > 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.
> > It limits the platform maximum pipe BPP passed to the function via
> > intel_crtc_state::pipe_bpp to the maximum EDID BPP _and_ the connector's
> > requested max-bpc x 3.
>
> Yes right thats true.
>
>
> >
> > > 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()).
> > Not sure. It's also good to see in one place how the debug-printed
> > max/target/edid BPPs are calculated.
>
> Hmm yes indeed make sense.
>
>
> >
> > In any case compute_sink_pipe_bpp() did compute both the target and max
> > BPPs implicitly even before, combining these to
> > intel_crtc_state::pipe_bpp, and this patch didn't change that. If
> > separate functions make more sense that should be a separate follow-up
> > change imo.
>
> You can ignore the comment. It seems better to have ctrc_state->max_pipe_bpp
> set and the relevant debug message in same place for now.
>
>
> >
> > > > +
> > > > 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.
> > Yes and then also reducing pipe.max_bpp further by every other source
> > and sink maximum BPP limits.
> >
> > > 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
>
>
> sorry I meant about pipe.max_bpp and the baseline BPP i.e.
> crtc_state->pipe_bpp (not crtc_state->max_pip_bpp).
>
>
> > > not yet clear.
> > The relationship is:
> >
> > crtc_state->max_pipe_bpp = min(platform_max_bpp, sink_edid_max_bpp);
> > limits->pipe.max_bpp = min(crtc_state->max_pipe_bpp,
> > dfp_max_bpp, vbt_edp_max_bpp,
> > sink_dsc_input_max_bpp,
> > src_dsc_input_max_bpp,
> > mst_max_24bpp);
> >
> > > 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.
> > crtc_state->pipe_bpp = min(crtc_state->max_pipe_bpp, requested_bpc * 3);
>
> Sorry, I am unable to find this line..
This line is in compute_sink_pipe_bpp(). So at this point in the code
crtc_state->pipe_bpp has the above min() value.
> perhaps you mean that we intend to
> make sure that crtc_state->pipe_bpp is in the range
> (crtc_state->max_pipe_bpp, requested_bpc * 3)?
No. crtc_state->pipe_bpp at this point in the code has exactly the above
min() value.
> OR, is there any place we are using crtc_state->max_pipe_bpp to set
> crtc_state->pipe_bpp, till this point?
Yes, in compute_sink_pipe_bpp().
> So, I am still not getting the line:
>
> "Since by now pipe.max_bpp is <= the above baseline BPP"
limits->pipe.max_bpp at this point in the code is <= the baseline
_maximum_ BPP that is crtc_state->max_pipe_bpp.
> Regards,
>
> Ankit
>
>
> >
> > and so
> >
> > limits->pipe.max_bpp should be set to crtc_state->pipe_bpp clamped
> > between limits->pipe.min_bpp and limits->pipe.max_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
