2013/9/22 Lin, Mengdong <mengdong.lin@xxxxxxxxx>: > Hi Daniel and Paulo, > > I think we need a confirmation from HW owner whether vblank wait can be skipped in audio enabling and disabling. I'll ping HW owner and involve you. > The original code just follows b-spec but there is no explanation why. Hi, any news on this? Please notice that even though the spec says we need to wait for a vblank, this code runs on a place where the pipe is disabled, so there's no vblank to wait. Bspec also suggests Audio should only be enabled at the very end of the mode set sequence, so the "proper" fix would probably be to move the place where haswell_write_eld is called. While this is not really done, I think we could probably merge this patch because the 50ms delay here seems pointless. Thanks, Paulo > > Thanks > Mengdong > >> -----Original Message----- >> From: intel-gfx-bounces+mengdong.lin=intel.com@xxxxxxxxxxxxxxxxxxxxx >> [mailto:intel-gfx-bounces+mengdong.lin=intel.com@xxxxxxxxxxxxxxxxxxxxx] On >> Behalf Of Daniel Vetter >> Sent: Friday, September 20, 2013 4:18 PM >> To: Paulo Zanoni >> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Zanoni, Paulo R >> Subject: Re: [PATCH 4/4] drm/i915: skip useless vblank wait on >> Haswell audio sequence >> >> On Thu, Sep 19, 2013 at 05:07:29PM -0300, Paulo Zanoni wrote: >> > From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> >> > >> > We call haswell_write_eld at mode_set time, not at crtc_enable time, >> > so the pipes are stopped, and it doesn't really make sense to wait for >> > a vblank: it just delays the modeset in 50ms. >> > >> > Leave the code there (commented with FIXME) for now since maybe we >> > need a bigger rework. >> > >> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> >> > --- >> > drivers/gpu/drm/i915/intel_display.c | 14 +++++++++++--- >> > 1 file changed, 11 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_display.c >> > b/drivers/gpu/drm/i915/intel_display.c >> > index 69e8bb6..b891a0c 100644 >> > --- a/drivers/gpu/drm/i915/intel_display.c >> > +++ b/drivers/gpu/drm/i915/intel_display.c >> > @@ -6662,7 +6662,6 @@ static void haswell_write_eld(struct >> > drm_connector *connector, { >> > struct drm_i915_private *dev_priv = connector->dev->dev_private; >> > uint8_t *eld = connector->eld; >> > - struct drm_device *dev = crtc->dev; >> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> > uint32_t eldv; >> > uint32_t i; >> > @@ -6684,8 +6683,17 @@ static void haswell_write_eld(struct >> drm_connector *connector, >> > tmp |= (AUDIO_OUTPUT_ENABLE_A << (pipe * 4)); >> > I915_WRITE(aud_cntrl_st2, tmp); >> > >> > - /* Wait for 1 vertical blank */ >> > - intel_wait_for_vblank(dev, pipe); >> > + /* >> > + * Wait for 1 vertical blank >> > + * >> > + * FIXME: We call this function at mode_set time, when the pipes are all >> > + * stopped, so it doesn't really make any sense to wait for a vblank >> > + * here: it just delays the modeset in 50ms. I'll leave the code here >> > + * because since the wait doesn't make sense at this point, maybe we >> > + * need a bigger rework. We need an Audio authority to audit this code. >> > + * >> > + * intel_wait_for_vblank(dev_priv->dev, pipe); >> > + */ >> >> This might be due to the generic recommendation that infoframes and related >> stuff (audio also gets transmitted when infoframes are) should only be changed >> after the vblank completed when the pipe is on. >> >> Imo we should just ditch this (and cc: the audio guys on the patch so they're >> aware). >> -Daniel >> >> > >> > /* Set ELD valid state */ >> > tmp = I915_READ(aud_cntrl_st2); >> > -- >> > 1.8.3.1 >> > >> > _______________________________________________ >> > Intel-gfx mailing list >> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
