[Intel-gfx] [PATCH] drm/i915: Replace has_bsd/blt/vebox with a mask

[Chris Wilson]

On Tue, Oct 15, 2013 at 08:03:25AM -0700, Ben Widawsky wrote:
> On Tue, Oct 15, 2013 at 09:50:39AM +0100, Chris Wilson wrote:
> > On Mon, Oct 14, 2013 at 08:46:22PM -0700, Ben Widawsky wrote:
> > > -cleanup_vebox_ring:
> > > -	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
> > > -cleanup_blt_ring:
> > > -	intel_cleanup_ring_buffer(&dev_priv->ring[BCS]);
> > > -cleanup_bsd_ring:
> > > -	intel_cleanup_ring_buffer(&dev_priv->ring[VCS]);
> > > -cleanup_render_ring:
> > > -	intel_cleanup_ring_buffer(&dev_priv->ring[RCS]);
> > > +cleanup:
> > > +	for_each_ring(ring, dev_priv, i) {
> > > +		if (!(INTEL_INFO(dev)->ring_mask & (1<<i)) ||
> > > +		    !ring->name)
> > > +			continue;
> > 
> > This looks dubious. You don't need to check ring_mask here as that will
> > be implicit in whatever we test for completeness. ring->name is set at
> > the start of initialisation and is not cleaned upon error. A better
> > choice is ring->obj, which we already check in
> > intel_cleanup_ring_buffer.
> > 
> > So this becomes:
> > cleanup:
> >   for_each_ring(ring, dev_priv, i)
> > > +		intel_cleanup_ring_buffer(ring);
> > 
> > -Chris
> > 
> 
> Actually, I just plopped this code in at the last minute because I
> wanted to provide a sample usage in the patch too. I do have some issues
> in the future of using for_each_ring(), hopefully you remember those...
> 
> In any event, I'll gladly drop this hunk. Can you review the rest?

No comments on the rest and lgtm, so
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre