| Message ID | 1402673891-14618-6-git-send-email-oscar.mateo@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 13, 2014 at 04:37:23PM +0100, oscar.mateo@intel.com wrote: > From: Oscar Mateo <oscar.mateo@intel.com> > > ... and namespace appropriately. > > It looks to me like it belongs logically there. > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > drivers/gpu/drm/i915/i915_gem_context.c | 23 +++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 25 +------------------------ > 3 files changed, 27 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index ec7e352..a15370c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2409,6 +2409,9 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, > struct drm_file *file); > int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, > struct drm_file *file); > +struct intel_context * > +i915_gem_context_validate(struct drm_device *dev, struct drm_file *file, > + struct intel_engine_cs *ring, const u32 ctx_id); > > /* i915_gem_render_state.c */ > int i915_gem_render_state_init(struct intel_engine_cs *ring); > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index f6c2538..801b891 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -824,3 +824,26 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, > DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id); > return 0; > } > + > +struct intel_context * > +i915_gem_context_validate(struct drm_device *dev, struct drm_file *file, > + struct intel_engine_cs *ring, const u32 ctx_id) > +{ > + struct intel_context *ctx = NULL; > + struct i915_ctx_hang_stats *hs; > + > + if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_ID) > + return ERR_PTR(-EINVAL); > + > + ctx = i915_gem_context_get(file->driver_priv, ctx_id); > + if (IS_ERR(ctx)) > + return ctx; > + > + hs = &ctx->hang_stats; > + if (hs->banned) { > + DRM_DEBUG("Context %u tried to submit while banned\n", ctx_id); > + return ERR_PTR(-EIO); Ugh. No. -Chris
> -----Original Message----- > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > Sent: Friday, June 13, 2014 6:11 PM > To: Mateo Lozano, Oscar > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 05/53] drm/i915: Move > i915_gem_validate_context() to i915_gem_context.c > > On Fri, Jun 13, 2014 at 04:37:23PM +0100, oscar.mateo@intel.com wrote: > > From: Oscar Mateo <oscar.mateo@intel.com> > > > > ... and namespace appropriately. > > > > It looks to me like it belongs logically there. > > > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > > drivers/gpu/drm/i915/i915_gem_context.c | 23 > +++++++++++++++++++++++ > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 25 > > +------------------------ > > 3 files changed, 27 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h index ec7e352..a15370c 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2409,6 +2409,9 @@ int i915_gem_context_create_ioctl(struct > drm_device *dev, void *data, > > struct drm_file *file); > > int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, > > struct drm_file *file); > > +struct intel_context * > > +i915_gem_context_validate(struct drm_device *dev, struct drm_file *file, > > + struct intel_engine_cs *ring, const u32 ctx_id); > > > > /* i915_gem_render_state.c */ > > int i915_gem_render_state_init(struct intel_engine_cs *ring); diff > > --git a/drivers/gpu/drm/i915/i915_gem_context.c > > b/drivers/gpu/drm/i915/i915_gem_context.c > > index f6c2538..801b891 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > > @@ -824,3 +824,26 @@ int i915_gem_context_destroy_ioctl(struct > drm_device *dev, void *data, > > DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id); > > return 0; > > } > > + > > +struct intel_context * > > +i915_gem_context_validate(struct drm_device *dev, struct drm_file *file, > > + struct intel_engine_cs *ring, const u32 ctx_id) { > > + struct intel_context *ctx = NULL; > > + struct i915_ctx_hang_stats *hs; > > + > > + if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_ID) > > + return ERR_PTR(-EINVAL); > > + > > + ctx = i915_gem_context_get(file->driver_priv, ctx_id); > > + if (IS_ERR(ctx)) > > + return ctx; > > + > > + hs = &ctx->hang_stats; > > + if (hs->banned) { > > + DRM_DEBUG("Context %u tried to submit while banned\n", > ctx_id); > > + return ERR_PTR(-EIO); > > Ugh. No. > -Chris D´oh! Why? - Oscar
[snip] On Mon, Jun 16, 2014 at 08:18:00AM -0700, Mateo Lozano, Oscar wrote: > > > +struct intel_context * > > > +i915_gem_context_validate(struct drm_device *dev, struct drm_file *file, > > > + struct intel_engine_cs *ring, const u32 ctx_id) { > > > + struct intel_context *ctx = NULL; > > > + struct i915_ctx_hang_stats *hs; > > > + > > > + if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_ID) > > > + return ERR_PTR(-EINVAL); > > > + > > > + ctx = i915_gem_context_get(file->driver_priv, ctx_id); > > > + if (IS_ERR(ctx)) > > > + return ctx; > > > + > > > + hs = &ctx->hang_stats; > > > + if (hs->banned) { > > > + DRM_DEBUG("Context %u tried to submit while banned\n", > > ctx_id); > > > + return ERR_PTR(-EIO); > > > > Ugh. No. > > -Chris > > D´oh! Why? > - Oscar Not sure if you got an answer on this. I'd guess the objection is that the function effectively implements part of the execbuf2 API contract rather than generic context behavior. So we'd want to just keep it as part of i915_gem_execbuffer.c. Brad
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ec7e352..a15370c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2409,6 +2409,9 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, struct drm_file *file); int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file *file); +struct intel_context * +i915_gem_context_validate(struct drm_device *dev, struct drm_file *file, + struct intel_engine_cs *ring, const u32 ctx_id); /* i915_gem_render_state.c */ int i915_gem_render_state_init(struct intel_engine_cs *ring); diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index f6c2538..801b891 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -824,3 +824,26 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id); return 0; } + +struct intel_context * +i915_gem_context_validate(struct drm_device *dev, struct drm_file *file, + struct intel_engine_cs *ring, const u32 ctx_id) +{ + struct intel_context *ctx = NULL; + struct i915_ctx_hang_stats *hs; + + if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_ID) + return ERR_PTR(-EINVAL); + + ctx = i915_gem_context_get(file->driver_priv, ctx_id); + if (IS_ERR(ctx)) + return ctx; + + hs = &ctx->hang_stats; + if (hs->banned) { + DRM_DEBUG("Context %u tried to submit while banned\n", ctx_id); + return ERR_PTR(-EIO); + } + + return ctx; +} diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 3a30133..58b3970 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -931,29 +931,6 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec, return 0; } -static struct intel_context * -i915_gem_validate_context(struct drm_device *dev, struct drm_file *file, - struct intel_engine_cs *ring, const u32 ctx_id) -{ - struct intel_context *ctx = NULL; - struct i915_ctx_hang_stats *hs; - - if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_ID) - return ERR_PTR(-EINVAL); - - ctx = i915_gem_context_get(file->driver_priv, ctx_id); - if (IS_ERR(ctx)) - return ctx; - - hs = &ctx->hang_stats; - if (hs->banned) { - DRM_DEBUG("Context %u tried to submit while banned\n", ctx_id); - return ERR_PTR(-EIO); - } - - return ctx; -} - static void i915_gem_execbuffer_move_to_active(struct list_head *vmas, struct intel_engine_cs *ring) @@ -1231,7 +1208,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, goto pre_mutex_err; } - ctx = i915_gem_validate_context(dev, file, ring, ctx_id); + ctx = i915_gem_context_validate(dev, file, ring, ctx_id); if (IS_ERR(ctx)) { mutex_unlock(&dev->struct_mutex); ret = PTR_ERR(ctx);