[Intel-gfx] [PATCH 02/50] drm/i915: for_each_ring

[Mateo Lozano, Oscar]

> -----Original Message-----
> From: Volkin, Bradley D
> Sent: Monday, May 19, 2014 5:34 PM
> To: Mateo Lozano, Oscar
> Cc: intel-gfx at lists.freedesktop.org; Ben Widawsky; Widawsky, Benjamin
> Subject: Re: [Intel-gfx] [PATCH 02/50] drm/i915: for_each_ring
> 
> On Fri, May 09, 2014 at 05:08:32AM -0700, oscar.mateo at intel.com wrote:
> > From: Ben Widawsky <benjamin.widawsky at intel.com>
> >
> > for_each_ring() iterates over all rings supported by the hardware, not
> > just those which have been initialized as in for_each_active_ring()
> 
> I think we should give this a new name; something like
> for_each_supported_ring.
> My concern is that, with all of the patches in flight, we'll merge something that
> uses for_each_ring when it should have been changed to for_each_active_ring.
> Better that such a patch not even compile.

I can kill this patch off completely: when we started with Execlists, it simplified a lot of things and made life easier in general, but with each new iteration it just becomes more and more useless...

> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > Acked-by: Oscar Mateo <oscar.mateo at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index a53a028..b1725c6 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1544,6 +1544,17 @@ static inline struct drm_i915_private
> *to_i915(const struct drm_device *dev)
> >  	return dev->dev_private;
> >  }
> >
> > +/* NB: Typically you want to use for_each_ring in init code before
> > +ringbuffers
> > + * are setup, or in debug code. for_each_active_ring is more suited
> > +for code
> > + * which is dynamically handling active rings, ie. normal code. In
> > +most
> > + * (currently all cases except on pre-production hardware)
> > +for_each_ring will
> > + * work even if it's a bad idea to use it - so be careful.
> > + */
> > +#define for_each_ring(ring__, dev_priv__, i__) \
> > +	for ((i__) = 0; (i__) < I915_NUM_RINGS; (i__)++) \
> > +		if (((ring__) = &(dev_priv__)->ring[(i__)]), \
> > +		    INTEL_INFO((dev_priv__)->dev)->ring_mask & (1<<(i__)))
> > +
> >  /* Iterate over initialised rings */
> >  #define for_each_active_ring(ring__, dev_priv__, i__) \
> >  	for ((i__) = 0; (i__) < I915_NUM_RINGS; (i__)++) \
> > --
> > 1.9.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx