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

[Tvrtko Ursulin]

On 02/05/2017 15:45, 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:

diff --git a/drivers/gpu/drm/i915/selftests/i915_syncmap.c 
b/drivers/gpu/drm/i915/selftests/i915_syncmap.c
index 1f8b594b4157..9fbc9e144833 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;
@@ -61,7 +61,7 @@ __sync_print(struct i915_syncmap *p,
        *sz -= len;

        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]);
                        buf += len;
@@ -76,11 +76,11 @@ __sync_print(struct i915_syncmap *p,
        *sz -= len;

        if (p->height) {
-               for (bits = p->bitmap; (i = ffs(bits)); ) {
-                       bits &= ~0u << i;
+               for_each_set_bit(i, (unsigned long *)&p->bitmap, KSYNCMAP) {
                        buf = __sync_print(__sync_child(p)[i - 1],
                                           buf, sz,
-                                          depth + 1, (last << 1) | !!bits,
+                                          depth + 1, (last << 1) |
+                                          !!(p->bitmap & (~0u << i)),
                                           i - 1);
                }
        }

Its a bit smaller line shrink in this version so I am not sure. Most 
importantly it is not getting rid of the ~0u << i business. Have it as 
you prefer it.

And thank you for not suggesting to use the horrible code generation of
for_each_set_bit() outside of the pretty printer. :)

P.S. Latest ascii graphs:
        0xXXXXXXXXXXXXXXXX
        0-> 0x0000000000XXXXXX
        |   0-> 0x0000000000000XXX
        |   |   0-> 0x00000000000000XX
        |   |   |   0-> 0x000000000000000X 0:0, 1:1, 2:2
        |   |   |   1-> 0x000000000000001X 0:10, 1:11
        |   |   2-> 0x000000000000020X 0:200, 1:201
        |   5-> 0x000000000050XXXX
        |       0-> 0x000000000050000X 0:500000, 1:500001
        |       3-> 0x000000000050300X 0:503000, 1:503001
        e-> 0xe00000000000000X e:e

I think that's better. And thank you for adding the graph!

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx