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
Wouldn't it be nicer (and safer) not to build macros that fold the loop
structure into the macro (in contravention of kernel programming
guidelines).
So how about NOT including the actual for() inside the macro, so that
instead of writing
for_each_engine(engine, dev_priv)
do_stuff(engine)
we would write it as
for (EACH_ENGINE(engine, dev_priv))
initialise(engine)
for (EACH_ACTIVE_ENGINE(engine, dev_priv)) {
service(engine)
restart(engine)
}
etc. The for-loop is visible and the scope doesn't give you any surprises.
[The EACH_ENGINE() macros expands to a semicolon-separated triplet of
expressions; still a violation of the "don't use macros to redefine C
syntax" guideline, but much less egregious than macros that contain
embedded 'for's and 'if's.
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
