From patchwork Wed Feb 27 17:02:37 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jani Nikula X-Patchwork-Id: 10831903 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id CD0D115AC for ; Wed, 27 Feb 2019 17:00:54 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B76412E79B for ; Wed, 27 Feb 2019 17:00:54 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id ABB2C2E7CA; Wed, 27 Feb 2019 17:00:54 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id B3C932E7DC for ; Wed, 27 Feb 2019 17:00:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 12D5E6E0FB; Wed, 27 Feb 2019 17:00:53 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id 52E7C6E0FD for ; Wed, 27 Feb 2019 17:00:51 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Feb 2019 09:00:51 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,420,1544515200"; d="scan'208";a="150498395" Received: from jnikula-mobl3.fi.intel.com (HELO localhost) ([10.237.66.172]) by fmsmga001.fm.intel.com with ESMTP; 27 Feb 2019 09:00:48 -0800 From: Jani Nikula To: Jani Nikula , intel-gfx@lists.freedesktop.org Date: Wed, 27 Feb 2019 19:02:37 +0200 Message-Id: X-Mailer: git-send-email 2.20.1 In-Reply-To: References: MIME-Version: 1.0 Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Subject: [Intel-gfx] [PATCH v3 2/3] drm/i915: deprecate _SHIFT in favor of _MASK passed to accessors X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP 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. We define REG_FIELD_GET() and REG_FIELD_PREP() wrappers for the above, in part to force u32 and for consistency with REG_BIT() and REG_GENMASK(), but also as we'll need to redefine REG_FIELD_PREP() in follow-up work to make it produce integer constant expressions. For the most part, REG_FIELD_GET() is shorter than masking followed by shift, and arguably has more clarity. REG_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. Convert power sequencer registers as an example. v2: - Add the REG_FIELD_GET() and REG_FIELD_PREP() wrappers to use them consistently from the start. Cc: Chris Wilson Cc: Joonas Lahtinen Cc: Michal Wajdeczko Cc: Mika Kuoppala Signed-off-by: Jani Nikula Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/i915_reg.h | 45 ++++++++++++++++++++----------- drivers/gpu/drm/i915/intel_dp.c | 40 +++++++++++---------------- drivers/gpu/drm/i915/intel_lvds.c | 40 +++++++++++++-------------- 3 files changed, 64 insertions(+), 61 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index e847a18067bc..1bd75770483a 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 #include /** @@ -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_GENMASK()`` 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_GENMASK(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_GENMASK(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) @@ -144,6 +144,30 @@ __builtin_constant_p(__low) && \ ((__low) < 0 || (__high) > 31 || (__low) > (__high))))) +/** + * REG_FIELD_PREP() - Prepare a u32 bitfield value + * @__mask: shifted mask defining the field's length and position + * @__val: value to put in the field + + * Local wrapper for FIELD_PREP() to force u32 and for consistency with + * REG_FIELD_GET(), REG_BIT() and REG_GENMASK(). + * + * @return: @__val masked and shifted into the field defined by @__mask. + */ +#define REG_FIELD_PREP(__mask, __val) ((u32)FIELD_PREP(__mask, __val)) + +/** + * REG_FIELD_GET() - Extract a u32 bitfield value + * @__mask: shifted mask defining the field's length and position + * @__val: value to extract the bitfield value from + * + * Local wrapper for FIELD_GET() to force u32 and for consistency with + * REG_FIELD_PREP(), REG_BIT() and REG_GENMASK(). + * + * @return: Masked and shifted value of the field defined by @__mask in @__val. + */ +#define REG_FIELD_GET(__mask, __val) ((u32)FIELD_GET(__mask, __val)) + typedef struct { u32 reg; } i915_reg_t; @@ -4726,7 +4750,6 @@ enum { #define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \ _PP_CONTROL_2) #define POWER_CYCLE_DELAY_MASK REG_GENMASK(8, 4) -#define POWER_CYCLE_DELAY_SHIFT 4 #define VDD_OVERRIDE_FORCE REG_BIT(3) #define BACKLIGHT_ENABLE REG_BIT(2) #define PWR_DOWN_ON_RESET REG_BIT(1) @@ -4743,7 +4766,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_GENMASK(3, 0) #define PP_SEQUENCE_STATE_OFF_IDLE (0x0 << 0) @@ -4769,7 +4791,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_GENMASK(31, 30) #define PANEL_PORT_SELECT_LVDS (0 << 30) #define PANEL_PORT_SELECT_DPA (1 << 30) @@ -4777,23 +4798,17 @@ enum { #define PANEL_PORT_SELECT_DPD (3 << 30) #define PANEL_PORT_SELECT_VLV(port) ((port) << 30) #define PANEL_POWER_UP_DELAY_MASK REG_GENMASK(28, 16) -#define PANEL_POWER_UP_DELAY_SHIFT 16 #define PANEL_LIGHT_ON_DELAY_MASK REG_GENMASK(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_GENMASK(28, 16) -#define PANEL_POWER_DOWN_DELAY_SHIFT 16 #define PANEL_LIGHT_OFF_DELAY_MASK REG_GENMASK(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_GENMASK(31, 8) -#define PP_REFERENCE_DIVIDER_SHIFT 8 #define PANEL_POWER_CYCLE_DELAY_MASK REG_GENMASK(4, 0) -#define PANEL_POWER_CYCLE_DELAY_SHIFT 0 /* Panel fitting */ #define PFIT_CONTROL _MMIO(DISPLAY_MMIO_BASE(dev_priv) + 0x61230) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index e1a051c0fbfe..d1d282bc4814 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6438,25 +6438,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 = REG_FIELD_GET(PANEL_POWER_UP_DELAY_MASK, pp_on); + seq->t8 = REG_FIELD_GET(PANEL_LIGHT_ON_DELAY_MASK, pp_on); + seq->t9 = REG_FIELD_GET(PANEL_LIGHT_OFF_DELAY_MASK, pp_off); + seq->t10 = REG_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 = REG_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 = REG_FIELD_GET(PANEL_POWER_CYCLE_DELAY_MASK, pp_div) * 1000; } } @@ -6616,22 +6607,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 = REG_FIELD_PREP(PANEL_POWER_UP_DELAY_MASK, seq->t1_t3) | + REG_FIELD_PREP(PANEL_LIGHT_ON_DELAY_MASK, seq->t8); + pp_off = REG_FIELD_PREP(PANEL_LIGHT_OFF_DELAY_MASK, seq->t9) | + REG_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 |= REG_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 = REG_FIELD_PREP(PP_REFERENCE_DIVIDER_MASK, + (100 * div) / 2 - 1); + pp_div |= REG_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 b4aa49768e90..646fc29e159a 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -152,24 +152,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 = REG_FIELD_GET(PANEL_PORT_SELECT_MASK, val); + pps->t1_t2 = REG_FIELD_GET(PANEL_POWER_UP_DELAY_MASK, val); + pps->t5 = REG_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 = REG_FIELD_GET(PANEL_POWER_DOWN_DELAY_MASK, val); + pps->tx = REG_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 = REG_FIELD_GET(PP_REFERENCE_DIVIDER_MASK, val); + val = REG_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 @@ -209,15 +202,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 = REG_FIELD_PREP(PANEL_PORT_SELECT_MASK, pps->port) | + REG_FIELD_PREP(PANEL_POWER_UP_DELAY_MASK, pps->t1_t2) | + REG_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 = REG_FIELD_PREP(PANEL_POWER_DOWN_DELAY_MASK, pps->t3) | + REG_FIELD_PREP(PANEL_LIGHT_OFF_DELAY_MASK, pps->tx); + I915_WRITE(PP_OFF_DELAYS(0), val); + + val = REG_FIELD_PREP(PP_REFERENCE_DIVIDER_MASK, pps->divider) | + REG_FIELD_PREP(PANEL_POWER_CYCLE_DELAY_MASK, + DIV_ROUND_UP(pps->t4, 1000) + 1); I915_WRITE(PP_DIVISOR(0), val); }