On Tue, Jul 22, 2025 at 11:25:32AM +0530, Nautiyal, Ankit K wrote:
>
> On 7/21/2025 5:11 PM, Imre Deak wrote:
> > On Mon, Jul 21, 2025 at 02:45:22PM +0530, Ankit Nautiyal wrote:
> > > The hardware cannot support DisplayPort configurations where the
> > > ceiling of the Link M/Link N ratio exceeds 10. This limitation has
> > > always existed, but it typically wasn't encountered without the use of
> > > joiners and DSC.
> > >
> > > With higher resolutions and combinations involving joiners and DSC,
> > > this constraint can now be hit in certain scenarios.
> > >
> > > Introduce a check during link rate configuration to ensure
> > > the computed M/N ratio does not exceed the hardware limit. If no valid
> > > link rate satisfies this constraint, the mode will be rejected.
> > >
> > > Note: This change applies the check only for SST. Support for MST will
> > > be added in a subsequent commit.
> > >
> > > v2:
> > > -Move the M/N ratio check to the link rate configuration phase instead
> > > of during M/N computation. (Ville)
> > > -Prune modes that cannot be supported even with highest link rate due to
> > > M/N ratio restriction.
> > >
> > > Signed-off-by: Ankit Nautiyal <ankit.k.nauti...@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_display.c | 1 -
> > > drivers/gpu/drm/i915/display/intel_display.h | 3 ++
> > > drivers/gpu/drm/i915/display/intel_dp.c | 33 +++++++++++++++++++-
> > > 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 671491a2a3b6..f32a4956c926 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -2506,7 +2506,6 @@ static void compute_m_n(u32 *ret_m, u32 *ret_n,
> > > intel_reduce_m_n_ratio(ret_m, ret_n);
> > > }
> > > -static
> > > void intel_display_get_link_m_n(u32 *link_m, u32 *link_n,
> > > u32 pixel_clock,
> > > u32 link_clock)
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> > > b/drivers/gpu/drm/i915/display/intel_display.h
> > > index 37e2ab301a80..bfa3db219b9c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > > @@ -559,5 +559,8 @@ bool assert_port_valid(struct intel_display *display,
> > > enum port port);
> > > bool intel_scanout_needs_vtd_wa(struct intel_display *display);
> > > int intel_crtc_num_joined_pipes(const struct intel_crtc_state
> > > *crtc_state);
> > > +void intel_display_get_link_m_n(u32 *link_m, u32 *link_n,
> > > + u32 pixel_clock,
> > > + u32 link_clock);
> > > #endif
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 54d88f24b689..4245dd65b2af 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -1407,6 +1407,28 @@ bool intel_dp_has_dsc(const struct intel_connector
> > > *connector)
> > > return true;
> > > }
> > > +static int
> > > +intel_dp_get_max_m_n_ratio(void)
> > > +{
> > > + return 10;
> > > +}
> > > +
> > > +static bool
> > > +intel_dp_can_support_m_n(int pixel_clock,
> > > + int link_rate)
> > > +{
> > > + int max_m_n_ratio = intel_dp_get_max_m_n_ratio();
> > > + u32 link_m, link_n;
> > > + int m_n_ratio;
> > > +
> > > + intel_display_get_link_m_n(&link_m, &link_n,
> > > + pixel_clock, link_rate);
> > > +
> > > + m_n_ratio = DIV_ROUND_UP(link_m, link_n);
> > > +
> > > + return m_n_ratio <= max_m_n_ratio;
> > > +}
> > > +
> > > static enum drm_mode_status
> > > intel_dp_mode_valid(struct drm_connector *_connector,
> > > const struct drm_display_mode *mode)
> > > @@ -1518,6 +1540,9 @@ intel_dp_mode_valid(struct drm_connector
> > > *_connector,
> > > if (status != MODE_OK)
> > > return status;
> > > + if (!intel_dp_can_support_m_n(target_clock, max_rate))
> > > + return MODE_CLOCK_HIGH;
> > > +
> > > return intel_mode_valid_max_plane_size(display, mode,
> > > num_joined_pipes);
> > > }
> > > @@ -1789,6 +1814,9 @@ intel_dp_compute_link_config_wide(struct intel_dp
> > > *intel_dp,
> > > link_rate > limits->max_rate)
> > > continue;
> > > + if (!intel_dp_can_support_m_n(clock, link_rate))
> > > + continue;
> > Could this limit be set in limits->min_rate, computed already in
> > intel_dp_compute_config_limits()? (And fail already there if this is
> > bigger than limits->max_rate.)
>
> Yes right this can be done in intel_dp_compute_config_limit.
>
> We can iterate over the array of common_rates and find the rate for which
> the ratio is within limits and bail out early if we cant find such a link
> rate.
>
> Or otherwise instead of iterating over rates, I guess theoretically we can
> calculate the minimum link rate for given pixelclock and ratio and set it to
> limits->min_rate and bail out early if this is more than the
> limits->max_rate.
Right, the first option to just iterate over common_rates sounds better.
limits->min_rate should be set to a validate rate, so it will need to be
looked up from common_rates anyway. There's also
intel_dp_rate_limit_len()/intel_dp_common_rate(), but the former returns
the index for a rate which is <= than the limit passed to it (while >=
would be needed here), so can't be used as-is.
I noticed that the mode's pixel clock is adjusted later based on the MSO
link count (in intel_dp_compute_config()) and the number of joined pipes
(in intel_joiner_adjust_timings()), but I suppose this doesn't matter,
since the link m/n values are calculated with the unadjusted pixel
clock. Could you confirm this nevertheless?
> However this might be bit tricky as symbol size is different for uhbr.
>
> I will check this out, and move this in intel_dp_compute_config_limits.
>
> Thanks & Regards,
>
> Ankit
>
>
> >
> > > +
> > > for (lane_count = limits->min_lane_count;
> > > lane_count <= limits->max_lane_count;
> > > lane_count <<= 1) {
> > > @@ -1796,7 +1824,6 @@ intel_dp_compute_link_config_wide(struct intel_dp
> > > *intel_dp,
> > >
> > > link_rate,
> > >
> > > lane_count);
> > > -
> > > if (mode_rate <= link_avail) {
> > > pipe_config->lane_count =
> > > lane_count;
> > > pipe_config->pipe_bpp = bpp;
> > > @@ -1983,6 +2010,10 @@ static int dsc_compute_link_config(struct intel_dp
> > > *intel_dp,
> > > if (link_rate < limits->min_rate || link_rate >
> > > limits->max_rate)
> > > continue;
> > > + if (!intel_dp_can_support_m_n(adjusted_mode->clock,
> > > + link_rate))
> > > + continue;
> > > +
> > > for (lane_count = limits->min_lane_count;
> > > lane_count <= limits->max_lane_count;
> > > lane_count <<= 1) {
> > > --
> > > 2.45.2
> > >
