From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/7] drm/i915: Move context special case to get() Date: Fri, 5 Apr 2013 13:41:11 +0300 Message-ID: <20130405104111.GC4469@intel.com> References: <1365118914-15753-1-git-send-email-ben@bwidawsk.net> <1365118914-15753-3-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id D426EE5CDA for ; Fri, 5 Apr 2013 03:41:17 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1365118914-15753-3-git-send-email-ben@bwidawsk.net> 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 Thu, Apr 04, 2013 at 04:41:48PM -0700, Ben Widawsky wrote: > This allows us to make upcoming refcounting code a bit simpler, and > cleaner. In addition, I think it makes the interface a bit nicer if the > caller doesn't need to figure out default contexts and such. > = > The interface works very similarly to the gem object ref counting, and > I believe it makes sense to do so as we'll use it in a very similar way > to objects (we currently use objects as a somewhat hackish way to manage > context lifecycles). > = > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_gem_context.c | 34 +++++++++++++++++++--------= ------ > 1 file changed, 20 insertions(+), 14 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i9= 15/i915_gem_context.c > index aa080ea..6211637 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -95,8 +95,6 @@ > */ > #define CONTEXT_ALIGN (64<<10) > = > -static struct i915_hw_context * > -i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id); > static int do_switch(struct i915_hw_context *to); > = > static int get_context_size(struct drm_device *dev) > @@ -291,9 +289,18 @@ void i915_gem_context_close(struct drm_device *dev, = struct drm_file *file) > } > = > static struct i915_hw_context * > -i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id) > +i915_gem_context_get(struct intel_ring_buffer *ring, > + struct drm_i915_file_private *file_priv, u32 id) > { > - return (struct i915_hw_context *)idr_find(&file_priv->context_idr, id); > + struct i915_hw_context *ctx; > + > + if (id =3D=3D DEFAULT_CONTEXT_ID) > + ctx =3D ring->default_context; > + else > + ctx =3D (struct i915_hw_context *) > + idr_find(&file_priv->context_idr, id); > + > + return ctx; > } > = > static inline int > @@ -440,16 +447,12 @@ int i915_switch_context(struct intel_ring_buffer *r= ing, > if (ring !=3D &dev_priv->ring[RCS]) > return 0; > = > - if (to_id =3D=3D DEFAULT_CONTEXT_ID) { > - to =3D ring->default_context; > - } else { > - if (file =3D=3D NULL) > - return -EINVAL; > + if (file =3D=3D NULL) > + return -EINVAL; This looks wrong. Before the NULL check was only done when to_id !=3D DEFAULT_CONTEXT_ID. Now it's done always, which means the call from i915_gpu_idle() will always fail. > = > - to =3D i915_gem_context_get(file->driver_priv, to_id); > - if (to =3D=3D NULL) > - return -ENOENT; > - } > + to =3D i915_gem_context_get(ring, file->driver_priv, to_id); > + if (to =3D=3D NULL) > + return -ENOENT; > = > return do_switch(to); > } > @@ -495,11 +498,14 @@ int i915_gem_context_destroy_ioctl(struct drm_devic= e *dev, void *data, > if (!(dev->driver->driver_features & DRIVER_GEM)) > return -ENODEV; > = > + if (args->ctx_id =3D=3D DEFAULT_CONTEXT_ID) > + return -ENOENT; > + > ret =3D i915_mutex_lock_interruptible(dev); > if (ret) > return ret; > = > - ctx =3D i915_gem_context_get(file_priv, args->ctx_id); > + ctx =3D i915_gem_context_get(NULL, file_priv, args->ctx_id); > if (!ctx) { > mutex_unlock(&dev->struct_mutex); > return -ENOENT; > -- = > 1.8.2 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC