intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Dave Gordon <david.s.gordon@intel.com>
To: Daniel Vetter <daniel@ffwll.ch>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	intel-gfx@lists.freedesktop.org
Subject: [RFC] drm/i915: for_each_engine()
Date: Thu, 10 Dec 2015 12:32:03 +0000	[thread overview]
Message-ID: <566970C3.6030602@intel.com> (raw)
In-Reply-To: <20151125092323.GP17050@phenom.ffwll.local>

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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2015-12-10 12:32 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-24 17:36 [RFC PATCH] drm/i915: fix potential dangling else problems in for_each_ macros Jani Nikula
2015-11-24 17:38 ` Jani Nikula
2015-11-24 18:25 ` Daniel Vetter
2015-11-24 22:26 ` Chris Wilson
2015-11-24 23:47   ` Chris Wilson
2015-11-25  9:10     ` Jani Nikula
2015-11-25  9:23     ` Daniel Vetter
2015-12-02 13:29       ` Dave Gordon
2015-12-02 13:46         ` Chris Wilson
2015-12-02 14:51           ` Dave Gordon
2015-12-02 14:58             ` Chris Wilson
2015-12-02 18:18               ` Dave Gordon
2015-12-10 12:32       ` Dave Gordon [this message]
2015-12-10 12:42         ` [RFC] drm/i915: for_each_engine() Chris Wilson
2016-03-23 18:19           ` [PATCH 0/2] Updating and simplifying for_each_engine() Dave Gordon
2016-03-23 18:19             ` [PATCH 1/2] drm/i915: introduce for_each_engine_id() Dave Gordon
2016-03-23 18:19             ` [PATCH 2/2] drm/i915: replace for_each_engine() Dave Gordon
2016-03-23 20:43               ` Chris Wilson
2016-03-24 11:20                 ` [PATCH 2/2 v2] " Dave Gordon
2016-03-24  9:06 ` ✗ Fi.CI.BAT: failure for drm/i915: fix potential dangling else problems in for_each_ macros (rev3) Patchwork
2016-03-24 11:38   ` Dave Gordon
2016-03-24 12:03 ` ✓ Fi.CI.BAT: success for drm/i915: fix potential dangling else problems in for_each_ macros (rev4) Patchwork
2016-03-24 14:35   ` Tvrtko Ursulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=566970C3.6030602@intel.com \
    --to=david.s.gordon@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).