Message ID | 20190718144340.1114-1-ville.syrjala@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/2] drm/i915: Refuse modes with hdisplay==4096 on pre-HSW DP | expand |
On Thu, Jul 18, 2019 at 05:43:39PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The DP port/pipe goes wonky if we try to use timings with > hdisplay==4096 on pre-HSW platforms. The link fails to train > and the pipe may not signal vblank interrupts. On HDMI such at > mode works just fine (tested on ELK/SNB/CHV). So let's refuse > such modes on DP on older platforms. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Makes sense if the link is not able to get trained that we reject this mode > --- > drivers/gpu/drm/i915/display/intel_dp.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 0eb5d66f87a7..c96030761eae 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -493,6 +493,25 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, > return 0; > } > > +static bool intel_dp_hdisplay_bad(struct drm_i915_private *dev_priv, > + int hdisplay) > +{ > + /* > + * Older platforms don't like hdisplay==4096 with DP. > + * > + * On ILK/SNB/IVB the pipe seems to be somewhat running (scanline > + * and frame counter increment), but we don't get vblank interrupts, > + * and the pipe underruns immediately. The link also doesn't seem > + * to get trained properly. > + * > + * On CHV the vblank interrupts don't seem to disappear but > + * otherwise the symptoms are similar. > + * > + * TODO: confirm the behaviour on HSW+ > + */ > + return hdisplay == 4096 && !HAS_DDI(dev_priv); > +} > + > static enum drm_mode_status > intel_dp_mode_valid(struct drm_connector *connector, > struct drm_display_mode *mode) > @@ -528,6 +547,9 @@ intel_dp_mode_valid(struct drm_connector *connector, > max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); > mode_rate = intel_dp_link_required(target_clock, 18); > > + if (intel_dp_hdisplay_bad(dev_priv, mode->hdisplay)) > + return MODE_H_ILLEGAL; > + > /* > * Output bpp is stored in 6.4 format so right shift by 4 to get the > * integer value since we support only integer values of bpp. > @@ -2208,6 +2230,9 @@ intel_dp_compute_config(struct intel_encoder *encoder, > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > return -EINVAL; > > + if (intel_dp_hdisplay_bad(dev_priv, adjusted_mode->crtc_hdisplay)) > + return -EINVAL; Why would we need another check in compute_config() since the mode would have been rejected already in intel_dp_mode_valid() Manasi > + > ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state); > if (ret < 0) > return ret; > -- > 2.21.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Jul 29, 2019 at 12:11:58PM -0700, Manasi Navare wrote: > On Thu, Jul 18, 2019 at 05:43:39PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > The DP port/pipe goes wonky if we try to use timings with > > hdisplay==4096 on pre-HSW platforms. The link fails to train > > and the pipe may not signal vblank interrupts. On HDMI such at > > mode works just fine (tested on ELK/SNB/CHV). So let's refuse > > such modes on DP on older platforms. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Makes sense if the link is not able to get trained that we reject this mode > > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index 0eb5d66f87a7..c96030761eae 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -493,6 +493,25 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, > > return 0; > > } > > > > +static bool intel_dp_hdisplay_bad(struct drm_i915_private *dev_priv, > > + int hdisplay) > > +{ > > + /* > > + * Older platforms don't like hdisplay==4096 with DP. > > + * > > + * On ILK/SNB/IVB the pipe seems to be somewhat running (scanline > > + * and frame counter increment), but we don't get vblank interrupts, > > + * and the pipe underruns immediately. The link also doesn't seem > > + * to get trained properly. > > + * > > + * On CHV the vblank interrupts don't seem to disappear but > > + * otherwise the symptoms are similar. > > + * > > + * TODO: confirm the behaviour on HSW+ > > + */ > > + return hdisplay == 4096 && !HAS_DDI(dev_priv); > > +} > > + > > static enum drm_mode_status > > intel_dp_mode_valid(struct drm_connector *connector, > > struct drm_display_mode *mode) > > @@ -528,6 +547,9 @@ intel_dp_mode_valid(struct drm_connector *connector, > > max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); > > mode_rate = intel_dp_link_required(target_clock, 18); > > > > + if (intel_dp_hdisplay_bad(dev_priv, mode->hdisplay)) > > + return MODE_H_ILLEGAL; > > + > > /* > > * Output bpp is stored in 6.4 format so right shift by 4 to get the > > * integer value since we support only integer values of bpp. > > @@ -2208,6 +2230,9 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > > return -EINVAL; > > > > + if (intel_dp_hdisplay_bad(dev_priv, adjusted_mode->crtc_hdisplay)) > > + return -EINVAL; > > Why would we need another check in compute_config() since the mode would have been > rejected already in intel_dp_mode_valid() The connector .mode_valid() won't be called for setcrtc/atomic ioctls. It's only used to filter out the modes from the connector's mode list. > > Manasi > > > + > > ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state); > > if (ret < 0) > > return ret; > > -- > > 2.21.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Aug 19, 2019 at 07:14:23PM +0300, Ville Syrjälä wrote: > On Mon, Jul 29, 2019 at 12:11:58PM -0700, Manasi Navare wrote: > > On Thu, Jul 18, 2019 at 05:43:39PM +0300, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > The DP port/pipe goes wonky if we try to use timings with > > > hdisplay==4096 on pre-HSW platforms. The link fails to train > > > and the pipe may not signal vblank interrupts. On HDMI such at > > > mode works just fine (tested on ELK/SNB/CHV). So let's refuse > > > such modes on DP on older platforms. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Makes sense if the link is not able to get trained that we reject this mode > > > > > --- > > > drivers/gpu/drm/i915/display/intel_dp.c | 25 +++++++++++++++++++++++++ > > > 1 file changed, 25 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 0eb5d66f87a7..c96030761eae 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -493,6 +493,25 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, > > > return 0; > > > } > > > > > > +static bool intel_dp_hdisplay_bad(struct drm_i915_private *dev_priv, > > > + int hdisplay) > > > +{ > > > + /* > > > + * Older platforms don't like hdisplay==4096 with DP. > > > + * > > > + * On ILK/SNB/IVB the pipe seems to be somewhat running (scanline > > > + * and frame counter increment), but we don't get vblank interrupts, > > > + * and the pipe underruns immediately. The link also doesn't seem > > > + * to get trained properly. > > > + * > > > + * On CHV the vblank interrupts don't seem to disappear but > > > + * otherwise the symptoms are similar. > > > + * > > > + * TODO: confirm the behaviour on HSW+ > > > + */ > > > + return hdisplay == 4096 && !HAS_DDI(dev_priv); > > > +} > > > + > > > static enum drm_mode_status > > > intel_dp_mode_valid(struct drm_connector *connector, > > > struct drm_display_mode *mode) > > > @@ -528,6 +547,9 @@ intel_dp_mode_valid(struct drm_connector *connector, > > > max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); > > > mode_rate = intel_dp_link_required(target_clock, 18); > > > > > > + if (intel_dp_hdisplay_bad(dev_priv, mode->hdisplay)) > > > + return MODE_H_ILLEGAL; > > > + > > > /* > > > * Output bpp is stored in 6.4 format so right shift by 4 to get the > > > * integer value since we support only integer values of bpp. > > > @@ -2208,6 +2230,9 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > > > return -EINVAL; > > > > > > + if (intel_dp_hdisplay_bad(dev_priv, adjusted_mode->crtc_hdisplay)) > > > + return -EINVAL; > > > > Why would we need another check in compute_config() since the mode would have been > > rejected already in intel_dp_mode_valid() > > The connector .mode_valid() won't be called for setcrtc/atomic ioctls. > It's only used to filter out the modes from the connector's mode list. > okay got it! Looks good to me in that case Reviewed-by: Manasi Navare <manasi.d.navare@intel.com> Manasi > > > > Manasi > > > > > + > > > ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state); > > > if (ret < 0) > > > return ret; > > > -- > > > 2.21.0 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 0eb5d66f87a7..c96030761eae 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -493,6 +493,25 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, return 0; } +static bool intel_dp_hdisplay_bad(struct drm_i915_private *dev_priv, + int hdisplay) +{ + /* + * Older platforms don't like hdisplay==4096 with DP. + * + * On ILK/SNB/IVB the pipe seems to be somewhat running (scanline + * and frame counter increment), but we don't get vblank interrupts, + * and the pipe underruns immediately. The link also doesn't seem + * to get trained properly. + * + * On CHV the vblank interrupts don't seem to disappear but + * otherwise the symptoms are similar. + * + * TODO: confirm the behaviour on HSW+ + */ + return hdisplay == 4096 && !HAS_DDI(dev_priv); +} + static enum drm_mode_status intel_dp_mode_valid(struct drm_connector *connector, struct drm_display_mode *mode) @@ -528,6 +547,9 @@ intel_dp_mode_valid(struct drm_connector *connector, max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); mode_rate = intel_dp_link_required(target_clock, 18); + if (intel_dp_hdisplay_bad(dev_priv, mode->hdisplay)) + return MODE_H_ILLEGAL; + /* * Output bpp is stored in 6.4 format so right shift by 4 to get the * integer value since we support only integer values of bpp. @@ -2208,6 +2230,9 @@ intel_dp_compute_config(struct intel_encoder *encoder, if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) return -EINVAL; + if (intel_dp_hdisplay_bad(dev_priv, adjusted_mode->crtc_hdisplay)) + return -EINVAL; + ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state); if (ret < 0) return ret;