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

Daniel Vetter
2011-04-14T17:43:41Z

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