Hi Daniel and Ville, Thanks for your feedback and please see comments below ... I'm very sorry I missed your mails for a long time. > -----Original Message----- > From: daniel.vetter@xxxxxxxx [mailto:daniel.vetter@xxxxxxxx] On Behalf Of > Daniel Vetter > Sent: Thursday, September 05, 2013 2:50 AM > To: Lin, Mengdong > Cc: intel-gfx; Arora, MukeshX > Subject: Re: [PATCH v2] drm/i915/hsw: Add display Audio codec > disable sequence for Haswell > > On Fri, Aug 30, 2013 at 1:50 AM, <mengdong.lin@xxxxxxxxx> wrote: > > + /* Wait for 2 vertical blanks */ > > + intel_wait_for_vblank(dev, pipe); > > + intel_wait_for_vblank(dev, pipe); > > + > > + /* Disable audio PD. This is optional as per Bspec. */ > > + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD); > > + temp &= ~(AUDIO_OUTPUT_ENABLE_A << (pipe * 4)); > > + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp); > > If this is optional do we really need the two vblank waits above? > Adding them just for fun when we generally try to rip out as many vblank waits > as possible from the modeset code isn't all that great ... The two vblank wait is requested by b-spec, not optional. Can we reserve them now until we got confirmation from HW owner that's safe to remove them? Or until we have better implementation of vblank wait? > Also I'd really like to see the audio stuff being tracked in the pipe config instead > of splattering these different ad-hoc state bits like intel_crtc->eld_vld all over > the place. > -Daniel How about adding a flag "has_audio" to intel_crtc->config? If okay, I'll write a patch to clean up checking on intel_crtc->eld_vld here and there. Thanks Mengdong _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
