[PATCH] drm/i915: Remove vblank wait from haswell_write_eld

[PATCH] drm/i915: Remove vblank wait from haswell_write_eld

[Daniel Vetter]

The pipe is off at that point in time, so a vblank wait is simply a
50ms wait. Caught by Jesse's verbose "make vblank wait timeouts WARN"
patch. We've probably had a few versions of this float around already.

To document assumptions put a pipe assert into the same place. And
also add a posting read.

If we ever decide to update the eld and infoframes while the pipe is
already on (e.g. for fastboot) then there's lots of work to do. So
better properly document all the hidden assumptions.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8b623a623f9c..82ad84eefc8d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7371,7 +7371,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;
 	uint32_t eldv;
 	uint32_t i;
 	int len;
@@ -7388,9 +7387,9 @@ static void haswell_write_eld(struct drm_connector *connector,
 	tmp = I915_READ(aud_cntrl_st2);
 	tmp |= (AUDIO_OUTPUT_ENABLE_A << (pipe * 4));
 	I915_WRITE(aud_cntrl_st2, tmp);
+	POSTING_READ(aud_cntrl_st2);
 
-	/* Wait for 1 vertical blank */
-	intel_wait_for_vblank(dev, pipe);
+	assert_pipe_disabled(dev_priv, to_intel_crtc(crtc)->pipe);
 
 	/* Set ELD valid state */
 	tmp = I915_READ(aud_cntrl_st2);
-- 
1.8.4.rc3

Re: [PATCH] drm/i915: Remove vblank wait from haswell_write_eld

[Ville Syrjälä]

On Wed, Apr 16, 2014 at 04:56:09PM +0200, Daniel Vetter wrote:
> The pipe is off at that point in time, so a vblank wait is simply a
> 50ms wait. Caught by Jesse's verbose "make vblank wait timeouts WARN"
> patch. We've probably had a few versions of this float around already.
> 
> To document assumptions put a pipe assert into the same place. And
> also add a posting read.
> 
> If we ever decide to update the eld and infoframes while the pipe is
> already on (e.g. for fastboot) then there's lots of work to do. So
> better properly document all the hidden assumptions.

IIRC the docs might say that eld (or some audio stuff) can't be enabled
until one or two vblanks after the pipe has been enabled (or something
to that effect). So we're probably doing it all wrong and we should
move the audio stuff to some vblank work thingy eventually.

But as this is currently done while the pipe is off, the vblank wait
is just a nop, and so we should just kill it.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8b623a623f9c..82ad84eefc8d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7371,7 +7371,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;
>  	uint32_t eldv;
>  	uint32_t i;
>  	int len;
> @@ -7388,9 +7387,9 @@ static void haswell_write_eld(struct drm_connector *connector,
>  	tmp = I915_READ(aud_cntrl_st2);
>  	tmp |= (AUDIO_OUTPUT_ENABLE_A << (pipe * 4));
>  	I915_WRITE(aud_cntrl_st2, tmp);
> +	POSTING_READ(aud_cntrl_st2);
>  
> -	/* Wait for 1 vertical blank */
> -	intel_wait_for_vblank(dev, pipe);
> +	assert_pipe_disabled(dev_priv, to_intel_crtc(crtc)->pipe);
>  
>  	/* Set ELD valid state */
>  	tmp = I915_READ(aud_cntrl_st2);
> -- 
> 1.8.4.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC