| Message ID | 1463780200-22813-1-git-send-email-cpaul@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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);
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
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
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
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);
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(-)