From patchwork Fri Oct 30 13:19:23 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mika Kuoppala X-Patchwork-Id: 7526921 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 139B4BEEA4 for ; Fri, 30 Oct 2015 13:20:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id ED400207C9 for ; Fri, 30 Oct 2015 13:20:44 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id BEF78207CF for ; Fri, 30 Oct 2015 13:20:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3FED0721BD; Fri, 30 Oct 2015 06:20:42 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTP id C11D9721D3 for ; Fri, 30 Oct 2015 06:20:15 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 30 Oct 2015 06:20:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,218,1444719600"; d="scan'208";a="823137681" Received: from gaia.fi.intel.com (HELO gaia) ([10.237.72.165]) by fmsmga001.fm.intel.com with ESMTP; 30 Oct 2015 06:20:13 -0700 Received: by gaia (Postfix, from userid 1000) id E9EC740605; Fri, 30 Oct 2015 15:19:23 +0200 (EET) From: Mika Kuoppala To: Chris Wilson , daniel.vetter@ffwll.ch In-Reply-To: <1445419281-28521-4-git-send-email-chris@chris-wilson.co.uk> References: <1445419281-28521-1-git-send-email-chris@chris-wilson.co.uk> <1445419281-28521-4-git-send-email-chris@chris-wilson.co.uk> User-Agent: Notmuch/0.20.2+75~gdca7220 (http://notmuchmail.org) Emacs/23.4.1 (i686-pc-linux-gnu) Date: Fri, 30 Oct 2015 15:19:23 +0200 Message-ID: <87611oqysk.fsf@gaia.fi.intel.com> MIME-Version: 1.0 Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v99 4/4] drm/i915: Treat ringbuffer writes as write to normal memory X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Chris Wilson writes: > Ringbuffers are now being written to either through LLC or WC paths, so > treating them as simply iomem is no longer adequate. However, for the > older !llc hardware, the hardware is documentated as treating the TAIL > register update as serialising, so we can relax the barriers when filling > the rings (but even if it were not, it is still an uncached register write > and so serialising anyway.). > > For simplicity, let's ignore the iomem annotation. > > v2: Remove iomem from ringbuffer->virtual_address > iomem annotation is for sparse. i915_irq.c still uses ioread32 thus mixing the address spaces. I see this has already r-b but something like this as a followup would make sparse quiet: u32 data) { -Mika > Signed-off-by: Chris Wilson > Reviewed-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_lrc.c | 7 +------ > drivers/gpu/drm/i915/intel_lrc.h | 6 +++--- > drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +------ > drivers/gpu/drm/i915/intel_ringbuffer.h | 19 +++++++++++++------ > 4 files changed, 18 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index d38746c5370d..10020505be75 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -713,13 +713,8 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request) > > static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf) > { > - uint32_t __iomem *virt; > int rem = ringbuf->size - ringbuf->tail; > - > - virt = ringbuf->virtual_start + ringbuf->tail; > - rem /= 4; > - while (rem--) > - iowrite32(MI_NOOP, virt++); > + memset(ringbuf->virtual_start + ringbuf->tail, 0, rem); > > ringbuf->tail = 0; > intel_ring_update_space(ringbuf); > diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h > index 64f89f9982a2..8b56fb934763 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.h > +++ b/drivers/gpu/drm/i915/intel_lrc.h > @@ -53,8 +53,9 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req); > */ > static inline void intel_logical_ring_advance(struct intel_ringbuffer *ringbuf) > { > - ringbuf->tail &= ringbuf->size - 1; > + intel_ringbuffer_advance(ringbuf); > } > + > /** > * intel_logical_ring_emit() - write a DWORD to the ringbuffer. > * @ringbuf: Ringbuffer to write to. > @@ -63,8 +64,7 @@ static inline void intel_logical_ring_advance(struct intel_ringbuffer *ringbuf) > static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf, > u32 data) > { > - iowrite32(data, ringbuf->virtual_start + ringbuf->tail); > - ringbuf->tail += 4; > + intel_ringbuffer_emit(ringbuf, data); > } > > /* Logical Ring Contexts */ > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 49aa52440db2..0eaaab92dea0 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -2180,13 +2180,8 @@ static int ring_wait_for_space(struct intel_engine_cs *ring, int n) > > static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf) > { > - uint32_t __iomem *virt; > int rem = ringbuf->size - ringbuf->tail; > - > - virt = ringbuf->virtual_start + ringbuf->tail; > - rem /= 4; > - while (rem--) > - iowrite32(MI_NOOP, virt++); > + memset(ringbuf->virtual_start + ringbuf->tail, 0, rem); > > ringbuf->tail = 0; > intel_ring_update_space(ringbuf); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 2e85fda94963..10f80df5f121 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -97,7 +97,7 @@ struct intel_ring_hangcheck { > > struct intel_ringbuffer { > struct drm_i915_gem_object *obj; > - void __iomem *virtual_start; > + void *virtual_start; > > struct intel_engine_cs *ring; > > @@ -427,17 +427,24 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request); > > int __must_check intel_ring_begin(struct drm_i915_gem_request *req, int n); > int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req); > +static inline void intel_ringbuffer_emit(struct intel_ringbuffer *rb, > + u32 data) > +{ > + *(uint32_t *)(rb->virtual_start + rb->tail) = data; > + rb->tail += 4; > +} > +static inline void intel_ringbuffer_advance(struct intel_ringbuffer *rb) > +{ > + rb->tail &= rb->size - 1; > +} > static inline void intel_ring_emit(struct intel_engine_cs *ring, > u32 data) > { > - struct intel_ringbuffer *ringbuf = ring->buffer; > - iowrite32(data, ringbuf->virtual_start + ringbuf->tail); > - ringbuf->tail += 4; > + intel_ringbuffer_emit(ring->buffer, data); > } > static inline void intel_ring_advance(struct intel_engine_cs *ring) > { > - struct intel_ringbuffer *ringbuf = ring->buffer; > - ringbuf->tail &= ringbuf->size - 1; > + intel_ringbuffer_advance(ring->buffer); > } > int __intel_ring_space(int head, int tail, int size); > void intel_ring_update_space(struct intel_ringbuffer *ringbuf); > -- > 2.6.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6e0a568..b5e9383 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2831,7 +2831,7 @@ semaphore_waits_for(struct intel_engine_cs *ring, u32 *seqno) head &= ring->buffer->size - 1; /* This here seems to blow up */ - cmd = ioread32(ring->buffer->virtual_start + head); + cmd = intel_ringbuffer_read(ring->buffer, head); if (cmd == ipehr) break; @@ -2841,11 +2841,11 @@ semaphore_waits_for(struct intel_engine_cs *ring, u32 *seqno) if (!i) return NULL; - *seqno = ioread32(ring->buffer->virtual_start + head + 4) + 1; + *seqno = intel_ringbuffer_read(ring->buffer, head + 4) + 1; if (INTEL_INFO(ring->dev)->gen >= 8) { - offset = ioread32(ring->buffer->virtual_start + head + 12); + offset = intel_ringbuffer_read(ring->buffer, head + 12); offset <<= 32; - offset = ioread32(ring->buffer->virtual_start + head + 8); + offset = intel_ringbuffer_read(ring->buffer, head + 8); } return semaphore_wait_to_signaller_ring(ring, ipehr, offset); } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index ea6f8a7..48fdfc3 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1999,7 +1999,7 @@ void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf) if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen) vunmap(ringbuf->virtual_start); else - iounmap(ringbuf->virtual_start); + iounmap((void __iomem *)ringbuf->virtual_start); ringbuf->virtual_start = NULL; i915_gem_object_ggtt_unpin(ringbuf->obj); } @@ -2059,7 +2059,7 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev, return ret; } - ringbuf->virtual_start = ioremap_wc(dev_priv->gtt.mappable_base + + ringbuf->virtual_start = (void __force *)ioremap_wc(dev_priv->gtt.mappable_base + i915_gem_obj_ggtt_offset(obj), ringbuf->size); if (ringbuf->virtual_start == NULL) { i915_gem_object_ggtt_unpin(obj); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index a15f6c2..335e632 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -448,6 +448,13 @@ static inline void intel_ringbuffer_advance(struct intel_ringbuffer *rb) rb->tail &= rb->size - 1; } +static inline u32 intel_ringbuffer_read(const struct intel_ringbuffer *rb, + const u32 offset) +{ + WARN_ON(offset >= rb->size); + return *(uint32_t *)(rb->virtual_start + offset); +} + static inline void intel_ring_emit(struct intel_engine_cs *ring,