On 04/04/16 20:14, Dave Gordon wrote:
On 04/04/16 17:51, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
As the vast majority of users does not use the domain id variable,
"do not use"
Yep.
we can eliminate it from the iterator and also change the latter
using the same principle as was recently done for for_each_engine.
For a couple of callers which do need the domain id, or the domain
mask, store the later in the domain itself at init time and use
both through it.
"For a couple of callers which do need the domain mask, store it
in the domain array (which already has the domain id), then both
can be retrieved thence." ?
Better yes.
Result is clearer code and smaller generated binary, especially
in the tight fw get/put loops. Also, relationship between domain
id and mask is no longer assumed in the macro.
"in the macro or elsewhere" ?
Just in the macro, it is still hardcoded in fw_domain_init.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
A few typos & clarifications above, plus a minor quibble about a macro
name (which you haven't actually changed, but might as well fix).
Otherwise LGTM, so
Reviewed-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
(even if you don't change the macro name)
Thanks, consistency sounds good so I will change it.
---
drivers/gpu/drm/i915/i915_debugfs.c | 5 ++---
drivers/gpu/drm/i915/i915_drv.h | 17 +++++++-------
drivers/gpu/drm/i915/intel_uncore.c | 45
+++++++++++++++++--------------------
3 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index a2e3af02c292..06fce014d0b4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1464,12 +1464,11 @@ static int i915_forcewake_domains(struct
seq_file *m, void *data)
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_uncore_forcewake_domain *fw_domain;
- int i;
spin_lock_irq(&dev_priv->uncore.lock);
- for_each_fw_domain(fw_domain, dev_priv, i) {
+ for_each_fw_domain(fw_domain, dev_priv) {
seq_printf(m, "%s.wake_count = %u\n",
- intel_uncore_forcewake_domain_to_str(i),
+ intel_uncore_forcewake_domain_to_str(fw_domain->id),
fw_domain->wake_count);
}
spin_unlock_irq(&dev_priv->uncore.lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 7d4c704d7d75..160f980f0368 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -665,6 +665,7 @@ struct intel_uncore {
struct intel_uncore_forcewake_domain {
struct drm_i915_private *i915;
enum forcewake_domain_id id;
+ enum forcewake_domains mask;
At present this mask will always have only one bit set, but I suppose
there might be some utility in allowing multiple bits (virtual domains?)
I did not like the name mask myself but couldn't think of anything
better. Do you have a suggestion?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
