bitfield.h defines FIELD_GET() and FIELD_PREP() macros to access
bitfields using the mask alone, with no need for separate shift. Indeed,
the shift is redundant.

For the most part, FIELD_GET() is shorter than masking followed by
shift, and arguably has more clarity.

FIELD_PREP() can get more verbose than simply shifting in place, but it
does provide masking to ensure we don't overflow the mask, something we
usually don't bother with currently.

Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
Cc: Mika Kuoppala <mika.kuopp...@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nik...@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   | 21 ++++++--------------
 drivers/gpu/drm/i915/intel_dp.c   | 40 ++++++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_lvds.c | 40 ++++++++++++++++++---------------------
 3 files changed, 40 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ac9258769435..dce4a6ac394c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -25,6 +25,7 @@
 #ifndef _I915_REG_H_
 #define _I915_REG_H_
 
+#include <linux/bitfield.h>
 #include <linux/bits.h>
 
 /**
@@ -61,11 +62,11 @@
  * significant to least significant bit. Indent the register content macros
  * using two extra spaces between ``#define`` and the macro name.
  *
- * For bit fields, define a ``_MASK`` and a ``_SHIFT`` macro. Use
- * ``REG_FIELD_MASK()`` to define _MASK. Define bit field contents so that they
- * are already shifted in place, and can be directly OR'd. For convenience,
- * function-like macros may be used to define bit fields, but do note that the
- * macros may be needed to read as well as write the register contents.
+ * Define bit fields using ``REG_FIELD_MASK(h, l)``. Define bit field contents
+ * so that they are already shifted in place, and can be directly OR'd. For
+ * convenience, function-like macros may be used to define bit fields, but do
+ * note that the macros may be needed to read as well as write the register
+ * contents.
  *
  * Define bits using ``REG_BIT(N)``. Do **not** add ``_BIT`` suffix to the 
name.
  *
@@ -107,7 +108,6 @@
  *  #define FOO(pipe)                   _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
  *  #define   FOO_ENABLE                REG_BIT(31)
  *  #define   FOO_MODE_MASK             REG_FIELD_MASK(19, 16)
- *  #define   FOO_MODE_SHIFT            16
  *  #define   FOO_MODE_BAR              (0 << 16)
  *  #define   FOO_MODE_BAZ              (1 << 16)
  *  #define   FOO_MODE_QUX_SNB          (2 << 16)
@@ -4636,7 +4636,6 @@ enum {
 #define   PP_SEQUENCE_NONE             (0 << 28)
 #define   PP_SEQUENCE_POWER_UP         (1 << 28)
 #define   PP_SEQUENCE_POWER_DOWN       (2 << 28)
-#define   PP_SEQUENCE_SHIFT            28
 #define   PP_CYCLE_DELAY_ACTIVE                REG_BIT(27)
 #define   PP_SEQUENCE_STATE_MASK       REG_FIELD_MASK(3, 0)
 #define   PP_SEQUENCE_STATE_OFF_IDLE   (0x0 << 0)
@@ -4654,7 +4653,6 @@ enum {
 #define  PANEL_UNLOCK_MASK             REG_FIELD_MASK(31, 16)
 #define  PANEL_UNLOCK_REGS             (0xabcd << 16)
 #define  BXT_POWER_CYCLE_DELAY_MASK    REG_FIELD_MASK(8, 4)
-#define  BXT_POWER_CYCLE_DELAY_SHIFT   4
 #define  EDP_FORCE_VDD                 REG_BIT(3)
 #define  EDP_BLC_ENABLE                        REG_BIT(2)
 #define  PANEL_POWER_RESET             REG_BIT(1)
@@ -4662,7 +4660,6 @@ enum {
 
 #define _PP_ON_DELAYS                  0x61208
 #define PP_ON_DELAYS(pps_idx)          _MMIO_PPS(pps_idx, _PP_ON_DELAYS)
-#define  PANEL_PORT_SELECT_SHIFT       30
 #define  PANEL_PORT_SELECT_MASK                REG_FIELD_MASK(31, 30)
 #define  PANEL_PORT_SELECT_LVDS                (0 << 30)
 #define  PANEL_PORT_SELECT_DPA         (1 << 30)
@@ -4670,23 +4667,17 @@ enum {
 #define  PANEL_PORT_SELECT_DPD         (3 << 30)
 #define  PANEL_PORT_SELECT_VLV(port)   ((port) << 30)
 #define  PANEL_POWER_UP_DELAY_MASK     REG_FIELD_MASK(28, 16)
-#define  PANEL_POWER_UP_DELAY_SHIFT    16
 #define  PANEL_LIGHT_ON_DELAY_MASK     REG_FIELD_MASK(12, 0)
-#define  PANEL_LIGHT_ON_DELAY_SHIFT    0
 
 #define _PP_OFF_DELAYS                 0x6120C
 #define PP_OFF_DELAYS(pps_idx)         _MMIO_PPS(pps_idx, _PP_OFF_DELAYS)
 #define  PANEL_POWER_DOWN_DELAY_MASK   REG_FIELD_MASK(28, 16)
-#define  PANEL_POWER_DOWN_DELAY_SHIFT  16
 #define  PANEL_LIGHT_OFF_DELAY_MASK    REG_FIELD_MASK(12, 0)
-#define  PANEL_LIGHT_OFF_DELAY_SHIFT   0
 
 #define _PP_DIVISOR                    0x61210
 #define PP_DIVISOR(pps_idx)            _MMIO_PPS(pps_idx, _PP_DIVISOR)
 #define  PP_REFERENCE_DIVIDER_MASK     REG_FIELD_MASK(31, 8)
-#define  PP_REFERENCE_DIVIDER_SHIFT    8
 #define  PANEL_POWER_CYCLE_DELAY_MASK  REG_FIELD_MASK(4, 0)
-#define  PANEL_POWER_CYCLE_DELAY_SHIFT 0
 
 /* Panel fitting */
 #define PFIT_CONTROL   _MMIO(dev_priv->info.display_mmio_offset + 0x61230)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 31eef9b0e33b..848ce42d7770 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5767,25 +5767,16 @@ intel_pps_readout_hw_state(struct intel_dp *intel_dp, 
struct edp_power_seq *seq)
        }
 
        /* Pull timing values out of registers */
-       seq->t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
-                    PANEL_POWER_UP_DELAY_SHIFT;
-
-       seq->t8 = (pp_on & PANEL_LIGHT_ON_DELAY_MASK) >>
-                 PANEL_LIGHT_ON_DELAY_SHIFT;
-
-       seq->t9 = (pp_off & PANEL_LIGHT_OFF_DELAY_MASK) >>
-                 PANEL_LIGHT_OFF_DELAY_SHIFT;
-
-       seq->t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >>
-                  PANEL_POWER_DOWN_DELAY_SHIFT;
+       seq->t1_t3 = FIELD_GET(PANEL_POWER_UP_DELAY_MASK, pp_on);
+       seq->t8 = FIELD_GET(PANEL_LIGHT_ON_DELAY_MASK, pp_on);
+       seq->t9 = FIELD_GET(PANEL_LIGHT_OFF_DELAY_MASK, pp_off);
+       seq->t10 = FIELD_GET(PANEL_POWER_DOWN_DELAY_MASK, pp_off);
 
        if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
            HAS_PCH_ICP(dev_priv)) {
-               seq->t11_t12 = ((pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
-                               BXT_POWER_CYCLE_DELAY_SHIFT) * 1000;
+               seq->t11_t12 = FIELD_GET(BXT_POWER_CYCLE_DELAY_MASK, pp_ctl) * 
1000;
        } else {
-               seq->t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
-                      PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
+               seq->t11_t12 = FIELD_GET(PANEL_POWER_CYCLE_DELAY_MASK, pp_div) 
* 1000;
        }
 }
 
@@ -5945,22 +5936,23 @@ intel_dp_init_panel_power_sequencer_registers(struct 
intel_dp *intel_dp,
                I915_WRITE(regs.pp_ctrl, pp);
        }
 
-       pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) |
-               (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT);
-       pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) |
-                (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT);
+       pp_on = FIELD_PREP(PANEL_POWER_UP_DELAY_MASK, seq->t1_t3) |
+               FIELD_PREP(PANEL_LIGHT_ON_DELAY_MASK, seq->t8);
+       pp_off = FIELD_PREP(PANEL_LIGHT_OFF_DELAY_MASK, seq->t9) |
+               FIELD_PREP(PANEL_POWER_DOWN_DELAY_MASK, seq->t10);
        /* Compute the divisor for the pp clock, simply match the Bspec
         * formula. */
        if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
            HAS_PCH_ICP(dev_priv)) {
                pp_div = I915_READ(regs.pp_ctrl);
                pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK;
-               pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
-                               << BXT_POWER_CYCLE_DELAY_SHIFT);
+               pp_div |= FIELD_PREP(BXT_POWER_CYCLE_DELAY_MASK,
+                                    DIV_ROUND_UP(seq->t11_t12, 1000));
        } else {
-               pp_div = ((100 * div)/2 - 1) << PP_REFERENCE_DIVIDER_SHIFT;
-               pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
-                               << PANEL_POWER_CYCLE_DELAY_SHIFT);
+               pp_div = FIELD_PREP(PP_REFERENCE_DIVIDER_MASK,
+                                   (100 * div) / 2 - 1);
+               pp_div |= FIELD_PREP(PANEL_POWER_CYCLE_DELAY_MASK,
+                                    DIV_ROUND_UP(seq->t11_t12, 1000));
        }
 
        /* Haswell doesn't have any port selection bits for the panel
diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
b/drivers/gpu/drm/i915/intel_lvds.c
index f9f3b0885ba5..285aee9e58f1 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -160,24 +160,17 @@ static void intel_lvds_pps_get_hw_state(struct 
drm_i915_private *dev_priv,
        pps->powerdown_on_reset = I915_READ(PP_CONTROL(0)) & PANEL_POWER_RESET;
 
        val = I915_READ(PP_ON_DELAYS(0));
-       pps->port = (val & PANEL_PORT_SELECT_MASK) >>
-                   PANEL_PORT_SELECT_SHIFT;
-       pps->t1_t2 = (val & PANEL_POWER_UP_DELAY_MASK) >>
-                    PANEL_POWER_UP_DELAY_SHIFT;
-       pps->t5 = (val & PANEL_LIGHT_ON_DELAY_MASK) >>
-                 PANEL_LIGHT_ON_DELAY_SHIFT;
+       pps->port = FIELD_GET(PANEL_PORT_SELECT_MASK, val);
+       pps->t1_t2 = FIELD_GET(PANEL_POWER_UP_DELAY_MASK, val);
+       pps->t5 = FIELD_GET(PANEL_LIGHT_ON_DELAY_MASK, val);
 
        val = I915_READ(PP_OFF_DELAYS(0));
-       pps->t3 = (val & PANEL_POWER_DOWN_DELAY_MASK) >>
-                 PANEL_POWER_DOWN_DELAY_SHIFT;
-       pps->tx = (val & PANEL_LIGHT_OFF_DELAY_MASK) >>
-                 PANEL_LIGHT_OFF_DELAY_SHIFT;
+       pps->t3 = FIELD_GET(PANEL_POWER_DOWN_DELAY_MASK, val);
+       pps->tx = FIELD_GET(PANEL_LIGHT_OFF_DELAY_MASK, val);
 
        val = I915_READ(PP_DIVISOR(0));
-       pps->divider = (val & PP_REFERENCE_DIVIDER_MASK) >>
-                      PP_REFERENCE_DIVIDER_SHIFT;
-       val = (val & PANEL_POWER_CYCLE_DELAY_MASK) >>
-             PANEL_POWER_CYCLE_DELAY_SHIFT;
+       pps->divider = FIELD_GET(PP_REFERENCE_DIVIDER_MASK, val);
+       val = FIELD_GET(PANEL_POWER_CYCLE_DELAY_MASK, val);
        /*
         * Remove the BSpec specified +1 (100ms) offset that accounts for a
         * too short power-cycle delay due to the asynchronous programming of
@@ -217,15 +210,18 @@ static void intel_lvds_pps_init_hw(struct 
drm_i915_private *dev_priv,
                val |= PANEL_POWER_RESET;
        I915_WRITE(PP_CONTROL(0), val);
 
-       I915_WRITE(PP_ON_DELAYS(0), (pps->port << PANEL_PORT_SELECT_SHIFT) |
-                                   (pps->t1_t2 << PANEL_POWER_UP_DELAY_SHIFT) |
-                                   (pps->t5 << PANEL_LIGHT_ON_DELAY_SHIFT));
-       I915_WRITE(PP_OFF_DELAYS(0), (pps->t3 << PANEL_POWER_DOWN_DELAY_SHIFT) |
-                                    (pps->tx << PANEL_LIGHT_OFF_DELAY_SHIFT));
+       val = FIELD_PREP(PANEL_PORT_SELECT_MASK, pps->port) |
+               FIELD_PREP(PANEL_POWER_UP_DELAY_MASK, pps->t1_t2) |
+               FIELD_PREP(PANEL_LIGHT_ON_DELAY_MASK, pps->t5);
+       I915_WRITE(PP_ON_DELAYS(0), val);
 
-       val = pps->divider << PP_REFERENCE_DIVIDER_SHIFT;
-       val |= (DIV_ROUND_UP(pps->t4, 1000) + 1) <<
-              PANEL_POWER_CYCLE_DELAY_SHIFT;
+       val = FIELD_PREP(PANEL_POWER_DOWN_DELAY_MASK, pps->t3) |
+               FIELD_PREP(PANEL_LIGHT_OFF_DELAY_MASK, pps->tx);
+       I915_WRITE(PP_OFF_DELAYS(0), val);
+
+       val = FIELD_PREP(PP_REFERENCE_DIVIDER_MASK, pps->divider) |
+               FIELD_PREP(PANEL_POWER_CYCLE_DELAY_MASK,
+                          DIV_ROUND_UP(pps->t4, 1000) + 1);
        I915_WRITE(PP_DIVISOR(0), val);
 }
 
-- 
2.11.0

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

Reply via email to