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 <chris@xxxxxxxxxxxxxxxxxx>
> ---
> 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ä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
