From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 20/26] drm/i915: ValleyView has limited cacheability Date: Thu, 22 Mar 2012 16:31:36 -0700 Message-ID: <20120322163136.243be072@jbarnes-desktop> References: <1332452348-8814-1-git-send-email-jbarnes@virtuousgeek.org> <1332452348-8814-21-git-send-email-jbarnes@virtuousgeek.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy7-pub.bluehost.com (oproxy7-pub.bluehost.com [67.222.55.9]) by gabe.freedesktop.org (Postfix) with SMTP id 79C7B9E70A for ; Thu, 22 Mar 2012 16:31:39 -0700 (PDT) Received: from c-67-161-37-189.hsd1.ca.comcast.net ([67.161.37.189] helo=jbarnes-desktop) by box514.bluehost.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.76) (envelope-from ) id 1SArTm-00067J-FB for intel-gfx@lists.freedesktop.org; Thu, 22 Mar 2012 17:31:38 -0600 In-Reply-To: <1332452348-8814-21-git-send-email-jbarnes@virtuousgeek.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 22 Mar 2012 14:39:02 -0700 Jesse Barnes 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 > --- > 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