[Intel-gfx] [PATCH 4/4] drm/i915: skip useless vblank wait on Haswell audio sequence

[Paulo Zanoni]

2013/9/22 Lin, Mengdong <mengdong.lin at intel.com>:
> 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 at lists.freedesktop.org
>> [mailto:intel-gfx-bounces+mengdong.lin=intel.com at lists.freedesktop.org] On
>> Behalf Of Daniel Vetter
>> Sent: Friday, September 20, 2013 4:18 PM
>> To: Paulo Zanoni
>> Cc: intel-gfx at lists.freedesktop.org; Zanoni, Paulo R
>> Subject: Re: [Intel-gfx] [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 at intel.com>
>> >
>> > 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 at intel.com>
>> > ---
>> >  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 at lists.freedesktop.org
>> > 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni