On Thu, 22 Mar 2012 14:39:02 -0700
Jesse Barnes <jbar...@virtuousgeek.org> wrote:

> The GT can snoop CPU writes, but doesn't snoop into the CPU cache when
> it does writes, so we can't use the cache bits the same way.
> 
> So map the status and pipe control pages as uncached on ValleyView, and
> only set the pages to cached if we're on a supported platform.
> 
> v2: add clarifying comments and don't use the LLC flag for ioremap vs
>     kmap (Daniel)
> 
> Signed-off-by: Jesse Barnes <jbar...@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c |   45 
> ++++++++++++++++++++++++++-----
>  1 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index ca3972f..9b26c9d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -319,6 +319,8 @@ init_pipe_control(struct intel_ring_buffer *ring)
>  {
>       struct pipe_control *pc;
>       struct drm_i915_gem_object *obj;
> +     int cache_level = HAS_LLC(ring->dev) ? I915_CACHE_LLC : I915_CACHE_NONE;
> +     struct drm_device *dev;
>       int ret;
>  
>       if (ring->private)
> @@ -335,14 +337,24 @@ init_pipe_control(struct intel_ring_buffer *ring)
>               goto err;
>       }
>  
> -     i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
> +     i915_gem_object_set_cache_level(obj, cache_level);
>  
>       ret = i915_gem_object_pin(obj, 4096, true);
>       if (ret)
>               goto err_unref;
> -
> +     dev = obj->base.dev;
>       pc->gtt_offset = obj->gtt_offset;
> -     pc->cpu_page =  kmap(obj->pages[0]);
> +     /*
> +      * On ValleyView, only CPU writes followed by GPU reads are snooped,
> +      * not GPU writes followed by CPU reads.  So we need to map status
> +      * pages as uncached.
> +      */
> +     if (IS_VALLEYVIEW(dev))
> +             pc->cpu_page = ioremap(dev->agp->base +
> +                                    obj->gtt_offset,
> +                                    PAGE_SIZE);
> +     else
> +             pc->cpu_page =  kmap(obj->pages[0]);
>       if (pc->cpu_page == NULL)
>               goto err_unpin;
>  
> @@ -364,12 +376,17 @@ cleanup_pipe_control(struct intel_ring_buffer *ring)
>  {
>       struct pipe_control *pc = ring->private;
>       struct drm_i915_gem_object *obj;
> +     struct drm_device *dev;
>  
>       if (!ring->private)
>               return;
>  
>       obj = pc->obj;
> -     kunmap(obj->pages[0]);
> +     dev = obj->base.dev;
> +     if (IS_VALLEYVIEW(dev))
> +             iounmap(pc->cpu_page);
> +     else
> +             kunmap(obj->pages[0]);
>       i915_gem_object_unpin(obj);
>       drm_gem_object_unreference(&obj->base);
>  
> @@ -929,7 +946,10 @@ static void cleanup_status_page(struct intel_ring_buffer 
> *ring)
>       if (obj == NULL)
>               return;
>  
> -     kunmap(obj->pages[0]);
> +     if (IS_VALLEYVIEW(dev_priv->dev))
> +             iounmap(ring->status_page.page_addr);
> +     else
> +             kunmap(obj->pages[0]);
>       i915_gem_object_unpin(obj);
>       drm_gem_object_unreference(&obj->base);
>       ring->status_page.obj = NULL;
> @@ -942,6 +962,7 @@ static int init_status_page(struct intel_ring_buffer 
> *ring)
>       struct drm_device *dev = ring->dev;
>       drm_i915_private_t *dev_priv = dev->dev_private;
>       struct drm_i915_gem_object *obj;
> +     int cache_level = HAS_LLC(ring->dev) ? I915_CACHE_LLC : I915_CACHE_NONE;

So yeah earlier chipsets where this was ok but that don't set .has_llc
will be affected; I'll just change this to IS_VALLEYVIEW.

But on the plus side, if we applied this they probably wouldn't see
"missed irq" messages (if that ever happened on those chipsets). :)

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to