Re: [Intel-gfx] [PATCH] drm/i915: Rename drm_i915_gem_request to i915_request

[Chris Wilson]

Quoting Joonas Lahtinen (2018-02-21 18:40:51)
> Quoting Chris Wilson (2018-02-21 11:56:36)
> > We want to de-emphasize the link between the request (dependency,
> > execution and fence tracking) from GEM and so rename the struct from
> > drm_i915_gem_request to i915_request. That is we may implement the GEM
> > user interface on top of requests, but they are an abstraction for
> > tracking execution rather than an implementation detail of GEM. (Since
> > they are not tied to HW, we keep the i915 prefix as opposed to intel.)
> > 
> > In short, the spatch:
> > @@
> > 
> > @@
> > - struct drm_i915_gem_request
> > + struct i915_request
> > 
> > A corollary to contracting the type name, we also harmonise on using
> > 'rq' shorthand for local variables where space if of the essence and
> > repetition makes 'request' unwieldy. For globals and struct members,
> > 'request' is still much preferred for its clarity.
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Joonas Lahtinen 
> > Cc: Mika Kuoppala 
> > Cc: Tvrtko Ursulin 
> > Cc: Michał Winiarski 
> > Cc: Michal Wajdeczko 
> 
> Right, should not cause a mayhem when merged now.
> 
> Acked-by: Joonas Lahtinen 

And pushed before the pain starts.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Rename drm_i915_gem_request to i915_request

[Joonas Lahtinen]

Quoting Chris Wilson (2018-02-21 11:56:36)
> We want to de-emphasize the link between the request (dependency,
> execution and fence tracking) from GEM and so rename the struct from
> drm_i915_gem_request to i915_request. That is we may implement the GEM
> user interface on top of requests, but they are an abstraction for
> tracking execution rather than an implementation detail of GEM. (Since
> they are not tied to HW, we keep the i915 prefix as opposed to intel.)
> 
> In short, the spatch:
> @@
> 
> @@
> - struct drm_i915_gem_request
> + struct i915_request
> 
> A corollary to contracting the type name, we also harmonise on using
> 'rq' shorthand for local variables where space if of the essence and
> repetition makes 'request' unwieldy. For globals and struct members,
> 'request' is still much preferred for its clarity.
> 
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 
> Cc: Mika Kuoppala 
> Cc: Tvrtko Ursulin 
> Cc: Michał Winiarski 
> Cc: Michal Wajdeczko 

Right, should not cause a mayhem when merged now.

Acked-by: Joonas Lahtinen 

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

Re: [Intel-gfx] [PATCH] drm/i915: Rename drm_i915_gem_request to i915_request

[Michał Winiarski]

On Wed, Feb 21, 2018 at 09:56:36AM +, Chris Wilson wrote:
> We want to de-emphasize the link between the request (dependency,
> execution and fence tracking) from GEM and so rename the struct from
> drm_i915_gem_request to i915_request. That is we may implement the GEM
> user interface on top of requests, but they are an abstraction for
> tracking execution rather than an implementation detail of GEM. (Since
> they are not tied to HW, we keep the i915 prefix as opposed to intel.)
> 
> In short, the spatch:
> @@
> 
> @@
> - struct drm_i915_gem_request
> + struct i915_request
> 
> A corollary to contracting the type name, we also harmonise on using
> 'rq' shorthand for local variables where space if of the essence and
> repetition makes 'request' unwieldy. For globals and struct members,
> 'request' is still much preferred for its clarity.
> 
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 
> Cc: Mika Kuoppala 
> Cc: Tvrtko Ursulin 
> Cc: Michał Winiarski 
> Cc: Michal Wajdeczko 

Reviewed-by: Michał Winiarski 

-Michał

> ---
>  drivers/gpu/drm/i915/Makefile  |   2 +-
>  drivers/gpu/drm/i915/gvt/scheduler.c   |  16 +-
>  drivers/gpu/drm/i915/gvt/scheduler.h   |   2 +-
>  drivers/gpu/drm/i915/i915_debugfs.c|   6 +-
>  drivers/gpu/drm/i915/i915_drv.c|   6 +-
>  drivers/gpu/drm/i915/i915_drv.h|  26 +-
>  drivers/gpu/drm/i915/i915_gem.c|  88 +++--
>  drivers/gpu/drm/i915/i915_gem_batch_pool.c |   2 +-
>  drivers/gpu/drm/i915/i915_gem_context.c|  18 +-
>  drivers/gpu/drm/i915/i915_gem_context.h|   2 +-
>  drivers/gpu/drm/i915/i915_gem_evict.c  |   4 +-
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  60 ++--
>  drivers/gpu/drm/i915/i915_gem_gtt.c|  38 +--
>  drivers/gpu/drm/i915/i915_gem_gtt.h|   5 +-
>  drivers/gpu/drm/i915/i915_gem_object.h |   2 +-
>  drivers/gpu/drm/i915/i915_gem_render_state.c   |   2 +-
>  drivers/gpu/drm/i915/i915_gem_render_state.h   |   4 +-
>  drivers/gpu/drm/i915/i915_gem_shrinker.c   |   4 +-
>  drivers/gpu/drm/i915/i915_gem_timeline.h   |   4 +-
>  drivers/gpu/drm/i915/i915_gpu_error.c  |  18 +-
>  drivers/gpu/drm/i915/i915_irq.c|   8 +-
>  drivers/gpu/drm/i915/i915_perf.c   |  28 +-
>  .../i915/{i915_gem_request.c => i915_request.c}| 378 
> +++--
>  .../i915/{i915_gem_request.h => i915_request.h}| 220 ++--
>  drivers/gpu/drm/i915/i915_trace.h  | 128 ---
>  drivers/gpu/drm/i915/i915_vma.c|   3 +-
>  drivers/gpu/drm/i915/i915_vma.h|   2 +-
>  drivers/gpu/drm/i915/intel_breadcrumbs.c   |  31 +-
>  drivers/gpu/drm/i915/intel_display.c   |   8 +-
>  drivers/gpu/drm/i915/intel_drv.h   |   3 +-
>  drivers/gpu/drm/i915/intel_engine_cs.c |  26 +-
>  drivers/gpu/drm/i915/intel_guc_submission.c|  27 +-
>  drivers/gpu/drm/i915/intel_lrc.c   | 115 +++
>  drivers/gpu/drm/i915/intel_mocs.c  |  28 +-
>  drivers/gpu/drm/i915/intel_mocs.h  |   2 +-
>  drivers/gpu/drm/i915/intel_overlay.c   |  82 ++---
>  drivers/gpu/drm/i915/intel_pm.c|   4 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c| 196 ++-
>  drivers/gpu/drm/i915/intel_ringbuffer.h|  78 ++---
>  drivers/gpu/drm/i915/selftests/huge_pages.c|   6 +-
>  .../gpu/drm/i915/selftests/i915_gem_coherency.c|   8 +-
>  drivers/gpu/drm/i915/selftests/i915_gem_context.c  |   8 +-
>  drivers/gpu/drm/i915/selftests/i915_gem_evict.c|   6 +-
>  drivers/gpu/drm/i915/selftests/i915_gem_object.c   |   6 +-
>  .../gpu/drm/i915/selftests/i915_live_selftests.h   |   2 +-
>  .../gpu/drm/i915/selftests/i915_mock_selftests.h   |   2 +-
>  .../{i915_gem_request.c => i915_request.c} | 125 ---
>  drivers/gpu/drm/i915/selftests/intel_hangcheck.c   | 117 ---
>  drivers/gpu/drm/i915/selftests/mock_engine.c   |  10 +-
>  drivers/gpu/drm/i915/selftests/mock_gem_device.c   |   2 +-
>  drivers/gpu/drm/i915/selftests/mock_request.c  |  10 +-
>  drivers/gpu/drm/i915/selftests/mock_request.h  |   8 +-
>  52 files changed, 990 insertions(+), 996 deletions(-)
>  rename drivers/gpu/drm/i915/{i915_gem_request.c => i915_request.c} (79%)
>  rename drivers/gpu/drm/i915/{i915_gem_request.h => i915_request.h} (79%)
>  rename drivers/gpu/drm/i915/selftests/{i915_gem_request.c => i915_request.c} 
> (87%)
___
Intel-gfx mailing list

Re: [Intel-gfx] [PATCH] drm/i915: Rename drm_i915_gem_request to i915_request

[Mika Kuoppala]

Chris Wilson  writes:

> We want to de-emphasize the link between the request (dependency,
> execution and fence tracking) from GEM and so rename the struct from
> drm_i915_gem_request to i915_request. That is we may implement the GEM
> user interface on top of requests, but they are an abstraction for
> tracking execution rather than an implementation detail of GEM. (Since
> they are not tied to HW, we keep the i915 prefix as opposed to intel.)
>
> In short, the spatch:
> @@
>
> @@
> - struct drm_i915_gem_request
> + struct i915_request
>
> A corollary to contracting the type name, we also harmonise on using
> 'rq' shorthand for local variables where space if of the essence and
> repetition makes 'request' unwieldy. For globals and struct members,
> 'request' is still much preferred for its clarity.
>
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 
> Cc: Mika Kuoppala 
> Cc: Tvrtko Ursulin 
> Cc: Michał Winiarski 
> Cc: Michal Wajdeczko 
> ---
>  drivers/gpu/drm/i915/Makefile  |   2 +-
>  drivers/gpu/drm/i915/gvt/scheduler.c   |  16 +-
>  drivers/gpu/drm/i915/gvt/scheduler.h   |   2 +-
>  drivers/gpu/drm/i915/i915_debugfs.c|   6 +-
>  drivers/gpu/drm/i915/i915_drv.c|   6 +-
>  drivers/gpu/drm/i915/i915_drv.h|  26 +-
>  drivers/gpu/drm/i915/i915_gem.c|  88 +++--
>  drivers/gpu/drm/i915/i915_gem_batch_pool.c |   2 +-
>  drivers/gpu/drm/i915/i915_gem_context.c|  18 +-
>  drivers/gpu/drm/i915/i915_gem_context.h|   2 +-
>  drivers/gpu/drm/i915/i915_gem_evict.c  |   4 +-
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  60 ++--
>  drivers/gpu/drm/i915/i915_gem_gtt.c|  38 +--
>  drivers/gpu/drm/i915/i915_gem_gtt.h|   5 +-
>  drivers/gpu/drm/i915/i915_gem_object.h |   2 +-
>  drivers/gpu/drm/i915/i915_gem_render_state.c   |   2 +-
>  drivers/gpu/drm/i915/i915_gem_render_state.h   |   4 +-
>  drivers/gpu/drm/i915/i915_gem_shrinker.c   |   4 +-
>  drivers/gpu/drm/i915/i915_gem_timeline.h   |   4 +-
>  drivers/gpu/drm/i915/i915_gpu_error.c  |  18 +-
>  drivers/gpu/drm/i915/i915_irq.c|   8 +-
>  drivers/gpu/drm/i915/i915_perf.c   |  28 +-
>  .../i915/{i915_gem_request.c => i915_request.c}| 378 
> +++--
>  .../i915/{i915_gem_request.h => i915_request.h}| 220 ++--
>  drivers/gpu/drm/i915/i915_trace.h  | 128 ---
>  drivers/gpu/drm/i915/i915_vma.c|   3 +-
>  drivers/gpu/drm/i915/i915_vma.h|   2 +-
>  drivers/gpu/drm/i915/intel_breadcrumbs.c   |  31 +-
>  drivers/gpu/drm/i915/intel_display.c   |   8 +-
>  drivers/gpu/drm/i915/intel_drv.h   |   3 +-
>  drivers/gpu/drm/i915/intel_engine_cs.c |  26 +-
>  drivers/gpu/drm/i915/intel_guc_submission.c|  27 +-
>  drivers/gpu/drm/i915/intel_lrc.c   | 115 +++
>  drivers/gpu/drm/i915/intel_mocs.c  |  28 +-
>  drivers/gpu/drm/i915/intel_mocs.h  |   2 +-
>  drivers/gpu/drm/i915/intel_overlay.c   |  82 ++---
>  drivers/gpu/drm/i915/intel_pm.c|   4 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c| 196 ++-
>  drivers/gpu/drm/i915/intel_ringbuffer.h|  78 ++---
>  drivers/gpu/drm/i915/selftests/huge_pages.c|   6 +-
>  .../gpu/drm/i915/selftests/i915_gem_coherency.c|   8 +-
>  drivers/gpu/drm/i915/selftests/i915_gem_context.c  |   8 +-
>  drivers/gpu/drm/i915/selftests/i915_gem_evict.c|   6 +-
>  drivers/gpu/drm/i915/selftests/i915_gem_object.c   |   6 +-
>  .../gpu/drm/i915/selftests/i915_live_selftests.h   |   2 +-
>  .../gpu/drm/i915/selftests/i915_mock_selftests.h   |   2 +-
>  .../{i915_gem_request.c => i915_request.c} | 125 ---
>  drivers/gpu/drm/i915/selftests/intel_hangcheck.c   | 117 ---
>  drivers/gpu/drm/i915/selftests/mock_engine.c   |  10 +-
>  drivers/gpu/drm/i915/selftests/mock_gem_device.c   |   2 +-
>  drivers/gpu/drm/i915/selftests/mock_request.c  |  10 +-
>  drivers/gpu/drm/i915/selftests/mock_request.h  |   8 +-
>  52 files changed, 990 insertions(+), 996 deletions(-)
>  rename drivers/gpu/drm/i915/{i915_gem_request.c => i915_request.c} (79%)
>  rename drivers/gpu/drm/i915/{i915_gem_request.h => i915_request.h} (79%)
>  rename drivers/gpu/drm/i915/selftests/{i915_gem_request.c => i915_request.c} 
> (87%)
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index f55cc028b2eb..881d7124c597 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ 

Re: [Intel-gfx] [PATCH] drm/i915: Rename drm_i915_gem_request to i915_request

[Mika Kuoppala]

Chris Wilson  writes:

> Quoting Joonas Lahtinen (2018-02-09 07:48:21)
>> Quoting Chris Wilson (2018-02-09 01:11:34)
>> > We want to de-emphasize the link between the request (dependency,
>> > execution and fence tracking) from GEM and so rename the struct from
>> > drm_i915_gem_request to i915_request. That is we may implement the GEM
>> > user interface on top of requests, but they are an abstraction for
>> > tracking execution rather than an implementation detail of GEM. (Since
>> > they are not tied to HW, we keep the i915 prefix as opposed to intel.)
>> 
>> There are also some req -> rq renames in addition to function renames.
>> 
>> If we're touching this much code, would it make sense to at least
>> consolidate the parameter names into "request" or "req" when touched
>> here.
>
> Never req. I always used rq in the pre-existing code as shorthand, and
> request otherwise.

One could argue that rq is too short but it is so fundamental concept
in gem code that obviousess/uniqueness is usually guaranteed.

My vote will go to 'rq' too.

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

Re: [Intel-gfx] [PATCH] drm/i915: Rename drm_i915_gem_request to i915_request

[Chris Wilson]

Quoting Joonas Lahtinen (2018-02-09 07:48:21)
> Quoting Chris Wilson (2018-02-09 01:11:34)
> > We want to de-emphasize the link between the request (dependency,
> > execution and fence tracking) from GEM and so rename the struct from
> > drm_i915_gem_request to i915_request. That is we may implement the GEM
> > user interface on top of requests, but they are an abstraction for
> > tracking execution rather than an implementation detail of GEM. (Since
> > they are not tied to HW, we keep the i915 prefix as opposed to intel.)
> 
> There are also some req -> rq renames in addition to function renames.
> 
> If we're touching this much code, would it make sense to at least
> consolidate the parameter names into "request" or "req" when touched
> here.

Never req. I always used rq in the pre-existing code as shorthand, and
request otherwise.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx