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

[Joonas Lahtinen]

On ke, 2016-07-20 at 14:11 +0100, 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.
> 

The cocci data would be useful, or sed expression. And would make me
more confident this is pure mechanical work.

Reviewed-by: Joonas Lahtinen 

Regards, Joonas

-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Dave Gordon]

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 


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 = >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.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Dave Gordon]

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 


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.


---
  drivers/gpu/drm/i915/i915_gem_context.c|  4 +-
  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 +-
  drivers/gpu/drm/i915/i915_gem_gtt.c|  6 +-
  drivers/gpu/drm/i915/i915_gem_request.c| 16 +++---
  drivers/gpu/drm/i915/i915_gem_request.h|  2 +-
  drivers/gpu/drm/i915/i915_gpu_error.c  | 20 +++
  drivers/gpu/drm/i915/intel_display.c   | 10 ++--
  drivers/gpu/drm/i915/intel_lrc.c   | 57 +-
  drivers/gpu/drm/i915/intel_mocs.c  | 36 ++--
  drivers/gpu/drm/i915/intel_overlay.c   |  8 +--
  drivers/gpu/drm/i915/intel_ringbuffer.c| 92 +++---
  11 files changed, 126 insertions(+), 129 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index b6d10bd763a0..16138c4ff7db 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -552,7 +552,7 @@ static inline int
  mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
  {
struct drm_i915_private *dev_priv = req->i915;
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
u32 flags = hw_flags | MI_MM_SPACE_GTT;
const int num_rings =
/* Use an extended w/a on ivb+ if signalling from other rings */
@@ -654,7 +654,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 
hw_flags)
  static int remap_l3(struct drm_i915_gem_request *req, int slice)
  {
u32 *remap_info = req->i915->l3_parity.remap_info[slice];
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
int i, ret;

if (!remap_info)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index e2c4d99a1e7f..501a1751d432 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1173,7 +1173,7 @@ i915_gem_execbuffer_retire_commands(struct 
i915_execbuffer_params *params)
  static int
  i915_reset_gen7_sol_offsets(struct drm_i915_gem_request *req)
  {
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
int ret, i;

if (!IS_GEN7(req->i915) || req->engine->id != RCS) {
@@ -1303,7 +1303,7 @@ i915_gem_ringbuffer_submission(struct 
i915_execbuffer_params *params,

if (params->engine->id == RCS &&
instp_mode != dev_priv->relative_constants_mode) {
-   struct intel_ringbuffer *ring = params->request->ringbuf;
+   struct intel_ringbuffer *ring = params->request->ring;

ret = intel_ring_begin(params->request, 4);
if (ret)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index abc439be2049..a48329baf432 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -669,7 +669,7 @@ static int gen8_write_pdp(struct drm_i915_gem_request *req,
  unsigned entry,
  dma_addr_t addr)
  {
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
int ret;

BUG_ON(entry >= 4);
@@ -1660,7 +1660,7 @@ static uint32_t get_pd_offset(struct i915_hw_ppgtt *ppgtt)
  static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
 struct drm_i915_gem_request *req)
  {
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
int ret;

/* NB: TLBs must be flushed and invalidated before a switch */
@@ -1688,7 +1688,7 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
  static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt,
  struct drm_i915_gem_request *req)
  {
-   struct intel_ringbuffer *ring = req->ringbuf;
+   

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

[Chris Wilson]

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 
---
 drivers/gpu/drm/i915/i915_gem_context.c|  4 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c|  6 +-
 drivers/gpu/drm/i915/i915_gem_request.c| 16 +++---
 drivers/gpu/drm/i915/i915_gem_request.h|  2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c  | 20 +++
 drivers/gpu/drm/i915/intel_display.c   | 10 ++--
 drivers/gpu/drm/i915/intel_lrc.c   | 57 +-
 drivers/gpu/drm/i915/intel_mocs.c  | 36 ++--
 drivers/gpu/drm/i915/intel_overlay.c   |  8 +--
 drivers/gpu/drm/i915/intel_ringbuffer.c| 92 +++---
 11 files changed, 126 insertions(+), 129 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index b6d10bd763a0..16138c4ff7db 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -552,7 +552,7 @@ static inline int
 mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
 {
struct drm_i915_private *dev_priv = req->i915;
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
u32 flags = hw_flags | MI_MM_SPACE_GTT;
const int num_rings =
/* Use an extended w/a on ivb+ if signalling from other rings */
@@ -654,7 +654,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 
hw_flags)
 static int remap_l3(struct drm_i915_gem_request *req, int slice)
 {
u32 *remap_info = req->i915->l3_parity.remap_info[slice];
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
int i, ret;
 
if (!remap_info)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index e2c4d99a1e7f..501a1751d432 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1173,7 +1173,7 @@ i915_gem_execbuffer_retire_commands(struct 
i915_execbuffer_params *params)
 static int
 i915_reset_gen7_sol_offsets(struct drm_i915_gem_request *req)
 {
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
int ret, i;
 
if (!IS_GEN7(req->i915) || req->engine->id != RCS) {
@@ -1303,7 +1303,7 @@ i915_gem_ringbuffer_submission(struct 
i915_execbuffer_params *params,
 
if (params->engine->id == RCS &&
instp_mode != dev_priv->relative_constants_mode) {
-   struct intel_ringbuffer *ring = params->request->ringbuf;
+   struct intel_ringbuffer *ring = params->request->ring;
 
ret = intel_ring_begin(params->request, 4);
if (ret)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index abc439be2049..a48329baf432 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -669,7 +669,7 @@ static int gen8_write_pdp(struct drm_i915_gem_request *req,
  unsigned entry,
  dma_addr_t addr)
 {
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
int ret;
 
BUG_ON(entry >= 4);
@@ -1660,7 +1660,7 @@ static uint32_t get_pd_offset(struct i915_hw_ppgtt *ppgtt)
 static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
 struct drm_i915_gem_request *req)
 {
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
int ret;
 
/* NB: TLBs must be flushed and invalidated before a switch */
@@ -1688,7 +1688,7 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
 static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt,
  struct drm_i915_gem_request *req)
 {
-   struct intel_ringbuffer *ring = req->ringbuf;
+   struct intel_ringbuffer *ring = req->ring;
int ret;
 
/* NB: TLBs must be flushed and invalidated before a switch */
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c 
b/drivers/gpu/drm/i915/i915_gem_request.c
index 60a3a343b3a8..0f415606a383 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -170,7 +170,7 @@ static void i915_gem_request_retire(struct 
drm_i915_gem_request *request)
 * Note this requires that we are always called in request
 * completion order.
 */
-   request->ringbuf->last_retired_head = request->postfix;
+   request->ring->last_retired_head = request->postfix;
 
i915_gem_request_remove_from_client(request);
 
@@ -425,7 +425,7 @@ void __i915_add_request(struct drm_i915_gem_request 
*request,
bool