Re: [PATCH 2/4] drm/i915: don't bother enabling swizzle bits on gen7+ v2

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Mar 14, 2014 at 02:37:09PM -0700, Jesse Barnes wrote:
> As of IVB, the memory controller does internal swizzling already, so we
> shouldn't need to enable these.  Based on an earlier fix from Kristian.
> 
> v2: preserve swizzling if BIOS had it set (Daniel)
> 
> Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.h        |    1 +
>  drivers/gpu/drm/i915/i915_gem.c        |    6 ++++++
>  drivers/gpu/drm/i915/i915_gem_tiling.c |    3 ++-
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c64f770..29cd977 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1508,6 +1508,7 @@ typedef struct drm_i915_private {
>  	struct intel_vbt_data vbt;
>  
>  	bool bios_ssc; /* BIOS had SSC enabled at boot? */
> +	bool bios_swizzle;
>  
>  	/* overlay */
>  	struct intel_overlay *overlay;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 92b0b41..87e34bc 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4313,6 +4313,9 @@ void i915_gem_init_swizzling(struct drm_device *dev)
>  	    dev_priv->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
>  		return;
>  
> +	if (INTEL_INFO(dev)->gen >= 7 && !dev_priv->bios_swizzle)
> +		return;
> +
>  	I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
>  				 DISP_TILE_SURFACE_SWIZZLING);
>  
> @@ -4454,6 +4457,9 @@ int i915_gem_init(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
>  
> +	if (I915_READ(DISP_ARB_CTL) & DISP_TILE_SURFACE_SWIZZLING)
> +		dev_priv->bios_swizzle = true;
> +
>  	mutex_lock(&dev->struct_mutex);
>  
>  	if (IS_VALLEYVIEW(dev)) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index eb99358..c6447de 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -91,7 +91,8 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
>  	uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
>  	uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
>  
> -	if (IS_VALLEYVIEW(dev)) {
> +	if (INTEL_INFO(dev)->gen >= 7 &&
> +	    !(I915_READ(TILECTL) & TILECTL_SWZCTL)) {

Not quite what I've had in mind. I think what we need to do instead is

- Walk all the inherited framebuffer configs and check whether any of them
  are tiled. If one is, then we need to preserve the BIOS swizzle setting,
  if not then we can pick whatever we want.

- If we decide to preserve the BIOS config, then we need to preserve both
  the swizzled and the unswizlled case, i.e. we need a nested if switch
  whichc either sets SWIZZE_NONE or the correct swizzle bits.

- We need to check the swizzle bits in DISP_ARB_CTL, not one of the other
  three - the DISP_ARB bit is the one relevant for the display controller,
  the other might be wrong or not set.

- This should be done for all gen6+ platforms, sicne those are the ones
  where swizzling is driver controlled.

Cheers, Daniel

>  		swizzle_x = I915_BIT_6_SWIZZLE_NONE;
>  		swizzle_y = I915_BIT_6_SWIZZLE_NONE;
>  	} else if (INTEL_INFO(dev)->gen >= 6) {
> -- 
> 1.7.9.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch