| Message ID | 1469467954-3920-9-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ma, 2016-07-25 at 18:31 +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@chris-wilson.co.uk> > Link: http://patchwork.freedesktop.org/patch/msgid/1469432687-22756-9-git-send-email-chris@chris-wilson.co.uk > --- > 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); I vote for engine_idx variable name, that much I dislike differing naming in signature and implementation. Regards, Joonas > > 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; > > - 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) > { > struct i915_vma *vma; > > - GEM_BUG_ON(obj->last_read_req[ring] == NULL); > - GEM_BUG_ON(!(obj->active & (1 << ring))); > + GEM_BUG_ON(obj->last_read_req[idx] == NULL); > + GEM_BUG_ON(!(obj->active & (1 << idx))); > > - list_del_init(&obj->engine_list[ring]); > - i915_gem_request_assign(&obj->last_read_req[ring], NULL); > + list_del_init(&obj->engine_list[idx]); > + i915_gem_request_assign(&obj->last_read_req[idx], NULL); > > - if (obj->last_write_req && obj->last_write_req->engine->id == ring) > + if (obj->last_write_req && obj->last_write_req->engine->id == idx) > i915_gem_object_retire__write(obj); > > - obj->active &= ~(1 << ring); > + obj->active &= ~(1 << idx); > if (obj->active) > return; > > @@ -4599,7 +4599,7 @@ int i915_gem_init(struct drm_device *dev) > > ret = i915_gem_init_hw(dev); > if (ret == -EIO) { > - /* Allow ring initialisation to fail by marking the GPU as > + /* Allow engine initialisation to fail by marking the GPU as > * wedged. But we only want to do this where the GPU is angry, > * for all other failure, such as an allocation failure, bail. > */
On Tue, Jul 26, 2016 at 08:02:25AM +0300, Joonas Lahtinen wrote: > On ma, 2016-07-25 at 18:31 +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@chris-wilson.co.uk> > > Link: http://patchwork.freedesktop.org/patch/msgid/1469432687-22756-9-git-send-email-chris@chris-wilson.co.uk > > --- > > 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); > > I vote for engine_idx variable name, that much I dislike differing > naming in signature and implementation. I think engine_idx is pretty repugnant. I'll wait until you get to "Refactor activity tracking for requests" so you see how this function looks closer to its final form (in this series at least). -Chris
On ti, 2016-07-26 at 09:12 +0100, Chris Wilson wrote: > On Tue, Jul 26, 2016 at 08:02:25AM +0300, Joonas Lahtinen wrote: > > > > On ma, 2016-07-25 at 18:31 +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@chris-wilson.co.uk> > > > Link: http://patchwork.freedesktop.org/patch/msgid/1469432687-22756-9-git-send-email-chris@chris-wilson.co.uk > > > --- > > > 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); > > I vote for engine_idx variable name, that much I dislike differing > > naming in signature and implementation. > I think engine_idx is pretty repugnant. I'll wait until you get to > "Refactor activity tracking for requests" so you see how this function > looks closer to its final form (in this series at least). Ok, the parameter completely changes (into request), so no need to debate really. So have my; Reviewed-by: Joonas Lahtien <joonas.lahtinen@linux.intel.com> CC'ing Dave here too. Regards, Joonas > -Chris >
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); 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; - 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) { struct i915_vma *vma; - GEM_BUG_ON(obj->last_read_req[ring] == NULL); - GEM_BUG_ON(!(obj->active & (1 << ring))); + GEM_BUG_ON(obj->last_read_req[idx] == NULL); + GEM_BUG_ON(!(obj->active & (1 << idx))); - list_del_init(&obj->engine_list[ring]); - i915_gem_request_assign(&obj->last_read_req[ring], NULL); + list_del_init(&obj->engine_list[idx]); + i915_gem_request_assign(&obj->last_read_req[idx], NULL); - if (obj->last_write_req && obj->last_write_req->engine->id == ring) + if (obj->last_write_req && obj->last_write_req->engine->id == idx) i915_gem_object_retire__write(obj); - obj->active &= ~(1 << ring); + obj->active &= ~(1 << idx); if (obj->active) return; @@ -4599,7 +4599,7 @@ int i915_gem_init(struct drm_device *dev) ret = i915_gem_init_hw(dev); if (ret == -EIO) { - /* Allow ring initialisation to fail by marking the GPU as + /* Allow engine initialisation to fail by marking the GPU as * wedged. But we only want to do this where the GPU is angry, * for all other failure, such as an allocation failure, bail. */
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@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1469432687-22756-9-git-send-email-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)