[Intel-gfx] [RFC] drm/i915: for_each_engine()

[Chris Wilson]

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