From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio Date: Tue, 24 May 2016 12:36:18 +0300 Message-ID: <87h9dndch9.fsf@intel.com> References: <1463780200-22813-1-git-send-email-cpaul@redhat.com> <20160523180623.GU4329@intel.com> <1464031815.26826.2.camel@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <1464031815.26826.2.camel@redhat.com> Sender: stable-owner@vger.kernel.org To: Lyude Paul , Ville =?utf-8?B?U3lyasOkbMOk?= Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, "open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list" , stable@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org On Mon, 23 May 2016, Lyude Paul wrote: > On Mon, 2016-05-23 at 21:06 +0300, Ville Syrj=C3=A4l=C3=A4 wrote: >> On Fri, May 20, 2016 at 05:36:40PM -0400, Lyude wrote: >> >=20 >> > 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=E2=80=A6 afaict waiting f= or 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 ou= t the audio stuff altogether? I.e. does the vblank wait happen to help independent of the audio? BR, Jani. > > Cheers, > Lyude > >>=20 >> 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 sequenc= e. >> They did appear to help sometimes, but with a enough repetitions it >> still failed. >>=20 >> >=20 >> >=20 >> > Cc: stable@vger.kernel.org >> > Signed-off-by: Lyude >> > --- >> > =C2=A0drivers/gpu/drm/i915/intel_audio.c | 8 ++------ >> > =C2=A01 file changed, 2 insertions(+), 6 deletions(-) >> >=20 >> > 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, >> > =C2=A0 if (WARN_ON(port =3D=3D PORT_A)) >> > =C2=A0 return; >> > =C2=A0 >> > - /* >> > - =C2=A0* FIXME: We're supposed to wait for vblank here, but we ha= ve >> > vblanks >> > - =C2=A0* disabled during the mode set. The proper fix would be to= push >> > the >> > - =C2=A0* rest of the setup into a vblank work item, queued here, = but the >> > - =C2=A0* infrastructure is not there yet. >> > - =C2=A0*/ >> > + /* Need to wait one vblank before enabling audio */ >> > + intel_wait_for_vblank(connector->dev, pipe); >> > =C2=A0 >> > =C2=A0 if (HAS_PCH_IBX(connector->dev)) { >> > =C2=A0 hdmiw_hdmiedid =3D IBX_HDMIW_HDMIEDID(pipe); >> > --=C2=A0 >> > 2.5.5 >> >=20 >> > _______________________________________________ >> > 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 --=20 Jani Nikula, Intel Open Source Technology Center From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com ([192.55.52.93]:48570 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752269AbcEXJgW convert rfc822-to-8bit (ORCPT ); Tue, 24 May 2016 05:36:22 -0400 From: Jani Nikula To: Lyude Paul , Ville =?utf-8?B?U3lyasOkbMOk?= Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, "open list\:INTEL DRM DRIVERS excluding Poulsbo\, Moorestow...\, linux-kernel\@vger.kernel.org open list" , stable@vger.kernel.org Subject: Re: [PATCH] drm/i915/ilk: Wait one vblank before enabling audio In-Reply-To: <1464031815.26826.2.camel@redhat.com> References: <1463780200-22813-1-git-send-email-cpaul@redhat.com> <20160523180623.GU4329@intel.com> <1464031815.26826.2.camel@redhat.com> Date: Tue, 24 May 2016 12:36:18 +0300 Message-ID: <87h9dndch9.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: stable-owner@vger.kernel.org List-ID: On Mon, 23 May 2016, Lyude Paul 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 >> > --- >> >  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