On 22/03/16 11:20, Dave Gordon wrote:
On 22/03/16 09:28, Chris Wilson wrote:
On Mon, Mar 21, 2016 at 03:23:57PM +0000, Dave Gordon wrote:
On 18/03/16 17:26, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
This anonymous struct was causing a good amount of overly
verbose code. Also, if we name it and cache the pointer locally
when there are multiple accesses to it, not only the code is
more readable, but the compiler manages to generate smaller
binary.
Along the way I also shortened access to dev_priv and eliminated
some unused variables and cached some where I spotted the
opportunity.
Name for the structure, intel_context_engine, and the local
variable name were borrowed from a similar patch by Chris Wilson.
v2: Hate the engine->dev surprises, really do.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_lrc.c | 94
+++++++++++++++++++++-------------------
2 files changed, 50 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 00c41a4bde2a..480639c39543 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -840,7 +840,7 @@ struct intel_context {
} legacy_hw_ctx;
/* Execlists */
- struct {
+ struct intel_context_engine {
Good idea, I had a version of this too, derived from Chris' patch
[157/190] drm/i915: Tidy execlists by using intel_context_engine locals.
The only thing to disagree with is the actual name; it should be
"intel_engine_context" (or some abbreviation thereof), because in
We have been using object_subobject.
-Chris
No we haven't. Examples of the above convention for structs include:
* "struct intel_device_info" - information about an intel device
* "struct i915_ctx_hang_stats" - statistics about hangs, per context
* "struct intel_uncore_funcs" - functions exported from uncore
* "struct i915_power_well_ops" - operations on power wells
* "struct i915_suspend_saved_registers" - registers saved at suspend
and for instance names:
* "struct list_head request_list" - a list of requests
* "struct i915_vma *lrc_vma" - the VMA for an LRC
* "uint32_t *lrc_reg_state" - the state of the registers in an LRC
Indeed it's quite difficult to find any compound names that do *not*
follow this convention. Maybe your version would be more natural in a
language where adjectives (or possessives) normally follow the noun they
describe?
One example is maybe:
struct drm_i915_error_state {
struct drm_i915_error_ring
So per-ring state of the total error state, which sounds equivalent to
per-engine state of a context.
Or:
struct intel_fbc {
struct intel_fbc_state_cache {
More verbose version like "struct intel_context_engine_state"?
"struct intel_engine_context" reads like a context of an engine to me,
which sounds opposite to what it should be - state of an engine for a
context.
?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
