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

[Dave Gordon]

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?

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?

.Dave.