| Message ID | 87mw1j6tgj.fsf@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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)
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)
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 >
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
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)