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 <chris@xxxxxxxxxxxxxxxxxx> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> --- 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx