[PATCH] drm/i915: Fix PCH port pipe select in CPT disable paths
Andy Whitcroft
apw at canonical.com
Thu Dec 22 16:56:13 UTC 2011
On Tue, Dec 20, 2011 at 05:19:49PM +0800, Keng-Yu Lin wrote:
> From: Keith Packard <keithp at keithp.com>
>
> CPT pipe select is different from previous generations (using two bits
> instead of one). All of the paths from intel_disable_pch_ports were
> not making this distinction.
>
> Mode setting with pipe A turned off would then also force all outputs
> on pipe B to get turned off as the disable code would mistakenly
> decide that all of these outputs were on pipe A and turn them off.
>
> This is an extension of the CPT DP disable fix (why didn't I fix this then?)
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
> Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> (cherry picked from commit 1519b9956eb4b4180fa3f47c73341463cdcfaa37)
>
> BugLink: http://bugs.launchpad.net/bugs/906756
>
> Signed-off-by: Keng-Yu Lin <kengyu at canonical.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 13 ++----
> drivers/gpu/drm/i915/intel_display.c | 81 +++++++++++++++++++++++++++++++---
> 2 files changed, 79 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 63162e6..9e69326 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1313,6 +1313,7 @@
> #define ADPA_PIPE_SELECT_MASK (1<<30)
> #define ADPA_PIPE_A_SELECT 0
> #define ADPA_PIPE_B_SELECT (1<<30)
> +#define ADPA_PIPE_SELECT(pipe) ((pipe) << 30)
> #define ADPA_USE_VGA_HVPOLARITY (1<<15)
> #define ADPA_SETS_HVPOLARITY 0
> #define ADPA_VSYNC_CNTL_DISABLE (1<<11)
> @@ -1455,6 +1456,7 @@
> /* Selects pipe B for LVDS data. Must be set on pre-965. */
> #define LVDS_PIPEB_SELECT (1 << 30)
> #define LVDS_PIPE_MASK (1 << 30)
> +#define LVDS_PIPE(pipe) ((pipe) << 30)
> /* LVDS dithering flag on 965/g4x platform */
> #define LVDS_ENABLE_DITHER (1 << 25)
> /* LVDS sync polarity flags. Set to invert (i.e. negative) */
> @@ -1494,9 +1496,6 @@
> #define LVDS_B0B3_POWER_DOWN (0 << 2)
> #define LVDS_B0B3_POWER_UP (3 << 2)
>
> -#define LVDS_PIPE_ENABLED(V, P) \
> - (((V) & (LVDS_PIPE_MASK | LVDS_PORT_EN)) == ((P) << 30 | LVDS_PORT_EN))
> -
> /* Video Data Island Packet control */
> #define VIDEO_DIP_DATA 0x61178
> #define VIDEO_DIP_CTL 0x61170
> @@ -3238,14 +3237,12 @@
> #define ADPA_CRT_HOTPLUG_VOLREF_475MV (1<<17)
> #define ADPA_CRT_HOTPLUG_FORCE_TRIGGER (1<<16)
>
> -#define ADPA_PIPE_ENABLED(V, P) \
> - (((V) & (ADPA_TRANS_SELECT_MASK | ADPA_DAC_ENABLE)) == ((P) << 30 | ADPA_DAC_ENABLE))
> -
> /* or SDVOB */
> #define HDMIB 0xe1140
> #define PORT_ENABLE (1 << 31)
> #define TRANSCODER_A (0)
> #define TRANSCODER_B (1 << 30)
> +#define TRANSCODER(pipe) ((pipe) << 30)
> #define TRANSCODER_MASK (1 << 30)
> #define COLOR_FORMAT_8bpc (0)
> #define COLOR_FORMAT_12bpc (3 << 26)
> @@ -3262,9 +3259,6 @@
> #define HSYNC_ACTIVE_HIGH (1 << 3)
> #define PORT_DETECTED (1 << 2)
>
> -#define HDMI_PIPE_ENABLED(V, P) \
> - (((V) & (TRANSCODER_MASK | PORT_ENABLE)) == ((P) << 30 | PORT_ENABLE))
> -
> /* PCH SDVOB multiplex with HDMIB */
> #define PCH_SDVOB HDMIB
>
> @@ -3331,6 +3325,7 @@
> #define PORT_TRANS_B_SEL_CPT (1<<29)
> #define PORT_TRANS_C_SEL_CPT (2<<29)
> #define PORT_TRANS_SEL_MASK (3<<29)
> +#define PORT_TRANS_SEL_CPT(pipe) ((pipe) << 29)
>
> #define TRANS_DP_CTL_A 0xe0300
> #define TRANS_DP_CTL_B 0xe1300
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 50e4dd3..80ec58e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -979,6 +979,71 @@ static void assert_transcoder_disabled(struct drm_i915_private *dev_priv,
> pipe_name(pipe));
> }
>
> +static bool dp_pipe_enabled(struct drm_i915_private *dev_priv, enum pipe pipe,
> + int reg, u32 port_sel, u32 val)
> +{
> + if ((val & DP_PORT_EN) == 0)
> + return false;
> +
> + if (HAS_PCH_CPT(dev_priv->dev)) {
> + u32 trans_dp_ctl_reg = TRANS_DP_CTL(pipe);
> + u32 trans_dp_ctl = I915_READ(trans_dp_ctl_reg);
> + if ((trans_dp_ctl & TRANS_DP_PORT_SEL_MASK) != port_sel)
> + return false;
> + } else {
> + if ((val & DP_PIPE_MASK) != (pipe << 30))
> + return false;
> + }
> + return true;
> +}
> +
> +static bool hdmi_pipe_enabled(struct drm_i915_private *dev_priv,
> + enum pipe pipe, u32 val)
> +{
> + if ((val & PORT_ENABLE) == 0)
> + return false;
> +
> + if (HAS_PCH_CPT(dev_priv->dev)) {
> + if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
> + return false;
> + } else {
> + if ((val & TRANSCODER_MASK) != TRANSCODER(pipe))
> + return false;
> + }
> + return true;
> +}
> +
> +static bool lvds_pipe_enabled(struct drm_i915_private *dev_priv,
> + enum pipe pipe, u32 val)
> +{
> + if ((val & LVDS_PORT_EN) == 0)
> + return false;
> +
> + if (HAS_PCH_CPT(dev_priv->dev)) {
> + if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
> + return false;
> + } else {
> + if ((val & LVDS_PIPE_MASK) != LVDS_PIPE(pipe))
> + return false;
> + }
> + return true;
> +}
> +
> +static bool adpa_pipe_enabled(struct drm_i915_private *dev_priv,
> + enum pipe pipe, u32 val)
> +{
> + if ((val & ADPA_DAC_ENABLE) == 0)
> + return false;
> + if (HAS_PCH_CPT(dev_priv->dev)) {
> + if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
> + return false;
> + } else {
> + if ((val & ADPA_PIPE_SELECT_MASK) != ADPA_PIPE_SELECT(pipe))
> + return false;
> + }
> + return true;
> +}
> +
> static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
> enum pipe pipe, int reg)
> {
> @@ -992,7 +1057,7 @@ static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv,
> enum pipe pipe, int reg)
> {
> u32 val = I915_READ(reg);
> - WARN(HDMI_PIPE_ENABLED(val, pipe),
> + WARN(hdmi_pipe_enabled(dev_priv, val, pipe),
> "PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n",
> reg, pipe_name(pipe));
> }
> @@ -1009,13 +1074,13 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
>
> reg = PCH_ADPA;
> val = I915_READ(reg);
> - WARN(ADPA_PIPE_ENABLED(val, pipe),
> + WARN(adpa_pipe_enabled(dev_priv, val, pipe),
> "PCH VGA enabled on transcoder %c, should be disabled\n",
> pipe_name(pipe));
>
> reg = PCH_LVDS;
> val = I915_READ(reg);
> - WARN(LVDS_PIPE_ENABLED(val, pipe),
> + WARN(lvds_pipe_enabled(dev_priv, val, pipe),
> "PCH LVDS enabled on transcoder %c, should be disabled\n",
> pipe_name(pipe));
>
> @@ -1345,8 +1410,11 @@ static void disable_pch_hdmi(struct drm_i915_private *dev_priv,
> enum pipe pipe, int reg)
> {
> u32 val = I915_READ(reg);
> - if (HDMI_PIPE_ENABLED(val, pipe))
> + if (hdmi_pipe_enabled(dev_priv, val, pipe)) {
> + DRM_DEBUG_KMS("Disabling pch HDMI %x on pipe %d\n",
> + reg, pipe);
> I915_WRITE(reg, val & ~PORT_ENABLE);
> + }
> }
>
> /* Disable any ports connected to this transcoder */
> @@ -1364,12 +1432,13 @@ static void intel_disable_pch_ports(struct drm_i915_private *dev_priv,
>
> reg = PCH_ADPA;
> val = I915_READ(reg);
> - if (ADPA_PIPE_ENABLED(val, pipe))
> + if (adpa_pipe_enabled(dev_priv, val, pipe))
> I915_WRITE(reg, val & ~ADPA_DAC_ENABLE);
>
> reg = PCH_LVDS;
> val = I915_READ(reg);
> - if (LVDS_PIPE_ENABLED(val, pipe)) {
> + if (lvds_pipe_enabled(dev_priv, val, pipe)) {
> + DRM_DEBUG_KMS("disable lvds on pipe %d val 0x%08x\n", pipe, val);
> I915_WRITE(reg, val & ~LVDS_PORT_EN);
> POSTING_READ(reg);
> udelay(100);
You'd think those enabled routines could be merged into something shorter
but hey. Sounds like it should be applied as we already applied the
previous bits he mentions right? So based on that:
Acked-by: Andy Whitcroft <apw at canonical.com>
-apw
More information about the kernel-team
mailing list