[Intel-gfx] [PATCH] drm/i915: Always use kref tracking for all contexts.

Chris Wilson chris at chris-wilson.co.uk
Sat Apr 5 22:14:56 CEST 2014


On Sat, Apr 05, 2014 at 12:18:05PM -0700, Ben Widawsky wrote:
> On Fri, Apr 04, 2014 at 02:40:06PM +0100, Chris Wilson wrote:
> > @@ -559,14 +519,13 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
> >  {
> >  	struct drm_i915_file_private *file_priv = file->driver_priv;
> >  
> > -	if (!HAS_HW_CONTEXTS(dev)) {
> > -		kfree(file_priv->private_default_ctx);
> > +	if (IS_ERR(file_priv->private_default_ctx))
> >  		return;
> > -	}
> 
> I still don't get this. How can file_priv->private_default_ctx be
> anything but a valid thing?

I honestly didn't check to see if close was never called along an error
path, or if context open failing prevented the whole file open, so just
left in this guard in case we do set it to the error pointer during
open.

> > @@ -793,7 +749,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
> >  	struct i915_hw_context *ctx;
> >  	int ret;
> >  
> > -	if (!HAS_HW_CONTEXTS(dev))
> > +	if (to_i915(dev)->hw_context_size == 0)
> >  		return -ENODEV;
> 
> You didn't like the idea of USES_Hw_CONTEXTS, or hw_contexts_enabled()?
> Just curious, I can live with this.

It basically came down to this single instance where we wanted to use
hw_context_size as hw_contexts_enabled() so I wasn't too fussed.  In
retrospect, it is exactly the sort of self-descriptive predicate I would
ask of others. Tsk, tsk, lazy me.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre



More information about the Intel-gfx mailing list