Ack: [PATCH] drm/i915: Fix PCH port pipe select in CPT disable paths
Seth Forshee
seth.forshee at canonical.com
Tue Jan 3 14:49:49 UTC 2012
On Tue, Dec 20, 2011 at 04:15:09PM +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>
I'd echo Sefan's comments, but otherwise:
Acked-by: Seth Forshee <seth.forshee 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);
> --
> 1.7.5.4
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
More information about the kernel-team
mailing list