Re: [PATCH 3/3] drm/i915/bdw: Use the GEN8 SRM when qeueing a flip

[Ben Widawsky <benjamin.widawsky@intel.com>]

On Mon, Apr 07, 2014 at 08:24:34PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3697433..a646ed4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8821,8 +8821,16 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
>  	}
>  
>  	len = 4;
> -	if (ring->id == RCS)
> +	if (ring->id == RCS) {
>  		len += 6;
> +		/*
> +		 * On Gen 8, SRM is now taking an extra dword to accommodate
> +		 * 48bits addresses, and we need a NOOP for the batch size to
> +		 * stay even.
> +		 */
> +		if (IS_GEN8(dev))
> +			len += 2;
> +	}
>  
>  	/*
>  	 * BSpec MI_DISPLAY_FLIP for IVB:
> @@ -8857,10 +8865,18 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
>  		intel_ring_emit(ring, ~(DERRMR_PIPEA_PRI_FLIP_DONE |
>  					DERRMR_PIPEB_PRI_FLIP_DONE |
>  					DERRMR_PIPEC_PRI_FLIP_DONE));
> -		intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) |
> -				MI_SRM_LRM_GLOBAL_GTT);
> +		if (IS_GEN8(dev))
> +			intel_ring_emit(ring, MI_STORE_REGISTER_MEM_GEN8(1) |
> +					      MI_SRM_LRM_GLOBAL_GTT);
> +		else
> +			intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) |
> +					      MI_SRM_LRM_GLOBAL_GTT);
>  		intel_ring_emit(ring, DERRMR);
>  		intel_ring_emit(ring, ring->scratch.gtt_offset + 256);
> +		if (IS_GEN8(dev)) {
> +			intel_ring_emit(ring, 0);
> +			intel_ring_emit(ring, MI_NOOP);
> +		}
>  	}
>  
>  	intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);

Cool. This explains the bad DERRMR values I was seeing in in error
states. I'm honestly didn't check if we actually need an SRM for BDW
still, but I'll assume you did check.

I also think it's worth to make a intel_gen8_queue_flip, but since I
don't touch this code much, I'll leave it to you to decide.

The series is:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

minus the above mentioned fact of whether it's needed.

-- 
Ben Widawsky, Intel Open Source Technology Center