On Mon, Feb 09, 2026 at 07:58:32PM +0530, Borah, Chaitanya Kumar wrote:
> 
> 
> On 2/9/2026 7:08 PM, Imre Deak wrote:
> > The pipe BPP value shouldn't be set outside of the source's / sink's
> > valid pipe BPP range, ensure this when increasing the minimum pipe BPP
> > value to 30 due to HDR.
> > 
> > While at it debug print if the HDR mode was requested for a connector by
> > setting the corresponding HDR connector property. This indicates
> > if the requested HDR mode could not be enabled, since the selected
> > pipe BPP is below 30, due to a sink capability or link BW limit.
> > 
> > v2:
> > - Also handle the case where the sink could support the target 30 BPP
> >    only in DSC mode due to a BW limit, but the sink doesn't support DSC
> >    or 30 BPP as a DSC input BPP. (Chaitanya)
> > - Debug print the connector's HDR mode in the link config dump, to
> >    indicate if a BPP >= 30 required by HDR couldn't be reached. (Ankit)
> > - Add Closes: trailer. (Ankit)
> > - Don't print the 30 BPP-outside of valid BPP range debug message if
> >    the min BPP is already > 30 (and so a target BPP >= 30 required
> >    for HDR is ensured).
> 
> Does this match the current implementation?

If you mean the logic that, after the point where the min/max pipe BPP
based on the source/sink caps has been determined, constrains when the
minimum pipe BPP is increased due to the requested HDR, then yes.

> > 
> > Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/7052
> 
> Also,
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15503

Ok, can add this as well, while applying the patch.

> > Fixes: ba49a4643cf53 ("drm/i915/dp: Set min_bpp limit to 30 in HDR mode")
> > Cc: Chaitanya Kumar Borah <[email protected]>
> > Cc: <[email protected]> # v6.18+
> > Reviewed-by: Ankit Nautiyal <[email protected]> # v1
> > Signed-off-by: Imre Deak <[email protected]>
> > ---
> >   drivers/gpu/drm/i915/display/intel_dp.c | 20 +++++++++++++++++---
> >   1 file changed, 17 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 4b786706ea2de..7fd20df10f26f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -2703,6 +2703,7 @@ intel_dp_compute_config_limits(struct intel_dp 
> > *intel_dp,
> >                            bool dsc,
> >                            struct link_config_limits *limits)
> >   {
> > +   struct intel_display *display = to_intel_display(intel_dp);
> >     bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> >     struct intel_connector *connector =
> >             to_intel_connector(conn_state->connector);
> > @@ -2715,8 +2716,7 @@ intel_dp_compute_config_limits(struct intel_dp 
> > *intel_dp,
> >     limits->min_lane_count = intel_dp_min_lane_count(intel_dp);
> >     limits->max_lane_count = intel_dp_max_lane_count(intel_dp);
> > -   limits->pipe.min_bpp = intel_dp_in_hdr_mode(conn_state) ? 30 :
> > -                           intel_dp_min_bpp(crtc_state->output_format);
> > +   limits->pipe.min_bpp = intel_dp_min_bpp(crtc_state->output_format);
> >     if (is_mst) {
> >             /*
> >              * FIXME: If all the streams can't fit into the link with their
> > @@ -2732,6 +2732,19 @@ intel_dp_compute_config_limits(struct intel_dp 
> > *intel_dp,
> >                                                     
> > respect_downstream_limits);
> >     }
> > +   if (!dsc && intel_dp_in_hdr_mode(conn_state)) {
> > +           if (intel_dp_supports_dsc(intel_dp, connector, crtc_state) &&
> > +               limits->pipe.max_bpp >= 30)
> > +                   limits->pipe.min_bpp = max(limits->pipe.min_bpp, 30);
> > +           else
> > +                   drm_dbg_kms(display->drm,
> > +                               "[CONNECTOR:%d:%s] Can't force 30 bpp for 
> > HDR (pipe bpp: %d-%d DSC-support: %s)\n",
> > +                               connector->base.base.id, 
> > connector->base.name,
> > +                               limits->pipe.min_bpp, limits->pipe.max_bpp,
> > +                               str_yes_no(intel_dp_supports_dsc(intel_dp, 
> > connector,
> > +                                                                
> > crtc_state)));
> > +   }
> > +
> 
> Nit: We could collect the output of intel_dp_supports_dsc() in a variable.

It's also an extra step to go back and check what that variable exactly
contains. I prefer a function call for that reason, unless using a
variable is unavoidable and the code stays readable.

> But not a blocker.
> 
> Reviewed-by: Chaitanya Kumar Borah <[email protected]>

Thanks.

> 
> >     if (dsc && !intel_dp_dsc_compute_pipe_bpp_limits(connector, limits))
> >             return false;
> > @@ -2869,10 +2882,11 @@ intel_dp_compute_link_for_joined_pipes(struct 
> > intel_encoder *encoder,
> >     }
> >     drm_dbg_kms(display->drm,
> > -               "DP lane count %d clock %d bpp input %d compressed " 
> > FXP_Q4_FMT " link rate required %d available %d\n",
> > +               "DP lane count %d clock %d bpp input %d compressed " 
> > FXP_Q4_FMT " HDR %s link rate required %d available %d\n",
> >                 pipe_config->lane_count, pipe_config->port_clock,
> >                 pipe_config->pipe_bpp,
> >                 FXP_Q4_ARGS(pipe_config->dsc.compressed_bpp_x16),
> > +               str_yes_no(intel_dp_in_hdr_mode(conn_state)),
> >                 intel_dp_config_required_rate(pipe_config),
> >                 intel_dp_max_link_data_rate(intel_dp,
> >                                             pipe_config->port_clock,
> 

Reply via email to