| Message ID | 1450713885-5828-4-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 21, 2015 at 04:04:42PM +0000, Dave Gordon wrote: > There are a number of places where the driver needs a request, but isn't > working on behalf of any specific user or in a specific context. For > such requests, we associate them with the default context for the engine > that the request will be submitted to. > > This patch provides a shorthand for doing such request allocations and > changes all such calls to use the new function. > > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/i915_gem.c | 36 ++++++++++++++++++++++++++++-------- > drivers/gpu/drm/i915/intel_display.c | 6 ++++-- > drivers/gpu/drm/i915/intel_overlay.c | 24 ++++++++++++------------ > 4 files changed, 46 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 666d07c..4955db9 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2270,6 +2270,8 @@ struct drm_i915_gem_request { > int i915_gem_request_alloc(struct intel_engine_cs *ring, > struct intel_context *ctx, > struct drm_i915_gem_request **req_out); > +struct drm_i915_gem_request * __must_check > + i915_gem_request_alloc_anon(struct intel_engine_cs *ring); > void i915_gem_request_cancel(struct drm_i915_gem_request *req); > void i915_gem_request_free(struct kref *req_ref); > int i915_gem_request_add_to_client(struct drm_i915_gem_request *req, > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index be1f984..9f9c0c0 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2751,6 +2751,21 @@ err: > return ret; > } > > +/* > + * Allocate a request associated with the default context for the given > + * ring. This can be used where the driver needs a request for internal > + * purposes not directly related to a user batch submission. > + */ > +struct drm_i915_gem_request * > +i915_gem_request_alloc_anon(struct intel_engine_cs *ring) > +{ As demonstrated, no. Contexts need to be considered properly first. > + struct drm_i915_gem_request *req; > + int err; > + > + err = i915_gem_request_alloc(ring, ring->default_context, &req); > + return err ? ERR_PTR(err) : req; > +} > + > void i915_gem_request_cancel(struct drm_i915_gem_request *req) > { > intel_ring_reserved_space_cancel(req->ringbuf); > @@ -3168,9 +3183,13 @@ __i915_gem_object_sync(struct drm_i915_gem_object *obj, > return 0; > > if (*to_req == NULL) { > - ret = i915_gem_request_alloc(to, to->default_context, to_req); > - if (ret) > - return ret; > + struct drm_i915_gem_request *req; > + > + req = i915_gem_request_alloc_anon(to); Wrong context. Please see patches to fix this mess. > + if (IS_ERR(req)) > + return PTR_ERR(req); > + > + *to_req = req; > } > > trace_i915_gem_ring_sync_to(*to_req, from, from_req); > @@ -3370,9 +3389,9 @@ int i915_gpu_idle(struct drm_device *dev) > if (!i915.enable_execlists) { > struct drm_i915_gem_request *req; > > - ret = i915_gem_request_alloc(ring, ring->default_context, &req); > - if (ret) > - return ret; > + req = i915_gem_request_alloc_anon(ring); > + if (IS_ERR(req)) > + return PTR_ERR(req); > > ret = i915_switch_context(req); > if (ret) { > @@ -4867,8 +4886,9 @@ i915_gem_init_hw(struct drm_device *dev) > for_each_ring(ring, dev_priv, i) { > struct drm_i915_gem_request *req; > > - ret = i915_gem_request_alloc(ring, ring->default_context, &req); > - if (ret) { > + req = i915_gem_request_alloc_anon(ring); > + if (IS_ERR(req)) { > + ret = PTR_ERR(req); > i915_gem_cleanup_ringbuffer(dev); > goto out; > } > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index abd2d29..5716f4a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11662,9 +11662,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > obj->last_write_req); > } else { > if (!request) { > - ret = i915_gem_request_alloc(ring, ring->default_context, &request); > - if (ret) Wrong context. -Chris
On 22/12/15 09:08, Chris Wilson wrote: > On Mon, Dec 21, 2015 at 04:04:42PM +0000, Dave Gordon wrote: >> There are a number of places where the driver needs a request, but isn't >> working on behalf of any specific user or in a specific context. For >> such requests, we associate them with the default context for the engine >> that the request will be submitted to. >> >> This patch provides a shorthand for doing such request allocations and >> changes all such calls to use the new function. >> >> Signed-off-by: Dave Gordon <david.s.gordon@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 2 ++ >> drivers/gpu/drm/i915/i915_gem.c | 36 ++++++++++++++++++++++++++++-------- >> drivers/gpu/drm/i915/intel_display.c | 6 ++++-- >> drivers/gpu/drm/i915/intel_overlay.c | 24 ++++++++++++------------ >> 4 files changed, 46 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 666d07c..4955db9 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2270,6 +2270,8 @@ struct drm_i915_gem_request { >> int i915_gem_request_alloc(struct intel_engine_cs *ring, >> struct intel_context *ctx, >> struct drm_i915_gem_request **req_out); >> +struct drm_i915_gem_request * __must_check >> + i915_gem_request_alloc_anon(struct intel_engine_cs *ring); >> void i915_gem_request_cancel(struct drm_i915_gem_request *req); >> void i915_gem_request_free(struct kref *req_ref); >> int i915_gem_request_add_to_client(struct drm_i915_gem_request *req, >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index be1f984..9f9c0c0 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -2751,6 +2751,21 @@ err: >> return ret; >> } >> >> +/* >> + * Allocate a request associated with the default context for the given >> + * ring. This can be used where the driver needs a request for internal >> + * purposes not directly related to a user batch submission. >> + */ >> +struct drm_i915_gem_request * >> +i915_gem_request_alloc_anon(struct intel_engine_cs *ring) >> +{ > > As demonstrated, no. Contexts need to be considered properly first. > >> + struct drm_i915_gem_request *req; >> + int err; >> + >> + err = i915_gem_request_alloc(ring, ring->default_context, &req); >> + return err ? ERR_PTR(err) : req; >> +} >> + >> void i915_gem_request_cancel(struct drm_i915_gem_request *req) >> { >> intel_ring_reserved_space_cancel(req->ringbuf); >> @@ -3168,9 +3183,13 @@ __i915_gem_object_sync(struct drm_i915_gem_object *obj, >> return 0; >> >> if (*to_req == NULL) { >> - ret = i915_gem_request_alloc(to, to->default_context, to_req); >> - if (ret) >> - return ret; >> + struct drm_i915_gem_request *req; >> + >> + req = i915_gem_request_alloc_anon(to); > > Wrong context. Please see patches to fix this mess. > >> + if (IS_ERR(req)) >> + return PTR_ERR(req); >> + >> + *to_req = req; >> } >> >> trace_i915_gem_ring_sync_to(*to_req, from, from_req); >> @@ -3370,9 +3389,9 @@ int i915_gpu_idle(struct drm_device *dev) >> if (!i915.enable_execlists) { >> struct drm_i915_gem_request *req; >> >> - ret = i915_gem_request_alloc(ring, ring->default_context, &req); >> - if (ret) >> - return ret; >> + req = i915_gem_request_alloc_anon(ring); >> + if (IS_ERR(req)) >> + return PTR_ERR(req); >> >> ret = i915_switch_context(req); >> if (ret) { >> @@ -4867,8 +4886,9 @@ i915_gem_init_hw(struct drm_device *dev) >> for_each_ring(ring, dev_priv, i) { >> struct drm_i915_gem_request *req; >> >> - ret = i915_gem_request_alloc(ring, ring->default_context, &req); >> - if (ret) { >> + req = i915_gem_request_alloc_anon(ring); >> + if (IS_ERR(req)) { >> + ret = PTR_ERR(req); >> i915_gem_cleanup_ringbuffer(dev); >> goto out; >> } >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index abd2d29..5716f4a 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -11662,9 +11662,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, >> obj->last_write_req); >> } else { >> if (!request) { >> - ret = i915_gem_request_alloc(ring, ring->default_context, &request); >> - if (ret) > > Wrong context. > -Chris What do you mean, "wrong context". The line above is the way it *currently* works, which I'm replacing with one that *doesn't* refer to the default context. This patch doesn't change any functionality, just simplifies it by reducing the number of instances of "default_context" so that patch 4/6 can actually get rid of ring->default_context entirely. I thought that was what you were aiming for? .Dave.
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 666d07c..4955db9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2270,6 +2270,8 @@ struct drm_i915_gem_request { int i915_gem_request_alloc(struct intel_engine_cs *ring, struct intel_context *ctx, struct drm_i915_gem_request **req_out); +struct drm_i915_gem_request * __must_check + i915_gem_request_alloc_anon(struct intel_engine_cs *ring); void i915_gem_request_cancel(struct drm_i915_gem_request *req); void i915_gem_request_free(struct kref *req_ref); int i915_gem_request_add_to_client(struct drm_i915_gem_request *req, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index be1f984..9f9c0c0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2751,6 +2751,21 @@ err: return ret; } +/* + * Allocate a request associated with the default context for the given + * ring. This can be used where the driver needs a request for internal + * purposes not directly related to a user batch submission. + */ +struct drm_i915_gem_request * +i915_gem_request_alloc_anon(struct intel_engine_cs *ring) +{ + struct drm_i915_gem_request *req; + int err; + + err = i915_gem_request_alloc(ring, ring->default_context, &req); + return err ? ERR_PTR(err) : req; +} + void i915_gem_request_cancel(struct drm_i915_gem_request *req) { intel_ring_reserved_space_cancel(req->ringbuf); @@ -3168,9 +3183,13 @@ __i915_gem_object_sync(struct drm_i915_gem_object *obj, return 0; if (*to_req == NULL) { - ret = i915_gem_request_alloc(to, to->default_context, to_req); - if (ret) - return ret; + struct drm_i915_gem_request *req; + + req = i915_gem_request_alloc_anon(to); + if (IS_ERR(req)) + return PTR_ERR(req); + + *to_req = req; } trace_i915_gem_ring_sync_to(*to_req, from, from_req); @@ -3370,9 +3389,9 @@ int i915_gpu_idle(struct drm_device *dev) if (!i915.enable_execlists) { struct drm_i915_gem_request *req; - ret = i915_gem_request_alloc(ring, ring->default_context, &req); - if (ret) - return ret; + req = i915_gem_request_alloc_anon(ring); + if (IS_ERR(req)) + return PTR_ERR(req); ret = i915_switch_context(req); if (ret) { @@ -4867,8 +4886,9 @@ i915_gem_init_hw(struct drm_device *dev) for_each_ring(ring, dev_priv, i) { struct drm_i915_gem_request *req; - ret = i915_gem_request_alloc(ring, ring->default_context, &req); - if (ret) { + req = i915_gem_request_alloc_anon(ring); + if (IS_ERR(req)) { + ret = PTR_ERR(req); i915_gem_cleanup_ringbuffer(dev); goto out; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index abd2d29..5716f4a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11662,9 +11662,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, obj->last_write_req); } else { if (!request) { - ret = i915_gem_request_alloc(ring, ring->default_context, &request); - if (ret) + request = i915_gem_request_alloc_anon(ring); + if (IS_ERR(request)) { + ret = PTR_ERR(request); goto cleanup_unpin; + } } ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request, diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c index 76f1980..57cd503 100644 --- a/drivers/gpu/drm/i915/intel_overlay.c +++ b/drivers/gpu/drm/i915/intel_overlay.c @@ -240,9 +240,9 @@ static int intel_overlay_on(struct intel_overlay *overlay) WARN_ON(overlay->active); WARN_ON(IS_I830(dev) && !(dev_priv->quirks & QUIRK_PIPEA_FORCE)); - ret = i915_gem_request_alloc(ring, ring->default_context, &req); - if (ret) - return ret; + req = i915_gem_request_alloc_anon(ring); + if (IS_ERR(req)) + return PTR_ERR(req); ret = intel_ring_begin(req, 4); if (ret) { @@ -283,9 +283,9 @@ static int intel_overlay_continue(struct intel_overlay *overlay, if (tmp & (1 << 17)) DRM_DEBUG("overlay underrun, DOVSTA: %x\n", tmp); - ret = i915_gem_request_alloc(ring, ring->default_context, &req); - if (ret) - return ret; + req = i915_gem_request_alloc_anon(ring); + if (IS_ERR(req)) + return PTR_ERR(req); ret = intel_ring_begin(req, 2); if (ret) { @@ -349,9 +349,9 @@ static int intel_overlay_off(struct intel_overlay *overlay) * of the hw. Do it in both cases */ flip_addr |= OFC_UPDATE; - ret = i915_gem_request_alloc(ring, ring->default_context, &req); - if (ret) - return ret; + req = i915_gem_request_alloc_anon(ring); + if (IS_ERR(req)) + return PTR_ERR(req); ret = intel_ring_begin(req, 6); if (ret) { @@ -423,9 +423,9 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay) /* synchronous slowpath */ struct drm_i915_gem_request *req; - ret = i915_gem_request_alloc(ring, ring->default_context, &req); - if (ret) - return ret; + req = i915_gem_request_alloc_anon(ring); + if (IS_ERR(req)) + return PTR_ERR(req); ret = intel_ring_begin(req, 2); if (ret) {
There are a number of places where the driver needs a request, but isn't working on behalf of any specific user or in a specific context. For such requests, we associate them with the default context for the engine that the request will be submitted to. This patch provides a shorthand for doing such request allocations and changes all such calls to use the new function. Signed-off-by: Dave Gordon <david.s.gordon@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_gem.c | 36 ++++++++++++++++++++++++++++-------- drivers/gpu/drm/i915/intel_display.c | 6 ++++-- drivers/gpu/drm/i915/intel_overlay.c | 24 ++++++++++++------------ 4 files changed, 46 insertions(+), 22 deletions(-)