[Intel-gfx] [PATCH 2/6] drm/i915: Allow passing any known pointer to for_each_engine()

2016-04-15 Thread Chris Wilson 
Rather than require the user to grab a drm_i915_private, allow them to
pass anything that we know how to derive such a pointer user to_i915()

Note this fixes a macro bug in for_each_engine_masked() which was not
using its dev_priv__ parameter.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.h | 18 +-
 drivers/gpu/drm/i915/i915_gem_context.c |  4 ++--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6569ebfe8cf1..0a62354ba53d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2020,24 +2020,24 @@ static inline struct drm_i915_private 
*guc_to_i915(struct intel_guc *guc)
 }
 
 /* Simple iterator over all initialised engines */
-#define for_each_engine(engine__, dev_priv__) \
-   for ((engine__) = &(dev_priv__)->engine[0]; \
-(engine__) < &(dev_priv__)->engine[I915_NUM_ENGINES]; \
+#define for_each_engine(engine__, ptr__) \
+   for ((engine__) = _i915(ptr__)->engine[0]; \
+(engine__) < _i915(ptr__)->engine[I915_NUM_ENGINES]; \
 (engine__)++) \
for_each_if (intel_engine_initialized(engine__))
 
 /* Iterator with engine_id */
-#define for_each_engine_id(engine__, dev_priv__, id__) \
-   for ((engine__) = &(dev_priv__)->engine[0], (id__) = 0; \
-(engine__) < &(dev_priv__)->engine[I915_NUM_ENGINES]; \
+#define for_each_engine_id(engine__, ptr__, id__) \
+   for ((engine__) = _i915(ptr__)->engine[0], (id__) = 0; \
+(engine__) < _i915(ptr__)->engine[I915_NUM_ENGINES]; \
 (engine__)++) \
for_each_if (((id__) = (engine__)->id, \
  intel_engine_initialized(engine__)))
 
 /* Iterator over subset of engines selected by mask */
-#define for_each_engine_masked(engine__, dev_priv__, mask__) \
-   for ((engine__) = &(dev_priv__)->engine[0]; \
-(engine__) < &(dev_priv__)->engine[I915_NUM_ENGINES]; \
+#define for_each_engine_masked(engine__, ptr__, mask__) \
+   for ((engine__) = _i915(ptr__)->engine[0]; \
+(engine__) < _i915(ptr__)->engine[I915_NUM_ENGINES]; \
 (engine__)++) \
for_each_if (((mask__) & intel_engine_flag(engine__)) && \
 intel_engine_initialized(engine__))
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index e5acc3916f75..b8439971c9bb 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -553,7 +553,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 
hw_flags)
 
intel_ring_emit(engine,
MI_LOAD_REGISTER_IMM(num_rings));
-   for_each_engine(signaller, to_i915(engine->dev)) {
+   for_each_engine(signaller, engine->dev) {
if (signaller == engine)
continue;
 
@@ -583,7 +583,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 
hw_flags)
 
intel_ring_emit(engine,
MI_LOAD_REGISTER_IMM(num_rings));
-   for_each_engine(signaller, to_i915(engine->dev)) {
+   for_each_engine(signaller, engine->dev) {
if (signaller == engine)
continue;
 
-- 
2.8.0.rc3

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

Re: [Intel-gfx] [PATCH 2/6] drm/i915: Allow passing any known pointer to for_each_engine()

2016-03-21 Thread Dave Gordon 

On 18/03/16 21:16, Chris Wilson wrote:

Rather than require the user to grab a drm_i915_private, allow them to
pass anything that we know how to derive such a pointer user to_i915()

Note this fixes a macro bug in for_each_engine_masked() which was not
using its dev_priv__ parameter.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
---
  drivers/gpu/drm/i915/i915_drv.h | 8 
  drivers/gpu/drm/i915/i915_gem_context.c | 4 ++--
  drivers/gpu/drm/i915/intel_mocs.c   | 3 +--
  3 files changed, 7 insertions(+), 8 deletions(-)


Hmm .. generally I'm quite keen on enhancing to_i915(), but I'm not sure 
about this iterator. The thing is, that the array of engines are 
associated with the device (or dev_priv/i915) as a whole, and not with 
an object (etc). In particular, some objects can have associations with 
one or more specific engines, either permanently (e.g. the kernel 
context subobjects) or transiently (objects being used by workloads). It 
therefore seems counterintuitive to use such an object as the basis for 
iterating over all (initialised) engines; I might just as reasonably 
expect the macro to iterate over all (but only) the engines associated 
with the object (whatever that might mean in a particular case).


So I think for_each_engine() should continue to require the caller to 
provide a (struct drm_i915_private *) so that it's clear that we're 
operating on the whole device. But the partially simplification of 
allowing to_i915(engine) rather than to_i915(engine->dev) is fine. And 
of course we still want the bug fix!



diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8606e2c7db04..0c9fe00d3e83 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1988,12 +1988,12 @@ static inline struct drm_i915_private 
*guc_to_i915(struct intel_guc *guc)
  }

  /* Iterate over initialised rings */
-#define for_each_engine(ring__, dev_priv__, i__) \
+#define for_each_engine(ring__, ptr__, i__) \
for ((i__) = 0; (i__) < I915_NUM_ENGINES; (i__)++) \
-   for_each_if ring__) = &(dev_priv__)->engine[(i__)]), 
intel_engine_initialized((ring__
+   for_each_if ring__) = _i915(ptr__)->engine[(i__)]), 
intel_engine_initialized((ring__

-#define for_each_engine_masked(engine__, dev_priv__, mask__) \
-   for ((engine__) = _priv->engine[0]; (engine__) < 
_priv->engine[I915_NUM_ENGINES]; (engine__)++) \
+#define for_each_engine_masked(engine__, ptr__, mask__) \
+   for ((engine__) = _i915(ptr__)->engine[0]; (engine__) < 
_i915(ptr__)->engine[I915_NUM_ENGINES]; (engine__)++) \
for_each_if (intel_engine_flag((engine__)) & (mask__) && 
intel_engine_initialized((engine__)))

  enum hdmi_force_audio {
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 394e525e55f1..a8afd0cee7f7 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -553,7 +553,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 
hw_flags)

intel_ring_emit(engine,
MI_LOAD_REGISTER_IMM(num_rings));
-   for_each_engine(signaller, to_i915(engine->dev), i) {
+   for_each_engine(signaller, engine->dev, i) {


for_each_engine(signaller, to_i915(engine), i) ?


if (signaller == engine)
continue;

@@ -582,7 +582,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 
hw_flags)

intel_ring_emit(engine,
MI_LOAD_REGISTER_IMM(num_rings));
-   for_each_engine(signaller, to_i915(engine->dev), i) {
+   for_each_engine(signaller, engine->dev, i) {


Also for_each_engine(signaller, to_i915(engine), i)


if (signaller == engine)
continue;

diff --git a/drivers/gpu/drm/i915/intel_mocs.c 
b/drivers/gpu/drm/i915/intel_mocs.c
index 3c725dde16ed..45200b93e9bb 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -323,12 +323,11 @@ int intel_rcs_context_init_mocs(struct 
drm_i915_gem_request *req)
int ret;

if (get_mocs_settings(req->engine->dev, )) {
-   struct drm_i915_private *dev_priv = req->i915;
struct intel_engine_cs *engine;
enum intel_engine_id ring_id;

/* Program the control registers */
-   for_each_engine(engine, dev_priv, ring_id) {
+   for_each_engine(engine, req->i915, ring_id) {


I'm quite happy with this one :)

.Dave.


ret = emit_mocs_control_table(req, , ring_id);
if (ret)
return ret;

[Intel-gfx] [PATCH 2/6] drm/i915: Allow passing any known pointer to for_each_engine()

2016-03-19 Thread Chris Wilson 
Rather than require the user to grab a drm_i915_private, allow them to
pass anything that we know how to derive such a pointer user to_i915()

Note this fixes a macro bug in for_each_engine_masked() which was not
using its dev_priv__ parameter.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.h | 8 
 drivers/gpu/drm/i915/i915_gem_context.c | 4 ++--
 drivers/gpu/drm/i915/intel_mocs.c   | 3 +--
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8606e2c7db04..0c9fe00d3e83 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1988,12 +1988,12 @@ static inline struct drm_i915_private 
*guc_to_i915(struct intel_guc *guc)
 }
 
 /* Iterate over initialised rings */
-#define for_each_engine(ring__, dev_priv__, i__) \
+#define for_each_engine(ring__, ptr__, i__) \
for ((i__) = 0; (i__) < I915_NUM_ENGINES; (i__)++) \
-   for_each_if ring__) = &(dev_priv__)->engine[(i__)]), 
intel_engine_initialized((ring__
+   for_each_if ring__) = _i915(ptr__)->engine[(i__)]), 
intel_engine_initialized((ring__
 
-#define for_each_engine_masked(engine__, dev_priv__, mask__) \
-   for ((engine__) = _priv->engine[0]; (engine__) < 
_priv->engine[I915_NUM_ENGINES]; (engine__)++) \
+#define for_each_engine_masked(engine__, ptr__, mask__) \
+   for ((engine__) = _i915(ptr__)->engine[0]; (engine__) < 
_i915(ptr__)->engine[I915_NUM_ENGINES]; (engine__)++) \
for_each_if (intel_engine_flag((engine__)) & (mask__) && 
intel_engine_initialized((engine__)))
 
 enum hdmi_force_audio {
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 394e525e55f1..a8afd0cee7f7 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -553,7 +553,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 
hw_flags)
 
intel_ring_emit(engine,
MI_LOAD_REGISTER_IMM(num_rings));
-   for_each_engine(signaller, to_i915(engine->dev), i) {
+   for_each_engine(signaller, engine->dev, i) {
if (signaller == engine)
continue;
 
@@ -582,7 +582,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 
hw_flags)
 
intel_ring_emit(engine,
MI_LOAD_REGISTER_IMM(num_rings));
-   for_each_engine(signaller, to_i915(engine->dev), i) {
+   for_each_engine(signaller, engine->dev, i) {
if (signaller == engine)
continue;
 
diff --git a/drivers/gpu/drm/i915/intel_mocs.c 
b/drivers/gpu/drm/i915/intel_mocs.c
index 3c725dde16ed..45200b93e9bb 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -323,12 +323,11 @@ int intel_rcs_context_init_mocs(struct 
drm_i915_gem_request *req)
int ret;
 
if (get_mocs_settings(req->engine->dev, )) {
-   struct drm_i915_private *dev_priv = req->i915;
struct intel_engine_cs *engine;
enum intel_engine_id ring_id;
 
/* Program the control registers */
-   for_each_engine(engine, dev_priv, ring_id) {
+   for_each_engine(engine, req->i915, ring_id) {
ret = emit_mocs_control_table(req, , ring_id);
if (ret)
return ret;
-- 
2.8.0.rc3

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