Re: [Intel-gfx] [PATCH 3/3] drm/i915: Treat ringbuffer writes as write to normal memory

2015-10-16 Thread Chris Wilson 
On Fri, Oct 16, 2015 at 06:06:09PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 08, 2015 at 01:39:56PM +0100, Chris Wilson wrote:
> > 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);
> 
> Hmm. The lrc copy looks identical to this one. But looks like most of
> intel_lrc.c ring handling is copy pasted anyway, so there's even more
> that could be thrown out.

Yes, only this time I am trying to be sneaky and only delete small
chunks at a time. It may take a few years to do what could be done in a
morning, but whatever.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Treat ringbuffer writes as write to normal memory

2015-10-16 Thread Ville Syrjälä 
On Thu, Oct 08, 2015 at 01:39:56PM +0100, Chris Wilson wrote:
> 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.
> 
> Signed-off-by: Chris Wilson 
> ---
>  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 | 17 -
>  4 files changed, 17 insertions(+), 20 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);

Hmm. The lrc copy looks identical to this one. But looks like most of
intel_lrc.c ring handling is copy pasted anyway, so there's even more
that could be thrown out.

This patch seems OK to me
Reviewed-by: Ville Syrjälä 

>  
>   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..ff290765e6c9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -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 __force *)(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;
> -

[Intel-gfx] [PATCH 3/3] drm/i915: Treat ringbuffer writes as write to normal memory

2015-10-08 Thread Chris Wilson 
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.

Signed-off-by: Chris Wilson 
---
 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 | 17 -
 4 files changed, 17 insertions(+), 20 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..ff290765e6c9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -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 __force *)(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