[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