Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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:
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,
u32 data)
{
-Mika
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
