> -----Original Message----- > From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of Kahola, > Mika > Sent: Tuesday, March 7, 2023 3:24 PM > To: De Marchi, Lucas <lucas.demar...@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v4 03/22] drm/i915/mtl: Create separate reg > file > for PICA registers > > > -----Original Message----- > > From: De Marchi, Lucas <lucas.demar...@intel.com> > > Sent: Wednesday, March 1, 2023 1:27 AM > > To: Kahola, Mika <mika.kah...@intel.com> > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH v4 03/22] drm/i915/mtl: Create > > separate reg file for PICA registers > > > > On Fri, Feb 24, 2023 at 12:13:37PM +0200, Mika Kahola wrote: > > >Create a separate file to store registers for PICA chips > > >C10 and C20. > > > > > >v2: Rename file (Jani) > > > > > >Signed-off-by: Radhakrishna Sripada <radhakrishna.srip...@intel.com> > > >Signed-off-by: Mika Kahola <mika.kah...@intel.com> > > >--- > > > .../gpu/drm/i915/display/intel_cx0_phy_regs.h | 136 > > >++++++++++++++++++ > > > 1 file changed, 136 insertions(+) > > > create mode 100644 drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > > > > > >diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > > >b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > > >new file mode 100644 > > >index 000000000000..d6b3709d3762 > > >--- /dev/null > > >+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > > >@@ -0,0 +1,136 @@ > > >+/* SPDX-License-Identifier: MIT > > >+ * > > >+ * Copyright © 2022 Intel Corporation */ > > >+ > > >+#ifndef __INTEL_CX0_PHY_REGS_H__ > > >+#define __INTEL_CX0_PHY_REGS_H__ > > >+ > > >+#define _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_A 0x64040 > > >+#define _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_B 0x64140 > > >+#define _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_USBC1 > > 0x16F240 > > >+#define _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_USBC2 > > 0x16F440 > > >+#define _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_USBC3 > > 0x16F640 > > >+#define _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_USBC4 > > 0x16F840 > > >+#define _XELPDP_PORT_M2P_MSGBUS_CTL(port, lane) > > (_PICK(port, \ > > >+ [PORT_A] = > > _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_A, \ > > >+ [PORT_B] = > > _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_B, \ > > >+ [PORT_TC1] = > > _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_USBC1, \ > > >+ [PORT_TC2] = > > _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_USBC2, \ > > >+ [PORT_TC3] = > > _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_USBC3, \ > > >+ [PORT_TC4] = > > _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_USBC4) + ((lane) * > > >+4)) > > >+ > > > > stray newline > > > > >+#define XELPDP_PORT_M2P_MSGBUS_CTL(port, lane) > > _MMIO(_XELPDP_PORT_M2P_MSGBUS_CTL(port, lane)) > > > > > > one of the reasons _PICK_EVEN_2RANGES() was added. We usually have 2 > > ranges for the ports due to the combo vs tc distinction. > > > > #define XELPDP_PORT_M2P_MSGBUS_CTL(port, lane) > > _MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, > > > > _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_A, > > > > _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_B, > > > > _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_USBC1, > > > > _XELPDP_PORT_M2P_MSGBUS_CTL_LN0_USBC2)) > > > Btw, are we missing something from upstream? It seems that this is not > compiling properly with upstream kernel?
Never mind this comment. I sorted this one out. > > Thanks for the review and comments! I'm working on the rest of these fixes. > > -Mika- > > > > >+#define XELPDP_PORT_M2P_TRANSACTION_PENDING > > REG_BIT(31) > > > > ^ wrong amount of spaces. See the example on top of > > i915_reg.h > > > > here and everywhere in this file > > > > >+#define XELPDP_PORT_M2P_COMMAND_TYPE_MASK > > REG_GENMASK(30, 27) > > >+#define XELPDP_PORT_M2P_COMMAND_WRITE_UNCOMMITTED > > REG_FIELD_PREP(XELPDP_PORT_M2P_COMMAND_TYPE_MASK, 0x1) > > >+#define XELPDP_PORT_M2P_COMMAND_WRITE_COMMITTED > > REG_FIELD_PREP(XELPDP_PORT_M2P_COMMAND_TYPE_MASK, 0x2) > > >+#define XELPDP_PORT_M2P_COMMAND_READ > > REG_FIELD_PREP(XELPDP_PORT_M2P_COMMAND_TYPE_MASK, 0x3) > > >+#define XELPDP_PORT_M2P_DATA_MASK > > REG_GENMASK(23, 16) > > >+#define XELPDP_PORT_M2P_DATA(val) > > REG_FIELD_PREP(XELPDP_PORT_M2P_DATA_MASK, val) > > >+#define XELPDP_PORT_M2P_TRANSACTION_RESET REG_BIT(15) > > >+#define XELPDP_PORT_M2P_ADDRESS_MASK > > REG_GENMASK(11, 0) > > >+#define XELPDP_PORT_M2P_ADDRESS(val) > > REG_FIELD_PREP(XELPDP_PORT_M2P_ADDRESS_MASK, val) > > >+ > > >+#define XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane) > > _MMIO(_XELPDP_PORT_M2P_MSGBUS_CTL(port, lane) + 8) > > >+#define XELPDP_PORT_P2M_RESPONSE_READY > > REG_BIT(31) > > >+#define XELPDP_PORT_P2M_COMMAND_TYPE_MASK > > REG_GENMASK(30, 27) > > >+#define XELPDP_PORT_P2M_COMMAND_READ_ACK 0x4 > > >+#define XELPDP_PORT_P2M_COMMAND_WRITE_ACK 0x5 > > >+#define XELPDP_PORT_P2M_DATA_MASK > > REG_GENMASK(23, 16) > > >+#define XELPDP_PORT_P2M_DATA(val) > > REG_FIELD_PREP(XELPDP_PORT_P2M_DATA_MASK, val) > > >+#define XELPDP_PORT_P2M_ERROR_SET REG_BIT(15) > > >+#define XELPDP_MSGBUS_TIMEOUT_SLOW 1 > > >+#define XELPDP_MSGBUS_TIMEOUT_FAST_US 2 > > >+ > > >+#define XELPDP_PCLK_PLL_ENABLE_TIMEOUT_US 3200 > > >+#define XELPDP_PCLK_PLL_DISABLE_TIMEOUT_US 20 > > >+#define XELPDP_PORT_BUF_SOC_READY_TIMEOUT_US 100 > > >+#define XELPDP_PORT_RESET_START_TIMEOUT_US 5 > > >+#define XELPDP_PORT_POWERDOWN_UPDATE_TIMEOUT_US > 100 > > >+#define XELPDP_PORT_RESET_END_TIMEOUT 15 > > >+#define XELPDP_REFCLK_ENABLE_TIMEOUT_US 1 > > >+ > > >+#define _XELPDP_PORT_BUF_CTL1_LN0_A 0x64004 > > >+#define _XELPDP_PORT_BUF_CTL1_LN0_B 0x64104 > > >+#define _XELPDP_PORT_BUF_CTL1_LN0_USBC1 > > 0x16F200 > > >+#define _XELPDP_PORT_BUF_CTL1_LN0_USBC2 > > 0x16F400 > > >+#define _XELPDP_PORT_BUF_CTL1_LN0_USBC3 > > 0x16F600 > > >+#define _XELPDP_PORT_BUF_CTL1_LN0_USBC4 > > 0x16F800 > > >+#define _XELPDP_PORT_BUF_CTL1(port) (_PICK(port, \ > > >+ [PORT_A] = > > _XELPDP_PORT_BUF_CTL1_LN0_A, \ > > >+ [PORT_B] = > > _XELPDP_PORT_BUF_CTL1_LN0_B, \ > > >+ [PORT_TC1] = > > _XELPDP_PORT_BUF_CTL1_LN0_USBC1, \ > > >+ [PORT_TC2] = > > _XELPDP_PORT_BUF_CTL1_LN0_USBC2, \ > > >+ [PORT_TC3] = > > _XELPDP_PORT_BUF_CTL1_LN0_USBC3, \ > > >+ [PORT_TC4] = > > _XELPDP_PORT_BUF_CTL1_LN0_USBC4)) > > > > same thing here, please use the new macro > > > > >+ > > >+#define XELPDP_PORT_BUF_CTL1(port) > > _MMIO(_XELPDP_PORT_BUF_CTL1(port)) > > >+#define XELPDP_PORT_BUF_SOC_PHY_READY > > REG_BIT(24) > > >+#define XELPDP_PORT_REVERSAL REG_BIT(16) > > >+ > > >+#define XELPDP_TC_PHY_OWNERSHIP REG_BIT(6) > > >+#define XELPDP_TCSS_POWER_REQUEST REG_BIT(5) > > >+#define XELPDP_TCSS_POWER_STATE REG_BIT(4) > > >+#define XELPDP_PORT_WIDTH_MASK > > REG_GENMASK(3, 1) > > >+#define XELPDP_PORT_WIDTH(val) > > REG_FIELD_PREP(XELPDP_PORT_WIDTH_MASK, val) > > >+ > > >+#define XELPDP_PORT_BUF_CTL2(port) > > _MMIO(_XELPDP_PORT_BUF_CTL1(port) + 4) > > >+#define XELPDP_LANE0_PIPE_RESET REG_BIT(31) > > >+#define XELPDP_LANE1_PIPE_RESET REG_BIT(30) > > >+#define XELPDP_LANE0_PHY_CURRENT_STATUS REG_BIT(29) > > >+#define XELPDP_LANE1_PHY_CURRENT_STATUS REG_BIT(28) > > >+#define XELPDP_LANE0_POWERDOWN_UPDATE > > REG_BIT(25) > > >+#define XELPDP_LANE1_POWERDOWN_UPDATE > > REG_BIT(24) > > >+#define XELPDP_LANE0_POWERDOWN_NEW_STATE_MASK > > REG_GENMASK(23, 20) > > >+#define XELPDP_LANE0_POWERDOWN_NEW_STATE(val) > > REG_FIELD_PREP(XELPDP_LANE0_POWERDOWN_NEW_STATE_MASK, > > val) > > >+#define XELPDP_LANE1_POWERDOWN_NEW_STATE_MASK > > REG_GENMASK(19, 16) > > >+#define XELPDP_LANE1_POWERDOWN_NEW_STATE(val) > > REG_FIELD_PREP(XELPDP_LANE1_POWERDOWN_NEW_STATE_MASK, > > val) > > >+#define XELPDP_POWER_STATE_READY_MASK > > REG_GENMASK(7, 4) > > >+#define XELPDP_POWER_STATE_READY(val) > > REG_FIELD_PREP(XELPDP_POWER_STATE_READY_MASK, val) > > >+ > > >+#define XELPDP_PORT_BUF_CTL3(port) > > _MMIO(_XELPDP_PORT_BUF_CTL1(port) + 8) > > >+#define XELPDP_PLL_LANE_STAGGERING_DELAY_MASK > > REG_GENMASK(15, 8) > > >+#define XELPDP_PLL_LANE_STAGGERING_DELAY(val) > > REG_FIELD_PREP(XELPDP_PLL_LANE_STAGGERING_DELAY_MASK, val) > > >+#define XELPDP_POWER_STATE_ACTIVE_MASK > > REG_GENMASK(3, 0) > > >+#define XELPDP_POWER_STATE_ACTIVE(val) > > REG_FIELD_PREP(XELPDP_POWER_STATE_ACTIVE_MASK, val) > > >+ > > >+#define _XELPDP_PORT_CLOCK_CTL_A 0x640E0 > > >+#define _XELPDP_PORT_CLOCK_CTL_B 0x641E0 > > >+#define _XELPDP_PORT_CLOCK_CTL_USBC1 0x16F260 > > >+#define _XELPDP_PORT_CLOCK_CTL_USBC2 0x16F460 > > >+#define _XELPDP_PORT_CLOCK_CTL_USBC3 0x16F660 > > >+#define _XELPDP_PORT_CLOCK_CTL_USBC4 0x16F860 > > >+#define XELPDP_PORT_CLOCK_CTL(port) > > _MMIO(_PICK(port, \ > > >+ [PORT_A] = > > _XELPDP_PORT_CLOCK_CTL_A, \ > > >+ [PORT_B] = > > _XELPDP_PORT_CLOCK_CTL_B, \ > > >+ [PORT_TC1] = > > _XELPDP_PORT_CLOCK_CTL_USBC1, \ > > >+ [PORT_TC2] = > > _XELPDP_PORT_CLOCK_CTL_USBC2, \ > > >+ [PORT_TC3] = > > _XELPDP_PORT_CLOCK_CTL_USBC3, \ > > >+ [PORT_TC4] = > > _XELPDP_PORT_CLOCK_CTL_USBC4)) > > > > and here > > > > Lucas De Marchi > > > > >+ > > >+#define XELPDP_LANE0_PCLK_PLL_REQUEST > REG_BIT(31) > > >+#define XELPDP_LANE0_PCLK_PLL_ACK REG_BIT(30) > > >+#define XELPDP_LANE0_PCLK_REFCLK_REQUEST REG_BIT(29) > > >+#define XELPDP_LANE0_PCLK_REFCLK_ACK REG_BIT(28) > > >+#define XELPDP_LANE1_PCLK_PLL_REQUEST > REG_BIT(27) > > >+#define XELPDP_LANE1_PCLK_PLL_ACK REG_BIT(26) > > >+#define XELPDP_LANE1_PCLK_REFCLK_REQUEST REG_BIT(25) > > >+#define XELPDP_LANE1_PCLK_REFCLK_ACK REG_BIT(24) > > >+#define XELPDP_TBT_CLOCK_REQUEST REG_BIT(19) > > >+#define XELPDP_TBT_CLOCK_ACK REG_BIT(18) > > >+#define XELPDP_DDI_CLOCK_SELECT_MASK > > REG_GENMASK(15, 12) > > >+#define XELPDP_DDI_CLOCK_SELECT(val) > > REG_FIELD_PREP(XELPDP_DDI_CLOCK_SELECT_MASK, val) > > >+#define XELPDP_DDI_CLOCK_SELECT_NONE 0x0 > > >+#define XELPDP_DDI_CLOCK_SELECT_MAXPCLK 0x8 > > >+#define XELPDP_DDI_CLOCK_SELECT_DIV18CLK 0x9 > > >+#define XELPDP_DDI_CLOCK_SELECT_TBT_162 0xc > > >+#define XELPDP_DDI_CLOCK_SELECT_TBT_270 0xd > > >+#define XELPDP_DDI_CLOCK_SELECT_TBT_540 0xe > > >+#define XELPDP_DDI_CLOCK_SELECT_TBT_810 0xf > > >+#define XELPDP_FORWARD_CLOCK_UNGATE > REG_BIT(10) > > >+#define XELPDP_LANE1_PHY_CLOCK_SELECT > REG_BIT(8) > > >+#define XELPDP_SSC_ENABLE_PLLA REG_BIT(1) > > >+#define XELPDP_SSC_ENABLE_PLLB REG_BIT(0) > > >+ > > >+#endif /* __INTEL_CX0_PHY_REGS_H__ */ > > >-- > > >2.34.1 > > >
