[2/2] drm/i915/audio: do not mess with audio registers if port is invalid
diff mbox

Message ID 1430749249-15565-2-git-send-email-jani.nikula@intel.com
State New
Headers show

Commit Message

Jani Nikula May 4, 2015, 2:20 p.m. UTC
We should no longer enter the codec enable/disable functions in question
with port A anyway, but to err on the safe side, keep the warnings. Just
bail out early without messing with the registers.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

Comments

Shuang He May 4, 2015, 8:45 p.m. UTC | #1
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6312
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  316/316              316/316
IVB                                  342/342              342/342
BYT                                  286/286              286/286
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
Sivakumar Thulasimani May 5, 2015, 5:33 a.m. UTC | #2
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

On 5/4/2015 7:50 PM, Jani Nikula wrote:
> We should no longer enter the codec enable/disable functions in question
> with port A anyway, but to err on the safe side, keep the warnings. Just
> bail out early without messing with the registers.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_audio.c | 20 ++++++++------------
>   1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index f72e93a45e11..c4312177b0ee 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -269,6 +269,9 @@ static void ilk_audio_codec_disable(struct intel_encoder *encoder)
>   	DRM_DEBUG_KMS("Disable audio codec on port %c, pipe %c\n",
>   		      port_name(port), pipe_name(pipe));
>   
> +	if (WARN_ON(port == PORT_A))
> +		return;
> +
>   	if (HAS_PCH_IBX(dev_priv->dev)) {
>   		aud_config = IBX_AUD_CFG(pipe);
>   		aud_cntrl_st2 = IBX_AUD_CNTL_ST2;
> @@ -290,12 +293,7 @@ static void ilk_audio_codec_disable(struct intel_encoder *encoder)
>   		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>   	I915_WRITE(aud_config, tmp);
>   
> -	if (WARN_ON(!port)) {
> -		eldv = IBX_ELD_VALID(PORT_B) | IBX_ELD_VALID(PORT_C) |
> -			IBX_ELD_VALID(PORT_D);
> -	} else {
> -		eldv = IBX_ELD_VALID(port);
> -	}
> +	eldv = IBX_ELD_VALID(port);
>   
>   	/* Invalidate ELD */
>   	tmp = I915_READ(aud_cntrl_st2);
> @@ -325,6 +323,9 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>   	DRM_DEBUG_KMS("Enable audio codec on port %c, pipe %c, %u bytes ELD\n",
>   		      port_name(port), pipe_name(pipe), drm_eld_size(eld));
>   
> +	if (WARN_ON(port == PORT_A))
> +		return;
> +
>   	/*
>   	 * FIXME: We're supposed to wait for vblank here, but we have vblanks
>   	 * disabled during the mode set. The proper fix would be to push the
> @@ -349,12 +350,7 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>   		aud_cntrl_st2 = CPT_AUD_CNTRL_ST2;
>   	}
>   
> -	if (WARN_ON(!port)) {
> -		eldv = IBX_ELD_VALID(PORT_B) | IBX_ELD_VALID(PORT_C) |
> -			IBX_ELD_VALID(PORT_D);
> -	} else {
> -		eldv = IBX_ELD_VALID(port);
> -	}
> +	eldv = IBX_ELD_VALID(port);
>   
>   	/* Invalidate ELD */
>   	tmp = I915_READ(aud_cntrl_st2);
Daniel Vetter May 6, 2015, 11:11 a.m. UTC | #3
On Tue, May 05, 2015 at 11:03:28AM +0530, Sivakumar Thulasimani wrote:
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> 
> On 5/4/2015 7:50 PM, Jani Nikula wrote:
> >We should no longer enter the codec enable/disable functions in question
> >with port A anyway, but to err on the safe side, keep the warnings. Just
> >bail out early without messing with the registers.
> >
> >Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> >---
> >  drivers/gpu/drm/i915/intel_audio.c | 20 ++++++++------------
> >  1 file changed, 8 insertions(+), 12 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> >index f72e93a45e11..c4312177b0ee 100644
> >--- a/drivers/gpu/drm/i915/intel_audio.c
> >+++ b/drivers/gpu/drm/i915/intel_audio.c
> >@@ -269,6 +269,9 @@ static void ilk_audio_codec_disable(struct intel_encoder *encoder)
> >  	DRM_DEBUG_KMS("Disable audio codec on port %c, pipe %c\n",
> >  		      port_name(port), pipe_name(pipe));
> >+	if (WARN_ON(port == PORT_A))
> >+		return;
> >+
> >  	if (HAS_PCH_IBX(dev_priv->dev)) {
> >  		aud_config = IBX_AUD_CFG(pipe);
> >  		aud_cntrl_st2 = IBX_AUD_CNTL_ST2;
> >@@ -290,12 +293,7 @@ static void ilk_audio_codec_disable(struct intel_encoder *encoder)
> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >  	I915_WRITE(aud_config, tmp);
> >-	if (WARN_ON(!port)) {
> >-		eldv = IBX_ELD_VALID(PORT_B) | IBX_ELD_VALID(PORT_C) |
> >-			IBX_ELD_VALID(PORT_D);
> >-	} else {
> >-		eldv = IBX_ELD_VALID(port);
> >-	}
> >+	eldv = IBX_ELD_VALID(port);
> >  	/* Invalidate ELD */
> >  	tmp = I915_READ(aud_cntrl_st2);
> >@@ -325,6 +323,9 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
> >  	DRM_DEBUG_KMS("Enable audio codec on port %c, pipe %c, %u bytes ELD\n",
> >  		      port_name(port), pipe_name(pipe), drm_eld_size(eld));
> >+	if (WARN_ON(port == PORT_A))
> >+		return;
> >+
> >  	/*
> >  	 * FIXME: We're supposed to wait for vblank here, but we have vblanks
> >  	 * disabled during the mode set. The proper fix would be to push the
> >@@ -349,12 +350,7 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
> >  		aud_cntrl_st2 = CPT_AUD_CNTRL_ST2;
> >  	}
> >-	if (WARN_ON(!port)) {
> >-		eldv = IBX_ELD_VALID(PORT_B) | IBX_ELD_VALID(PORT_C) |
> >-			IBX_ELD_VALID(PORT_D);
> >-	} else {
> >-		eldv = IBX_ELD_VALID(port);
> >-	}
> >+	eldv = IBX_ELD_VALID(port);
> >  	/* Invalidate ELD */
> >  	tmp = I915_READ(aud_cntrl_st2);
> 
> -- 
> regards,
> Sivakumar
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index f72e93a45e11..c4312177b0ee 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -269,6 +269,9 @@  static void ilk_audio_codec_disable(struct intel_encoder *encoder)
 	DRM_DEBUG_KMS("Disable audio codec on port %c, pipe %c\n",
 		      port_name(port), pipe_name(pipe));
 
+	if (WARN_ON(port == PORT_A))
+		return;
+
 	if (HAS_PCH_IBX(dev_priv->dev)) {
 		aud_config = IBX_AUD_CFG(pipe);
 		aud_cntrl_st2 = IBX_AUD_CNTL_ST2;
@@ -290,12 +293,7 @@  static void ilk_audio_codec_disable(struct intel_encoder *encoder)
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	I915_WRITE(aud_config, tmp);
 
-	if (WARN_ON(!port)) {
-		eldv = IBX_ELD_VALID(PORT_B) | IBX_ELD_VALID(PORT_C) |
-			IBX_ELD_VALID(PORT_D);
-	} else {
-		eldv = IBX_ELD_VALID(port);
-	}
+	eldv = IBX_ELD_VALID(port);
 
 	/* Invalidate ELD */
 	tmp = I915_READ(aud_cntrl_st2);
@@ -325,6 +323,9 @@  static void ilk_audio_codec_enable(struct drm_connector *connector,
 	DRM_DEBUG_KMS("Enable audio codec on port %c, pipe %c, %u bytes ELD\n",
 		      port_name(port), pipe_name(pipe), drm_eld_size(eld));
 
+	if (WARN_ON(port == PORT_A))
+		return;
+
 	/*
 	 * FIXME: We're supposed to wait for vblank here, but we have vblanks
 	 * disabled during the mode set. The proper fix would be to push the
@@ -349,12 +350,7 @@  static void ilk_audio_codec_enable(struct drm_connector *connector,
 		aud_cntrl_st2 = CPT_AUD_CNTRL_ST2;
 	}
 
-	if (WARN_ON(!port)) {
-		eldv = IBX_ELD_VALID(PORT_B) | IBX_ELD_VALID(PORT_C) |
-			IBX_ELD_VALID(PORT_D);
-	} else {
-		eldv = IBX_ELD_VALID(port);
-	}
+	eldv = IBX_ELD_VALID(port);
 
 	/* Invalidate ELD */
 	tmp = I915_READ(aud_cntrl_st2);