On Thu, Dec 10, 2015 at 12:32:03PM +0000, Dave Gordon wrote: > On 25/11/15 09:23, Daniel Vetter wrote: > >On Tue, Nov 24, 2015 at 11:47:26PM +0000, Chris Wilson wrote: > >>On Tue, Nov 24, 2015 at 10:26:01PM +0000, Chris Wilson wrote: > >>>On Tue, Nov 24, 2015 at 07:36:25PM +0200, Jani Nikula wrote: > >>>> /* Iterate over initialised rings */ > >>>> #define for_each_ring(ring__, dev_priv__, i__) \ > >>>> for ((i__) = 0; (i__) < I915_NUM_RINGS; (i__)++) \ > >>>>- if (((ring__) = &(dev_priv__)->ring[(i__)]), intel_ring_initialized((ring__))) > >>>>+ for_each_if ((((ring__) = &(dev_priv__)->ring[(i__)]), intel_ring_initialized((ring__)))) > >>> > >>>Idly wondering if we would be happy with > >>> > >>>for_each_ring(ring__, dev_priv__) > >>> for ((ring__) = &(dev_priv__)->ring[0]; > >>> (ring__) <= &(dev_priv__)->ring[I915_NUM_RINGS]; > >>> (ring__)++) > >>> for_each_if(intel_ring_initialized(ring__)) > >>> > >>>? > >>> > >>>The downside is that we have used i__ in several places rather than > >>>ring->id. > >> > >>Fwiw, 13 files changed, 113 insertions(+), 140 deletions(-) > >> > >>Seems a reasonable shrinkage. > > > >Maybe for_each_engine even, and phase out for_each_ring completely? > >-Daniel > > Hi, > > I've done an implementation of for_each_engine(ring, dev_priv), and > converted a few uses of for_each_ring(ring, dev_priv, unused) to get > rid of the unused dummy variable. That works fine, so now I'm > looking at for_each_ring() for the cases where the variable IS used. > > The comments above imply to me that the loop variable shouldn't > really be the index in dev_priv->ring[i], but rather the value of > engine->id. Is this correct? Yes, that is more common and I think looks better for the slight abstraction. > Presumably there is at present no difference, i.e. > dev_priv->ring[i].id == i > (at least if the ring has been initialised?). So is the reason that > converting from index to id might give more flexibility in how to > organise the ring structures? It would. Primary motivation today is consistency. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
