Re: [PATCH v2 15/16] drm/i915: Add pipe B force quirk for 830M

[Thomas Richter <richter@rus.uni-stuttgart.de>]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 830M has problems when some of the pipes are disabled. Namely if a
> plane, DVO port etc. is currently assigned to a disabled pipe, it
> can't moved to the other pipe until the current pipe is also enabled.
> To keep things simple just leave both pipes running all the time.
>
> Ideally I think should turn the pipes off if neither is active, and
> when either becomes active we enable both. But that would reuquire
> proper atomic modeset support, and probably a bit of extra care in
> the order things get enabled.
>
> v2: Reorder wrt. double wide handling changes
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Tested-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/i915_drv.h      |  1 +
>   drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++-----------
>   2 files changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 54895a6..b1ed71e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -706,6 +706,7 @@ enum intel_sbi_destination {
>   #define QUIRK_LVDS_SSC_DISABLE (1<<1)
>   #define QUIRK_INVERT_BRIGHTNESS (1<<2)
>   #define QUIRK_BACKLIGHT_PRESENT (1<<3)
> +#define QUIRK_PIPEB_FORCE (1<<4)
>
>   struct intel_fbdev;
>   struct intel_fbc_work;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e1c0c0b..92baf6f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1194,8 +1194,9 @@ void assert_pipe(struct drm_i915_private *dev_priv,
>   	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
>   								      pipe);
>
> -	/* if we need the pipe A quirk it must be always on */
> -	if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
> +	/* if we need the pipe quirk it must be always on */
> +	if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> +	    (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
>   		state = true;
>
>   	if (!intel_display_power_enabled(dev_priv,
> @@ -1626,8 +1627,9 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
>   		}
>   	}
>
> -	/* Don't disable pipe A or pipe A PLLs if needed */
> -	if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
> +	/* Don't disable pipe or pipe PLLs if needed */
> +	if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> +	    (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
>   		return;
>
>   	/* Make sure the pipe isn't still relying on us */
> @@ -1995,8 +1997,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>   	reg = PIPECONF(cpu_transcoder);
>   	val = I915_READ(reg);
>   	if (val & PIPECONF_ENABLE) {
> -		WARN_ON(!(pipe == PIPE_A &&
> -			  dev_priv->quirks & QUIRK_PIPEA_FORCE));
> +		WARN_ON(!((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> +			  (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE)));
>   		return;
>   	}
>
> @@ -2043,7 +2045,8 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
>   		val &= ~PIPECONF_DOUBLE_WIDE;
>
>   	/* Don't disable pipe or pipe PLLs if needed */
> -	if (!(pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE))
> +	if (!(pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) &&
> +	    !(pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
>   		val &= ~PIPECONF_ENABLE;
>
>   	I915_WRITE(reg, val);
> @@ -5968,9 +5971,9 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>
>   	pipeconf = 0;
>
> -	if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
> -	    I915_READ(PIPECONF(intel_crtc->pipe)) & PIPECONF_ENABLE)
> -		pipeconf |= PIPECONF_ENABLE;
> +	if ((intel_crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> +	    (intel_crtc->pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
> +		pipeconf |= I915_READ(PIPECONF(intel_crtc->pipe)) & PIPECONF_ENABLE;
>
>   	if (intel_crtc->config.double_wide)
>   		pipeconf |= PIPECONF_DOUBLE_WIDE;
> @@ -10680,8 +10683,9 @@ check_crtc_state(struct drm_device *dev)
>   		active = dev_priv->display.get_pipe_config(crtc,
>   							   &pipe_config);
>
> -		/* hw state is inconsistent with the pipe A quirk */
> -		if (crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
> +		/* hw state is inconsistent with the pipe quirk */
> +		if ((crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> +		    (crtc->pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
>   			active = crtc->active;
>
>   		for_each_intel_encoder(dev, encoder) {
> @@ -12429,6 +12433,14 @@ static void quirk_pipea_force(struct drm_device *dev)
>   	DRM_INFO("applying pipe a force quirk\n");
>   }
>
> +static void quirk_pipeb_force(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	dev_priv->quirks |= QUIRK_PIPEB_FORCE;
> +	DRM_INFO("applying pipe b force quirk\n");
> +}
> +
>   /*
>    * Some machines (Lenovo U160) do not work with SSC on LVDS for some reason
>    */
> @@ -12506,6 +12518,9 @@ static struct intel_quirk intel_quirks[] = {
>   	/* 830 needs to leave pipe A & dpll A up */
>   	{ 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
>
> +	/* 830 needs to leave pipe B & dpll B up */
> +	{ 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipeb_force },
> +
>   	/* Lenovo U160 cannot use SSC on LVDS */
>   	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
>
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx