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

Message ID 87mw1j6tgj.fsf@intel.com
State New
Headers show

Commit Message

Jani Nikula May 5, 2015, 1:32 p.m. UTC
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(-)

Comments

Sivakumar Thulasimani May 5, 2015, 1:38 p.m. UTC | #1
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)
Sivakumar Thulasimani May 5, 2015, 1:43 p.m. UTC | #2
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)
Jani Nikula May 6, 2015, 9:30 a.m. UTC | #3
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
>
Daniel Vetter May 6, 2015, 11:11 a.m. UTC | #4
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

Patch
diff mbox

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)