[PATCH] drm/i915: Assert that we don't overflow frontbuffer tracking bits

[PATCH] drm/i915: Assert that we don't overflow frontbuffer tracking bits

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add some compile time assrts to the frontbuffer tracking to make sure
that we have enough bits per pipe to cover all the planes, and that we
have enough total bits to cover all the planes across all pipes.

We'll ignore any potential clash between the overlay bit and the
plane bits because that will allow us to keep using a total of 32
bits for the foreseeable future.

While at it change the macros to use BIT() and GENMASK(). The latter
gets rid of the hardcoded 0xff and thus means we can change the
number of bits per pipe by just changing
INTEL_FRONTBUFFER_BITS_PER_PIPE.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 454d8f937fae..98f346c7b0b3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2411,12 +2411,16 @@ enum hdmi_force_audio {
  * We have one bit per pipe and per scanout plane type.
  */
 #define INTEL_FRONTBUFFER_BITS_PER_PIPE 8
-#define INTEL_FRONTBUFFER(pipe, plane_id) \
-	(1 << ((plane_id) + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
+#define INTEL_FRONTBUFFER(pipe, plane_id) ({ \
+	BUILD_BUG_ON(INTEL_FRONTBUFFER_BITS_PER_PIPE * I915_MAX_PIPES > 32); \
+	BUILD_BUG_ON(I915_MAX_PLANES > INTEL_FRONTBUFFER_BITS_PER_PIPE); \
+	BIT((plane_id) + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)); \
+})
 #define INTEL_FRONTBUFFER_OVERLAY(pipe) \
-	(1 << (INTEL_FRONTBUFFER_BITS_PER_PIPE - 1 + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
+	BIT(INTEL_FRONTBUFFER_BITS_PER_PIPE - 1 + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))
 #define INTEL_FRONTBUFFER_ALL_MASK(pipe) \
-	(0xff << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
+	GENMASK(INTEL_FRONTBUFFER_BITS_PER_PIPE * ((pipe) + 1) - 1, \
+		INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))
 
 /*
  * Optimised SGL iterator for GEM objects
-- 
2.13.6

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

Re: [PATCH] drm/i915: Assert that we don't overflow frontbuffer tracking bits

[Chris Wilson]

Quoting Ville Syrjala (2018-01-24 18:36:42)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add some compile time assrts to the frontbuffer tracking to make sure
> that we have enough bits per pipe to cover all the planes, and that we
> have enough total bits to cover all the planes across all pipes.
> 
> We'll ignore any potential clash between the overlay bit and the
> plane bits because that will allow us to keep using a total of 32
> bits for the foreseeable future.
> 
> While at it change the macros to use BIT() and GENMASK(). The latter
> gets rid of the hardcoded 0xff and thus means we can change the
> number of bits per pipe by just changing
> INTEL_FRONTBUFFER_BITS_PER_PIPE.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 454d8f937fae..98f346c7b0b3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2411,12 +2411,16 @@ enum hdmi_force_audio {
>   * We have one bit per pipe and per scanout plane type.
>   */
>  #define INTEL_FRONTBUFFER_BITS_PER_PIPE 8
> -#define INTEL_FRONTBUFFER(pipe, plane_id) \
> -       (1 << ((plane_id) + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> +#define INTEL_FRONTBUFFER(pipe, plane_id) ({ \
> +       BUILD_BUG_ON(INTEL_FRONTBUFFER_BITS_PER_PIPE * I915_MAX_PIPES > 32); \
> +       BUILD_BUG_ON(I915_MAX_PLANES > INTEL_FRONTBUFFER_BITS_PER_PIPE); \
> +       BIT((plane_id) + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)); \
> +})

Next endeavour will be to convert to unsigned long just so we can use
BITS_PER_LONG here ; (These masks are now defined as ul, seems a natural
progression to use unsigned long throughout. Alternatively typedef
intel_frontbuffer_mask_t and 8*sizeof().)

>  #define INTEL_FRONTBUFFER_OVERLAY(pipe) \
> -       (1 << (INTEL_FRONTBUFFER_BITS_PER_PIPE - 1 + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> +       BIT(INTEL_FRONTBUFFER_BITS_PER_PIPE - 1 + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))
>  #define INTEL_FRONTBUFFER_ALL_MASK(pipe) \
> -       (0xff << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> +       GENMASK(INTEL_FRONTBUFFER_BITS_PER_PIPE * ((pipe) + 1) - 1, \
> +               INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))

Inclusive ends, check.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Assert that we don't overflow frontbuffer tracking bits

[Patchwork]

== Series Details ==

Series: drm/i915: Assert that we don't overflow frontbuffer tracking bits
URL   : https://patchwork.freedesktop.org/series/37057/
State : success

== Summary ==

Series 37057v1 drm/i915: Assert that we don't overflow frontbuffer tracking bits
https://patchwork.freedesktop.org/api/1.0/series/37057/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> DMESG-FAIL (fi-elk-e7500) fdo#103989 +1
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713
                incomplete -> PASS       (fi-bdw-5557u) fdo#104162

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#104162 https://bugs.freedesktop.org/show_bug.cgi?id=104162

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:422s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:426s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:373s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:489s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:283s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:482s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:474s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:453s
fi-elk-e7500     total:224  pass:168  dwarn:9   dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:278s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:510s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:391s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:399s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:410s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:459s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:413s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:459s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:495s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:453s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:503s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:584s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:425s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:507s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:529s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:489s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:475s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:416s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:428s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:395s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:565s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:476s

d33d70e5449f1477b09de4963fc7397890da1263 drm-tip: 2018y-01m-24d-18h-16m-27s UTC integration manifest
19b64b896bf6 drm/i915: Assert that we don't overflow frontbuffer tracking bits

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7771/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for drm/i915: Assert that we don't overflow frontbuffer tracking bits

[Patchwork]

== Series Details ==

Series: drm/i915: Assert that we don't overflow frontbuffer tracking bits
URL   : https://patchwork.freedesktop.org/series/37057/
State : failure

== Summary ==

Test perf:
        Subgroup enable-disable:
                fail       -> PASS       (shard-apl) fdo#103715
        Subgroup oa-exponents:
                pass       -> FAIL       (shard-apl) fdo#102254
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                fail       -> PASS       (shard-snb) fdo#101623 +1
Test kms_cursor_crc:
        Subgroup cursor-64x64-random:
                pass       -> DMESG-FAIL (shard-hsw)
        Subgroup cursor-256x256-suspend:
                pass       -> SKIP       (shard-hsw) fdo#103375
Test kms_cursor_legacy:
        Subgroup flip-vs-cursor-crc-atomic:
                fail       -> PASS       (shard-apl)

fdo#103715 https://bugs.freedesktop.org/show_bug.cgi?id=103715
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

shard-apl        total:2838 pass:1752 dwarn:1   dfail:0   fail:23  skip:1062 time:12615s
shard-hsw        total:2838 pass:1733 dwarn:1   dfail:1   fail:11  skip:1091 time:11870s
shard-snb        total:2838 pass:1330 dwarn:1   dfail:0   fail:10  skip:1497 time:6593s
Blacklisted hosts:
shard-kbl        total:2838 pass:1855 dwarn:18  dfail:1   fail:24  skip:940 time:9521s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7771/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Assert that we don't overflow frontbuffer tracking bits

[Ville Syrjälä]

On Wed, Jan 24, 2018 at 06:50:45PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-01-24 18:36:42)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Add some compile time assrts to the frontbuffer tracking to make sure
> > that we have enough bits per pipe to cover all the planes, and that we
> > have enough total bits to cover all the planes across all pipes.
> > 
> > We'll ignore any potential clash between the overlay bit and the
> > plane bits because that will allow us to keep using a total of 32
> > bits for the foreseeable future.
> > 
> > While at it change the macros to use BIT() and GENMASK(). The latter
> > gets rid of the hardcoded 0xff and thus means we can change the
> > number of bits per pipe by just changing
> > INTEL_FRONTBUFFER_BITS_PER_PIPE.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 454d8f937fae..98f346c7b0b3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2411,12 +2411,16 @@ enum hdmi_force_audio {
> >   * We have one bit per pipe and per scanout plane type.
> >   */
> >  #define INTEL_FRONTBUFFER_BITS_PER_PIPE 8
> > -#define INTEL_FRONTBUFFER(pipe, plane_id) \
> > -       (1 << ((plane_id) + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> > +#define INTEL_FRONTBUFFER(pipe, plane_id) ({ \
> > +       BUILD_BUG_ON(INTEL_FRONTBUFFER_BITS_PER_PIPE * I915_MAX_PIPES > 32); \
> > +       BUILD_BUG_ON(I915_MAX_PLANES > INTEL_FRONTBUFFER_BITS_PER_PIPE); \
> > +       BIT((plane_id) + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)); \
> > +})
> 
> Next endeavour will be to convert to unsigned long just so we can use
> BITS_PER_LONG here ; (These masks are now defined as ul, seems a natural
> progression to use unsigned long throughout.

longs always worry me because they can lead to funky bugs on 32bit
systems. Also we still depend on manual verification to make the
same type is used consistently.

> Alternatively typedef
> intel_frontbuffer_mask_t and 8*sizeof().)

typedef might be decent. Still no help from the compiler though.
I guess the only way to get such help would be to start using
macros/functions to manipulate bitmasks.

> 
> >  #define INTEL_FRONTBUFFER_OVERLAY(pipe) \
> > -       (1 << (INTEL_FRONTBUFFER_BITS_PER_PIPE - 1 + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> > +       BIT(INTEL_FRONTBUFFER_BITS_PER_PIPE - 1 + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))
> >  #define INTEL_FRONTBUFFER_ALL_MASK(pipe) \
> > -       (0xff << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> > +       GENMASK(INTEL_FRONTBUFFER_BITS_PER_PIPE * ((pipe) + 1) - 1, \
> > +               INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))
> 
> Inclusive ends, check.
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx