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

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Sep 19, 2013 at 05:07:29PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@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@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@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