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