| Message ID | 20250203160834.2708027-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/dp: Add support for DP UHBR SST DSC | expand |
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@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Imre Deak <imre.deak@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. > --- > 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 >
On Tue, 04 Feb 2025, Imre Deak <imre.deak@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@intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > Reviewed-by: Imre Deak <imre.deak@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! > >> --- >> 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 >>
Hi, > -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani > Nikula > Sent: Tuesday, 4 February 2025 18.40 > To: Deak, Imre <imre.deak@intel.com> > Cc: intel-gfx@lists.freedesktop.org; intel-xe@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@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@intel.com> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > > > Reviewed-by: Imre Deak <imre.deak@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
On Tue, 04 Feb 2025, "Saarinen, Jani" <jani.saarinen@intel.com> wrote: > Hi, > >> -----Original Message----- >> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani >> Nikula >> Sent: Tuesday, 4 February 2025 18.40 >> To: Deak, Imre <imre.deak@intel.com> >> Cc: intel-gfx@lists.freedesktop.org; intel-xe@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@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@intel.com> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> > >> > Reviewed-by: Imre Deak <imre.deak@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! And pushed to din, thanks for the review! > >> > > 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
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);
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@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 35 +++++++++++++++++++------ 1 file changed, 27 insertions(+), 8 deletions(-)