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

Joonas Lahtinen
2016-07-26T05:02:27Z

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.
>    */
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation