All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix MST audio enabling
@ 2016-03-11 15:33 Ander Conselvan de Oliveira
  2016-03-11 15:37 ` Ville Syrjälä
  2016-03-11 17:03 ` ✗ Fi.CI.BAT: failure for " Patchwork
  0 siblings, 2 replies; 5+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-11 15:33 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira, Libin Yang, drm-intel-fixes

Commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") erroneously
added a check for MST encoder to ilk_audio_codec_enable instead of the
hsw+ equivalent. That causes the HDMI path to be used, resulting in a
wrong register value.

Cc: Libin Yang <libin.yang@linux.intel.com>
Cc: drm-intel-fixes@lists.freedesktop.org
Fixes: commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio")
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 30f9214..f5fccf7 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -329,7 +329,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
 	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) ||
+	    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	else
 		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
@@ -476,8 +477,7 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
 	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) ||
-	    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
+	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	else
 		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
-- 
2.4.3

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Fix MST audio enabling
  2016-03-11 15:33 [PATCH] drm/i915: Fix MST audio enabling Ander Conselvan de Oliveira
@ 2016-03-11 15:37 ` Ville Syrjälä
  2016-03-11 15:52   ` Ander Conselvan De Oliveira
  2016-03-11 17:03 ` ✗ Fi.CI.BAT: failure for " Patchwork
  1 sibling, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2016-03-11 15:37 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx, drm-intel-fixes, Libin Yang

On Fri, Mar 11, 2016 at 05:33:06PM +0200, Ander Conselvan de Oliveira wrote:
> Commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") erroneously
> added a check for MST encoder to ilk_audio_codec_enable instead of the
> hsw+ equivalent. That causes the HDMI path to be used, resulting in a
> wrong register value.
> 
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Cc: drm-intel-fixes@lists.freedesktop.org
> Fixes: commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio")
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 30f9214..f5fccf7 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -329,7 +329,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> -	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
> +	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) ||
> +	    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))

Humm. I thought had I asked all of these to be changed to has_dp_encoder.
I wonder if I imagined it or if I was just ignored.

>  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>  	else
>  		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> @@ -476,8 +477,7 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> -	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) ||
> -	    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
> +	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
>  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>  	else
>  		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Fix MST audio enabling
  2016-03-11 15:37 ` Ville Syrjälä
@ 2016-03-11 15:52   ` Ander Conselvan De Oliveira
  2016-03-14  6:40     ` Jani Nikula
  0 siblings, 1 reply; 5+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-03-11 15:52 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, drm-intel-fixes, Libin Yang

On Fri, 2016-03-11 at 17:37 +0200, Ville Syrjälä wrote:
> On Fri, Mar 11, 2016 at 05:33:06PM +0200, Ander Conselvan de Oliveira wrote:
> > Commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") erroneously
> > added a check for MST encoder to ilk_audio_codec_enable instead of the
> > hsw+ equivalent. That causes the HDMI path to be used, resulting in a
> > wrong register value.
> > 
> > Cc: Libin Yang <libin.yang@linux.intel.com>
> > Cc: drm-intel-fixes@lists.freedesktop.org
> > Fixes: commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio")
> > Signed-off-by: Ander Conselvan de Oliveira <
> > ander.conselvan.de.oliveira@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_audio.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 30f9214..f5fccf7 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -329,7 +329,8 @@ static void hsw_audio_codec_enable(struct drm_connector
> > *connector,
> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> >  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> > -	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
> > +	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) ||
> > +	    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
> 
> Humm. I thought had I asked all of these to be changed to has_dp_encoder.
> I wonder if I imagined it or if I was just ignored.

Ah, yes. Libin sent a patch to fix the same bug and you asked for that [1] and
even R-b the follow up version even though it didn't use it everywhere. But it
looks like Daniel didn't merge it since he wanted all uses converted to
has_dp_encoders.

[1] https://patchwork.freedesktop.org/patch/68902/
[2] https://patchwork.freedesktop.org/patch/69525/


> 
> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >  	else
> >  		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> > @@ -476,8 +477,7 @@ static void ilk_audio_codec_enable(struct drm_connector
> > *connector,
> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> >  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> >  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> > -	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) ||
> > -	    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
> > +	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >  	else
> >  		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Fix MST audio enabling
  2016-03-11 15:33 [PATCH] drm/i915: Fix MST audio enabling Ander Conselvan de Oliveira
  2016-03-11 15:37 ` Ville Syrjälä
@ 2016-03-11 17:03 ` Patchwork
  1 sibling, 0 replies; 5+ messages in thread
From: Patchwork @ 2016-03-11 17:03 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Fix MST audio enabling
URL   : https://patchwork.freedesktop.org/series/4370/
State : failure

== Summary ==

Series 4370v1 drm/i915: Fix MST audio enabling
http://patchwork.freedesktop.org/api/1.0/series/4370/revisions/1/mbox/

Test kms_addfb_basic:
        Subgroup clobberred-modifier:
                incomplete -> PASS       (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-plain-flip:
                pass       -> INCOMPLETE (bsw-nuc-2)
                pass       -> DMESG-WARN (hsw-gt2)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-c:
                pass       -> DMESG-WARN (hsw-gt2)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                pass       -> DMESG-WARN (snb-x220t)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-fail -> FAIL       (snb-x220t)
                dmesg-warn -> PASS       (byt-nuc)

bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:194  pass:173  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:4    pass:1    dwarn:0   dfail:0   fail:0   skip:2  
byt-nuc          total:194  pass:155  dwarn:4   dfail:0   fail:0   skip:35 
hsw-brixbox      total:194  pass:170  dwarn:2   dfail:0   fail:0   skip:22 
hsw-gt2          total:194  pass:175  dwarn:2   dfail:0   fail:0   skip:17 
ivb-t430s        total:194  pass:169  dwarn:0   dfail:0   fail:0   skip:25 
skl-i5k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:194  pass:183  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:194  pass:159  dwarn:1   dfail:0   fail:0   skip:34 
snb-x220t        total:194  pass:159  dwarn:1   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1582/

571147d0be8b04cbbe99db761e82ef105c6f82ad drm-intel-nightly: 2016y-03m-11d-13h-31m-03s UTC integration manifest
b8a315fba364c1a6c9fb2986b6fad6c6afb23686 drm/i915: Fix MST audio enabling

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Fix MST audio enabling
  2016-03-11 15:52   ` Ander Conselvan De Oliveira
@ 2016-03-14  6:40     ` Jani Nikula
  0 siblings, 0 replies; 5+ messages in thread
From: Jani Nikula @ 2016-03-14  6:40 UTC (permalink / raw)
  To: Ander Conselvan De Oliveira, Ville Syrjälä
  Cc: intel-gfx, drm-intel-fixes, Libin Yang

On Fri, 11 Mar 2016, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote:
> [ text/plain ]
> On Fri, 2016-03-11 at 17:37 +0200, Ville Syrjälä wrote:
>> On Fri, Mar 11, 2016 at 05:33:06PM +0200, Ander Conselvan de Oliveira wrote:
>> > Commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") erroneously
>> > added a check for MST encoder to ilk_audio_codec_enable instead of the
>> > hsw+ equivalent. That causes the HDMI path to be used, resulting in a
>> > wrong register value.
>> > 
>> > Cc: Libin Yang <libin.yang@linux.intel.com>
>> > Cc: drm-intel-fixes@lists.freedesktop.org
>> > Fixes: commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio")
>> > Signed-off-by: Ander Conselvan de Oliveira <
>> > ander.conselvan.de.oliveira@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_audio.c | 6 +++---
>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> > b/drivers/gpu/drm/i915/intel_audio.c
>> > index 30f9214..f5fccf7 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -329,7 +329,8 @@ static void hsw_audio_codec_enable(struct drm_connector
>> > *connector,
>> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> >  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
>> > -	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
>> > +	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) ||
>> > +	    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
>> 
>> Humm. I thought had I asked all of these to be changed to has_dp_encoder.
>> I wonder if I imagined it or if I was just ignored.
>
> Ah, yes. Libin sent a patch to fix the same bug and you asked for that [1] and
> even R-b the follow up version even though it didn't use it everywhere. But it
> looks like Daniel didn't merge it since he wanted all uses converted to
> has_dp_encoders.

We want the minimal fix first now because by bikeshedding we let the bug
slip to v4.5, and we need cc: stable on this.

BR,
Jani.


>
> [1] https://patchwork.freedesktop.org/patch/68902/
> [2] https://patchwork.freedesktop.org/patch/69525/
>
>
>> 
>> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >  	else
>> >  		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
>> > @@ -476,8 +477,7 @@ static void ilk_audio_codec_enable(struct drm_connector
>> > *connector,
>> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> >  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>> >  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
>> > -	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) ||
>> > -	    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
>> > +	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
>> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >  	else
>> >  		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
>> > -- 
>> > 2.4.3
>> > 
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-03-14  6:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-11 15:33 [PATCH] drm/i915: Fix MST audio enabling Ander Conselvan de Oliveira
2016-03-11 15:37 ` Ville Syrjälä
2016-03-11 15:52   ` Ander Conselvan De Oliveira
2016-03-14  6:40     ` Jani Nikula
2016-03-11 17:03 ` ✗ Fi.CI.BAT: failure for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.