From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 13/18] drm/i915/context: create & destroy ioctls Date: Thu, 29 Mar 2012 21:35:35 +0200 Message-ID: <20120329193535.GG27737@phenom.ffwll.local> References: <1332103198-25852-1-git-send-email-ben@bwidawsk.net> <1332103198-25852-14-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f171.google.com (mail-wi0-f171.google.com [209.85.212.171]) by gabe.freedesktop.org (Postfix) with ESMTP id 9F3A19E78C for ; Thu, 29 Mar 2012 12:34:52 -0700 (PDT) Received: by wibhj13 with SMTP id hj13so297875wib.12 for ; Thu, 29 Mar 2012 12:34:51 -0700 (PDT) In-Reply-To: <1332103198-25852-14-git-send-email-ben@bwidawsk.net> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Sun, Mar 18, 2012 at 01:39:53PM -0700, Ben Widawsky wrote: > Add the interfaces to allow user space to create and destroy contexts. > Contexts are destroyed automatically if the file descriptor for the dri > device is closed. > > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_dma.c | 2 + > drivers/gpu/drm/i915/i915_drv.h | 4 ++ > drivers/gpu/drm/i915/i915_gem_context.c | 64 +++++++++++++++++++++++++++++++ > include/drm/i915_drm.h | 16 ++++++++ > 4 files changed, 86 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 4c7c1dc..fb3fccb 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -2333,6 +2333,8 @@ struct drm_ioctl_desc i915_ioctls[] = { > DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), > DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_COLORKEY, intel_sprite_set_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), > DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_COLORKEY, intel_sprite_get_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), > + DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED), > + DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED), > }; > > int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index c6c2ada..d49615e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1312,6 +1312,10 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file); > int i915_switch_context(struct intel_ring_buffer *ring, > struct drm_file *file, > int to_id, u32 seqno, u32 flags); > +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); > > /* i915_gem_gtt.c */ > int __must_check i915_gem_init_aliasing_ppgtt(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index d9a08f2..accb3de 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -411,3 +411,67 @@ out: > kref_put(&to->nref, destroy_hw_context); > return ret; > } > + > +int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_i915_gem_context_create *args = data; > + struct drm_i915_file_private *file_priv = file->driver_priv; > + struct intel_ring_buffer *ring = &dev_priv->ring[RCS]; > + struct i915_hw_context *ctx; > + int ret; > + > + if (dev_priv->hw_contexts_disabled) > + return -EPERM; > + > + ret = create_hw_context(dev, file_priv, &ctx); Note that the gem_alloc_object call in here is rather unsafe, due to the unsafe statistics-keeping in i915_gem_info_add_obj. Pre-existing looking goof-up, but can I maybe volunteer you to fix this? I think we need an mm stats spinlock because these counters could be rather big (and atomic_t is only 32 bits). > + if (ret) > + return ret; > + > + /* We need to do a special switch (save-only) either now, or before the > + * context is actually used in order to keep the context switch request > + * in execbuffer fairly simple. > + */ > + mutex_lock(&dev->struct_mutex); > + ret = i915_switch_context(ring, file, ctx->id, ring->get_seqno(ring), > + I915_CONTEXT_INITIAL_SWITCH); With the hw_context->is_intialized change you could ditch this (imo). > + mutex_unlock(&dev->struct_mutex); > + if (ret) > + return ret; > + > + args->ctx_id = ctx->id; > + DRM_DEBUG_DRIVER("HW context %d created\n", args->ctx_id); > + return ret; > +} > + > +int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file) > +{ > + struct drm_i915_gem_context_destroy *args = data; > + struct drm_i915_file_private *file_priv = file->driver_priv; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct i915_hw_context *ctx; > + > + if (dev_priv->hw_contexts_disabled) > + return -EPERM; > + > + mutex_lock(&dev->struct_mutex); > + ctx = i915_gem_context_get(file_priv, args->ctx_id); > + if (!ctx) { > + mutex_unlock(&dev->struct_mutex); > + return -EINVAL; > + } > + > + /* The first put is for the context ref we got just up above. The second > + * put should unref the original ref (and will therefore destroy the context > + * unless someone else holds a reference > + */ > + kref_put(&ctx->nref, destroy_hw_context); > + kref_put(&ctx->nref, destroy_hw_context); > + > + mutex_unlock(&dev->struct_mutex); > + > + DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id); > + return 0; > +} > diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h > index da929bb..bead13e 100644 > --- a/include/drm/i915_drm.h > +++ b/include/drm/i915_drm.h > @@ -200,6 +200,8 @@ typedef struct _drm_i915_sarea { > #define DRM_I915_GEM_EXECBUFFER2 0x29 > #define DRM_I915_GET_SPRITE_COLORKEY 0x2a > #define DRM_I915_SET_SPRITE_COLORKEY 0x2b > +#define DRM_I915_GEM_CONTEXT_CREATE 0x2c > +#define DRM_I915_GEM_CONTEXT_DESTROY 0x2d > > #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) > #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) > @@ -243,6 +245,9 @@ typedef struct _drm_i915_sarea { > #define DRM_IOCTL_I915_OVERLAY_ATTRS DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_OVERLAY_ATTRS, struct drm_intel_overlay_attrs) > #define DRM_IOCTL_I915_SET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey) > #define DRM_IOCTL_I915_GET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey) > +#define DRM_IOCTL_I915_GEM_CONTEXT_CREATE DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_CREATE, struct drm_i915_gem_context_create) > +#define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy) > + > > /* Allow drivers to submit batchbuffers directly to hardware, relying > * on the security mechanisms provided by hardware. > @@ -885,4 +890,15 @@ struct drm_intel_sprite_colorkey { > __u32 flags; > }; > > +struct drm_i915_gem_context_create { > + /* output: id of new context*/ > + __u32 ctx_id; > + __u32 pad; > +}; > + > +struct drm_i915_gem_context_destroy { > + __u32 ctx_id; > + __u32 pad; > +}; > + > #endif /* _I915_DRM_H_ */ > -- > 1.7.9.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48