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

[Jani Nikula <jani.nikula@linux.intel.com>]

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