Re: [PATCH 02/18] drm/i915: Rename request->ringbuf to request->ring

Dave Gordon
2016-07-20T14:51:47Z

On 20/07/16 15:12, Dave Gordon wrote:
> On 20/07/16 14:11, Chris Wilson wrote:
>> Now that we have disambuigated ring and engine, we can use the clearer
>> and more consistent name for the intel_ringbuffer pointer in the
>> request.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> You missed a few instances of 'ring' meaning engine:
>
> i915_gem_execbuffer.c: struct intel_engine_cs **ring)
> intel_mocs.h:int intel_mocs_init_engine(struct intel_engine_cs *ring);
> intel_ringbuffer.c:gen5_seqno_barrier(struct intel_engine_cs *ring)
> intel_ringbuffer.h: void (*irq_enable)(struct intel_engine_cs
> *ring);
> intel_ringbuffer.h: void (*irq_disable)(struct intel_engine_cs
> *ring);
> intel_ringbuffer.h: int (*init_hw)(struct intel_engine_cs *ring);
> intel_ringbuffer.h: void (*irq_seqno_barrier)(struct
> intel_engine_cs *ring);
> intel_ringbuffer.h: void (*cleanup)(struct intel_engine_cs
> *ring);
>
> I think we have to purge every last trace of this usage before using
> 'ring' as shorthand for 'ringbuf[fer]'.
>
> .Dave.

Oh yes, also there are lots of other things called 'ring' which aren't
ringbuffers, such as an engine:

#define RING_ELSP(ring) _MMIO((ring)->mmio_base + 0x230)

or an engine id:

static i915_reg_t mocs_register(enum intel_engine_id ring, int index)
i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring)
int ring = req->engine->id;

or a different structure entirely:

struct drm_i915_error_ring *ring = &error->ring[ring_idx];

I could probably write some Cocci to find-and-rename all the things
called 'ring' that weren't ringbuffers, but it would be easier not to
overload the identifier with a host of different meanings in the first
place. So I think adding any more instances of things called 'ring'
should wait until the name has no other meanings, if ringbuffers are the
thing you want it to unambiguously identify.

.Dave.