All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
@ 2016-05-20 21:36 ` Lyude
  0 siblings, 0 replies; 11+ messages in thread
From: Lyude @ 2016-05-20 21:36 UTC (permalink / raw)
  To: intel-gfx
  Cc: Lyude, stable, Daniel Vetter, Jani Nikula, David Airlie,
	open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...),
	linux-kernel@vger.kernel.org (open list)

We no longer call ilk_audio_codec_enable() while we have vblanks
disabled. As such, we can finally fix this and stop the occasional pipe
underruns I'm seeing on this Dell OptiPlex 990.

Cc: stable@vger.kernel.org
Signed-off-by: Lyude <cpaul@redhat.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 7d281b4..0d685fe 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
 	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
-	 * rest of the setup into a vblank work item, queued here, but the
-	 * infrastructure is not there yet.
-	 */
+	/* Need to wait one vblank before enabling audio */
+	intel_wait_for_vblank(connector->dev, pipe);
 
 	if (HAS_PCH_IBX(connector->dev)) {
 		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
@ 2016-05-20 21:36 ` Lyude
  0 siblings, 0 replies; 11+ messages in thread
From: Lyude @ 2016-05-20 21:36 UTC (permalink / raw)
  To: intel-gfx
  Cc: stable, open list:INTEL DRM DRIVERS excluding Poulsbo,
	Moorestow...,
	linux-kernel@vger.kernel.org open list, Daniel Vetter, Lyude

We no longer call ilk_audio_codec_enable() while we have vblanks
disabled. As such, we can finally fix this and stop the occasional pipe
underruns I'm seeing on this Dell OptiPlex 990.

Cc: stable@vger.kernel.org
Signed-off-by: Lyude <cpaul@redhat.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 7d281b4..0d685fe 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
 	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
-	 * rest of the setup into a vblank work item, queued here, but the
-	 * infrastructure is not there yet.
-	 */
+	/* Need to wait one vblank before enabling audio */
+	intel_wait_for_vblank(connector->dev, pipe);
 
 	if (HAS_PCH_IBX(connector->dev)) {
 		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 11+ messages in thread
* ✗ Ro.CI.BAT: failure for drm/i915/ilk: Wait one vblank before enabling audio
  2016-05-20 21:36 ` Lyude
  (?)
@ 2016-05-21  5:21 ` Patchwork
  -1 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2016-05-21  5:21 UTC (permalink / raw)
  To: cpaul; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/ilk: Wait one vblank before enabling audio
URL   : https://patchwork.freedesktop.org/series/7488/
State : failure

== Summary ==

Series 7488v1 drm/i915/ilk: Wait one vblank before enabling audio
http://patchwork.freedesktop.org/api/1.0/series/7488/revisions/1/mbox

Test drv_module_reload_basic:
                pass       -> DMESG-WARN (fi-byt-n2820)
Test gem_exec_gttfill:
        Subgroup basic:
                skip       -> PASS       (fi-hsw-i7-4770k)
                skip       -> PASS       (fi-hsw-i7-4770r)
                skip       -> FAIL       (fi-skl-i7-6700k)
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (fi-byt-n2820)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-byt-n2820)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup hang-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-n2820)
Test kms_sink_crc_basic:
                pass       -> SKIP       (ro-skl-i7-6700hq)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (fi-byt-n2820)

fi-byt-n2820     total:209  pass:148  dwarn:21  dfail:0   fail:2   skip:38 
fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-i7-6700k  total:209  pass:183  dwarn:0   dfail:0   fail:1   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:209  pass:171  dwarn:0   dfail:0   fail:0   skip:38 
ro-bdw-i7-5557U  total:209  pass:196  dwarn:0   dfail:0   fail:0   skip:13 
ro-bdw-i7-5600u  total:209  pass:179  dwarn:0   dfail:0   fail:1   skip:29 
ro-bsw-n3050     total:209  pass:166  dwarn:0   dfail:0   fail:3   skip:40 
ro-byt-n2820     total:209  pass:168  dwarn:0   dfail:0   fail:3   skip:38 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:209  pass:185  dwarn:0   dfail:0   fail:0   skip:24 
ro-ilk-i7-620lm  total:209  pass:145  dwarn:0   dfail:0   fail:1   skip:63 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:209  pass:176  dwarn:0   dfail:0   fail:0   skip:33 
ro-ivb2-i7-3770  total:209  pass:180  dwarn:0   dfail:0   fail:0   skip:29 
ro-skl-i7-6700hq total:204  pass:181  dwarn:0   dfail:0   fail:0   skip:23 
ro-snb-i7-2620M  total:209  pass:170  dwarn:0   dfail:0   fail:1   skip:38 
fi-bdw-i7-5557u failed to connect after reboot
fi-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_953/

f1eaed1 drm-intel-nightly: 2016y-05m-20d-14h-35m-29s UTC integration manifest
376347a drm/i915/ilk: Wait one vblank before enabling audio

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

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
  2016-05-20 21:36 ` Lyude
@ 2016-05-23  6:32   ` Jani Nikula
  -1 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2016-05-23  6:32 UTC (permalink / raw)
  To: Lyude, intel-gfx
  Cc: Lyude, stable, Daniel Vetter, David Airlie,
	open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...),
	linux-kernel@vger.kernel.org (open list),
	Syrjala, Ville

On Sat, 21 May 2016, Lyude <cpaul@redhat.com> wrote:
> We no longer call ilk_audio_codec_enable() while we have vblanks
> disabled. As such, we can finally fix this and stop the occasional pipe
> underruns I'm seeing on this Dell OptiPlex 990.
>
> Cc: stable@vger.kernel.org

Even if this were the right fix now, I'd be wary of adding a blanket cc:
stable to kernels where we still have vblanks disabled when this
function gets called.

Whoever pushes this, please ensure the cc: stable gets dropped from the
commit.

Thanks,
Jani.

> Signed-off-by: Lyude <cpaul@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 7d281b4..0d685fe 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>  	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
> -	 * rest of the setup into a vblank work item, queued here, but the
> -	 * infrastructure is not there yet.
> -	 */
> +	/* Need to wait one vblank before enabling audio */
> +	intel_wait_for_vblank(connector->dev, pipe);
>  
>  	if (HAS_PCH_IBX(connector->dev)) {
>  		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
@ 2016-05-23  6:32   ` Jani Nikula
  0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2016-05-23  6:32 UTC (permalink / raw)
  To: Lyude, intel-gfx
  Cc: Syrjala, Ville, David Airlie,
	open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...),
	linux-kernel@vger.kernel.org (open list),
	stable, Daniel Vetter

On Sat, 21 May 2016, Lyude <cpaul@redhat.com> wrote:
> We no longer call ilk_audio_codec_enable() while we have vblanks
> disabled. As such, we can finally fix this and stop the occasional pipe
> underruns I'm seeing on this Dell OptiPlex 990.
>
> Cc: stable@vger.kernel.org

Even if this were the right fix now, I'd be wary of adding a blanket cc:
stable to kernels where we still have vblanks disabled when this
function gets called.

Whoever pushes this, please ensure the cc: stable gets dropped from the
commit.

Thanks,
Jani.

> Signed-off-by: Lyude <cpaul@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 7d281b4..0d685fe 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>  	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
> -	 * rest of the setup into a vblank work item, queued here, but the
> -	 * infrastructure is not there yet.
> -	 */
> +	/* Need to wait one vblank before enabling audio */
> +	intel_wait_for_vblank(connector->dev, pipe);
>  
>  	if (HAS_PCH_IBX(connector->dev)) {
>  		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);

-- 
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] 11+ messages in thread
* Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
  2016-05-20 21:36 ` Lyude
@ 2016-05-23 18:06   ` Ville Syrjälä
  -1 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2016-05-23 18:06 UTC (permalink / raw)
  To: Lyude
  Cc: intel-gfx, stable, open list:INTEL DRM DRIVERS excluding Poulsbo,
	Moorestow...,
	linux-kernel@vger.kernel.org open list, Daniel Vetter

On Fri, May 20, 2016 at 05:36:40PM -0400, Lyude wrote:
> We no longer call ilk_audio_codec_enable() while we have vblanks
> disabled. As such, we can finally fix this and stop the occasional pipe
> underruns I'm seeing on this Dell OptiPlex 990.

Hmm. Are you still getting underruns on -nightly?

I basically tried this same thing (+ a bunch of other tweaks to the
audio enable sequence) when I was hunting the remaining underruns on
my ILK, but in the end audio was a red herring. So I never found
any real benefit from extra vblank waits in the audio enable sequence.
They did appear to help sometimes, but with a enough repetitions it
still failed.

> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Lyude <cpaul@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 7d281b4..0d685fe 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>  	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
> -	 * rest of the setup into a vblank work item, queued here, but the
> -	 * infrastructure is not there yet.
> -	 */
> +	/* Need to wait one vblank before enabling audio */
> +	intel_wait_for_vblank(connector->dev, pipe);
>  
>  	if (HAS_PCH_IBX(connector->dev)) {
>  		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrj�l�
Intel OTC

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
@ 2016-05-23 18:06   ` Ville Syrjälä
  0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2016-05-23 18:06 UTC (permalink / raw)
  To: Lyude
  Cc: Daniel Vetter, intel-gfx,
	open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
	linux-kernel@vger.kernel.org open list, stable

On Fri, May 20, 2016 at 05:36:40PM -0400, Lyude wrote:
> We no longer call ilk_audio_codec_enable() while we have vblanks
> disabled. As such, we can finally fix this and stop the occasional pipe
> underruns I'm seeing on this Dell OptiPlex 990.

Hmm. Are you still getting underruns on -nightly?

I basically tried this same thing (+ a bunch of other tweaks to the
audio enable sequence) when I was hunting the remaining underruns on
my ILK, but in the end audio was a red herring. So I never found
any real benefit from extra vblank waits in the audio enable sequence.
They did appear to help sometimes, but with a enough repetitions it
still failed.

> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Lyude <cpaul@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 7d281b4..0d685fe 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>  	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
> -	 * rest of the setup into a vblank work item, queued here, but the
> -	 * infrastructure is not there yet.
> -	 */
> +	/* Need to wait one vblank before enabling audio */
> +	intel_wait_for_vblank(connector->dev, pipe);
>  
>  	if (HAS_PCH_IBX(connector->dev)) {
>  		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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] 11+ messages in thread
* Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
  2016-05-23 18:06   ` Ville Syrjälä
@ 2016-05-23 19:30     ` Lyude Paul
  -1 siblings, 0 replies; 11+ messages in thread
From: Lyude Paul @ 2016-05-23 19:30 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, stable, open list:INTEL DRM DRIVERS excluding Poulsbo,
	Moorestow...,
	 linux-kernel@vger.kernel.org open list, Daniel Vetter

On Mon, 2016-05-23 at 21:06 +0300, Ville Syrjälä wrote:
> On Fri, May 20, 2016 at 05:36:40PM -0400, Lyude wrote:
> > 
> > We no longer call ilk_audio_codec_enable() while we have vblanks
> > disabled. As such, we can finally fix this and stop the occasional pipe
> > underruns I'm seeing on this Dell OptiPlex 990.
> Hmm. Are you still getting underruns on -nightly?
For me the problem isn't just underruns, a lot of times the modeset results in a
blank monitor without this fix… afaict waiting for 1 vblank seems to fix the
issue entirely here, but I'll check more thoroughly in a bit and update you if I
manage to get it to underrun.

Cheers,
Lyude

> 
> I basically tried this same thing (+ a bunch of other tweaks to the
> audio enable sequence) when I was hunting the remaining underruns on
> my ILK, but in the end audio was a red herring. So I never found
> any real benefit from extra vblank waits in the audio enable sequence.
> They did appear to help sometimes, but with a enough repetitions it
> still failed.
> 
> > 
> > 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lyude <cpaul@redhat.com>
> > ---
> >  drivers/gpu/drm/i915/intel_audio.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 7d281b4..0d685fe 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector
> > *connector,
> >  	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
> > -	 * rest of the setup into a vblank work item, queued here, but the
> > -	 * infrastructure is not there yet.
> > -	 */
> > +	/* Need to wait one vblank before enabling audio */
> > +	intel_wait_for_vblank(connector->dev, pipe);
> >  
> >  	if (HAS_PCH_IBX(connector->dev)) {
> >  		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);
> > -- 
> > 2.5.5
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
@ 2016-05-23 19:30     ` Lyude Paul
  0 siblings, 0 replies; 11+ messages in thread
From: Lyude Paul @ 2016-05-23 19:30 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Daniel Vetter, intel-gfx,
	open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
	linux-kernel@vger.kernel.org open list, stable

On Mon, 2016-05-23 at 21:06 +0300, Ville Syrjälä wrote:
> On Fri, May 20, 2016 at 05:36:40PM -0400, Lyude wrote:
> > 
> > We no longer call ilk_audio_codec_enable() while we have vblanks
> > disabled. As such, we can finally fix this and stop the occasional pipe
> > underruns I'm seeing on this Dell OptiPlex 990.
> Hmm. Are you still getting underruns on -nightly?
For me the problem isn't just underruns, a lot of times the modeset results in a
blank monitor without this fix… afaict waiting for 1 vblank seems to fix the
issue entirely here, but I'll check more thoroughly in a bit and update you if I
manage to get it to underrun.

Cheers,
Lyude

> 
> I basically tried this same thing (+ a bunch of other tweaks to the
> audio enable sequence) when I was hunting the remaining underruns on
> my ILK, but in the end audio was a red herring. So I never found
> any real benefit from extra vblank waits in the audio enable sequence.
> They did appear to help sometimes, but with a enough repetitions it
> still failed.
> 
> > 
> > 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lyude <cpaul@redhat.com>
> > ---
> >  drivers/gpu/drm/i915/intel_audio.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 7d281b4..0d685fe 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector
> > *connector,
> >  	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
> > -	 * rest of the setup into a vblank work item, queued here, but the
> > -	 * infrastructure is not there yet.
> > -	 */
> > +	/* Need to wait one vblank before enabling audio */
> > +	intel_wait_for_vblank(connector->dev, pipe);
> >  
> >  	if (HAS_PCH_IBX(connector->dev)) {
> >  		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);
> > -- 
> > 2.5.5
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
  2016-05-23 19:30     ` Lyude Paul
@ 2016-05-24  9:36       ` Jani Nikula
  -1 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2016-05-24  9:36 UTC (permalink / raw)
  To: Lyude Paul, Ville Syrjälä
  Cc: Daniel Vetter, intel-gfx,
	open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
	linux-kernel@vger.kernel.org open list, stable

On Mon, 23 May 2016, Lyude Paul <cpaul@redhat.com> wrote:
> On Mon, 2016-05-23 at 21:06 +0300, Ville Syrjälä wrote:
>> On Fri, May 20, 2016 at 05:36:40PM -0400, Lyude wrote:
>> > 
>> > We no longer call ilk_audio_codec_enable() while we have vblanks
>> > disabled. As such, we can finally fix this and stop the occasional pipe
>> > underruns I'm seeing on this Dell OptiPlex 990.
>> Hmm. Are you still getting underruns on -nightly?
> For me the problem isn't just underruns, a lot of times the modeset
> results in a blank monitor without this fix… afaict waiting for 1
> vblank seems to fix the issue entirely here, but I'll check more
> thoroughly in a bit and update you if I manage to get it to underrun.

I suppose the question is, do you see the problem also when you rule out
the audio stuff altogether? I.e. does the vblank wait happen to help
independent of the audio?

BR,
Jani.


>
> Cheers,
> Lyude
>
>> 
>> I basically tried this same thing (+ a bunch of other tweaks to the
>> audio enable sequence) when I was hunting the remaining underruns on
>> my ILK, but in the end audio was a red herring. So I never found
>> any real benefit from extra vblank waits in the audio enable sequence.
>> They did appear to help sometimes, but with a enough repetitions it
>> still failed.
>> 
>> > 
>> > 
>> > Cc: stable@vger.kernel.org
>> > Signed-off-by: Lyude <cpaul@redhat.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_audio.c | 8 ++------
>> >  1 file changed, 2 insertions(+), 6 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> > b/drivers/gpu/drm/i915/intel_audio.c
>> > index 7d281b4..0d685fe 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector
>> > *connector,
>> >  	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
>> > -	 * rest of the setup into a vblank work item, queued here, but the
>> > -	 * infrastructure is not there yet.
>> > -	 */
>> > +	/* Need to wait one vblank before enabling audio */
>> > +	intel_wait_for_vblank(connector->dev, pipe);
>> >  
>> >  	if (HAS_PCH_IBX(connector->dev)) {
>> >  		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);
>> > -- 
>> > 2.5.5
>> > 
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
@ 2016-05-24  9:36       ` Jani Nikula
  0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2016-05-24  9:36 UTC (permalink / raw)
  To: Lyude Paul, Ville Syrjälä
  Cc: Daniel Vetter, intel-gfx,
	open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
	linux-kernel@vger.kernel.org open list, stable

On Mon, 23 May 2016, Lyude Paul <cpaul@redhat.com> wrote:
> On Mon, 2016-05-23 at 21:06 +0300, Ville Syrjälä wrote:
>> On Fri, May 20, 2016 at 05:36:40PM -0400, Lyude wrote:
>> > 
>> > We no longer call ilk_audio_codec_enable() while we have vblanks
>> > disabled. As such, we can finally fix this and stop the occasional pipe
>> > underruns I'm seeing on this Dell OptiPlex 990.
>> Hmm. Are you still getting underruns on -nightly?
> For me the problem isn't just underruns, a lot of times the modeset
> results in a blank monitor without this fix… afaict waiting for 1
> vblank seems to fix the issue entirely here, but I'll check more
> thoroughly in a bit and update you if I manage to get it to underrun.

I suppose the question is, do you see the problem also when you rule out
the audio stuff altogether? I.e. does the vblank wait happen to help
independent of the audio?

BR,
Jani.


>
> Cheers,
> Lyude
>
>> 
>> I basically tried this same thing (+ a bunch of other tweaks to the
>> audio enable sequence) when I was hunting the remaining underruns on
>> my ILK, but in the end audio was a red herring. So I never found
>> any real benefit from extra vblank waits in the audio enable sequence.
>> They did appear to help sometimes, but with a enough repetitions it
>> still failed.
>> 
>> > 
>> > 
>> > Cc: stable@vger.kernel.org
>> > Signed-off-by: Lyude <cpaul@redhat.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_audio.c | 8 ++------
>> >  1 file changed, 2 insertions(+), 6 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> > b/drivers/gpu/drm/i915/intel_audio.c
>> > index 7d281b4..0d685fe 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector
>> > *connector,
>> >  	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
>> > -	 * rest of the setup into a vblank work item, queued here, but the
>> > -	 * infrastructure is not there yet.
>> > -	 */
>> > +	/* Need to wait one vblank before enabling audio */
>> > +	intel_wait_for_vblank(connector->dev, pipe);
>> >  
>> >  	if (HAS_PCH_IBX(connector->dev)) {
>> >  		hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);
>> > -- 
>> > 2.5.5
>> > 
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-05-24  9:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-20 21:36 [PATCH] drm/i915/ilk: Wait one vblank before enabling audio Lyude
2016-05-20 21:36 ` Lyude
2016-05-21  5:21 ` ✗ Ro.CI.BAT: failure for " Patchwork
2016-05-23  6:32 ` [PATCH] " Jani Nikula
2016-05-23  6:32   ` Jani Nikula
2016-05-23 18:06 ` Ville Syrjälä
2016-05-23 18:06   ` Ville Syrjälä
2016-05-23 19:30   ` Lyude Paul
2016-05-23 19:30     ` Lyude Paul
2016-05-24  9:36     ` Jani Nikula
2016-05-24  9:36       ` Jani Nikula

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.