[Intel-gfx] [PATCH] drm/i915: Allow DBLSCAN user modes with eDP/LVDS/DSI

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Wed Jun 13 14:48:29 UTC 2018


Op 24-05-18 om 14:54 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> When encountering a connector with the scaling mode property both
> intel and modesetting ddxs sometimes add tons of DBLSCAN modes
> to the output's mode list. The idea presumably being that since the
> output will be going through the panel fitter anyway we can pretend
> to use any kind of mode.
>
> Sadly that means we can't reject user modes with the DBLSCAN flag
> until we know whether we're going to be using the panel's native
> mode or the user mode directly. Doing otherwise means X clients using
> xf86vidmode/xrandr will get a protocol error (and often self
> terminate as a result) when the kernel refuses to use the requested
> mode with the DBLSCAN flag.
>
> To undo the regression we'll move the DBLSCAN checks into the
> connector->mode_valid() and encoder->compute_config() hooks.
>
> Cc: Vito Caputo <vcaputo at pengaru.com>
> Reported-by: Vito Caputo <vcaputo at pengaru.com>
> Fixes: e995ca0b8139 ("drm/i915: Provide a device level .mode_valid() hook")
> References: https://lkml.org/lkml/2018/5/21/715
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c     | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c | 16 +++++++++++++---
>  drivers/gpu/drm/i915/intel_dp.c      |  6 ++++++
>  drivers/gpu/drm/i915/intel_dp_mst.c  |  6 ++++++
>  drivers/gpu/drm/i915/intel_dsi.c     |  6 ++++++
>  drivers/gpu/drm/i915/intel_dvo.c     |  6 ++++++
>  drivers/gpu/drm/i915/intel_hdmi.c    |  6 ++++++
>  drivers/gpu/drm/i915/intel_lvds.c    |  5 +++++
>  drivers/gpu/drm/i915/intel_sdvo.c    |  6 ++++++
>  drivers/gpu/drm/i915/intel_tv.c      | 12 ++++++++++--
>  10 files changed, 84 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 211d601cd1b1..95aa29cf2d9c 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -304,6 +304,9 @@ intel_crt_mode_valid(struct drm_connector *connector,
>  	int max_dotclk = dev_priv->max_dotclk_freq;
>  	int max_clock;
>  
> +	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return MODE_NO_DBLESCAN;
> +
>  	if (mode->clock < 25000)
>  		return MODE_CLOCK_LOW;
>  
> @@ -337,6 +340,12 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
>  				     struct intel_crtc_state *pipe_config,
>  				     struct drm_connector_state *conn_state)
>  {
> +	struct drm_display_mode *adjusted_mode =
> +		&pipe_config->base.adjusted_mode;
> +
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
> +
>  	return true;
>  }
>  
> @@ -344,6 +353,12 @@ static bool pch_crt_compute_config(struct intel_encoder *encoder,
>  				   struct intel_crtc_state *pipe_config,
>  				   struct drm_connector_state *conn_state)
>  {
> +	struct drm_display_mode *adjusted_mode =
> +		&pipe_config->base.adjusted_mode;
> +
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
> +
>  	pipe_config->has_pch_encoder = true;
>  
>  	return true;
> @@ -354,6 +369,11 @@ static bool hsw_crt_compute_config(struct intel_encoder *encoder,
>  				   struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct drm_display_mode *adjusted_mode =
> +		&pipe_config->base.adjusted_mode;
> +
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
>  
>  	pipe_config->has_pch_encoder = true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8b385176ce3c..02651298a99b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14443,12 +14443,22 @@ static enum drm_mode_status
>  intel_mode_valid(struct drm_device *dev,
>  		 const struct drm_display_mode *mode)
>  {
> +	/*
> +	 * Can't reject DBLSCAN here because Xorg ddxen can add piles
> +	 * of DBLSCAN modes to the output's mode list when they detect
> +	 * the scaling mode property on the connector. And they don't
> +	 * ask the kernel to validate those modes in any way until
> +	 * modeset time at which point the client gets a protocol error.
> +	 * So in order to not upset those clients we silently ignore the
> +	 * DBLSCAN flag on such connectors. For other connectors we will
> +	 * reject modes with the DBLSCAN flag in encoder->compute_config().
> +	 * And we always reject DBLSCAN modes in connector->mode_valid()
> +	 * as we never want such modes on the connector's mode list.
> +	 */
> +
>  	if (mode->vscan > 1)
>  		return MODE_NO_VSCAN;
>  
> -	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> -		return MODE_NO_DBLESCAN;
> -
>  	if (mode->flags & DRM_MODE_FLAG_HSKEW)
>  		return MODE_H_ILLEGAL;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ce07bd794aed..91885f2500af 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -420,6 +420,9 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  	int max_rate, mode_rate, max_lanes, max_link_clock;
>  	int max_dotclk;
>  
> +	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return MODE_NO_DBLESCAN;
> +
>  	max_dotclk = intel_dp_downstream_max_dotclock(intel_dp);
>  
>  	if (intel_dp_is_edp(intel_dp) && fixed_mode) {
> @@ -1862,6 +1865,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  						conn_state->scaling_mode);
>  	}
>  
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
> +
>  	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
>  	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
>  		return false;
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 9e6956c08688..5890500a3a8b 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -48,6 +48,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
>  					   DP_DPCD_QUIRK_LIMITED_M_N);
>  
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
> +
>  	pipe_config->has_pch_encoder = false;
>  	bpp = 24;
>  	if (intel_dp->compliance.test_data.bpc) {
> @@ -366,6 +369,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
>  	if (!intel_dp)
>  		return MODE_ERROR;
>  
> +	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return MODE_NO_DBLESCAN;
> +
>  	max_link_clock = intel_dp_max_link_rate(intel_dp);
>  	max_lanes = intel_dp_max_lane_count(intel_dp);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index cf39ca90d887..f349b3920199 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -326,6 +326,9 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>  						conn_state->scaling_mode);
>  	}
>  
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
> +
>  	/* DSI uses short packets for sync events, so clear mode flags for DSI */
>  	adjusted_mode->flags = 0;
>  
> @@ -1266,6 +1269,9 @@ intel_dsi_mode_valid(struct drm_connector *connector,
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return MODE_NO_DBLESCAN;
> +
>  	if (fixed_mode) {
>  		if (mode->hdisplay > fixed_mode->hdisplay)
>  			return MODE_PANEL;
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 7b942b6c1700..27f16db8953a 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -215,6 +215,9 @@ intel_dvo_mode_valid(struct drm_connector *connector,
>  	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>  	int target_clock = mode->clock;
>  
> +	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return MODE_NO_DBLESCAN;
> +
>  	/* XXX: Validate clock range */
>  
>  	if (fixed_mode) {
> @@ -250,6 +253,9 @@ static bool intel_dvo_compute_config(struct intel_encoder *encoder,
>  	if (fixed_mode)
>  		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
>  
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
> +
>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 0ca4cc877520..47e6cca3e208 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1544,6 +1544,9 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>  	bool force_dvi =
>  		READ_ONCE(to_intel_digital_connector_state(connector->state)->force_audio) == HDMI_AUDIO_OFF_DVI;
>  
> +	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return MODE_NO_DBLESCAN;
> +
>  	clock = mode->clock;
>  
>  	if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == DRM_MODE_FLAG_3D_FRAME_PACKING)
> @@ -1664,6 +1667,9 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	int desired_bpp;
>  	bool force_dvi = intel_conn_state->force_audio == HDMI_AUDIO_OFF_DVI;
>  
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
> +
>  	pipe_config->has_hdmi_sink = !force_dvi && intel_hdmi->has_hdmi_sink;
>  
>  	if (pipe_config->has_hdmi_sink)
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index bacad88ad7ae..5b4a35299531 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -378,6 +378,8 @@ intel_lvds_mode_valid(struct drm_connector *connector,
>  	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
>  	int max_pixclk = to_i915(connector->dev)->max_dotclk_freq;
>  
> +	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return MODE_NO_DBLESCAN;
>  	if (mode->hdisplay > fixed_mode->hdisplay)
>  		return MODE_PANEL;
>  	if (mode->vdisplay > fixed_mode->vdisplay)
> @@ -427,6 +429,9 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
>  	intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
>  			       adjusted_mode);
>  
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
> +
>  	if (HAS_PCH_SPLIT(dev_priv)) {
>  		pipe_config->has_pch_encoder = true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index a02e4d73c7a4..e6a64b3ecd91 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1160,6 +1160,9 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  							   adjusted_mode);
>  	}
>  
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
> +
>  	/*
>  	 * Make the CRTC code factor in the SDVO pixel multiplier.  The
>  	 * SDVO device will factor out the multiplier during mode_set.
> @@ -1631,6 +1634,9 @@ intel_sdvo_mode_valid(struct drm_connector *connector,
>  	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
>  	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>  
> +	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return MODE_NO_DBLESCAN;
> +
>  	if (intel_sdvo->pixel_clock_min > mode->clock)
>  		return MODE_CLOCK_LOW;
>  
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 99bc2368dda0..24dc368fdaa1 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -846,6 +846,9 @@ intel_tv_mode_valid(struct drm_connector *connector,
>  	const struct tv_mode *tv_mode = intel_tv_mode_find(connector->state);
>  	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>  
> +	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return MODE_NO_DBLESCAN;
> +
>  	if (mode->clock > max_dotclk)
>  		return MODE_CLOCK_HIGH;
>  
> @@ -873,16 +876,21 @@ intel_tv_compute_config(struct intel_encoder *encoder,
>  			struct drm_connector_state *conn_state)
>  {
>  	const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state);
> +	struct drm_display_mode *adjusted_mode =
> +		&pipe_config->base.adjusted_mode;
>  
>  	if (!tv_mode)
>  		return false;
>  
> -	pipe_config->base.adjusted_mode.crtc_clock = tv_mode->clock;
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> +		return false;
> +
> +	adjusted_mode->crtc_clock = tv_mode->clock;
>  	DRM_DEBUG_KMS("forcing bpc to 8 for TV\n");
>  	pipe_config->pipe_bpp = 8*3;
>  
>  	/* TV has it's own notion of sync and other mode flags, so clear them. */
> -	pipe_config->base.adjusted_mode.flags = 0;
> +	adjusted_mode->flags = 0;
>  
>  	/*
>  	 * FIXME: We don't check whether the input mode is actually what we want


Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>



More information about the Intel-gfx mailing list