| Message ID | 20200511231915.28069-1-shawn.c.lee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/mst: filter out the display mode exceed sink's capability | expand |
sorry it's been taking me a while to get back to you with these, crunch time at my work started so i've been really busy over the last week. On Tue, 2020-05-12 at 07:19 +0800, Lee Shawn C wrote: > So far, max dot clock rate for MST mode rely on physcial > bandwidth limitation. It would caused compatibility issue > if source display resolution exceed MST hub output ability. > > For example, source DUT had DP 1.2 output capability. > And MST docking just support HDMI 1.4 spec. When a HDMI 2.0 > monitor connected. Source would retrieve EDID from external > and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps > because it did not surpass DP physical bandwidth limitation. > Do modeset to 4k@60fps, source output display data but MST > docking can't output HDMI properly due to this resolution > already over HDMI 1.4 spec. > > Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn > instead of available_pbn for bandwidth checks"). > Source driver should refer to full_pbn to evaluate sink > output capability. And filter out the resolution surpass > sink output limitation. > > v2: Using mgr->base.lock to protect full_pbn. > > Cc: Manasi Navare <manasi.d.navare@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Cooper Chiou <cooper.chiou@intel.com> > Cc: Lyude Paul <lyude@redhat.com> > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 ++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 74559379384a..6b1864ce3771 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -611,6 +611,26 @@ static int intel_dp_mst_get_modes(struct drm_connector > *connector) > return intel_dp_mst_get_ddc_modes(connector); > } > > +static bool > +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector *connector, > int clock, int bpp) > +{ > + struct intel_connector *intel_connector = to_intel_connector(connector); > + struct intel_dp *intel_dp = intel_connector->mst_port; > + struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr; > + struct drm_dp_mst_port *port = (to_intel_connector(connector))->port; > + bool ret = false; > + > + if (!mgr) > + return ret; > + > + drm_modeset_lock(&mgr->base.lock, NULL); > + if (port->full_pbn) > + ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port- > >full_pbn); > + drm_modeset_unlock(&mgr->base.lock); You're so close, but not quite! I'm fairly sure ->mode_valid() gets called with drm_device.mode_config.connection_mutex already locked, which means that you need to use the same drm_modeset_acquire context (if there isn't one, you need to make it) so that you can safely roll back and retry in case there's a deadlock between drm_dp_mst_topology_mgr.base.lock and drm_device.mode_config.connection_mutex . fwiw - I'm sensing some confusion here about ww_mutexes (wouldn't be the first time, they're unusual). If you haven't already, it's probably worth reading up on: https://lwn.net/Articles/548909/ Keep in mind when you pipe down the context by adding a new version of the ->mode_valid() hook you won't want to drop any locks once you're done in this function. They'll get dropped automatically when the caller of the new ->mode_valid() hook (so, the one where you get the drm_modeset_acquire_ctx from) calls drm_modeset_drop_locks(). > + > + return ret; > +} > + > static enum drm_mode_status > intel_dp_mst_mode_valid(struct drm_connector *connector, > struct drm_display_mode *mode) > @@ -633,7 +653,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector, > max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); > mode_rate = intel_dp_link_required(mode->clock, 18); > > - /* TODO - validate mode against available PBN for link */ > + if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode->clock, > 24)) > + return MODE_CLOCK_HIGH; > + > if (mode->clock < 10000) > return MODE_CLOCK_LOW; >
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 74559379384a..6b1864ce3771 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -611,6 +611,26 @@ static int intel_dp_mst_get_modes(struct drm_connector *connector) return intel_dp_mst_get_ddc_modes(connector); } +static bool +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector *connector, int clock, int bpp) +{ + struct intel_connector *intel_connector = to_intel_connector(connector); + struct intel_dp *intel_dp = intel_connector->mst_port; + struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr; + struct drm_dp_mst_port *port = (to_intel_connector(connector))->port; + bool ret = false; + + if (!mgr) + return ret; + + drm_modeset_lock(&mgr->base.lock, NULL); + if (port->full_pbn) + ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port->full_pbn); + drm_modeset_unlock(&mgr->base.lock); + + return ret; +} + static enum drm_mode_status intel_dp_mst_mode_valid(struct drm_connector *connector, struct drm_display_mode *mode) @@ -633,7 +653,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector, max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); mode_rate = intel_dp_link_required(mode->clock, 18); - /* TODO - validate mode against available PBN for link */ + if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode->clock, 24)) + return MODE_CLOCK_HIGH; + if (mode->clock < 10000) return MODE_CLOCK_LOW;
So far, max dot clock rate for MST mode rely on physcial bandwidth limitation. It would caused compatibility issue if source display resolution exceed MST hub output ability. For example, source DUT had DP 1.2 output capability. And MST docking just support HDMI 1.4 spec. When a HDMI 2.0 monitor connected. Source would retrieve EDID from external and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps because it did not surpass DP physical bandwidth limitation. Do modeset to 4k@60fps, source output display data but MST docking can't output HDMI properly due to this resolution already over HDMI 1.4 spec. Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn instead of available_pbn for bandwidth checks"). Source driver should refer to full_pbn to evaluate sink output capability. And filter out the resolution surpass sink output limitation. v2: Using mgr->base.lock to protect full_pbn. Cc: Manasi Navare <manasi.d.navare@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Cooper Chiou <cooper.chiou@intel.com> Cc: Lyude Paul <lyude@redhat.com> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 ++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)