[PATCH] drm/i915/dp: Add support for DP UHBR SST DSC

[Saarinen, Jani]

Hi, 

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Jani
> Nikula
> Sent: Tuesday, 4 February 2025 18.40
> To: Deak, Imre <imre.deak at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915/dp: Add support for DP UHBR SST DSC
> 
> On Tue, 04 Feb 2025, Imre Deak <imre.deak at intel.com> wrote:
> > On Mon, Feb 03, 2025 at 06:08:34PM +0200, Jani Nikula wrote:
> >> Drop the UHBR limitation from DP SST DSC, and handle SST DSC
> >> bandwidth computation for UHBR using
> intel_dp_mtp_tu_compute_config().
> >>
> >> Cc: Imre Deak <imre.deak at intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> >
> > Reviewed-by: Imre Deak <imre.deak at intel.com>
> >
> > With the DPT bpp and bpp_step fixes on the list, this seems to work on
> > a
> > DP2.0 dock on (SST) UHBR link/DSC mode.
> 
> \o/
> 
> Thanks for the review and testing!
Awesome, thanks Jani N and Imre! 

> >
Br,
Jani
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_dp.c | 35
> >> +++++++++++++++++++------
> >>  1 file changed, 27 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >> b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index cc6aba353c11..eb8f6806166c 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -1958,15 +1958,37 @@ static int dsc_compute_link_config(struct
> intel_dp *intel_dp,
> >>  		for (lane_count = limits->min_lane_count;
> >>  		     lane_count <= limits->max_lane_count;
> >>  		     lane_count <<= 1) {
> >> -			if (!is_bw_sufficient_for_dsc_config(dsc_bpp_x16,
> link_rate,
> >> -							     lane_count,
> adjusted_mode->clock,
> >> -							     pipe_config-
> >output_format,
> >> -							     timeslots))
> >> -				continue;
> >>
> >> +			/*
> >> +			 * FIXME: intel_dp_mtp_tu_compute_config()
> requires
> >> +			 * ->lane_count and ->port_clock set before we know
> >> +			 * they'll work. If we end up failing altogether,
> >> +			 * they'll remain in crtc state. This shouldn't matter,
> >> +			 * as we'd then bail out from compute config, but it's
> >> +			 * just ugly.
> >> +			 */
> >>  			pipe_config->lane_count = lane_count;
> >>  			pipe_config->port_clock = link_rate;
> >>
> >> +			if (drm_dp_is_uhbr_rate(link_rate)) {
> >> +				int ret;
> >> +
> >> +				ret =
> intel_dp_mtp_tu_compute_config(intel_dp,
> >> +
> pipe_config,
> >> +								     conn_state,
> >> +
> dsc_bpp_x16,
> >> +
> dsc_bpp_x16,
> >> +								     0, true);
> >> +				if (ret)
> >> +					continue;
> >> +			} else {
> >> +				if
> (!is_bw_sufficient_for_dsc_config(dsc_bpp_x16, link_rate,
> >> +								     lane_count,
> adjusted_mode->clock,
> >> +
> pipe_config->output_format,
> >> +								     timeslots))
> >> +					continue;
> >> +			}
> >> +
> >>  			return 0;
> >>  		}
> >>  	}
> >> @@ -2493,9 +2515,6 @@ intel_dp_compute_config_limits(struct intel_dp
> *intel_dp,
> >>  	limits->min_rate = intel_dp_min_link_rate(intel_dp);
> >>  	limits->max_rate = intel_dp_max_link_rate(intel_dp);
> >>
> >> -	/* FIXME 128b/132b SST+DSC support missing */
> >> -	if (!is_mst && dsc)
> >> -		limits->max_rate = min(limits->max_rate, 810000);
> >>  	limits->min_rate = min(limits->min_rate, limits->max_rate);
> >>
> >>  	limits->min_lane_count = intel_dp_min_lane_count(intel_dp);
> >> --
> >> 2.39.5
> >>
> 
> --
> Jani Nikula, Intel