[PATCH] drm/i915/ilk: Wait one vblank before enabling audio

[PATCH] drm/i915/ilk: Wait one vblank before enabling audio

[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

✗ Ro.CI.BAT: failure for drm/i915/ilk: Wait one vblank before enabling audio

[Patchwork]

== 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

Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio

[Jani Nikula]

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

Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio

[Ville Syrjälä]

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

Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio

[Lyude Paul]

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

Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio

[Jani Nikula]

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