On 13/03/2015 12:46, John Harrison wrote:
On 05/03/2015 17:57, Tomas Elf wrote:
On 19/02/2015 17:17, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
The final step in removing the OLR from i915_gem_init_hw() is to pass
the newly
allocated request structure in to each step rather than passing a ring
structure. This patch updates both i915_ppgtt_init_ring() and
i915_gem_context_enable() to take request pointers.
For: VIZ-5115
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 4 ++--
drivers/gpu/drm/i915/i915_gem_context.c | 7 ++++---
drivers/gpu/drm/i915/i915_gem_gtt.c | 6 +++---
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +-
5 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index ea0da6b..618a841 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2990,7 +2990,7 @@ int __must_check i915_gem_context_init(struct
drm_device *dev);
void i915_gem_context_fini(struct drm_device *dev);
void i915_gem_context_reset(struct drm_device *dev);
int i915_gem_context_open(struct drm_device *dev, struct drm_file
*file);
-int i915_gem_context_enable(struct intel_engine_cs *ring);
+int i915_gem_context_enable(struct drm_i915_gem_request *req);
void i915_gem_context_close(struct drm_device *dev, struct drm_file
*file);
int i915_switch_context(struct intel_engine_cs *ring,
struct intel_context *to);
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index efed49a..379bf44 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4877,7 +4877,7 @@ i915_gem_init_hw(struct drm_device *dev)
i915_gem_l3_remap(ring, i);
}
- ret = i915_ppgtt_init_ring(ring);
+ ret = i915_ppgtt_init_ring(req);
if (ret && ret != -EIO) {
DRM_ERROR("PPGTT enable ring #%d failed %d\n", i, ret);
i915_gem_request_unreference(req);
@@ -4885,7 +4885,7 @@ i915_gem_init_hw(struct drm_device *dev)
return ret;
}
- ret = i915_gem_context_enable(ring);
+ ret = i915_gem_context_enable(req);
if (ret && ret != -EIO) {
DRM_ERROR("Context enable ring #%d failed %d\n", i, ret);
i915_gem_request_unreference(req);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index dd83d61..04d2a20 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -403,17 +403,18 @@ void i915_gem_context_fini(struct drm_device *dev)
i915_gem_context_unreference(dctx);
}
-int i915_gem_context_enable(struct intel_engine_cs *ring)
+int i915_gem_context_enable(struct drm_i915_gem_request *req)
{
+ struct intel_engine_cs *ring = req->ring;
int ret;
if (i915.enable_execlists) {
if (ring->init_context == NULL)
return 0;
- ret = ring->init_context(ring, ring->default_context);
+ ret = ring->init_context(req->ring, ring->default_context);
} else
- ret = i915_switch_context(ring, ring->default_context);
+ ret = i915_switch_context(req->ring, ring->default_context);
You don't have to make any more changes to this function aside from
setting up the ring variable at the top. ring = req->ring and if you
don't change these lines the will by default use ring like they always
did.
if (ret) {
DRM_ERROR("ring init context: %d\n", ret);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 428d2f6..cd00080 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1227,15 +1227,15 @@ int i915_ppgtt_init_hw(struct drm_device *dev)
return 0;
}
-int i915_ppgtt_init_ring(struct intel_engine_cs *ring)
+int i915_ppgtt_init_ring(struct drm_i915_gem_request *req)
{
- struct drm_i915_private *dev_priv = ring->dev->dev_private;
+ struct drm_i915_private *dev_priv = req->ring->dev->dev_private;
struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
if (!ppgtt)
return 0;
- return ppgtt->switch_mm(ppgtt, ring);
+ return ppgtt->switch_mm(ppgtt, req->ring);
}
If you want to uphold a pattern that you've already established you
could just make a single change in the function above by setting up
ring = req->ring and then make no more changes to the function body.
In this case it's one new line vs. two changes of existing code so
it's doesn't make that much of a difference but it is nice to stick to
patterns. Also, you wouldn't have to make a 4-level indirection
(req->ring->dev->dev_private), only a 3-level one.
It all depends how often the ring variable would get used. In this case,
there are only two references. One of which, the switch_mm(), will
disappear later in the series anyway. So for the sake of a single usage,
it isn't worth adding in the extra line. The dev_priv is only really
there because if you don't use an explicit variable, you have to do a
messy type cast in the middle of the dereference chain.
If switch_mm won't require the ring parameter later in the patch series
(which I should've noticed) then it admittedly becomes kind of
unnecessary to have a local ring variable. Never mind!
Thanks,
Tomas
struct i915_hw_ppgtt *
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 5a6cef9..e7e202f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -300,7 +300,7 @@ void i915_global_gtt_cleanup(struct drm_device
*dev);
int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt
*ppgtt);
int i915_ppgtt_init_hw(struct drm_device *dev);
-int i915_ppgtt_init_ring(struct intel_engine_cs *ring);
+int i915_ppgtt_init_ring(struct drm_i915_gem_request *req);
void i915_ppgtt_release(struct kref *kref);
struct i915_hw_ppgtt *i915_ppgtt_create(struct drm_device *dev,
struct drm_i915_file_private *fpriv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
