On Mon, 2025-04-28 at 16:31 +0300, Imre Deak wrote: > Atm TGL supports only a fixed set of valid DSC compressed bpps > (6,8,10,12,15), but this is not taken into account while looking for a > bpp in the minimum..maximum compressed bpp range. > > This happened to work only by chance since atm from the above min..max > range it's always the maximum bpp that is selected, which is one of the > above valid bpps (see mst_stream_dsc_compute_link_config() -> > intel_dp_dsc_nearest_valid_bpp()). Before selecting a bpp however, the > bpp's BW requirement should be checked wrt. to the MST total link BW; > after doing that - in a follow-up change - the validity of any bpp in > the min..max range must be ensured before the bpp is selected, do that > here. > > Cc: Jani Nikula <[email protected]> > Signed-off-by: Imre Deak <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 9 ++++++--- > drivers/gpu/drm/i915/display/intel_dp.h | 1 + > drivers/gpu/drm/i915/display/intel_dp_mst.c | 6 ++++++ > 3 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index d63aea7ee9c80..5c206faadf93a 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -2110,8 +2110,11 @@ static int intel_dp_dsc_bpp_step_x16(const struct > intel_connector *connector) > return fxp_q4_from_int(1) / incr; > } > > -/* Note: This is not universally usable! */ > -static bool intel_dp_dsc_valid_bpp(struct intel_dp *intel_dp, int bpp_x16) > +/* > + * Note: for bpp_x16 to be valid it must be also within the source/sink's > + * min..max bpp capability range. > + */ > +bool intel_dp_dsc_valid_compressed_bpp(struct intel_dp *intel_dp, int > bpp_x16) > { > struct intel_display *display = to_intel_display(intel_dp); > int i; > @@ -2175,7 +2178,7 @@ static int dsc_compute_compressed_bpp(struct intel_dp > *intel_dp, > max_bpp_x16 = min(max_bpp_x16, fxp_q4_from_int(output_bpp) - > bpp_step_x16); > > for (bpp_x16 = max_bpp_x16; bpp_x16 >= min_bpp_x16; bpp_x16 -= > bpp_step_x16) { > - if (!intel_dp_dsc_valid_bpp(intel_dp, bpp_x16)) > + if (!intel_dp_dsc_valid_compressed_bpp(intel_dp, bpp_x16)) > continue; > > ret = dsc_compute_link_config(intel_dp, > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h > b/drivers/gpu/drm/i915/display/intel_dp.h > index 98f90955fdb1d..a9dd9ed1afc9d 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.h > +++ b/drivers/gpu/drm/i915/display/intel_dp.h > @@ -147,6 +147,7 @@ int intel_dp_dsc_sink_min_compressed_bpp(const struct > intel_crtc_state *pipe_con > int intel_dp_dsc_sink_max_compressed_bpp(const struct intel_connector > *connector, > const struct intel_crtc_state > *pipe_config, > int bpc); > +bool intel_dp_dsc_valid_compressed_bpp(struct intel_dp *intel_dp, int > bpp_x16); > u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector, > int mode_clock, int mode_hdisplay, > int num_joined_pipes); > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index d8033e55dc093..8e1ed3b38217d 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -330,6 +330,12 @@ int intel_dp_mtp_tu_compute_config(struct intel_dp > *intel_dp, > > drm_dbg_kms(display->drm, "Trying bpp " FXP_Q4_FMT "\n", > FXP_Q4_ARGS(bpp_x16)); > > + if (dsc && !intel_dp_dsc_valid_compressed_bpp(intel_dp, > bpp_x16)) { > + /* SST must have validated the single bpp tried here > already earlier. */ > + drm_WARN_ON(display->drm, !is_mst); > + continue; > + } > + > link_bpp_x16 = dsc ? bpp_x16 : > > fxp_q4_from_int(intel_dp_output_bpp(crtc_state->output_format, > > fxp_q4_to_int(bpp_x16)));
Reviewed-by: Luca Coelho <[email protected]> -- Cheers, Luca.
