Re: [PATCH 12/13] drm/i915: Prevent mixing of snooped and tiling modes for old chipsets

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Apr 14, 2011 at 10:03:46AM +0100, Chris Wilson wrote:
> Older chipsets do not support snooping (i.e. cache sharing between the
> CPU and GPU) on tiled surfaces. So prevent userspace from being silly
> should we one day expose the ability to change cache levels from
> userspace.
> 
> It does enforce a strict ordering for mode changing though. So in order
> to change a buffer to snooped, the driver has to clear any tiling first
> and then change the cache level. This is consistent with how we flush
> and update the PTEs and seems a reasonable imposition on the driver.
> Deferring the check until use, whilst providing flexibilty here, implies
> forcing extra unbinds and a more complicated error message from, for
> example, execbuffer.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Small error in the debug output below.

> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index 281ad3d..ca69fd4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -331,6 +331,14 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
>  	}
>  
>  	mutex_lock(&dev->struct_mutex);
> +	if (INTEL_INFO(dev)->gen < 6 &&
> +	    args->tiling_mode != I915_TILING_NONE &&
> +	    obj->cache_level != I915_CACHE_NONE) {
> +		DRM_DEBUG("can't not set a tiling mode on snooped memory,"

One negation too much.

> +			  "it must be linear for pre-SandyBridge chipsets\n");
> +		ret = -EINVAL;
> +		goto err;
> +	}
>  	if (args->tiling_mode != obj->tiling_mode ||
>  	    args->stride != obj->stride) {
>  		/* We need to rebind the object if its current allocation
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48