On Thu, 22 Mar 2012 14:39:02 -0700
Jesse Barnes <jbarnes at 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 <jbarnes at 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
