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. 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
