[Intel-gfx] [PATCH v14] drm/i915: Squash repeated awaits on the same fence

[Chris Wilson]

On Tue, May 02, 2017 at 03:45:23PM +0100, Chris Wilson wrote:
> On Tue, May 02, 2017 at 01:24:58PM +0100, Tvrtko Ursulin wrote:
> > On 28/04/2017 20:02, Chris Wilson wrote:
> > >+	if (!p->height) {
> > >+		for (bits = p->bitmap; (i = ffs(bits)); bits &= ~0u << i) {
> > 
> > Would for_each_set_bit be more readable?
> 
> Downside is that we have to cast bitmap to unsigned long:
> 
> Something like:

Well that forgot that for_each_set_bit was 0-based and not off-by-one
like ffs(). Let's try again:


diff --git a/drivers/gpu/drm/i915/selftests/i915_syncmap.c b/drivers/gpu/drm/i915/selftests/i915_syncmap.c
index 7b9c6eeaf62c..f279347ab218 100644
--- a/drivers/gpu/drm/i915/selftests/i915_syncmap.c
+++ b/drivers/gpu/drm/i915/selftests/i915_syncmap.c
@@ -33,7 +33,7 @@ __sync_print(struct i915_syncmap *p,
             unsigned int idx)
 {
        unsigned long len;
-       unsigned i, bits, X;
+       unsigned i, X;
 
        if (depth) {
                unsigned int d;
@@ -60,9 +60,9 @@ __sync_print(struct i915_syncmap *p,
        scnprintf(buf - X, *sz + X, "%*s", X, "XXXXXXXXXXXXXXXXX");
 
        if (!p->height) {
-               for (bits = p->bitmap; (i = ffs(bits)); bits &= ~0u << i) {
+               for_each_set_bit(i, (unsigned long *)&p->bitmap, KSYNCMAP) {
                        len = scnprintf(buf, *sz, " %x:%x,",
-                                      i - 1, __sync_seqno(p)[i - 1]);
+                                      i, __sync_seqno(p)[i]);
                        buf += len;
                        *sz -= len;
                }
@@ -75,12 +75,12 @@ __sync_print(struct i915_syncmap *p,
        *sz -= len;
 
        if (p->height) {
-               for (bits = p->bitmap; (i = ffs(bits)); ) {
-                       bits &= ~0u << i;
-                       buf = __sync_print(__sync_child(p)[i - 1],
-                                          buf, sz,
-                                          depth + 1, (last << 1) | !!bits,
-                                          i - 1);
+               for_each_set_bit(i, (unsigned long *)&p->bitmap, KSYNCMAP) {
+                       buf = __sync_print(__sync_child(p)[i], buf, sz,
+                                          depth + 1,
+                                          (last << 1) |
+                                          !!(p->bitmap & (~0u << (i + 1))),
+                                          i);
                }
        }
 

I'm in favour of the cast over gratiutious use of ffs()
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre