[PATCH 1/2] drm/i915/dp: there is no audio on port A on non-DDI platforms

[PATCH 1/2] drm/i915/dp: there is no audio on port A on non-DDI platforms

[Jani Nikula]

The eDP port A register on PCH split platforms has a slightly different
register layout from the other ports, with bit 6 being either alternate
scrambler reset or reserved, depending on the generation. Our
misinterpretation of the bit as audio has lead to warnings.

Fix this by not enabling audio on port A on non-DDI platforms. The check
could be made more specific, but this corresponds to the audio codec
enabling code which emits the warning for port A.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89958
Reported-and-tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 937ba31d8dde..fa71fb4d6542 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1355,7 +1355,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->has_dp_encoder = true;
 	pipe_config->has_drrs = false;
-	pipe_config->has_audio = intel_dp->has_audio;
+	if (HAS_DDI(dev) || port != PORT_A)
+		pipe_config->has_audio = intel_dp->has_audio;
 
 	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
 		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
@@ -2228,7 +2229,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	int dotclock;
 
 	tmp = I915_READ(intel_dp->output_reg);
-	if (tmp & DP_AUDIO_OUTPUT_ENABLE)
+	if (port != PORT_A && tmp & DP_AUDIO_OUTPUT_ENABLE)
 		pipe_config->has_audio = true;
 
 	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
-- 
2.1.4

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

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

[Jani Nikula]

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);
-- 
2.1.4

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

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

[shuang.he]

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 '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dp: there is no audio on port A on non-DDI platforms

[Sivakumar Thulasimani]

On 5/4/2015 7:50 PM, Jani Nikula wrote:
> The eDP port A register on PCH split platforms has a slightly different
> register layout from the other ports, with bit 6 being either alternate
> scrambler reset or reserved, depending on the generation. Our
> misinterpretation of the bit as audio has lead to warnings.
>
> Fix this by not enabling audio on port A on non-DDI platforms. The check
> could be made more specific, but this corresponds to the audio codec
> enabling code which emits the warning for port A.
shouldn't we disable audio for eDP on all platforms , including DDI ones ?
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89958
> Reported-and-tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 937ba31d8dde..fa71fb4d6542 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1355,7 +1355,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>   
>   	pipe_config->has_dp_encoder = true;
>   	pipe_config->has_drrs = false;
> -	pipe_config->has_audio = intel_dp->has_audio;
> +	if (HAS_DDI(dev) || port != PORT_A)
> +		pipe_config->has_audio = intel_dp->has_audio;
>   
this still enables audio for PORT_A on for HSW/BDW/etc .  the following 
is better IMO

+	if (!is_edp(encoder))
+		pipe_config->has_audio = intel_dp->has_audio;



-- 
regards,
Sivakumar

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

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

[Sivakumar Thulasimani]

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);

-- 
regards,
Sivakumar

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

Re: [PATCH 1/2] drm/i915/dp: there is no audio on port A on non-DDI platforms

[Jani Nikula]

On Tue, 05 May 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> On 5/4/2015 7:50 PM, Jani Nikula wrote:
>> The eDP port A register on PCH split platforms has a slightly different
>> register layout from the other ports, with bit 6 being either alternate
>> scrambler reset or reserved, depending on the generation. Our
>> misinterpretation of the bit as audio has lead to warnings.
>>
>> Fix this by not enabling audio on port A on non-DDI platforms. The check
>> could be made more specific, but this corresponds to the audio codec
>> enabling code which emits the warning for port A.
> shouldn't we disable audio for eDP on all platforms , including DDI ones ?
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89958
>> Reported-and-tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 937ba31d8dde..fa71fb4d6542 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1355,7 +1355,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>   
>>   	pipe_config->has_dp_encoder = true;
>>   	pipe_config->has_drrs = false;
>> -	pipe_config->has_audio = intel_dp->has_audio;
>> +	if (HAS_DDI(dev) || port != PORT_A)
>> +		pipe_config->has_audio = intel_dp->has_audio;
>>   
> this still enables audio for PORT_A on for HSW/BDW/etc .  the following 
> is better IMO
>
> +	if (!is_edp(encoder))
> +		pipe_config->has_audio = intel_dp->has_audio;

I wanted the check to match what's in intel_audio.c for these
platforms. And those only need the pipe.

Perhaps it's a theoretical question, but is it impossible for edp to
have audio?

Jani.




>
>
>
> -- 
> regards,
> Sivakumar
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dp: there is no audio on port A on non-DDI platforms

[Sivakumar Thulasimani]

two points
1) The eDP spec says Audio is optional so it is allowed to have audio, 
but i am yet to come across any eDP panel that supports Audio.
2) Also, there is no support for audio in DDI A port as well :)

So please change the check to

if (!is_edp(encoder))

On 5/5/2015 2:39 PM, Jani Nikula wrote:
> On Tue, 05 May 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>> On 5/4/2015 7:50 PM, Jani Nikula wrote:
>>> The eDP port A register on PCH split platforms has a slightly different
>>> register layout from the other ports, with bit 6 being either alternate
>>> scrambler reset or reserved, depending on the generation. Our
>>> misinterpretation of the bit as audio has lead to warnings.
>>>
>>> Fix this by not enabling audio on port A on non-DDI platforms. The check
>>> could be made more specific, but this corresponds to the audio codec
>>> enabling code which emits the warning for port A.
>> shouldn't we disable audio for eDP on all platforms , including DDI ones ?
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89958
>>> Reported-and-tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_dp.c | 5 +++--
>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 937ba31d8dde..fa71fb4d6542 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -1355,7 +1355,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>>    
>>>    	pipe_config->has_dp_encoder = true;
>>>    	pipe_config->has_drrs = false;
>>> -	pipe_config->has_audio = intel_dp->has_audio;
>>> +	if (HAS_DDI(dev) || port != PORT_A)
>>> +		pipe_config->has_audio = intel_dp->has_audio;
>>>    
>> this still enables audio for PORT_A on for HSW/BDW/etc .  the following
>> is better IMO
>>
>> +	if (!is_edp(encoder))
>> +		pipe_config->has_audio = intel_dp->has_audio;
> I wanted the check to match what's in intel_audio.c for these
> platforms. And those only need the pipe.
>
> Perhaps it's a theoretical question, but is it impossible for edp to
> have audio?
>
> Jani.
>
>
>
>
>>
>>
>> -- 
>> regards,
>> Sivakumar
>>

-- 
regards,
Sivakumar

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

Re: [PATCH 1/2] drm/i915/dp: there is no audio on port A on non-DDI platforms

[Jani Nikula]

On Tue, 05 May 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> two points
> 1) The eDP spec says Audio is optional so it is allowed to have audio, 
> but i am yet to come across any eDP panel that supports Audio.
> 2) Also, there is no support for audio in DDI A port as well :)
>
> So please change the check to
>
> if (!is_edp(encoder))

Because the actual limitation we have is "no audio on port A" instead of
"no audio on eDP", I insist we check for port A. Patch below.

BR,
Jani.


From ca5d2444065ac91db3ad6cfcdbf8a012a4d44556 Mon Sep 17 00:00:00 2001
From: Jani Nikula <jani.nikula@intel.com>
Date: Thu, 9 Apr 2015 12:41:53 +0300
Subject: [PATCH] drm/i915/dp: there is no audio on port A
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Cc: Jani Nikula <jani.nikula@intel.com>

The eDP port A register on PCH split platforms has a slightly different
register layout from the other ports, with bit 6 being either alternate
scrambler reset or reserved, depending on the generation. Our
misinterpretation of the bit as audio has lead to warning.

Fix this by not enabling audio on port A, since none of our platforms
support audio on port A anyway.

v2: DDI doesn't have audio on port A either (Sivakumar Thulasimani)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89958
Reported-and-tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index caeb6ed99ad2..b61bf9b2f76f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1361,7 +1361,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->has_dp_encoder = true;
 	pipe_config->has_drrs = false;
-	pipe_config->has_audio = intel_dp->has_audio;
+	pipe_config->has_audio = intel_dp->has_audio && port != PORT_A;
 
 	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
 		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
@@ -2234,8 +2234,8 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	int dotclock;
 
 	tmp = I915_READ(intel_dp->output_reg);
-	if (tmp & DP_AUDIO_OUTPUT_ENABLE)
-		pipe_config->has_audio = true;
+
+	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
 
 	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
 		if (tmp & DP_SYNC_HS_HIGH)
-- 
2.1.4


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dp: there is no audio on port A on non-DDI platforms

[Sivakumar Thulasimani]

sure, you can check for port A alone then. DDI A will have edp in all 
SKUs so checking for eDP should ideally be the same as DDIA.

On 5/5/2015 7:02 PM, Jani Nikula wrote:
> On Tue, 05 May 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>> two points
>> 1) The eDP spec says Audio is optional so it is allowed to have audio,
>> but i am yet to come across any eDP panel that supports Audio.
>> 2) Also, there is no support for audio in DDI A port as well :)
>>
>> So please change the check to
>>
>> if (!is_edp(encoder))
> Because the actual limitation we have is "no audio on port A" instead of
> "no audio on eDP", I insist we check for port A. Patch below.
>
> BR,
> Jani.
>
>
>  From ca5d2444065ac91db3ad6cfcdbf8a012a4d44556 Mon Sep 17 00:00:00 2001
> From: Jani Nikula <jani.nikula@intel.com>
> Date: Thu, 9 Apr 2015 12:41:53 +0300
> Subject: [PATCH] drm/i915/dp: there is no audio on port A
> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
> Cc: Jani Nikula <jani.nikula@intel.com>
>
> The eDP port A register on PCH split platforms has a slightly different
> register layout from the other ports, with bit 6 being either alternate
> scrambler reset or reserved, depending on the generation. Our
> misinterpretation of the bit as audio has lead to warning.
>
> Fix this by not enabling audio on port A, since none of our platforms
> support audio on port A anyway.
>
> v2: DDI doesn't have audio on port A either (Sivakumar Thulasimani)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89958
> Reported-and-tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index caeb6ed99ad2..b61bf9b2f76f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1361,7 +1361,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>   
>   	pipe_config->has_dp_encoder = true;
>   	pipe_config->has_drrs = false;
> -	pipe_config->has_audio = intel_dp->has_audio;
> +	pipe_config->has_audio = intel_dp->has_audio && port != PORT_A;
>   
>   	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>   		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> @@ -2234,8 +2234,8 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>   	int dotclock;
>   
>   	tmp = I915_READ(intel_dp->output_reg);
> -	if (tmp & DP_AUDIO_OUTPUT_ENABLE)
> -		pipe_config->has_audio = true;
> +
> +	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
>   
>   	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
>   		if (tmp & DP_SYNC_HS_HIGH)

-- 
regards,
Sivakumar

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

Re: [PATCH 1/2] drm/i915/dp: there is no audio on port A on non-DDI platforms

[Sivakumar Thulasimani]

[-- Attachment #1.1: Type: text/plain, Size: 2848 bytes --]

missed the changes attached. so adding rb tag

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>


On 5/5/2015 7:02 PM, Jani Nikula wrote:
> On Tue, 05 May 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>> two points
>> 1) The eDP spec says Audio is optional so it is allowed to have audio,
>> but i am yet to come across any eDP panel that supports Audio.
>> 2) Also, there is no support for audio in DDI A port as well :)
>>
>> So please change the check to
>>
>> if (!is_edp(encoder))
> Because the actual limitation we have is "no audio on port A" instead of
> "no audio on eDP", I insist we check for port A. Patch below.
>
> BR,
> Jani.
>
>
>  From ca5d2444065ac91db3ad6cfcdbf8a012a4d44556 Mon Sep 17 00:00:00 2001
> From: Jani Nikula <jani.nikula@intel.com>
> Date: Thu, 9 Apr 2015 12:41:53 +0300
> Subject: [PATCH] drm/i915/dp: there is no audio on port A
> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
> Cc: Jani Nikula <jani.nikula@intel.com>
>
> The eDP port A register on PCH split platforms has a slightly different
> register layout from the other ports, with bit 6 being either alternate
> scrambler reset or reserved, depending on the generation. Our
> misinterpretation of the bit as audio has lead to warning.
>
> Fix this by not enabling audio on port A, since none of our platforms
> support audio on port A anyway.
>
> v2: DDI doesn't have audio on port A either (Sivakumar Thulasimani)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89958
> Reported-and-tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index caeb6ed99ad2..b61bf9b2f76f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1361,7 +1361,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>   
>   	pipe_config->has_dp_encoder = true;
>   	pipe_config->has_drrs = false;
> -	pipe_config->has_audio = intel_dp->has_audio;
> +	pipe_config->has_audio = intel_dp->has_audio && port != PORT_A;
>   
>   	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>   		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> @@ -2234,8 +2234,8 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>   	int dotclock;
>   
>   	tmp = I915_READ(intel_dp->output_reg);
> -	if (tmp & DP_AUDIO_OUTPUT_ENABLE)
> -		pipe_config->has_audio = true;
> +
> +	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
>   
>   	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
>   		if (tmp & DP_SYNC_HS_HIGH)

-- 
regards,
Sivakumar


[-- Attachment #1.2: Type: text/html, Size: 4091 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 1/2] drm/i915/dp: there is no audio on port A on non-DDI platforms

[Jani Nikula]

On Tue, 05 May 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> missed the changes attached. so adding rb tag
>
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

Pardon my stubbornness, and thanks for the review! Pushed to
drm-intel-fixes.

BR,
Jani.


>
>
> On 5/5/2015 7:02 PM, Jani Nikula wrote:
>> On Tue, 05 May 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>>> two points
>>> 1) The eDP spec says Audio is optional so it is allowed to have audio,
>>> but i am yet to come across any eDP panel that supports Audio.
>>> 2) Also, there is no support for audio in DDI A port as well :)
>>>
>>> So please change the check to
>>>
>>> if (!is_edp(encoder))
>> Because the actual limitation we have is "no audio on port A" instead of
>> "no audio on eDP", I insist we check for port A. Patch below.
>>
>> BR,
>> Jani.
>>
>>
>>  From ca5d2444065ac91db3ad6cfcdbf8a012a4d44556 Mon Sep 17 00:00:00 2001
>> From: Jani Nikula <jani.nikula@intel.com>
>> Date: Thu, 9 Apr 2015 12:41:53 +0300
>> Subject: [PATCH] drm/i915/dp: there is no audio on port A
>> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
>> Cc: Jani Nikula <jani.nikula@intel.com>
>>
>> The eDP port A register on PCH split platforms has a slightly different
>> register layout from the other ports, with bit 6 being either alternate
>> scrambler reset or reserved, depending on the generation. Our
>> misinterpretation of the bit as audio has lead to warning.
>>
>> Fix this by not enabling audio on port A, since none of our platforms
>> support audio on port A anyway.
>>
>> v2: DDI doesn't have audio on port A either (Sivakumar Thulasimani)
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89958
>> Reported-and-tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index caeb6ed99ad2..b61bf9b2f76f 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1361,7 +1361,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>   
>>   	pipe_config->has_dp_encoder = true;
>>   	pipe_config->has_drrs = false;
>> -	pipe_config->has_audio = intel_dp->has_audio;
>> +	pipe_config->has_audio = intel_dp->has_audio && port != PORT_A;
>>   
>>   	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>>   		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
>> @@ -2234,8 +2234,8 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>>   	int dotclock;
>>   
>>   	tmp = I915_READ(intel_dp->output_reg);
>> -	if (tmp & DP_AUDIO_OUTPUT_ENABLE)
>> -		pipe_config->has_audio = true;
>> +
>> +	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
>>   
>>   	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
>>   		if (tmp & DP_SYNC_HS_HIGH)
>
> -- 
> regards,
> Sivakumar
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dp: there is no audio on port A on non-DDI platforms

[Daniel Vetter]

On Tue, May 05, 2015 at 04:32:12PM +0300, Jani Nikula wrote:
> On Tue, 05 May 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> > two points
> > 1) The eDP spec says Audio is optional so it is allowed to have audio, 
> > but i am yet to come across any eDP panel that supports Audio.
> > 2) Also, there is no support for audio in DDI A port as well :)
> >
> > So please change the check to
> >
> > if (!is_edp(encoder))
> 
> Because the actual limitation we have is "no audio on port A" instead of
> "no audio on eDP", I insist we check for port A. Patch below.

Yes this is the correct check. On desktop all-in-ones edp is usually on
port D, which does support audio.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx