Re: [PATCH 08/31] drm/i915: Remove stray intel_engine_cs ring identifiers from i915_gem.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25/07/16 09:49, Chris Wilson wrote:
On Mon, Jul 25, 2016 at 11:45:42AM +0300, Joonas Lahtinen wrote:
On ma, 2016-07-25 at 08:44 +0100, Chris Wilson wrote:
A few places we use ring when referring to the struct intel_engine_cs. An
anachronism we are pruning out.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
 drivers/gpu/drm/i915/i915_gem.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e155e8dd28ed..7bfce1d5c61b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -46,7 +46,7 @@ static void i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *o
 static void
 i915_gem_object_retire__write(struct drm_i915_gem_object *obj);
 static void
-i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring);
+i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int engine);

'engine' rather than 'engine_id' ?
Elsewhere 'engine' is a pointer-to-intel_engine_cs.


 static bool cpu_cache_is_coherent(struct drm_device *dev,
 				  enum i915_cache_level level)
@@ -1385,10 +1385,10 @@ static void
 i915_gem_object_retire_request(struct drm_i915_gem_object *obj,
 			       struct drm_i915_gem_request *req)
 {
-	int ring = req->engine->id;
+	int idx = req->engine->id;

See below.


-	if (obj->last_read_req[ring] == req)
-		i915_gem_object_retire__read(obj, ring);
+	if (obj->last_read_req[idx] == req)
+		i915_gem_object_retire__read(obj, idx);
 	else if (obj->last_write_req == req)
 		i915_gem_object_retire__write(obj);

@@ -2381,20 +2381,20 @@ i915_gem_object_retire__write(struct drm_i915_gem_object *obj)
 }

 static void
-i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring)
+i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int idx)

I do not fancy declaring different variable names than are used. Also,
idx is very cryptic in this function signature (one would think of
object index).

If you look at the later patches (posted earlier ;) using index is the
less cryptic option as it really does refer to the index of the tracker.

There was a desired to avoid using ring and here I was trying to avoid
confusion with activity tracking.
-Chris

'engine_id' would be unambiguous, although a bit ugly.

I'd rather have the 'id' bit at the end of the name rather than the start, but maybe we can find a good abbreviation for the engine_ part of the name? It's not a word that contracts easily :(

.Dave.

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux