diff mbox

[1/6] drm/i915: Warn if a pipe is enabled with a bogus port

Message ID 1362670228-19494-1-git-send-email-damien.lespiau@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lespiau, Damien March 7, 2013, 3:30 p.m. UTC
If TRANS_DDI_FUNC_CTL has been wrongly programmed with an incorrect
port, we are currently trying to read PORT_CLK_SEL(port) with an
initialized value.

Handle that case by returning PORT_CLK_SEL_NONE and warning about it.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

to add to first
---
 drivers/gpu/drm/i915/intel_ddi.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

Comments

Paulo Zanoni March 22, 2013, 9:04 p.m. UTC | #1
Hi

2013/3/7 Damien Lespiau <damien.lespiau@intel.com>:
> If TRANS_DDI_FUNC_CTL has been wrongly programmed with an incorrect
> port, we are currently trying to read PORT_CLK_SEL(port) with an
> initialized value.

s/initialized/uninitialized/g

>
> Handle that case by returning PORT_CLK_SEL_NONE and warning about it.
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
>
> to add to first

First!

> ---
>  drivers/gpu/drm/i915/intel_ddi.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 56bb7cb..3b5838d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1157,7 +1157,7 @@ static uint32_t intel_ddi_get_crtc_pll(struct drm_i915_private *dev_priv,
>                                        enum pipe pipe)
>  {
>         uint32_t temp, ret;
> -       enum port port;
> +       enum port port = I915_MAX_PORTS;
>         enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
>                                                                       pipe);
>         int i;
> @@ -1173,7 +1173,10 @@ static uint32_t intel_ddi_get_crtc_pll(struct drm_i915_private *dev_priv,
>                                 port = i;
>         }
>
> -       ret = I915_READ(PORT_CLK_SEL(port));
> +       if (port == I915_MAX_PORTS)
> +               ret = PORT_CLK_SEL_NONE;

I would put the WARN here (see below).

> +       else
> +               ret = I915_READ(PORT_CLK_SEL(port));
>
>         DRM_DEBUG_KMS("Pipe %c connected to port %c using clock 0x%08x\n",
>                       pipe_name(pipe), port_name(port), ret);
> @@ -1207,6 +1210,10 @@ void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
>                 case PORT_CLK_SEL_WRPLL2:
>                         dev_priv->ddi_plls.wrpll2_refcount++;
>                         break;
> +               case PORT_CLK_SEL_NONE:
> +                       WARN(1, "Pipe %c enabled but no clock selected\n",
> +                            pipe_name(pipe));

I would put this WARN inside intel_ddi_get_crtc_pll, because that's
the function that really detects the "problem". Currently the function
has only 1 caller, but this could change in the future, and other
callers should also get their WARNs.


> +                       break;
>                 }
>         }
>  }
> --
> 1.7.7.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 56bb7cb..3b5838d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1157,7 +1157,7 @@  static uint32_t intel_ddi_get_crtc_pll(struct drm_i915_private *dev_priv,
 				       enum pipe pipe)
 {
 	uint32_t temp, ret;
-	enum port port;
+	enum port port = I915_MAX_PORTS;
 	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
 								      pipe);
 	int i;
@@ -1173,7 +1173,10 @@  static uint32_t intel_ddi_get_crtc_pll(struct drm_i915_private *dev_priv,
 				port = i;
 	}
 
-	ret = I915_READ(PORT_CLK_SEL(port));
+	if (port == I915_MAX_PORTS)
+		ret = PORT_CLK_SEL_NONE;
+	else
+		ret = I915_READ(PORT_CLK_SEL(port));
 
 	DRM_DEBUG_KMS("Pipe %c connected to port %c using clock 0x%08x\n",
 		      pipe_name(pipe), port_name(port), ret);
@@ -1207,6 +1210,10 @@  void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
 		case PORT_CLK_SEL_WRPLL2:
 			dev_priv->ddi_plls.wrpll2_refcount++;
 			break;
+		case PORT_CLK_SEL_NONE:
+			WARN(1, "Pipe %c enabled but no clock selected\n",
+			     pipe_name(pipe));
+			break;
 		}
 	}
 }