* [PATCH] drm/i915: encourage BIT() macro usage in register definitions @ 2018-06-27 14:41 Jani Nikula 2018-06-27 15:51 ` Michal Wajdeczko ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Jani Nikula @ 2018-06-27 14:41 UTC (permalink / raw) To: intel-gfx; +Cc: jani.nikula, Paulo Zanoni There's already some BIT() usage here and there, embrace it. Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 476118f46cf3..64b9c270045d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -65,9 +65,10 @@ * but do note that the macros may be needed to read as well as write the * register contents. * - * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We may change this in - * the future, but this is the prevailing style. Do **not** add ``_BIT`` suffix - * to the name. + * Define bits using ``BIT(N)`` instead of ``(1 << N)``. Do **not** add ``_BIT`` + * suffix to the name. Exception to ``BIT()`` usage: Value 1 for a bit field + * should be defined using ``(1 << N)`` to be in line with other values such as + * ``(2 << N)`` for the same field. * * Group the register and its contents together without blank lines, separate * from other registers and their contents with one blank line. @@ -105,7 +106,7 @@ * #define _FOO_A 0xf000 * #define _FOO_B 0xf001 * #define FOO(pipe) _MMIO_PIPE(pipe, _FOO_A, _FOO_B) - * #define FOO_ENABLE (1 << 31) + * #define FOO_ENABLE BIT(31) * #define FOO_MODE_MASK (0xf << 16) * #define FOO_MODE_SHIFT 16 * #define FOO_MODE_BAR (0 << 16) -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: encourage BIT() macro usage in register definitions 2018-06-27 14:41 [PATCH] drm/i915: encourage BIT() macro usage in register definitions Jani Nikula @ 2018-06-27 15:51 ` Michal Wajdeczko 2018-06-27 15:59 ` Chris Wilson 2018-06-27 17:25 ` ✓ Fi.CI.BAT: success for " Patchwork ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Michal Wajdeczko @ 2018-06-27 15:51 UTC (permalink / raw) To: intel-gfx, Jani Nikula; +Cc: Paulo Zanoni On Wed, 27 Jun 2018 16:41:13 +0200, Jani Nikula <jani.nikula@intel.com> wrote: > There's already some BIT() usage here and there, embrace it. > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 476118f46cf3..64b9c270045d 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -65,9 +65,10 @@ > * but do note that the macros may be needed to read as well as write > the > * register contents. > * > - * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We may change > this in > - * the future, but this is the prevailing style. Do **not** add > ``_BIT`` suffix > - * to the name. > + * Define bits using ``BIT(N)`` instead of ``(1 << N)``. Do **not** add > ``_BIT`` > + * suffix to the name. Exception to ``BIT()`` usage: Value 1 for a bit > field > + * should be defined using ``(1 << N)`` to be in line with other values > such as > + * ``(2 << N)`` for the same field. > * > * Group the register and its contents together without blank lines, > separate > * from other registers and their contents with one blank line. > @@ -105,7 +106,7 @@ > * #define _FOO_A 0xf000 > * #define _FOO_B 0xf001 > * #define FOO(pipe) _MMIO_PIPE(pipe, _FOO_A, _FOO_B) > - * #define FOO_ENABLE (1 << 31) > + * #define FOO_ENABLE BIT(31) hmm, this breaks nice consistency between one- and multi-bit fields .. > * #define FOO_MODE_MASK (0xf << 16) .. but if you want to use macro for single bit, then maybe you should also consider other existing macro for the mask definition: #define FOO_MODE_MASK GENMASK(19, 16) > * #define FOO_MODE_SHIFT 16 > * #define FOO_MODE_BAR (0 << 16) .. but we still don't have any macro for defining multi-bit values so I'm not sure if this change will make code really easier to read Michal _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: encourage BIT() macro usage in register definitions 2018-06-27 15:51 ` Michal Wajdeczko @ 2018-06-27 15:59 ` Chris Wilson 2018-06-28 12:03 ` Jani Nikula 0 siblings, 1 reply; 9+ messages in thread From: Chris Wilson @ 2018-06-27 15:59 UTC (permalink / raw) To: Michal Wajdeczko, intel-gfx, Jani Nikula; +Cc: Paulo Zanoni Quoting Michal Wajdeczko (2018-06-27 16:51:42) > On Wed, 27 Jun 2018 16:41:13 +0200, Jani Nikula <jani.nikula@intel.com> > wrote: > > > There's already some BIT() usage here and there, embrace it. > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 476118f46cf3..64b9c270045d 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -65,9 +65,10 @@ > > * but do note that the macros may be needed to read as well as write > > the > > * register contents. > > * > > - * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We may change > > this in > > - * the future, but this is the prevailing style. Do **not** add > > ``_BIT`` suffix > > - * to the name. > > + * Define bits using ``BIT(N)`` instead of ``(1 << N)``. Do **not** add > > ``_BIT`` > > + * suffix to the name. Exception to ``BIT()`` usage: Value 1 for a bit > > field > > + * should be defined using ``(1 << N)`` to be in line with other values > > such as > > + * ``(2 << N)`` for the same field. > > * > > * Group the register and its contents together without blank lines, > > separate > > * from other registers and their contents with one blank line. > > @@ -105,7 +106,7 @@ > > * #define _FOO_A 0xf000 > > * #define _FOO_B 0xf001 > > * #define FOO(pipe) _MMIO_PIPE(pipe, _FOO_A, _FOO_B) > > - * #define FOO_ENABLE (1 << 31) > > + * #define FOO_ENABLE BIT(31) > > hmm, this breaks nice consistency between one- and multi-bit fields .. > > > * #define FOO_MODE_MASK (0xf << 16) > > .. but if you want to use macro for single bit, then maybe you should > also consider other existing macro for the mask definition: > > #define FOO_MODE_MASK GENMASK(19, 16) > > > * #define FOO_MODE_SHIFT 16 > > * #define FOO_MODE_BAR (0 << 16) > > .. but we still don't have any macro for defining multi-bit values > so I'm not sure if this change will make code really easier to read #include <linux/bitfield.h> I'm not sure if I'm ready to embrace that yet, but it does seem to be the direction we should be heading in. Primarily to check the invalid range checking & usage. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: encourage BIT() macro usage in register definitions 2018-06-27 15:59 ` Chris Wilson @ 2018-06-28 12:03 ` Jani Nikula 2018-06-28 17:45 ` Paulo Zanoni 0 siblings, 1 reply; 9+ messages in thread From: Jani Nikula @ 2018-06-28 12:03 UTC (permalink / raw) To: Chris Wilson, Michal Wajdeczko, intel-gfx; +Cc: Paulo Zanoni On Wed, 27 Jun 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Michal Wajdeczko (2018-06-27 16:51:42) >> On Wed, 27 Jun 2018 16:41:13 +0200, Jani Nikula <jani.nikula@intel.com> >> wrote: >> >> > There's already some BIT() usage here and there, embrace it. >> > >> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> > --- >> > drivers/gpu/drm/i915/i915_reg.h | 9 +++++---- >> > 1 file changed, 5 insertions(+), 4 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h >> > b/drivers/gpu/drm/i915/i915_reg.h >> > index 476118f46cf3..64b9c270045d 100644 >> > --- a/drivers/gpu/drm/i915/i915_reg.h >> > +++ b/drivers/gpu/drm/i915/i915_reg.h >> > @@ -65,9 +65,10 @@ >> > * but do note that the macros may be needed to read as well as write >> > the >> > * register contents. >> > * >> > - * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We may change >> > this in >> > - * the future, but this is the prevailing style. Do **not** add >> > ``_BIT`` suffix >> > - * to the name. >> > + * Define bits using ``BIT(N)`` instead of ``(1 << N)``. Do **not** add >> > ``_BIT`` >> > + * suffix to the name. Exception to ``BIT()`` usage: Value 1 for a bit >> > field >> > + * should be defined using ``(1 << N)`` to be in line with other values >> > such as >> > + * ``(2 << N)`` for the same field. >> > * >> > * Group the register and its contents together without blank lines, >> > separate >> > * from other registers and their contents with one blank line. >> > @@ -105,7 +106,7 @@ >> > * #define _FOO_A 0xf000 >> > * #define _FOO_B 0xf001 >> > * #define FOO(pipe) _MMIO_PIPE(pipe, _FOO_A, _FOO_B) >> > - * #define FOO_ENABLE (1 << 31) >> > + * #define FOO_ENABLE BIT(31) >> >> hmm, this breaks nice consistency between one- and multi-bit fields .. >> >> > * #define FOO_MODE_MASK (0xf << 16) >> >> .. but if you want to use macro for single bit, then maybe you should >> also consider other existing macro for the mask definition: >> >> #define FOO_MODE_MASK GENMASK(19, 16) >> >> > * #define FOO_MODE_SHIFT 16 >> > * #define FOO_MODE_BAR (0 << 16) >> >> .. but we still don't have any macro for defining multi-bit values >> so I'm not sure if this change will make code really easier to read > > #include <linux/bitfield.h> > > I'm not sure if I'm ready to embrace that yet, but it does seem to be > the direction we should be heading in. Primarily to check the invalid > range checking & usage. I guess there are two things here. Using bitfield.h macros to define our own stuff is one thing, like so: #define FOO_ENABLE BIT(31) #define FOO_MODE_MASK GENMASK(19, 16) #define FOO_MODE_SHIFT 16 #define FOO_MODE_BAR FIELD_PREP(FOO_MODE_MASK, 0) #define FOO_MODE_BAZ FIELD_PREP(FOO_MODE_MASK, 1) #define FOO_MODE_QUX_SNB FIELD_PREP(FOO_MODE_MASK, 2) The range checking is indeed an advantage. Using FIELD_PREP() or FIELD_GET() in code is another, because we currently don't define the *unshifted* field values. Everything is defined with the shift. Defining everything unshifted and then moving the FIELD_PREP() and FIELD_GET() in code would be quite the change. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: encourage BIT() macro usage in register definitions 2018-06-28 12:03 ` Jani Nikula @ 2018-06-28 17:45 ` Paulo Zanoni 2018-06-28 19:05 ` Rodrigo Vivi 0 siblings, 1 reply; 9+ messages in thread From: Paulo Zanoni @ 2018-06-28 17:45 UTC (permalink / raw) To: Jani Nikula, Chris Wilson, Michal Wajdeczko, intel-gfx Em Qui, 2018-06-28 às 15:03 +0300, Jani Nikula escreveu: > On Wed, 27 Jun 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Michal Wajdeczko (2018-06-27 16:51:42) > > > On Wed, 27 Jun 2018 16:41:13 +0200, Jani Nikula <jani.nikula@inte > > > l.com> > > > wrote: > > > > > > > There's already some BIT() usage here and there, embrace it. > > > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Since I'm CC'd I guess my opinion counts here :) > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++---- > > > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > b/drivers/gpu/drm/i915/i915_reg.h > > > > index 476118f46cf3..64b9c270045d 100644 > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > @@ -65,9 +65,10 @@ > > > > * but do note that the macros may be needed to read as well > > > > as write > > > > the > > > > * register contents. > > > > * > > > > - * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We > > > > may change > > > > this in > > > > - * the future, but this is the prevailing style. Do **not** > > > > add > > > > ``_BIT`` suffix > > > > - * to the name. > > > > + * Define bits using ``BIT(N)`` instead of ``(1 << N)``. Do > > > > **not** add > > > > ``_BIT`` > > > > + * suffix to the name. Exception to ``BIT()`` usage: Value 1 > > > > for a bit > > > > field > > > > + * should be defined using ``(1 << N)`` to be in line with > > > > other values > > > > such as > > > > + * ``(2 << N)`` for the same field. > > > > * > > > > * Group the register and its contents together without blank > > > > lines, > > > > separate > > > > * from other registers and their contents with one blank > > > > line. > > > > @@ -105,7 +106,7 @@ > > > > * #define _FOO_A 0xf000 > > > > * #define _FOO_B 0xf001 > > > > * #define FOO(pipe) _MMIO_PIPE(pipe, > > > > _FOO_A, _FOO_B) > > > > - * #define FOO_ENABLE (1 << 31) > > > > + * #define FOO_ENABLE BIT(31) > > > > > > hmm, this breaks nice consistency between one- and multi-bit > > > fields .. I'll agree with Michal and Chris here: I'm not a huge fan of mixing BIT() and (x << y), I would prefer to keep the current standard, especially since BIT() is easily blacklistable. Or fully embrace the helper macros and abolish all sorts of (x << y). Consistency wins IMHO. > > > > > > > * #define FOO_MODE_MASK (0xf << 16) > > > > > > .. but if you want to use macro for single bit, then maybe you > > > should > > > also consider other existing macro for the mask definition: > > > > > > #define FOO_MODE_MASK GENMASK(19, 16) > > > > > > > * #define FOO_MODE_SHIFT 16 > > > > * #define FOO_MODE_BAR (0 << 16) > > > > > > .. but we still don't have any macro for defining multi-bit > > > values > > > so I'm not sure if this change will make code really easier to > > > read > > > > #include <linux/bitfield.h> > > > > I'm not sure if I'm ready to embrace that yet, but it does seem to > > be > > the direction we should be heading in. Primarily to check the > > invalid > > range checking & usage. > > I guess there are two things here. Using bitfield.h macros to define > our > own stuff is one thing, like so: > > #define FOO_ENABLE BIT(31) > #define FOO_MODE_MASK GENMASK(19, 16) > #define FOO_MODE_SHIFT 16 > #define FOO_MODE_BAR FIELD_PREP(FOO_MODE_MASK, 0) > #define FOO_MODE_BAZ FIELD_PREP(FOO_MODE_MASK, 1) > #define FOO_MODE_QUX_SNB FIELD_PREP(FOO_MODE_MASK, 2) > > The range checking is indeed an advantage. > > Using FIELD_PREP() or FIELD_GET() in code is another, because we > currently don't define the *unshifted* field values. Everything is > defined with the shift. Defining everything unshifted and then moving > the FIELD_PREP() and FIELD_GET() in code would be quite the change. Can't we create simple macros that cover our cases then? (yes, that would perhaps be more divergence from the Kernel coding standards, which could be worse than using (x << y) that are not hidden by nonstandard macros) > > BR, > Jani. > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: encourage BIT() macro usage in register definitions 2018-06-28 17:45 ` Paulo Zanoni @ 2018-06-28 19:05 ` Rodrigo Vivi 0 siblings, 0 replies; 9+ messages in thread From: Rodrigo Vivi @ 2018-06-28 19:05 UTC (permalink / raw) To: Paulo Zanoni; +Cc: Jani Nikula, intel-gfx On Thu, Jun 28, 2018 at 10:45:02AM -0700, Paulo Zanoni wrote: > Em Qui, 2018-06-28 às 15:03 +0300, Jani Nikula escreveu: > > On Wed, 27 Jun 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > Quoting Michal Wajdeczko (2018-06-27 16:51:42) > > > > On Wed, 27 Jun 2018 16:41:13 +0200, Jani Nikula <jani.nikula@inte > > > > l.com> > > > > wrote: > > > > > > > > > There's already some BIT() usage here and there, embrace it. > > > > > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Since I'm CC'd I guess my opinion counts here :) > > > > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++---- > > > > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > > b/drivers/gpu/drm/i915/i915_reg.h > > > > > index 476118f46cf3..64b9c270045d 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > @@ -65,9 +65,10 @@ > > > > > * but do note that the macros may be needed to read as well > > > > > as write > > > > > the > > > > > * register contents. > > > > > * > > > > > - * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We > > > > > may change > > > > > this in > > > > > - * the future, but this is the prevailing style. Do **not** > > > > > add > > > > > ``_BIT`` suffix > > > > > - * to the name. > > > > > + * Define bits using ``BIT(N)`` instead of ``(1 << N)``. Do > > > > > **not** add > > > > > ``_BIT`` > > > > > + * suffix to the name. Exception to ``BIT()`` usage: Value 1 > > > > > for a bit > > > > > field > > > > > + * should be defined using ``(1 << N)`` to be in line with > > > > > other values > > > > > such as > > > > > + * ``(2 << N)`` for the same field. > > > > > * > > > > > * Group the register and its contents together without blank > > > > > lines, > > > > > separate > > > > > * from other registers and their contents with one blank > > > > > line. > > > > > @@ -105,7 +106,7 @@ > > > > > * #define _FOO_A 0xf000 > > > > > * #define _FOO_B 0xf001 > > > > > * #define FOO(pipe) _MMIO_PIPE(pipe, > > > > > _FOO_A, _FOO_B) > > > > > - * #define FOO_ENABLE (1 << 31) > > > > > + * #define FOO_ENABLE BIT(31) > > > > > > > > hmm, this breaks nice consistency between one- and multi-bit > > > > fields .. > > I'll agree with Michal and Chris here: I'm not a huge fan of mixing > BIT() and (x << y), I would prefer to keep the current standard, > especially since BIT() is easily blacklistable. Or fully embrace the > helper macros and abolish all sorts of (x << y). Consistency wins IMHO. +1 for the Consistency. > > > > > > > > > > * #define FOO_MODE_MASK (0xf << 16) > > > > > > > > .. but if you want to use macro for single bit, then maybe you > > > > should > > > > also consider other existing macro for the mask definition: > > > > > > > > #define FOO_MODE_MASK GENMASK(19, 16) > > > > > > > > > * #define FOO_MODE_SHIFT 16 > > > > > * #define FOO_MODE_BAR (0 << 16) > > > > > > > > .. but we still don't have any macro for defining multi-bit > > > > values > > > > so I'm not sure if this change will make code really easier to > > > > read > > > > > > #include <linux/bitfield.h> > > > > > > I'm not sure if I'm ready to embrace that yet, but it does seem to > > > be > > > the direction we should be heading in. Primarily to check the > > > invalid > > > range checking & usage. > > > > I guess there are two things here. Using bitfield.h macros to define > > our > > own stuff is one thing, like so: > > > > #define FOO_ENABLE BIT(31) > > #define FOO_MODE_MASK GENMASK(19, 16) > > #define FOO_MODE_SHIFT 16 > > #define FOO_MODE_BAR FIELD_PREP(FOO_MODE_MASK, 0) > > #define FOO_MODE_BAZ FIELD_PREP(FOO_MODE_MASK, 1) > > #define FOO_MODE_QUX_SNB FIELD_PREP(FOO_MODE_MASK, 2) > > > > The range checking is indeed an advantage. > > > > Using FIELD_PREP() or FIELD_GET() in code is another, because we > > currently don't define the *unshifted* field values. Everything is > > defined with the shift. Defining everything unshifted and then moving > > the FIELD_PREP() and FIELD_GET() in code would be quite the change. > > Can't we create simple macros that cover our cases then? > > (yes, that would perhaps be more divergence from the Kernel coding > standards, which could be worse than using (x << y) that are not hidden > by nonstandard macros) I fully agree with them, so I'm actually dropping my ack while we don't find rough agreement on how to move forward without more consistency. > > > > > > BR, > > Jani. > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: encourage BIT() macro usage in register definitions 2018-06-27 14:41 [PATCH] drm/i915: encourage BIT() macro usage in register definitions Jani Nikula 2018-06-27 15:51 ` Michal Wajdeczko @ 2018-06-27 17:25 ` Patchwork 2018-06-27 20:07 ` ✓ Fi.CI.IGT: " Patchwork 2018-06-28 4:57 ` [PATCH] " Rodrigo Vivi 3 siblings, 0 replies; 9+ messages in thread From: Patchwork @ 2018-06-27 17:25 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx == Series Details == Series: drm/i915: encourage BIT() macro usage in register definitions URL : https://patchwork.freedesktop.org/series/45498/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4388 -> Patchwork_9446 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/45498/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_9446 that come from known issues: === IGT changes === ==== Possible fixes ==== igt@kms_chamelium@dp-edid-read: fi-kbl-7500u: FAIL (fdo#103841) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: INCOMPLETE (fdo#107054, fdo#103927) -> PASS fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#107054 https://bugs.freedesktop.org/show_bug.cgi?id=107054 == Participating hosts (45 -> 40) == Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4388 -> Patchwork_9446 CI_DRM_4388: b3654d40e9004f17bb612e71aee129347ea2c4aa @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4530: 0e98bf69f146eb72fe3a7c3b19a049b5786f0ca3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9446: d22f87f4e6805dea2e32f6b9e0a78f1717cff8f4 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == d22f87f4e680 drm/i915: encourage BIT() macro usage in register definitions == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9446/issues.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: encourage BIT() macro usage in register definitions 2018-06-27 14:41 [PATCH] drm/i915: encourage BIT() macro usage in register definitions Jani Nikula 2018-06-27 15:51 ` Michal Wajdeczko 2018-06-27 17:25 ` ✓ Fi.CI.BAT: success for " Patchwork @ 2018-06-27 20:07 ` Patchwork 2018-06-28 4:57 ` [PATCH] " Rodrigo Vivi 3 siblings, 0 replies; 9+ messages in thread From: Patchwork @ 2018-06-27 20:07 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx == Series Details == Series: drm/i915: encourage BIT() macro usage in register definitions URL : https://patchwork.freedesktop.org/series/45498/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4388_full -> Patchwork_9446_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_9446_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9446_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9446_full: === IGT changes === ==== Warnings ==== igt@kms_cursor_legacy@cursora-vs-flipb-atomic: shard-hsw: SKIP -> PASS == Known issues == Here are the changes found in Patchwork_9446_full that come from known issues: === IGT changes === ==== Issues hit ==== igt@gem_exec_await@wide-contexts: shard-glk: PASS -> FAIL (fdo#105900) igt@gem_exec_schedule@pi-ringfull-blt: {shard-glk9}: NOTRUN -> FAIL (fdo#103158) igt@gem_exec_schedule@pi-ringfull-vebox: shard-glk: NOTRUN -> FAIL (fdo#103158) igt@gem_mmap_gtt@coherency: shard-glk: NOTRUN -> FAIL (fdo#100587) igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic: shard-glk: NOTRUN -> FAIL (fdo#106509, fdo#105454) igt@kms_flip@2x-plain-flip-fb-recreate: {shard-glk9}: NOTRUN -> FAIL (fdo#100368) igt@kms_flip@plain-flip-fb-recreate: shard-glk: PASS -> FAIL (fdo#100368) igt@kms_flip_tiling@flip-to-x-tiled: shard-glk: PASS -> FAIL (fdo#104724, fdo#103822) igt@kms_flip_tiling@flip-y-tiled: {shard-glk9}: NOTRUN -> FAIL (fdo#104724) igt@kms_setmode@basic: shard-kbl: PASS -> FAIL (fdo#99912) igt@testdisplay: shard-glk: NOTRUN -> INCOMPLETE (fdo#103359, k.org#198133) ==== Possible fixes ==== igt@drv_selftest@live_gtt: shard-glk: INCOMPLETE (fdo#103359, k.org#198133) -> PASS shard-apl: INCOMPLETE (fdo#103927) -> PASS igt@drv_suspend@shrink: shard-snb: FAIL (fdo#107054, fdo#106886) -> PASS igt@gem_ctx_switch@basic-all-light: shard-hsw: INCOMPLETE (fdo#103540) -> PASS igt@gem_softpin@noreloc-s3: shard-kbl: INCOMPLETE (fdo#103665) -> PASS igt@kms_flip@dpms-vs-vblank-race: shard-kbl: FAIL (fdo#103060) -> PASS igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-glk: FAIL (fdo#105363) -> PASS igt@kms_flip_tiling@flip-x-tiled: shard-glk: FAIL (fdo#104724, fdo#103822) -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#100587 https://bugs.freedesktop.org/show_bug.cgi?id=100587 fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060 fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158 fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454 fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900 fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#107054 https://bugs.freedesktop.org/show_bug.cgi?id=107054 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133 == Participating hosts (6 -> 6) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4388 -> Patchwork_9446 CI_DRM_4388: b3654d40e9004f17bb612e71aee129347ea2c4aa @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4530: 0e98bf69f146eb72fe3a7c3b19a049b5786f0ca3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9446: d22f87f4e6805dea2e32f6b9e0a78f1717cff8f4 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9446/shards.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: encourage BIT() macro usage in register definitions 2018-06-27 14:41 [PATCH] drm/i915: encourage BIT() macro usage in register definitions Jani Nikula ` (2 preceding siblings ...) 2018-06-27 20:07 ` ✓ Fi.CI.IGT: " Patchwork @ 2018-06-28 4:57 ` Rodrigo Vivi 3 siblings, 0 replies; 9+ messages in thread From: Rodrigo Vivi @ 2018-06-28 4:57 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx, Paulo Zanoni On Wed, Jun 27, 2018 at 05:41:13PM +0300, Jani Nikula wrote: > There's already some BIT() usage here and there, embrace it. > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 476118f46cf3..64b9c270045d 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -65,9 +65,10 @@ > * but do note that the macros may be needed to read as well as write the > * register contents. > * > - * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We may change this in > - * the future, but this is the prevailing style. Do **not** add ``_BIT`` suffix > - * to the name. > + * Define bits using ``BIT(N)`` instead of ``(1 << N)``. Do **not** add ``_BIT`` > + * suffix to the name. Exception to ``BIT()`` usage: Value 1 for a bit field > + * should be defined using ``(1 << N)`` to be in line with other values such as > + * ``(2 << N)`` for the same field. > * > * Group the register and its contents together without blank lines, separate > * from other registers and their contents with one blank line. > @@ -105,7 +106,7 @@ > * #define _FOO_A 0xf000 > * #define _FOO_B 0xf001 > * #define FOO(pipe) _MMIO_PIPE(pipe, _FOO_A, _FOO_B) > - * #define FOO_ENABLE (1 << 31) > + * #define FOO_ENABLE BIT(31) Oh! I'm sure that I will miss the (1 << n) notation... and right now we had just converted everything to the documented style... any chance of get a script modifying all at once? Anyways: Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > * #define FOO_MODE_MASK (0xf << 16) > * #define FOO_MODE_SHIFT 16 > * #define FOO_MODE_BAR (0 << 16) > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-06-28 19:06 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-27 14:41 [PATCH] drm/i915: encourage BIT() macro usage in register definitions Jani Nikula 2018-06-27 15:51 ` Michal Wajdeczko 2018-06-27 15:59 ` Chris Wilson 2018-06-28 12:03 ` Jani Nikula 2018-06-28 17:45 ` Paulo Zanoni 2018-06-28 19:05 ` Rodrigo Vivi 2018-06-27 17:25 ` ✓ Fi.CI.BAT: success for " Patchwork 2018-06-27 20:07 ` ✓ Fi.CI.IGT: " Patchwork 2018-06-28 4:57 ` [PATCH] " Rodrigo Vivi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.