[Intel-gfx] [PATCH v2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell

[Lin, Mengdong]

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 at ffwll.ch [mailto:daniel.vetter at ffwll.ch] On Behalf Of
> Daniel Vetter
> Sent: Thursday, September 05, 2013 2:50 AM
> To: Lin, Mengdong
> Cc: intel-gfx; Arora, MukeshX
> Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/hsw: Add display Audio codec
> disable sequence for Haswell
> 
> On Fri, Aug 30, 2013 at 1:50 AM,  <mengdong.lin at intel.com> 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