* [Intel-gfx] [PATCH v7 1/5] drm/i915/rkl: Handle new DPCLKA_CFGCR0 layout
2020-06-17 3:30 [Intel-gfx] [PATCH v7 0/5] Remaining RKL patches Matt Roper
@ 2020-06-17 3:30 ` Matt Roper
2020-07-08 0:34 ` Souza, Jose
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 2/5] drm/i915/rkl: Add DPLL4 support Matt Roper
` (5 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Matt Roper @ 2020-06-17 3:30 UTC (permalink / raw)
To: intel-gfx
RKL uses a slightly different bit layout for the DPCLKA_CFGCR0 register.
v2:
- Fix inverted mask application when updating ICL_DPCLKA_CFGCR0
- Checkpatch style fixes
Bspec: 50287
Cc: Aditya Swarup <aditya.swarup@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 18 +++++++++++++++---
drivers/gpu/drm/i915/display/intel_display.c | 15 ++++++++++++---
drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
3 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index ca7bb2294d2b..8790f221dc77 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2770,7 +2770,9 @@ hsw_set_signal_levels(struct intel_dp *intel_dp)
static u32 icl_dpclka_cfgcr0_clk_off(struct drm_i915_private *dev_priv,
enum phy phy)
{
- if (intel_phy_is_combo(dev_priv, phy)) {
+ if (IS_ROCKETLAKE(dev_priv)) {
+ return RKL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy);
+ } else if (intel_phy_is_combo(dev_priv, phy)) {
return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy);
} else if (intel_phy_is_tc(dev_priv, phy)) {
enum tc_port tc_port = intel_port_to_tc(dev_priv,
@@ -2797,6 +2799,16 @@ static void icl_map_plls_to_ports(struct intel_encoder *encoder,
(val & icl_dpclka_cfgcr0_clk_off(dev_priv, phy)) == 0);
if (intel_phy_is_combo(dev_priv, phy)) {
+ u32 mask, sel;
+
+ if (IS_ROCKETLAKE(dev_priv)) {
+ mask = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
+ sel = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy);
+ } else {
+ mask = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
+ sel = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy);
+ }
+
/*
* Even though this register references DDIs, note that we
* want to pass the PHY rather than the port (DDI). For
@@ -2807,8 +2819,8 @@ static void icl_map_plls_to_ports(struct intel_encoder *encoder,
* Clock Select chooses the PLL for both DDIA and DDID and
* drives port A in all cases."
*/
- val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
- val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy);
+ val &= ~mask;
+ val |= sel;
intel_de_write(dev_priv, ICL_DPCLKA_CFGCR0, val);
intel_de_posting_read(dev_priv, ICL_DPCLKA_CFGCR0);
}
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7457813ef273..6c2bb3354b86 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10785,9 +10785,18 @@ static void icl_get_ddi_pll(struct drm_i915_private *dev_priv, enum port port,
u32 temp;
if (intel_phy_is_combo(dev_priv, phy)) {
- temp = intel_de_read(dev_priv, ICL_DPCLKA_CFGCR0) &
- ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
- id = temp >> ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy);
+ u32 mask, shift;
+
+ if (IS_ROCKETLAKE(dev_priv)) {
+ mask = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
+ shift = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy);
+ } else {
+ mask = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
+ shift = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy);
+ }
+
+ temp = intel_de_read(dev_priv, ICL_DPCLKA_CFGCR0) & mask;
+ id = temp >> shift;
port_dpll_id = ICL_PORT_DPLL_DEFAULT;
} else if (intel_phy_is_tc(dev_priv, phy)) {
u32 clk_sel = intel_de_read(dev_priv, DDI_CLK_SEL(port)) & DDI_CLK_SEL_MASK;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f09120cac89a..45bda5819abd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10195,12 +10195,18 @@ enum skl_power_gate {
#define ICL_DPCLKA_CFGCR0 _MMIO(0x164280)
#define ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy) (1 << _PICK(phy, 10, 11, 24))
+#define RKL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy) REG_BIT((phy) + 10)
#define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) < PORT_TC4 ? \
(tc_port) + 12 : \
(tc_port) - PORT_TC4 + 21))
#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) ((phy) * 2)
#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) (3 << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) ((pll) << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
+#define RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) _PICK(phy, 0, 2, 4, 27)
+#define RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) \
+ (3 << RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
+#define RKL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) \
+ ((pll) << RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
/* CNL PLL */
#define DPLL0_ENABLE 0x46010
--
2.24.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 1/5] drm/i915/rkl: Handle new DPCLKA_CFGCR0 layout
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 1/5] drm/i915/rkl: Handle new DPCLKA_CFGCR0 layout Matt Roper
@ 2020-07-08 0:34 ` Souza, Jose
0 siblings, 0 replies; 19+ messages in thread
From: Souza, Jose @ 2020-07-08 0:34 UTC (permalink / raw)
To: Roper, Matthew D, intel-gfx
On Tue, 2020-06-16 at 20:30 -0700, Matt Roper wrote:
> RKL uses a slightly different bit layout for the DPCLKA_CFGCR0 register.
>
> v2:
> - Fix inverted mask application when updating ICL_DPCLKA_CFGCR0
> - Checkpatch style fixes
>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> Bspec: 50287
> Cc: Aditya Swarup <aditya.swarup@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 18 +++++++++++++++---
> drivers/gpu/drm/i915/display/intel_display.c | 15 ++++++++++++---
> drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
> 3 files changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index ca7bb2294d2b..8790f221dc77 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2770,7 +2770,9 @@ hsw_set_signal_levels(struct intel_dp *intel_dp)
> static u32 icl_dpclka_cfgcr0_clk_off(struct drm_i915_private *dev_priv,
> enum phy phy)
> {
> - if (intel_phy_is_combo(dev_priv, phy)) {
> + if (IS_ROCKETLAKE(dev_priv)) {
> + return RKL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy);
> + } else if (intel_phy_is_combo(dev_priv, phy)) {
> return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy);
> } else if (intel_phy_is_tc(dev_priv, phy)) {
> enum tc_port tc_port = intel_port_to_tc(dev_priv,
> @@ -2797,6 +2799,16 @@ static void icl_map_plls_to_ports(struct intel_encoder *encoder,
> (val & icl_dpclka_cfgcr0_clk_off(dev_priv, phy)) == 0);
>
> if (intel_phy_is_combo(dev_priv, phy)) {
> + u32 mask, sel;
> +
> + if (IS_ROCKETLAKE(dev_priv)) {
> + mask = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
> + sel = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy);
> + } else {
> + mask = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
> + sel = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy);
> + }
> +
> /*
> * Even though this register references DDIs, note that we
> * want to pass the PHY rather than the port (DDI). For
> @@ -2807,8 +2819,8 @@ static void icl_map_plls_to_ports(struct intel_encoder *encoder,
> * Clock Select chooses the PLL for both DDIA and DDID and
> * drives port A in all cases."
> */
> - val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
> - val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy);
> + val &= ~mask;
> + val |= sel;
> intel_de_write(dev_priv, ICL_DPCLKA_CFGCR0, val);
> intel_de_posting_read(dev_priv, ICL_DPCLKA_CFGCR0);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7457813ef273..6c2bb3354b86 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -10785,9 +10785,18 @@ static void icl_get_ddi_pll(struct drm_i915_private *dev_priv, enum port port,
> u32 temp;
>
> if (intel_phy_is_combo(dev_priv, phy)) {
> - temp = intel_de_read(dev_priv, ICL_DPCLKA_CFGCR0) &
> - ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
> - id = temp >> ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy);
> + u32 mask, shift;
> +
> + if (IS_ROCKETLAKE(dev_priv)) {
> + mask = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
> + shift = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy);
> + } else {
> + mask = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
> + shift = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy);
> + }
> +
> + temp = intel_de_read(dev_priv, ICL_DPCLKA_CFGCR0) & mask;
> + id = temp >> shift;
> port_dpll_id = ICL_PORT_DPLL_DEFAULT;
> } else if (intel_phy_is_tc(dev_priv, phy)) {
> u32 clk_sel = intel_de_read(dev_priv, DDI_CLK_SEL(port)) & DDI_CLK_SEL_MASK;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f09120cac89a..45bda5819abd 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10195,12 +10195,18 @@ enum skl_power_gate {
>
> #define ICL_DPCLKA_CFGCR0 _MMIO(0x164280)
> #define ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy) (1 << _PICK(phy, 10, 11, 24))
> +#define RKL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy) REG_BIT((phy) + 10)
> #define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) < PORT_TC4 ? \
> (tc_port) + 12 : \
> (tc_port) - PORT_TC4 + 21))
> #define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) ((phy) * 2)
> #define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) (3 << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
> #define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) ((pll) << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
> +#define RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) _PICK(phy, 0, 2, 4, 27)
> +#define RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) \
> + (3 << RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
> +#define RKL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) \
> + ((pll) << RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
>
> /* CNL PLL */
> #define DPLL0_ENABLE 0x46010
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH v7 2/5] drm/i915/rkl: Add DPLL4 support
2020-06-17 3:30 [Intel-gfx] [PATCH v7 0/5] Remaining RKL patches Matt Roper
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 1/5] drm/i915/rkl: Handle new DPCLKA_CFGCR0 layout Matt Roper
@ 2020-06-17 3:30 ` Matt Roper
2020-06-17 18:22 ` Lucas De Marchi
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 3/5] drm/i915/rkl: Handle HTI Matt Roper
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Matt Roper @ 2020-06-17 3:30 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
Rocket Lake has a third DPLL (called 'DPLL4') that must be used to
enable a third display. Unlike EHL's variant of DPLL4, the RKL variant
behaves the same as DPLL0/1. And despite its name, the DPLL4 registers
are offset as if it were DPLL2.
To allow the TGL register selectors like TGL_DPLL_CFGCR0 to be used
seamlessly on all gen12 platforms, we set the non-MG PLL ID's to match
how the registers are laid out: DPLL0, DPLL1, DPLL4 (RKL-only), TBT.
This means just renumbering TBT to be ID '3' rather than being another
ID '2' like DPLL4. With this change, we can build our register
selectors with _MMIO_PLL rather than _MMIO_PLL3 since the register
offsets are evenly-spaced. MGPLL's don't need any specific ID's
(they're just used to translate back to a tc_port), so we let them float
at the top of the enum.
v2:
- Add new .update_ref_clks() hook.
v3:
- Renumber TBT PLL to '3' and switch _MMIO_PLL3 to _MMIO_PLL (Lucas)
Bspec: 49202
Bspec: 49443
Bspec: 50288
Bspec: 50289
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++++++++++++++++--
drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 14 ++++-----
drivers/gpu/drm/i915/i915_reg.h | 15 +++-------
3 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index b45185b80bec..b5f4d4cef682 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3506,13 +3506,19 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
return false;
}
- if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A)
+ if (IS_ROCKETLAKE(dev_priv)) {
dpll_mask =
BIT(DPLL_ID_EHL_DPLL4) |
BIT(DPLL_ID_ICL_DPLL1) |
BIT(DPLL_ID_ICL_DPLL0);
- else
+ } else if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) {
+ dpll_mask =
+ BIT(DPLL_ID_EHL_DPLL4) |
+ BIT(DPLL_ID_ICL_DPLL1) |
+ BIT(DPLL_ID_ICL_DPLL0);
+ } else {
dpll_mask = BIT(DPLL_ID_ICL_DPLL1) | BIT(DPLL_ID_ICL_DPLL0);
+ }
port_dpll->pll = intel_find_shared_dpll(state, crtc,
&port_dpll->hw_state,
@@ -4275,6 +4281,21 @@ static const struct intel_dpll_mgr tgl_pll_mgr = {
.dump_hw_state = icl_dump_hw_state,
};
+static const struct dpll_info rkl_plls[] = {
+ { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
+ { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
+ { "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
+ { },
+};
+
+static const struct intel_dpll_mgr rkl_pll_mgr = {
+ .dpll_info = rkl_plls,
+ .get_dplls = icl_get_dplls,
+ .put_dplls = icl_put_dplls,
+ .update_ref_clks = icl_update_dpll_ref_clks,
+ .dump_hw_state = icl_dump_hw_state,
+};
+
/**
* intel_shared_dpll_init - Initialize shared DPLLs
* @dev: drm device
@@ -4288,7 +4309,9 @@ void intel_shared_dpll_init(struct drm_device *dev)
const struct dpll_info *dpll_info;
int i;
- if (INTEL_GEN(dev_priv) >= 12)
+ if (IS_ROCKETLAKE(dev_priv))
+ dpll_mgr = &rkl_pll_mgr;
+ else if (INTEL_GEN(dev_priv) >= 12)
dpll_mgr = &tgl_pll_mgr;
else if (IS_ELKHARTLAKE(dev_priv))
dpll_mgr = &ehl_pll_mgr;
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
index 5d9a2bc371e7..49367847bfb5 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
@@ -125,35 +125,35 @@ enum intel_dpll_id {
/**
* @DPLL_ID_ICL_TBTPLL: ICL/TGL TBT PLL
*/
- DPLL_ID_ICL_TBTPLL = 2,
+ DPLL_ID_ICL_TBTPLL = 3,
/**
* @DPLL_ID_ICL_MGPLL1: ICL MG PLL 1 port 1 (C),
* TGL TC PLL 1 port 1 (TC1)
*/
- DPLL_ID_ICL_MGPLL1 = 3,
+ DPLL_ID_ICL_MGPLL1,
/**
* @DPLL_ID_ICL_MGPLL2: ICL MG PLL 1 port 2 (D)
* TGL TC PLL 1 port 2 (TC2)
*/
- DPLL_ID_ICL_MGPLL2 = 4,
+ DPLL_ID_ICL_MGPLL2,
/**
* @DPLL_ID_ICL_MGPLL3: ICL MG PLL 1 port 3 (E)
* TGL TC PLL 1 port 3 (TC3)
*/
- DPLL_ID_ICL_MGPLL3 = 5,
+ DPLL_ID_ICL_MGPLL3,
/**
* @DPLL_ID_ICL_MGPLL4: ICL MG PLL 1 port 4 (F)
* TGL TC PLL 1 port 4 (TC4)
*/
- DPLL_ID_ICL_MGPLL4 = 6,
+ DPLL_ID_ICL_MGPLL4,
/**
* @DPLL_ID_TGL_MGPLL5: TGL TC PLL port 5 (TC5)
*/
- DPLL_ID_TGL_MGPLL5 = 7,
+ DPLL_ID_TGL_MGPLL5,
/**
* @DPLL_ID_TGL_MGPLL6: TGL TC PLL port 6 (TC6)
*/
- DPLL_ID_TGL_MGPLL6 = 8,
+ DPLL_ID_TGL_MGPLL6,
};
#define I915_NUM_PLLS 9
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 45bda5819abd..34f8698ac3aa 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -242,7 +242,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
#define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
-#define _MMIO_PLL3(pll, a, b, c) _MMIO(_PICK(pll, a, b, c))
/*
* Device info offset array based helpers for groups of registers with unevenly
@@ -10427,19 +10426,13 @@ enum skl_power_gate {
#define _TGL_DPLL0_CFGCR0 0x164284
#define _TGL_DPLL1_CFGCR0 0x16428C
-/* TODO: add DPLL4 */
-#define _TGL_TBTPLL_CFGCR0 0x16429C
-#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR0, \
- _TGL_DPLL1_CFGCR0, \
- _TGL_TBTPLL_CFGCR0)
+#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR0, \
+ _TGL_DPLL1_CFGCR0)
#define _TGL_DPLL0_CFGCR1 0x164288
#define _TGL_DPLL1_CFGCR1 0x164290
-/* TODO: add DPLL4 */
-#define _TGL_TBTPLL_CFGCR1 0x1642A0
-#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
- _TGL_DPLL1_CFGCR1, \
- _TGL_TBTPLL_CFGCR1)
+#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR1, \
+ _TGL_DPLL1_CFGCR1)
#define _DKL_PHY1_BASE 0x168000
#define _DKL_PHY2_BASE 0x169000
--
2.24.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 2/5] drm/i915/rkl: Add DPLL4 support
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 2/5] drm/i915/rkl: Add DPLL4 support Matt Roper
@ 2020-06-17 18:22 ` Lucas De Marchi
2020-06-17 20:00 ` Matt Roper
0 siblings, 1 reply; 19+ messages in thread
From: Lucas De Marchi @ 2020-06-17 18:22 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Tue, Jun 16, 2020 at 08:30:57PM -0700, Matt Roper wrote:
>Rocket Lake has a third DPLL (called 'DPLL4') that must be used to
>enable a third display. Unlike EHL's variant of DPLL4, the RKL variant
>behaves the same as DPLL0/1. And despite its name, the DPLL4 registers
>are offset as if it were DPLL2.
>
>To allow the TGL register selectors like TGL_DPLL_CFGCR0 to be used
>seamlessly on all gen12 platforms, we set the non-MG PLL ID's to match
>how the registers are laid out: DPLL0, DPLL1, DPLL4 (RKL-only), TBT.
>This means just renumbering TBT to be ID '3' rather than being another
>ID '2' like DPLL4. With this change, we can build our register
>selectors with _MMIO_PLL rather than _MMIO_PLL3 since the register
>offsets are evenly-spaced. MGPLL's don't need any specific ID's
>(they're just used to translate back to a tc_port), so we let them float
>at the top of the enum.
>
>v2:
> - Add new .update_ref_clks() hook.
>
>v3:
> - Renumber TBT PLL to '3' and switch _MMIO_PLL3 to _MMIO_PLL (Lucas)
>
>Bspec: 49202
>Bspec: 49443
>Bspec: 50288
>Bspec: 50289
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++++++++++++++++--
> drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 14 ++++-----
> drivers/gpu/drm/i915/i915_reg.h | 15 +++-------
> 3 files changed, 37 insertions(+), 21 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>index b45185b80bec..b5f4d4cef682 100644
>--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>@@ -3506,13 +3506,19 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
> return false;
> }
>
>- if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A)
>+ if (IS_ROCKETLAKE(dev_priv)) {
> dpll_mask =
> BIT(DPLL_ID_EHL_DPLL4) |
> BIT(DPLL_ID_ICL_DPLL1) |
> BIT(DPLL_ID_ICL_DPLL0);
>- else
>+ } else if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) {
>+ dpll_mask =
>+ BIT(DPLL_ID_EHL_DPLL4) |
>+ BIT(DPLL_ID_ICL_DPLL1) |
>+ BIT(DPLL_ID_ICL_DPLL0);
>+ } else {
> dpll_mask = BIT(DPLL_ID_ICL_DPLL1) | BIT(DPLL_ID_ICL_DPLL0);
>+ }
>
> port_dpll->pll = intel_find_shared_dpll(state, crtc,
> &port_dpll->hw_state,
>@@ -4275,6 +4281,21 @@ static const struct intel_dpll_mgr tgl_pll_mgr = {
> .dump_hw_state = icl_dump_hw_state,
> };
>
>+static const struct dpll_info rkl_plls[] = {
>+ { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
>+ { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
>+ { "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
>+ { },
>+};
>+
>+static const struct intel_dpll_mgr rkl_pll_mgr = {
>+ .dpll_info = rkl_plls,
>+ .get_dplls = icl_get_dplls,
>+ .put_dplls = icl_put_dplls,
>+ .update_ref_clks = icl_update_dpll_ref_clks,
>+ .dump_hw_state = icl_dump_hw_state,
>+};
>+
> /**
> * intel_shared_dpll_init - Initialize shared DPLLs
> * @dev: drm device
>@@ -4288,7 +4309,9 @@ void intel_shared_dpll_init(struct drm_device *dev)
> const struct dpll_info *dpll_info;
> int i;
>
>- if (INTEL_GEN(dev_priv) >= 12)
>+ if (IS_ROCKETLAKE(dev_priv))
>+ dpll_mgr = &rkl_pll_mgr;
>+ else if (INTEL_GEN(dev_priv) >= 12)
> dpll_mgr = &tgl_pll_mgr;
> else if (IS_ELKHARTLAKE(dev_priv))
> dpll_mgr = &ehl_pll_mgr;
>diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
>index 5d9a2bc371e7..49367847bfb5 100644
>--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
>+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
>@@ -125,35 +125,35 @@ enum intel_dpll_id {
> /**
> * @DPLL_ID_ICL_TBTPLL: ICL/TGL TBT PLL
> */
>- DPLL_ID_ICL_TBTPLL = 2,
>+ DPLL_ID_ICL_TBTPLL = 3,
> /**
> * @DPLL_ID_ICL_MGPLL1: ICL MG PLL 1 port 1 (C),
> * TGL TC PLL 1 port 1 (TC1)
> */
>- DPLL_ID_ICL_MGPLL1 = 3,
>+ DPLL_ID_ICL_MGPLL1,
> /**
> * @DPLL_ID_ICL_MGPLL2: ICL MG PLL 1 port 2 (D)
> * TGL TC PLL 1 port 2 (TC2)
> */
>- DPLL_ID_ICL_MGPLL2 = 4,
>+ DPLL_ID_ICL_MGPLL2,
> /**
> * @DPLL_ID_ICL_MGPLL3: ICL MG PLL 1 port 3 (E)
> * TGL TC PLL 1 port 3 (TC3)
> */
>- DPLL_ID_ICL_MGPLL3 = 5,
>+ DPLL_ID_ICL_MGPLL3,
> /**
> * @DPLL_ID_ICL_MGPLL4: ICL MG PLL 1 port 4 (F)
> * TGL TC PLL 1 port 4 (TC4)
> */
>- DPLL_ID_ICL_MGPLL4 = 6,
>+ DPLL_ID_ICL_MGPLL4,
> /**
> * @DPLL_ID_TGL_MGPLL5: TGL TC PLL port 5 (TC5)
> */
>- DPLL_ID_TGL_MGPLL5 = 7,
>+ DPLL_ID_TGL_MGPLL5,
> /**
> * @DPLL_ID_TGL_MGPLL6: TGL TC PLL port 6 (TC6)
> */
>- DPLL_ID_TGL_MGPLL6 = 8,
>+ DPLL_ID_TGL_MGPLL6,
> };
>
> #define I915_NUM_PLLS 9
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index 45bda5819abd..34f8698ac3aa 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -242,7 +242,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
>-#define _MMIO_PLL3(pll, a, b, c) _MMIO(_PICK(pll, a, b, c))
See my series adding DPLL support for DG1. We will need it again for a
different reason, with a slightly different form. I'd let this here
to avoid removing and adding it back.
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
thanks
Lucas De Marchi
>
> /*
> * Device info offset array based helpers for groups of registers with unevenly
>@@ -10427,19 +10426,13 @@ enum skl_power_gate {
>
> #define _TGL_DPLL0_CFGCR0 0x164284
> #define _TGL_DPLL1_CFGCR0 0x16428C
>-/* TODO: add DPLL4 */
>-#define _TGL_TBTPLL_CFGCR0 0x16429C
>-#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR0, \
>- _TGL_DPLL1_CFGCR0, \
>- _TGL_TBTPLL_CFGCR0)
>+#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR0, \
>+ _TGL_DPLL1_CFGCR0)
>
> #define _TGL_DPLL0_CFGCR1 0x164288
> #define _TGL_DPLL1_CFGCR1 0x164290
>-/* TODO: add DPLL4 */
>-#define _TGL_TBTPLL_CFGCR1 0x1642A0
>-#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
>- _TGL_DPLL1_CFGCR1, \
>- _TGL_TBTPLL_CFGCR1)
>+#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR1, \
>+ _TGL_DPLL1_CFGCR1)
>
> #define _DKL_PHY1_BASE 0x168000
> #define _DKL_PHY2_BASE 0x169000
>--
>2.24.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 2/5] drm/i915/rkl: Add DPLL4 support
2020-06-17 18:22 ` Lucas De Marchi
@ 2020-06-17 20:00 ` Matt Roper
2020-06-17 20:41 ` Lucas De Marchi
2020-06-18 5:57 ` Matt Roper
0 siblings, 2 replies; 19+ messages in thread
From: Matt Roper @ 2020-06-17 20:00 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Wed, Jun 17, 2020 at 11:22:58AM -0700, Lucas De Marchi wrote:
> On Tue, Jun 16, 2020 at 08:30:57PM -0700, Matt Roper wrote:
> > Rocket Lake has a third DPLL (called 'DPLL4') that must be used to
> > enable a third display. Unlike EHL's variant of DPLL4, the RKL variant
> > behaves the same as DPLL0/1. And despite its name, the DPLL4 registers
> > are offset as if it were DPLL2.
> >
> > To allow the TGL register selectors like TGL_DPLL_CFGCR0 to be used
> > seamlessly on all gen12 platforms, we set the non-MG PLL ID's to match
> > how the registers are laid out: DPLL0, DPLL1, DPLL4 (RKL-only), TBT.
> > This means just renumbering TBT to be ID '3' rather than being another
> > ID '2' like DPLL4. With this change, we can build our register
> > selectors with _MMIO_PLL rather than _MMIO_PLL3 since the register
> > offsets are evenly-spaced. MGPLL's don't need any specific ID's
> > (they're just used to translate back to a tc_port), so we let them float
> > at the top of the enum.
> >
> > v2:
> > - Add new .update_ref_clks() hook.
> >
> > v3:
> > - Renumber TBT PLL to '3' and switch _MMIO_PLL3 to _MMIO_PLL (Lucas)
> >
> > Bspec: 49202
> > Bspec: 49443
> > Bspec: 50288
> > Bspec: 50289
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++++++++++++++++--
> > drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 14 ++++-----
> > drivers/gpu/drm/i915/i915_reg.h | 15 +++-------
> > 3 files changed, 37 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index b45185b80bec..b5f4d4cef682 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -3506,13 +3506,19 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
> > return false;
> > }
> >
> > - if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A)
> > + if (IS_ROCKETLAKE(dev_priv)) {
> > dpll_mask =
> > BIT(DPLL_ID_EHL_DPLL4) |
> > BIT(DPLL_ID_ICL_DPLL1) |
> > BIT(DPLL_ID_ICL_DPLL0);
> > - else
> > + } else if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) {
> > + dpll_mask =
> > + BIT(DPLL_ID_EHL_DPLL4) |
> > + BIT(DPLL_ID_ICL_DPLL1) |
> > + BIT(DPLL_ID_ICL_DPLL0);
> > + } else {
> > dpll_mask = BIT(DPLL_ID_ICL_DPLL1) | BIT(DPLL_ID_ICL_DPLL0);
> > + }
> >
> > port_dpll->pll = intel_find_shared_dpll(state, crtc,
> > &port_dpll->hw_state,
> > @@ -4275,6 +4281,21 @@ static const struct intel_dpll_mgr tgl_pll_mgr = {
> > .dump_hw_state = icl_dump_hw_state,
> > };
> >
> > +static const struct dpll_info rkl_plls[] = {
> > + { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> > + { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> > + { "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
> > + { },
> > +};
> > +
> > +static const struct intel_dpll_mgr rkl_pll_mgr = {
> > + .dpll_info = rkl_plls,
> > + .get_dplls = icl_get_dplls,
> > + .put_dplls = icl_put_dplls,
> > + .update_ref_clks = icl_update_dpll_ref_clks,
> > + .dump_hw_state = icl_dump_hw_state,
> > +};
> > +
> > /**
> > * intel_shared_dpll_init - Initialize shared DPLLs
> > * @dev: drm device
> > @@ -4288,7 +4309,9 @@ void intel_shared_dpll_init(struct drm_device *dev)
> > const struct dpll_info *dpll_info;
> > int i;
> >
> > - if (INTEL_GEN(dev_priv) >= 12)
> > + if (IS_ROCKETLAKE(dev_priv))
> > + dpll_mgr = &rkl_pll_mgr;
> > + else if (INTEL_GEN(dev_priv) >= 12)
> > dpll_mgr = &tgl_pll_mgr;
> > else if (IS_ELKHARTLAKE(dev_priv))
> > dpll_mgr = &ehl_pll_mgr;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > index 5d9a2bc371e7..49367847bfb5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > @@ -125,35 +125,35 @@ enum intel_dpll_id {
> > /**
> > * @DPLL_ID_ICL_TBTPLL: ICL/TGL TBT PLL
> > */
> > - DPLL_ID_ICL_TBTPLL = 2,
> > + DPLL_ID_ICL_TBTPLL = 3,
> > /**
> > * @DPLL_ID_ICL_MGPLL1: ICL MG PLL 1 port 1 (C),
> > * TGL TC PLL 1 port 1 (TC1)
> > */
> > - DPLL_ID_ICL_MGPLL1 = 3,
> > + DPLL_ID_ICL_MGPLL1,
> > /**
> > * @DPLL_ID_ICL_MGPLL2: ICL MG PLL 1 port 2 (D)
> > * TGL TC PLL 1 port 2 (TC2)
> > */
> > - DPLL_ID_ICL_MGPLL2 = 4,
> > + DPLL_ID_ICL_MGPLL2,
> > /**
> > * @DPLL_ID_ICL_MGPLL3: ICL MG PLL 1 port 3 (E)
> > * TGL TC PLL 1 port 3 (TC3)
> > */
> > - DPLL_ID_ICL_MGPLL3 = 5,
> > + DPLL_ID_ICL_MGPLL3,
> > /**
> > * @DPLL_ID_ICL_MGPLL4: ICL MG PLL 1 port 4 (F)
> > * TGL TC PLL 1 port 4 (TC4)
> > */
> > - DPLL_ID_ICL_MGPLL4 = 6,
> > + DPLL_ID_ICL_MGPLL4,
> > /**
> > * @DPLL_ID_TGL_MGPLL5: TGL TC PLL port 5 (TC5)
> > */
> > - DPLL_ID_TGL_MGPLL5 = 7,
> > + DPLL_ID_TGL_MGPLL5,
> > /**
> > * @DPLL_ID_TGL_MGPLL6: TGL TC PLL port 6 (TC6)
> > */
> > - DPLL_ID_TGL_MGPLL6 = 8,
> > + DPLL_ID_TGL_MGPLL6,
> > };
> >
> > #define I915_NUM_PLLS 9
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 45bda5819abd..34f8698ac3aa 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -242,7 +242,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> > #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> > #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> > #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
> > -#define _MMIO_PLL3(pll, a, b, c) _MMIO(_PICK(pll, a, b, c))
>
> See my series adding DPLL support for DG1. We will need it again for a
> different reason, with a slightly different form. I'd let this here
> to avoid removing and adding it back.
>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
With the renumbering of TBT to 3, we now have a "hole" in the DPLLs
exposed on TGL (0, 1, 3) which WARNs:
<4>[ 6.165705] i915 0000:00:02.0: drm_WARN_ON(i != dpll_info[i].id)
<4>[ 6.166050] WARNING: CPU: 7 PID: 335 at drivers/gpu/drm/i915/display/intel_dpll_mgr.c:4360 intel_shared_dpll_init+0xa6/0x1d0 [i915]
I remember having a hole was a problem back when we just passed min/max
DPLLs for initialization, but I think it should be safe now ever since:
commit 2a86972f60fcfaa0daa02b9fe461935ea2063791
Author: Matt Roper <matthew.d.roper@intel.com>
Date: Tue Oct 8 10:29:20 2019 -0700
drm/i915: Select DPLL's via mask
so I'll send another version that drops that WARN and keeps the
_MMIO_PLL3 definition here.
Thanks.
Matt
>
> thanks
> Lucas De Marchi
>
> >
> > /*
> > * Device info offset array based helpers for groups of registers with unevenly
> > @@ -10427,19 +10426,13 @@ enum skl_power_gate {
> >
> > #define _TGL_DPLL0_CFGCR0 0x164284
> > #define _TGL_DPLL1_CFGCR0 0x16428C
> > -/* TODO: add DPLL4 */
> > -#define _TGL_TBTPLL_CFGCR0 0x16429C
> > -#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR0, \
> > - _TGL_DPLL1_CFGCR0, \
> > - _TGL_TBTPLL_CFGCR0)
> > +#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR0, \
> > + _TGL_DPLL1_CFGCR0)
> >
> > #define _TGL_DPLL0_CFGCR1 0x164288
> > #define _TGL_DPLL1_CFGCR1 0x164290
> > -/* TODO: add DPLL4 */
> > -#define _TGL_TBTPLL_CFGCR1 0x1642A0
> > -#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
> > - _TGL_DPLL1_CFGCR1, \
> > - _TGL_TBTPLL_CFGCR1)
> > +#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR1, \
> > + _TGL_DPLL1_CFGCR1)
> >
> > #define _DKL_PHY1_BASE 0x168000
> > #define _DKL_PHY2_BASE 0x169000
> > --
> > 2.24.1
> >
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 2/5] drm/i915/rkl: Add DPLL4 support
2020-06-17 20:00 ` Matt Roper
@ 2020-06-17 20:41 ` Lucas De Marchi
2020-06-18 5:57 ` Matt Roper
1 sibling, 0 replies; 19+ messages in thread
From: Lucas De Marchi @ 2020-06-17 20:41 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Wed, Jun 17, 2020 at 01:00:38PM -0700, Matt Roper wrote:
>On Wed, Jun 17, 2020 at 11:22:58AM -0700, Lucas De Marchi wrote:
>> On Tue, Jun 16, 2020 at 08:30:57PM -0700, Matt Roper wrote:
>> > Rocket Lake has a third DPLL (called 'DPLL4') that must be used to
>> > enable a third display. Unlike EHL's variant of DPLL4, the RKL variant
>> > behaves the same as DPLL0/1. And despite its name, the DPLL4 registers
>> > are offset as if it were DPLL2.
>> >
>> > To allow the TGL register selectors like TGL_DPLL_CFGCR0 to be used
>> > seamlessly on all gen12 platforms, we set the non-MG PLL ID's to match
>> > how the registers are laid out: DPLL0, DPLL1, DPLL4 (RKL-only), TBT.
>> > This means just renumbering TBT to be ID '3' rather than being another
>> > ID '2' like DPLL4. With this change, we can build our register
>> > selectors with _MMIO_PLL rather than _MMIO_PLL3 since the register
>> > offsets are evenly-spaced. MGPLL's don't need any specific ID's
>> > (they're just used to translate back to a tc_port), so we let them float
>> > at the top of the enum.
>> >
>> > v2:
>> > - Add new .update_ref_clks() hook.
>> >
>> > v3:
>> > - Renumber TBT PLL to '3' and switch _MMIO_PLL3 to _MMIO_PLL (Lucas)
>> >
>> > Bspec: 49202
>> > Bspec: 49443
>> > Bspec: 50288
>> > Bspec: 50289
>> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++++++++++++++++--
>> > drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 14 ++++-----
>> > drivers/gpu/drm/i915/i915_reg.h | 15 +++-------
>> > 3 files changed, 37 insertions(+), 21 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> > index b45185b80bec..b5f4d4cef682 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> > @@ -3506,13 +3506,19 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
>> > return false;
>> > }
>> >
>> > - if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A)
>> > + if (IS_ROCKETLAKE(dev_priv)) {
>> > dpll_mask =
>> > BIT(DPLL_ID_EHL_DPLL4) |
>> > BIT(DPLL_ID_ICL_DPLL1) |
>> > BIT(DPLL_ID_ICL_DPLL0);
>> > - else
>> > + } else if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) {
>> > + dpll_mask =
>> > + BIT(DPLL_ID_EHL_DPLL4) |
>> > + BIT(DPLL_ID_ICL_DPLL1) |
>> > + BIT(DPLL_ID_ICL_DPLL0);
>> > + } else {
>> > dpll_mask = BIT(DPLL_ID_ICL_DPLL1) | BIT(DPLL_ID_ICL_DPLL0);
>> > + }
>> >
>> > port_dpll->pll = intel_find_shared_dpll(state, crtc,
>> > &port_dpll->hw_state,
>> > @@ -4275,6 +4281,21 @@ static const struct intel_dpll_mgr tgl_pll_mgr = {
>> > .dump_hw_state = icl_dump_hw_state,
>> > };
>> >
>> > +static const struct dpll_info rkl_plls[] = {
>> > + { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
>> > + { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
>> > + { "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
>> > + { },
>> > +};
>> > +
>> > +static const struct intel_dpll_mgr rkl_pll_mgr = {
>> > + .dpll_info = rkl_plls,
>> > + .get_dplls = icl_get_dplls,
>> > + .put_dplls = icl_put_dplls,
>> > + .update_ref_clks = icl_update_dpll_ref_clks,
>> > + .dump_hw_state = icl_dump_hw_state,
>> > +};
>> > +
>> > /**
>> > * intel_shared_dpll_init - Initialize shared DPLLs
>> > * @dev: drm device
>> > @@ -4288,7 +4309,9 @@ void intel_shared_dpll_init(struct drm_device *dev)
>> > const struct dpll_info *dpll_info;
>> > int i;
>> >
>> > - if (INTEL_GEN(dev_priv) >= 12)
>> > + if (IS_ROCKETLAKE(dev_priv))
>> > + dpll_mgr = &rkl_pll_mgr;
>> > + else if (INTEL_GEN(dev_priv) >= 12)
>> > dpll_mgr = &tgl_pll_mgr;
>> > else if (IS_ELKHARTLAKE(dev_priv))
>> > dpll_mgr = &ehl_pll_mgr;
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
>> > index 5d9a2bc371e7..49367847bfb5 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
>> > @@ -125,35 +125,35 @@ enum intel_dpll_id {
>> > /**
>> > * @DPLL_ID_ICL_TBTPLL: ICL/TGL TBT PLL
>> > */
>> > - DPLL_ID_ICL_TBTPLL = 2,
>> > + DPLL_ID_ICL_TBTPLL = 3,
>> > /**
>> > * @DPLL_ID_ICL_MGPLL1: ICL MG PLL 1 port 1 (C),
>> > * TGL TC PLL 1 port 1 (TC1)
>> > */
>> > - DPLL_ID_ICL_MGPLL1 = 3,
>> > + DPLL_ID_ICL_MGPLL1,
>> > /**
>> > * @DPLL_ID_ICL_MGPLL2: ICL MG PLL 1 port 2 (D)
>> > * TGL TC PLL 1 port 2 (TC2)
>> > */
>> > - DPLL_ID_ICL_MGPLL2 = 4,
>> > + DPLL_ID_ICL_MGPLL2,
>> > /**
>> > * @DPLL_ID_ICL_MGPLL3: ICL MG PLL 1 port 3 (E)
>> > * TGL TC PLL 1 port 3 (TC3)
>> > */
>> > - DPLL_ID_ICL_MGPLL3 = 5,
>> > + DPLL_ID_ICL_MGPLL3,
>> > /**
>> > * @DPLL_ID_ICL_MGPLL4: ICL MG PLL 1 port 4 (F)
>> > * TGL TC PLL 1 port 4 (TC4)
>> > */
>> > - DPLL_ID_ICL_MGPLL4 = 6,
>> > + DPLL_ID_ICL_MGPLL4,
>> > /**
>> > * @DPLL_ID_TGL_MGPLL5: TGL TC PLL port 5 (TC5)
>> > */
>> > - DPLL_ID_TGL_MGPLL5 = 7,
>> > + DPLL_ID_TGL_MGPLL5,
>> > /**
>> > * @DPLL_ID_TGL_MGPLL6: TGL TC PLL port 6 (TC6)
>> > */
>> > - DPLL_ID_TGL_MGPLL6 = 8,
>> > + DPLL_ID_TGL_MGPLL6,
>> > };
>> >
>> > #define I915_NUM_PLLS 9
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> > index 45bda5819abd..34f8698ac3aa 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -242,7 +242,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>> > #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
>> > #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
>> > #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
>> > -#define _MMIO_PLL3(pll, a, b, c) _MMIO(_PICK(pll, a, b, c))
>>
>> See my series adding DPLL support for DG1. We will need it again for a
>> different reason, with a slightly different form. I'd let this here
>> to avoid removing and adding it back.
>>
>> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
>With the renumbering of TBT to 3, we now have a "hole" in the DPLLs
>exposed on TGL (0, 1, 3) which WARNs:
>
> <4>[ 6.165705] i915 0000:00:02.0: drm_WARN_ON(i != dpll_info[i].id)
> <4>[ 6.166050] WARNING: CPU: 7 PID: 335 at drivers/gpu/drm/i915/display/intel_dpll_mgr.c:4360 intel_shared_dpll_init+0xa6/0x1d0 [i915]
>
>I remember having a hole was a problem back when we just passed min/max
>DPLLs for initialization, but I think it should be safe now ever since:
yep, I think we had a change like that when we had DPLL4 for TGL
implemented.
>
> commit 2a86972f60fcfaa0daa02b9fe461935ea2063791
> Author: Matt Roper <matthew.d.roper@intel.com>
> Date: Tue Oct 8 10:29:20 2019 -0700
>
> drm/i915: Select DPLL's via mask
>
>so I'll send another version that drops that WARN and keeps the
>_MMIO_PLL3 definition here.
thanks
Lucas De Marchi
>
>Thanks.
>
>
>Matt
>
>>
>> thanks
>> Lucas De Marchi
>>
>> >
>> > /*
>> > * Device info offset array based helpers for groups of registers with unevenly
>> > @@ -10427,19 +10426,13 @@ enum skl_power_gate {
>> >
>> > #define _TGL_DPLL0_CFGCR0 0x164284
>> > #define _TGL_DPLL1_CFGCR0 0x16428C
>> > -/* TODO: add DPLL4 */
>> > -#define _TGL_TBTPLL_CFGCR0 0x16429C
>> > -#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR0, \
>> > - _TGL_DPLL1_CFGCR0, \
>> > - _TGL_TBTPLL_CFGCR0)
>> > +#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR0, \
>> > + _TGL_DPLL1_CFGCR0)
>> >
>> > #define _TGL_DPLL0_CFGCR1 0x164288
>> > #define _TGL_DPLL1_CFGCR1 0x164290
>> > -/* TODO: add DPLL4 */
>> > -#define _TGL_TBTPLL_CFGCR1 0x1642A0
>> > -#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
>> > - _TGL_DPLL1_CFGCR1, \
>> > - _TGL_TBTPLL_CFGCR1)
>> > +#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR1, \
>> > + _TGL_DPLL1_CFGCR1)
>> >
>> > #define _DKL_PHY1_BASE 0x168000
>> > #define _DKL_PHY2_BASE 0x169000
>> > --
>> > 2.24.1
>> >
>
>--
>Matt Roper
>Graphics Software Engineer
>VTT-OSGC Platform Enablement
>Intel Corporation
>(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 2/5] drm/i915/rkl: Add DPLL4 support
2020-06-17 20:00 ` Matt Roper
2020-06-17 20:41 ` Lucas De Marchi
@ 2020-06-18 5:57 ` Matt Roper
2020-06-18 6:38 ` Lucas De Marchi
1 sibling, 1 reply; 19+ messages in thread
From: Matt Roper @ 2020-06-18 5:57 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Wed, Jun 17, 2020 at 01:00:38PM -0700, Matt Roper wrote:
> On Wed, Jun 17, 2020 at 11:22:58AM -0700, Lucas De Marchi wrote:
> > On Tue, Jun 16, 2020 at 08:30:57PM -0700, Matt Roper wrote:
> > > Rocket Lake has a third DPLL (called 'DPLL4') that must be used to
> > > enable a third display. Unlike EHL's variant of DPLL4, the RKL variant
> > > behaves the same as DPLL0/1. And despite its name, the DPLL4 registers
> > > are offset as if it were DPLL2.
> > >
> > > To allow the TGL register selectors like TGL_DPLL_CFGCR0 to be used
> > > seamlessly on all gen12 platforms, we set the non-MG PLL ID's to match
> > > how the registers are laid out: DPLL0, DPLL1, DPLL4 (RKL-only), TBT.
> > > This means just renumbering TBT to be ID '3' rather than being another
> > > ID '2' like DPLL4. With this change, we can build our register
> > > selectors with _MMIO_PLL rather than _MMIO_PLL3 since the register
> > > offsets are evenly-spaced. MGPLL's don't need any specific ID's
> > > (they're just used to translate back to a tc_port), so we let them float
> > > at the top of the enum.
> > >
> > > v2:
> > > - Add new .update_ref_clks() hook.
> > >
> > > v3:
> > > - Renumber TBT PLL to '3' and switch _MMIO_PLL3 to _MMIO_PLL (Lucas)
> > >
> > > Bspec: 49202
> > > Bspec: 49443
> > > Bspec: 50288
> > > Bspec: 50289
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++++++++++++++++--
> > > drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 14 ++++-----
> > > drivers/gpu/drm/i915/i915_reg.h | 15 +++-------
> > > 3 files changed, 37 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > index b45185b80bec..b5f4d4cef682 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > @@ -3506,13 +3506,19 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
> > > return false;
> > > }
> > >
> > > - if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A)
> > > + if (IS_ROCKETLAKE(dev_priv)) {
> > > dpll_mask =
> > > BIT(DPLL_ID_EHL_DPLL4) |
> > > BIT(DPLL_ID_ICL_DPLL1) |
> > > BIT(DPLL_ID_ICL_DPLL0);
> > > - else
> > > + } else if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) {
> > > + dpll_mask =
> > > + BIT(DPLL_ID_EHL_DPLL4) |
> > > + BIT(DPLL_ID_ICL_DPLL1) |
> > > + BIT(DPLL_ID_ICL_DPLL0);
> > > + } else {
> > > dpll_mask = BIT(DPLL_ID_ICL_DPLL1) | BIT(DPLL_ID_ICL_DPLL0);
> > > + }
> > >
> > > port_dpll->pll = intel_find_shared_dpll(state, crtc,
> > > &port_dpll->hw_state,
> > > @@ -4275,6 +4281,21 @@ static const struct intel_dpll_mgr tgl_pll_mgr = {
> > > .dump_hw_state = icl_dump_hw_state,
> > > };
> > >
> > > +static const struct dpll_info rkl_plls[] = {
> > > + { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> > > + { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> > > + { "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
> > > + { },
> > > +};
> > > +
> > > +static const struct intel_dpll_mgr rkl_pll_mgr = {
> > > + .dpll_info = rkl_plls,
> > > + .get_dplls = icl_get_dplls,
> > > + .put_dplls = icl_put_dplls,
> > > + .update_ref_clks = icl_update_dpll_ref_clks,
> > > + .dump_hw_state = icl_dump_hw_state,
> > > +};
> > > +
> > > /**
> > > * intel_shared_dpll_init - Initialize shared DPLLs
> > > * @dev: drm device
> > > @@ -4288,7 +4309,9 @@ void intel_shared_dpll_init(struct drm_device *dev)
> > > const struct dpll_info *dpll_info;
> > > int i;
> > >
> > > - if (INTEL_GEN(dev_priv) >= 12)
> > > + if (IS_ROCKETLAKE(dev_priv))
> > > + dpll_mgr = &rkl_pll_mgr;
> > > + else if (INTEL_GEN(dev_priv) >= 12)
> > > dpll_mgr = &tgl_pll_mgr;
> > > else if (IS_ELKHARTLAKE(dev_priv))
> > > dpll_mgr = &ehl_pll_mgr;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > > index 5d9a2bc371e7..49367847bfb5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > > @@ -125,35 +125,35 @@ enum intel_dpll_id {
> > > /**
> > > * @DPLL_ID_ICL_TBTPLL: ICL/TGL TBT PLL
> > > */
> > > - DPLL_ID_ICL_TBTPLL = 2,
> > > + DPLL_ID_ICL_TBTPLL = 3,
> > > /**
> > > * @DPLL_ID_ICL_MGPLL1: ICL MG PLL 1 port 1 (C),
> > > * TGL TC PLL 1 port 1 (TC1)
> > > */
> > > - DPLL_ID_ICL_MGPLL1 = 3,
> > > + DPLL_ID_ICL_MGPLL1,
> > > /**
> > > * @DPLL_ID_ICL_MGPLL2: ICL MG PLL 1 port 2 (D)
> > > * TGL TC PLL 1 port 2 (TC2)
> > > */
> > > - DPLL_ID_ICL_MGPLL2 = 4,
> > > + DPLL_ID_ICL_MGPLL2,
> > > /**
> > > * @DPLL_ID_ICL_MGPLL3: ICL MG PLL 1 port 3 (E)
> > > * TGL TC PLL 1 port 3 (TC3)
> > > */
> > > - DPLL_ID_ICL_MGPLL3 = 5,
> > > + DPLL_ID_ICL_MGPLL3,
> > > /**
> > > * @DPLL_ID_ICL_MGPLL4: ICL MG PLL 1 port 4 (F)
> > > * TGL TC PLL 1 port 4 (TC4)
> > > */
> > > - DPLL_ID_ICL_MGPLL4 = 6,
> > > + DPLL_ID_ICL_MGPLL4,
> > > /**
> > > * @DPLL_ID_TGL_MGPLL5: TGL TC PLL port 5 (TC5)
> > > */
> > > - DPLL_ID_TGL_MGPLL5 = 7,
> > > + DPLL_ID_TGL_MGPLL5,
> > > /**
> > > * @DPLL_ID_TGL_MGPLL6: TGL TC PLL port 6 (TC6)
> > > */
> > > - DPLL_ID_TGL_MGPLL6 = 8,
> > > + DPLL_ID_TGL_MGPLL6,
> > > };
> > >
> > > #define I915_NUM_PLLS 9
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 45bda5819abd..34f8698ac3aa 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -242,7 +242,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> > > #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> > > #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> > > #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
> > > -#define _MMIO_PLL3(pll, a, b, c) _MMIO(_PICK(pll, a, b, c))
> >
> > See my series adding DPLL support for DG1. We will need it again for a
> > different reason, with a slightly different form. I'd let this here
> > to avoid removing and adding it back.
> >
> > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
> With the renumbering of TBT to 3, we now have a "hole" in the DPLLs
> exposed on TGL (0, 1, 3) which WARNs:
>
> <4>[ 6.165705] i915 0000:00:02.0: drm_WARN_ON(i != dpll_info[i].id)
> <4>[ 6.166050] WARNING: CPU: 7 PID: 335 at drivers/gpu/drm/i915/display/intel_dpll_mgr.c:4360 intel_shared_dpll_init+0xa6/0x1d0 [i915]
>
> I remember having a hole was a problem back when we just passed min/max
> DPLLs for initialization, but I think it should be safe now ever since:
>
> commit 2a86972f60fcfaa0daa02b9fe461935ea2063791
> Author: Matt Roper <matthew.d.roper@intel.com>
> Date: Tue Oct 8 10:29:20 2019 -0700
>
> drm/i915: Select DPLL's via mask
Actually it turns out there are still some assumptions that DPLLs have
contiguous ID's and the dpll array index matches the ID. I'll need to
make some other changes to allow us to break those assumptions; I'll
work on that some more tomorrow when I get free time.
Matt
>
> so I'll send another version that drops that WARN and keeps the
> _MMIO_PLL3 definition here.
>
> Thanks.
>
>
> Matt
>
> >
> > thanks
> > Lucas De Marchi
> >
> > >
> > > /*
> > > * Device info offset array based helpers for groups of registers with unevenly
> > > @@ -10427,19 +10426,13 @@ enum skl_power_gate {
> > >
> > > #define _TGL_DPLL0_CFGCR0 0x164284
> > > #define _TGL_DPLL1_CFGCR0 0x16428C
> > > -/* TODO: add DPLL4 */
> > > -#define _TGL_TBTPLL_CFGCR0 0x16429C
> > > -#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR0, \
> > > - _TGL_DPLL1_CFGCR0, \
> > > - _TGL_TBTPLL_CFGCR0)
> > > +#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR0, \
> > > + _TGL_DPLL1_CFGCR0)
> > >
> > > #define _TGL_DPLL0_CFGCR1 0x164288
> > > #define _TGL_DPLL1_CFGCR1 0x164290
> > > -/* TODO: add DPLL4 */
> > > -#define _TGL_TBTPLL_CFGCR1 0x1642A0
> > > -#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
> > > - _TGL_DPLL1_CFGCR1, \
> > > - _TGL_TBTPLL_CFGCR1)
> > > +#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR1, \
> > > + _TGL_DPLL1_CFGCR1)
> > >
> > > #define _DKL_PHY1_BASE 0x168000
> > > #define _DKL_PHY2_BASE 0x169000
> > > --
> > > 2.24.1
> > >
>
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 2/5] drm/i915/rkl: Add DPLL4 support
2020-06-18 5:57 ` Matt Roper
@ 2020-06-18 6:38 ` Lucas De Marchi
0 siblings, 0 replies; 19+ messages in thread
From: Lucas De Marchi @ 2020-06-18 6:38 UTC (permalink / raw)
To: Matt Roper; +Cc: Intel Graphics, Lucas De Marchi
On Wed, Jun 17, 2020 at 10:57 PM Matt Roper <matthew.d.roper@intel.com> wrote:
>
> On Wed, Jun 17, 2020 at 01:00:38PM -0700, Matt Roper wrote:
> > On Wed, Jun 17, 2020 at 11:22:58AM -0700, Lucas De Marchi wrote:
> > > On Tue, Jun 16, 2020 at 08:30:57PM -0700, Matt Roper wrote:
> > > > Rocket Lake has a third DPLL (called 'DPLL4') that must be used to
> > > > enable a third display. Unlike EHL's variant of DPLL4, the RKL variant
> > > > behaves the same as DPLL0/1. And despite its name, the DPLL4 registers
> > > > are offset as if it were DPLL2.
> > > >
> > > > To allow the TGL register selectors like TGL_DPLL_CFGCR0 to be used
> > > > seamlessly on all gen12 platforms, we set the non-MG PLL ID's to match
> > > > how the registers are laid out: DPLL0, DPLL1, DPLL4 (RKL-only), TBT.
> > > > This means just renumbering TBT to be ID '3' rather than being another
> > > > ID '2' like DPLL4. With this change, we can build our register
> > > > selectors with _MMIO_PLL rather than _MMIO_PLL3 since the register
> > > > offsets are evenly-spaced. MGPLL's don't need any specific ID's
> > > > (they're just used to translate back to a tc_port), so we let them float
> > > > at the top of the enum.
> > > >
> > > > v2:
> > > > - Add new .update_ref_clks() hook.
> > > >
> > > > v3:
> > > > - Renumber TBT PLL to '3' and switch _MMIO_PLL3 to _MMIO_PLL (Lucas)
> > > >
> > > > Bspec: 49202
> > > > Bspec: 49443
> > > > Bspec: 50288
> > > > Bspec: 50289
> > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++++++++++++++++--
> > > > drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 14 ++++-----
> > > > drivers/gpu/drm/i915/i915_reg.h | 15 +++-------
> > > > 3 files changed, 37 insertions(+), 21 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > > index b45185b80bec..b5f4d4cef682 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > > @@ -3506,13 +3506,19 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
> > > > return false;
> > > > }
> > > >
> > > > - if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A)
> > > > + if (IS_ROCKETLAKE(dev_priv)) {
> > > > dpll_mask =
> > > > BIT(DPLL_ID_EHL_DPLL4) |
> > > > BIT(DPLL_ID_ICL_DPLL1) |
> > > > BIT(DPLL_ID_ICL_DPLL0);
> > > > - else
> > > > + } else if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) {
> > > > + dpll_mask =
> > > > + BIT(DPLL_ID_EHL_DPLL4) |
> > > > + BIT(DPLL_ID_ICL_DPLL1) |
> > > > + BIT(DPLL_ID_ICL_DPLL0);
> > > > + } else {
> > > > dpll_mask = BIT(DPLL_ID_ICL_DPLL1) | BIT(DPLL_ID_ICL_DPLL0);
> > > > + }
> > > >
> > > > port_dpll->pll = intel_find_shared_dpll(state, crtc,
> > > > &port_dpll->hw_state,
> > > > @@ -4275,6 +4281,21 @@ static const struct intel_dpll_mgr tgl_pll_mgr = {
> > > > .dump_hw_state = icl_dump_hw_state,
> > > > };
> > > >
> > > > +static const struct dpll_info rkl_plls[] = {
> > > > + { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> > > > + { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> > > > + { "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
> > > > + { },
> > > > +};
> > > > +
> > > > +static const struct intel_dpll_mgr rkl_pll_mgr = {
> > > > + .dpll_info = rkl_plls,
> > > > + .get_dplls = icl_get_dplls,
> > > > + .put_dplls = icl_put_dplls,
> > > > + .update_ref_clks = icl_update_dpll_ref_clks,
> > > > + .dump_hw_state = icl_dump_hw_state,
> > > > +};
> > > > +
> > > > /**
> > > > * intel_shared_dpll_init - Initialize shared DPLLs
> > > > * @dev: drm device
> > > > @@ -4288,7 +4309,9 @@ void intel_shared_dpll_init(struct drm_device *dev)
> > > > const struct dpll_info *dpll_info;
> > > > int i;
> > > >
> > > > - if (INTEL_GEN(dev_priv) >= 12)
> > > > + if (IS_ROCKETLAKE(dev_priv))
> > > > + dpll_mgr = &rkl_pll_mgr;
> > > > + else if (INTEL_GEN(dev_priv) >= 12)
> > > > dpll_mgr = &tgl_pll_mgr;
> > > > else if (IS_ELKHARTLAKE(dev_priv))
> > > > dpll_mgr = &ehl_pll_mgr;
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > > > index 5d9a2bc371e7..49367847bfb5 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > > > @@ -125,35 +125,35 @@ enum intel_dpll_id {
> > > > /**
> > > > * @DPLL_ID_ICL_TBTPLL: ICL/TGL TBT PLL
> > > > */
> > > > - DPLL_ID_ICL_TBTPLL = 2,
> > > > + DPLL_ID_ICL_TBTPLL = 3,
> > > > /**
> > > > * @DPLL_ID_ICL_MGPLL1: ICL MG PLL 1 port 1 (C),
> > > > * TGL TC PLL 1 port 1 (TC1)
> > > > */
> > > > - DPLL_ID_ICL_MGPLL1 = 3,
> > > > + DPLL_ID_ICL_MGPLL1,
> > > > /**
> > > > * @DPLL_ID_ICL_MGPLL2: ICL MG PLL 1 port 2 (D)
> > > > * TGL TC PLL 1 port 2 (TC2)
> > > > */
> > > > - DPLL_ID_ICL_MGPLL2 = 4,
> > > > + DPLL_ID_ICL_MGPLL2,
> > > > /**
> > > > * @DPLL_ID_ICL_MGPLL3: ICL MG PLL 1 port 3 (E)
> > > > * TGL TC PLL 1 port 3 (TC3)
> > > > */
> > > > - DPLL_ID_ICL_MGPLL3 = 5,
> > > > + DPLL_ID_ICL_MGPLL3,
> > > > /**
> > > > * @DPLL_ID_ICL_MGPLL4: ICL MG PLL 1 port 4 (F)
> > > > * TGL TC PLL 1 port 4 (TC4)
> > > > */
> > > > - DPLL_ID_ICL_MGPLL4 = 6,
> > > > + DPLL_ID_ICL_MGPLL4,
> > > > /**
> > > > * @DPLL_ID_TGL_MGPLL5: TGL TC PLL port 5 (TC5)
> > > > */
> > > > - DPLL_ID_TGL_MGPLL5 = 7,
> > > > + DPLL_ID_TGL_MGPLL5,
> > > > /**
> > > > * @DPLL_ID_TGL_MGPLL6: TGL TC PLL port 6 (TC6)
> > > > */
> > > > - DPLL_ID_TGL_MGPLL6 = 8,
> > > > + DPLL_ID_TGL_MGPLL6,
> > > > };
> > > >
> > > > #define I915_NUM_PLLS 9
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > > index 45bda5819abd..34f8698ac3aa 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -242,7 +242,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> > > > #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> > > > #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> > > > #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
> > > > -#define _MMIO_PLL3(pll, a, b, c) _MMIO(_PICK(pll, a, b, c))
> > >
> > > See my series adding DPLL support for DG1. We will need it again for a
> > > different reason, with a slightly different form. I'd let this here
> > > to avoid removing and adding it back.
> > >
> > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >
> > With the renumbering of TBT to 3, we now have a "hole" in the DPLLs
> > exposed on TGL (0, 1, 3) which WARNs:
> >
> > <4>[ 6.165705] i915 0000:00:02.0: drm_WARN_ON(i != dpll_info[i].id)
> > <4>[ 6.166050] WARNING: CPU: 7 PID: 335 at drivers/gpu/drm/i915/display/intel_dpll_mgr.c:4360 intel_shared_dpll_init+0xa6/0x1d0 [i915]
> >
> > I remember having a hole was a problem back when we just passed min/max
> > DPLLs for initialization, but I think it should be safe now ever since:
> >
> > commit 2a86972f60fcfaa0daa02b9fe461935ea2063791
> > Author: Matt Roper <matthew.d.roper@intel.com>
> > Date: Tue Oct 8 10:29:20 2019 -0700
> >
> > drm/i915: Select DPLL's via mask
>
> Actually it turns out there are still some assumptions that DPLLs have
> contiguous ID's and the dpll array index matches the ID. I'll need to
> make some other changes to allow us to break those assumptions; I'll
> work on that some more tomorrow when I get free time.
I think we had removed that long ago...? Maybe we missed some patch? I
think that was the intention of
https://patchwork.freedesktop.org/series/55378/#rev1
So.... either that or remove the hole. The new IDs could be used by
TGL and RKL and leave ICL/EHL alone.
For DG1 we are defining new IDs for clarity nonetheless.
Lucas De Marchi
>
>
>
> Matt
>
> >
> > so I'll send another version that drops that WARN and keeps the
> > _MMIO_PLL3 definition here.
> >
> > Thanks.
> >
> >
> > Matt
> >
> > >
> > > thanks
> > > Lucas De Marchi
> > >
> > > >
> > > > /*
> > > > * Device info offset array based helpers for groups of registers with unevenly
> > > > @@ -10427,19 +10426,13 @@ enum skl_power_gate {
> > > >
> > > > #define _TGL_DPLL0_CFGCR0 0x164284
> > > > #define _TGL_DPLL1_CFGCR0 0x16428C
> > > > -/* TODO: add DPLL4 */
> > > > -#define _TGL_TBTPLL_CFGCR0 0x16429C
> > > > -#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR0, \
> > > > - _TGL_DPLL1_CFGCR0, \
> > > > - _TGL_TBTPLL_CFGCR0)
> > > > +#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR0, \
> > > > + _TGL_DPLL1_CFGCR0)
> > > >
> > > > #define _TGL_DPLL0_CFGCR1 0x164288
> > > > #define _TGL_DPLL1_CFGCR1 0x164290
> > > > -/* TODO: add DPLL4 */
> > > > -#define _TGL_TBTPLL_CFGCR1 0x1642A0
> > > > -#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
> > > > - _TGL_DPLL1_CFGCR1, \
> > > > - _TGL_TBTPLL_CFGCR1)
> > > > +#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR1, \
> > > > + _TGL_DPLL1_CFGCR1)
> > > >
> > > > #define _DKL_PHY1_BASE 0x168000
> > > > #define _DKL_PHY2_BASE 0x169000
> > > > --
> > > > 2.24.1
> > > >
> >
> > --
> > Matt Roper
> > Graphics Software Engineer
> > VTT-OSGC Platform Enablement
> > Intel Corporation
> > (916) 356-2795
>
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH v7 3/5] drm/i915/rkl: Handle HTI
2020-06-17 3:30 [Intel-gfx] [PATCH v7 0/5] Remaining RKL patches Matt Roper
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 1/5] drm/i915/rkl: Handle new DPCLKA_CFGCR0 layout Matt Roper
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 2/5] drm/i915/rkl: Add DPLL4 support Matt Roper
@ 2020-06-17 3:30 ` Matt Roper
2020-07-08 0:39 ` Souza, Jose
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 4/5] drm/i915/rkl: Add initial workarounds Matt Roper
` (3 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Matt Roper @ 2020-06-17 3:30 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
If HTI (also sometimes called HDPORT) is enabled at startup, it may be
using some of the PHYs and DPLLs making them unavailable for general
usage. Let's read out the HDPORT_STATE register and avoid making use of
resources that HTI is already using.
v2:
- Fix minor checkpatch warnings
Bspec: 49189
Bspec: 53707
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 30 ++++++++++++++++---
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 21 +++++++++++++
drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 1 +
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/i915_reg.h | 6 ++++
5 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6c2bb3354b86..f16512eddc58 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -46,6 +46,7 @@
#include "display/intel_ddi.h"
#include "display/intel_dp.h"
#include "display/intel_dp_mst.h"
+#include "display/intel_dpll_mgr.h"
#include "display/intel_dsi.h"
#include "display/intel_dvo.h"
#include "display/intel_gmbus.h"
@@ -16814,6 +16815,13 @@ static void intel_pps_init(struct drm_i915_private *dev_priv)
intel_pps_unlock_regs_wa(dev_priv);
}
+static bool hti_uses_phy(u32 hdport_state, enum phy phy)
+{
+ return hdport_state & HDPORT_ENABLED &&
+ (hdport_state & HDPORT_PHY_USED_DP(phy) ||
+ hdport_state & HDPORT_PHY_USED_HDMI(phy));
+}
+
static void intel_setup_outputs(struct drm_i915_private *dev_priv)
{
struct intel_encoder *encoder;
@@ -16825,10 +16833,22 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
return;
if (IS_ROCKETLAKE(dev_priv)) {
- intel_ddi_init(dev_priv, PORT_A);
- intel_ddi_init(dev_priv, PORT_B);
- intel_ddi_init(dev_priv, PORT_D); /* DDI TC1 */
- intel_ddi_init(dev_priv, PORT_E); /* DDI TC2 */
+ /*
+ * If HTI (aka HDPORT) is enabled at boot, it may have taken
+ * over some of the PHYs and made them unavailable to the
+ * driver. In that case we should skip initializing the
+ * corresponding outputs.
+ */
+ u32 hdport_state = intel_de_read(dev_priv, HDPORT_STATE);
+
+ if (!hti_uses_phy(hdport_state, PHY_A))
+ intel_ddi_init(dev_priv, PORT_A);
+ if (!hti_uses_phy(hdport_state, PHY_B))
+ intel_ddi_init(dev_priv, PORT_B);
+ if (!hti_uses_phy(hdport_state, PHY_C))
+ intel_ddi_init(dev_priv, PORT_D); /* DDI TC1 */
+ if (!hti_uses_phy(hdport_state, PHY_D))
+ intel_ddi_init(dev_priv, PORT_E); /* DDI TC2 */
} else if (INTEL_GEN(dev_priv) >= 12) {
intel_ddi_init(dev_priv, PORT_A);
intel_ddi_init(dev_priv, PORT_B);
@@ -18376,6 +18396,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
intel_dpll_readout_hw_state(dev_priv);
+ dev_priv->hti_pll_mask = intel_get_hti_plls(dev_priv);
+
for_each_intel_encoder(dev, encoder) {
pipe = 0;
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index b5f4d4cef682..6f59f9ec453b 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -265,6 +265,24 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
mutex_unlock(&dev_priv->dpll.lock);
}
+/*
+ * HTI (aka HDPORT) may be using some of the platform's PLL's, making them
+ * unavailable for use.
+ */
+u32 intel_get_hti_plls(struct drm_i915_private *dev_priv)
+{
+ u32 hdport_state;
+
+ if (!IS_ROCKETLAKE(dev_priv))
+ return 0;
+
+ hdport_state = intel_de_read(dev_priv, HDPORT_STATE);
+ if (!(hdport_state & HDPORT_ENABLED))
+ return 0;
+
+ return REG_FIELD_GET(HDPORT_DPLL_USED_MASK, hdport_state);
+}
+
static struct intel_shared_dpll *
intel_find_shared_dpll(struct intel_atomic_state *state,
const struct intel_crtc *crtc,
@@ -280,6 +298,9 @@ intel_find_shared_dpll(struct intel_atomic_state *state,
drm_WARN_ON(&dev_priv->drm, dpll_mask & ~(BIT(I915_NUM_PLLS) - 1));
+ /* Eliminate DPLLs from consideration if reserved by HTI */
+ dpll_mask &= ~dev_priv->hti_pll_mask;
+
for_each_set_bit(i, &dpll_mask, I915_NUM_PLLS) {
pll = &dev_priv->dpll.shared_dplls[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
index 49367847bfb5..edcc43f4670f 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
@@ -390,6 +390,7 @@ void intel_shared_dpll_swap_state(struct intel_atomic_state *state);
void intel_shared_dpll_init(struct drm_device *dev);
void intel_dpll_readout_hw_state(struct drm_i915_private *dev_priv);
void intel_dpll_sanitize_state(struct drm_i915_private *dev_priv);
+u32 intel_get_hti_plls(struct drm_i915_private *dev_priv);
void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv,
const struct intel_dpll_hw_state *hw_state);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5649f8e502fe..b836032fa0de 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1037,6 +1037,9 @@ struct drm_i915_private {
struct intel_l3_parity l3_parity;
+ /* Mask of PLLs reserved for use by HTI and unavailable to driver. */
+ u32 hti_pll_mask;
+
/*
* edram size in MB.
* Cannot be determined by PCIID. You must always read a register.
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 34f8698ac3aa..34b2ec04ccd8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2908,6 +2908,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define MBUS_BBOX_CTL_S1 _MMIO(0x45040)
#define MBUS_BBOX_CTL_S2 _MMIO(0x45044)
+#define HDPORT_STATE _MMIO(0x45050)
+#define HDPORT_DPLL_USED_MASK REG_GENMASK(14, 12)
+#define HDPORT_PHY_USED_DP(phy) REG_BIT(2 * (phy) + 2)
+#define HDPORT_PHY_USED_HDMI(phy) REG_BIT(2 * (phy) + 1)
+#define HDPORT_ENABLED REG_BIT(0)
+
/* Make render/texture TLB fetches lower priorty than associated data
* fetches. This is not turned on by default
*/
--
2.24.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 3/5] drm/i915/rkl: Handle HTI
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 3/5] drm/i915/rkl: Handle HTI Matt Roper
@ 2020-07-08 0:39 ` Souza, Jose
2020-07-15 23:13 ` Matt Roper
0 siblings, 1 reply; 19+ messages in thread
From: Souza, Jose @ 2020-07-08 0:39 UTC (permalink / raw)
To: Roper, Matthew D, intel-gfx; +Cc: De Marchi, Lucas
On Tue, 2020-06-16 at 20:30 -0700, Matt Roper wrote:
> If HTI (also sometimes called HDPORT) is enabled at startup, it may be
> using some of the PHYs and DPLLs making them unavailable for general
> usage. Let's read out the HDPORT_STATE register and avoid making use of
> resources that HTI is already using.
>
> v2:
> - Fix minor checkpatch warnings
>
> Bspec: 49189
> Bspec: 53707
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 30 ++++++++++++++++---
> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 21 +++++++++++++
> drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 3 ++
> drivers/gpu/drm/i915/i915_reg.h | 6 ++++
> 5 files changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6c2bb3354b86..f16512eddc58 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -46,6 +46,7 @@
> #include "display/intel_ddi.h"
> #include "display/intel_dp.h"
> #include "display/intel_dp_mst.h"
> +#include "display/intel_dpll_mgr.h"
> #include "display/intel_dsi.h"
> #include "display/intel_dvo.h"
> #include "display/intel_gmbus.h"
> @@ -16814,6 +16815,13 @@ static void intel_pps_init(struct drm_i915_private *dev_priv)
> intel_pps_unlock_regs_wa(dev_priv);
> }
>
> +static bool hti_uses_phy(u32 hdport_state, enum phy phy)
> +{
> + return hdport_state & HDPORT_ENABLED &&
> + (hdport_state & HDPORT_PHY_USED_DP(phy) ||
> + hdport_state & HDPORT_PHY_USED_HDMI(phy));
> +}
> +
> static void intel_setup_outputs(struct drm_i915_private *dev_priv)
> {
> struct intel_encoder *encoder;
> @@ -16825,10 +16833,22 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
> return;
>
> if (IS_ROCKETLAKE(dev_priv)) {
> - intel_ddi_init(dev_priv, PORT_A);
> - intel_ddi_init(dev_priv, PORT_B);
> - intel_ddi_init(dev_priv, PORT_D); /* DDI TC1 */
> - intel_ddi_init(dev_priv, PORT_E); /* DDI TC2 */
> + /*
> + * If HTI (aka HDPORT) is enabled at boot, it may have taken
> + * over some of the PHYs and made them unavailable to the
> + * driver. In that case we should skip initializing the
> + * corresponding outputs.
> + */
> + u32 hdport_state = intel_de_read(dev_priv, HDPORT_STATE);
> +
> + if (!hti_uses_phy(hdport_state, PHY_A))
> + intel_ddi_init(dev_priv, PORT_A);
> + if (!hti_uses_phy(hdport_state, PHY_B))
> + intel_ddi_init(dev_priv, PORT_B);
> + if (!hti_uses_phy(hdport_state, PHY_C))
> + intel_ddi_init(dev_priv, PORT_D); /* DDI TC1 */
> + if (!hti_uses_phy(hdport_state, PHY_D))
> + intel_ddi_init(dev_priv, PORT_E); /* DDI TC2 */
> } else if (INTEL_GEN(dev_priv) >= 12) {
> intel_ddi_init(dev_priv, PORT_A);
> intel_ddi_init(dev_priv, PORT_B);
> @@ -18376,6 +18396,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>
> intel_dpll_readout_hw_state(dev_priv);
>
> + dev_priv->hti_pll_mask = intel_get_hti_plls(dev_priv);
Why not do this in intel_shared_dpll_init()?
> +
> for_each_intel_encoder(dev, encoder) {
> pipe = 0;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index b5f4d4cef682..6f59f9ec453b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -265,6 +265,24 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
> mutex_unlock(&dev_priv->dpll.lock);
> }
>
> +/*
> + * HTI (aka HDPORT) may be using some of the platform's PLL's, making them
> + * unavailable for use.
> + */
> +u32 intel_get_hti_plls(struct drm_i915_private *dev_priv)
No need to export this function, check above.
> +{
> + u32 hdport_state;
> +
> + if (!IS_ROCKETLAKE(dev_priv))
> + return 0;
> +
> + hdport_state = intel_de_read(dev_priv, HDPORT_STATE);
> + if (!(hdport_state & HDPORT_ENABLED))
> + return 0;
> +
> + return REG_FIELD_GET(HDPORT_DPLL_USED_MASK, hdport_state);
> +}
> +
> static struct intel_shared_dpll *
> intel_find_shared_dpll(struct intel_atomic_state *state,
> const struct intel_crtc *crtc,
> @@ -280,6 +298,9 @@ intel_find_shared_dpll(struct intel_atomic_state *state,
>
> drm_WARN_ON(&dev_priv->drm, dpll_mask & ~(BIT(I915_NUM_PLLS) - 1));
>
> + /* Eliminate DPLLs from consideration if reserved by HTI */
> + dpll_mask &= ~dev_priv->hti_pll_mask;
This should be done in icl_get_combo_phy_dpll() for RKL only.
> +
> for_each_set_bit(i, &dpll_mask, I915_NUM_PLLS) {
> pll = &dev_priv->dpll.shared_dplls[i];
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> index 49367847bfb5..edcc43f4670f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> @@ -390,6 +390,7 @@ void intel_shared_dpll_swap_state(struct intel_atomic_state *state);
> void intel_shared_dpll_init(struct drm_device *dev);
> void intel_dpll_readout_hw_state(struct drm_i915_private *dev_priv);
> void intel_dpll_sanitize_state(struct drm_i915_private *dev_priv);
> +u32 intel_get_hti_plls(struct drm_i915_private *dev_priv);
>
> void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv,
> const struct intel_dpll_hw_state *hw_state);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5649f8e502fe..b836032fa0de 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1037,6 +1037,9 @@ struct drm_i915_private {
>
> struct intel_l3_parity l3_parity;
>
> + /* Mask of PLLs reserved for use by HTI and unavailable to driver. */
> + u32 hti_pll_mask;
> +
> /*
> * edram size in MB.
> * Cannot be determined by PCIID. You must always read a register.
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 34f8698ac3aa..34b2ec04ccd8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2908,6 +2908,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define MBUS_BBOX_CTL_S1 _MMIO(0x45040)
> #define MBUS_BBOX_CTL_S2 _MMIO(0x45044)
>
> +#define HDPORT_STATE _MMIO(0x45050)
> +#define HDPORT_DPLL_USED_MASK REG_GENMASK(14, 12)
> +#define HDPORT_PHY_USED_DP(phy) REG_BIT(2 * (phy) + 2)
> +#define HDPORT_PHY_USED_HDMI(phy) REG_BIT(2 * (phy) + 1)
> +#define HDPORT_ENABLED REG_BIT(0)
> +
> /* Make render/texture TLB fetches lower priorty than associated data
> * fetches. This is not turned on by default
> */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 3/5] drm/i915/rkl: Handle HTI
2020-07-08 0:39 ` Souza, Jose
@ 2020-07-15 23:13 ` Matt Roper
0 siblings, 0 replies; 19+ messages in thread
From: Matt Roper @ 2020-07-15 23:13 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx, De Marchi, Lucas
On Tue, Jul 07, 2020 at 05:39:14PM -0700, Souza, Jose wrote:
> On Tue, 2020-06-16 at 20:30 -0700, Matt Roper wrote:
> > If HTI (also sometimes called HDPORT) is enabled at startup, it may be
> > using some of the PHYs and DPLLs making them unavailable for general
> > usage. Let's read out the HDPORT_STATE register and avoid making use of
> > resources that HTI is already using.
> >
> > v2:
> > - Fix minor checkpatch warnings
> >
> > Bspec: 49189
> > Bspec: 53707
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 30 ++++++++++++++++---
> > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 21 +++++++++++++
> > drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 1 +
> > drivers/gpu/drm/i915/i915_drv.h | 3 ++
> > drivers/gpu/drm/i915/i915_reg.h | 6 ++++
> > 5 files changed, 57 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 6c2bb3354b86..f16512eddc58 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -46,6 +46,7 @@
> > #include "display/intel_ddi.h"
> > #include "display/intel_dp.h"
> > #include "display/intel_dp_mst.h"
> > +#include "display/intel_dpll_mgr.h"
> > #include "display/intel_dsi.h"
> > #include "display/intel_dvo.h"
> > #include "display/intel_gmbus.h"
> > @@ -16814,6 +16815,13 @@ static void intel_pps_init(struct drm_i915_private *dev_priv)
> > intel_pps_unlock_regs_wa(dev_priv);
> > }
> >
> > +static bool hti_uses_phy(u32 hdport_state, enum phy phy)
> > +{
> > + return hdport_state & HDPORT_ENABLED &&
> > + (hdport_state & HDPORT_PHY_USED_DP(phy) ||
> > + hdport_state & HDPORT_PHY_USED_HDMI(phy));
> > +}
> > +
> > static void intel_setup_outputs(struct drm_i915_private *dev_priv)
> > {
> > struct intel_encoder *encoder;
> > @@ -16825,10 +16833,22 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
> > return;
> >
> > if (IS_ROCKETLAKE(dev_priv)) {
> > - intel_ddi_init(dev_priv, PORT_A);
> > - intel_ddi_init(dev_priv, PORT_B);
> > - intel_ddi_init(dev_priv, PORT_D); /* DDI TC1 */
> > - intel_ddi_init(dev_priv, PORT_E); /* DDI TC2 */
> > + /*
> > + * If HTI (aka HDPORT) is enabled at boot, it may have taken
> > + * over some of the PHYs and made them unavailable to the
> > + * driver. In that case we should skip initializing the
> > + * corresponding outputs.
> > + */
> > + u32 hdport_state = intel_de_read(dev_priv, HDPORT_STATE);
> > +
> > + if (!hti_uses_phy(hdport_state, PHY_A))
> > + intel_ddi_init(dev_priv, PORT_A);
> > + if (!hti_uses_phy(hdport_state, PHY_B))
> > + intel_ddi_init(dev_priv, PORT_B);
> > + if (!hti_uses_phy(hdport_state, PHY_C))
> > + intel_ddi_init(dev_priv, PORT_D); /* DDI TC1 */
> > + if (!hti_uses_phy(hdport_state, PHY_D))
> > + intel_ddi_init(dev_priv, PORT_E); /* DDI TC2 */
> > } else if (INTEL_GEN(dev_priv) >= 12) {
> > intel_ddi_init(dev_priv, PORT_A);
> > intel_ddi_init(dev_priv, PORT_B);
> > @@ -18376,6 +18396,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >
> > intel_dpll_readout_hw_state(dev_priv);
> >
> > + dev_priv->hti_pll_mask = intel_get_hti_plls(dev_priv);
>
> Why not do this in intel_shared_dpll_init()?
I think I had it that way initially, but that failed because we don't
have the runtime PM references so the hardware is powered down. I could
grab the necessary references and wake up the hardware, but it probably
makes more sense to read this out at the same time we're reading out
everything else from the registers.
But it doesn't really make sense to read the register twice (once here
and then again in intel_setup_outputs). I'll just readout the whole
register once and have it parsed in the two spots in the next version.
Matt
>
> > +
> > for_each_intel_encoder(dev, encoder) {
> > pipe = 0;
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index b5f4d4cef682..6f59f9ec453b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -265,6 +265,24 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
> > mutex_unlock(&dev_priv->dpll.lock);
> > }
> >
> > +/*
> > + * HTI (aka HDPORT) may be using some of the platform's PLL's, making them
> > + * unavailable for use.
> > + */
> > +u32 intel_get_hti_plls(struct drm_i915_private *dev_priv)
>
> No need to export this function, check above.
>
> > +{
> > + u32 hdport_state;
> > +
> > + if (!IS_ROCKETLAKE(dev_priv))
> > + return 0;
> > +
> > + hdport_state = intel_de_read(dev_priv, HDPORT_STATE);
> > + if (!(hdport_state & HDPORT_ENABLED))
> > + return 0;
> > +
> > + return REG_FIELD_GET(HDPORT_DPLL_USED_MASK, hdport_state);
> > +}
> > +
> > static struct intel_shared_dpll *
> > intel_find_shared_dpll(struct intel_atomic_state *state,
> > const struct intel_crtc *crtc,
> > @@ -280,6 +298,9 @@ intel_find_shared_dpll(struct intel_atomic_state *state,
> >
> > drm_WARN_ON(&dev_priv->drm, dpll_mask & ~(BIT(I915_NUM_PLLS) - 1));
> >
> > + /* Eliminate DPLLs from consideration if reserved by HTI */
> > + dpll_mask &= ~dev_priv->hti_pll_mask;
>
> This should be done in icl_get_combo_phy_dpll() for RKL only.
>
> > +
> > for_each_set_bit(i, &dpll_mask, I915_NUM_PLLS) {
> > pll = &dev_priv->dpll.shared_dplls[i];
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > index 49367847bfb5..edcc43f4670f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > @@ -390,6 +390,7 @@ void intel_shared_dpll_swap_state(struct intel_atomic_state *state);
> > void intel_shared_dpll_init(struct drm_device *dev);
> > void intel_dpll_readout_hw_state(struct drm_i915_private *dev_priv);
> > void intel_dpll_sanitize_state(struct drm_i915_private *dev_priv);
> > +u32 intel_get_hti_plls(struct drm_i915_private *dev_priv);
> >
> > void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv,
> > const struct intel_dpll_hw_state *hw_state);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 5649f8e502fe..b836032fa0de 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1037,6 +1037,9 @@ struct drm_i915_private {
> >
> > struct intel_l3_parity l3_parity;
> >
> > + /* Mask of PLLs reserved for use by HTI and unavailable to driver. */
> > + u32 hti_pll_mask;
> > +
> > /*
> > * edram size in MB.
> > * Cannot be determined by PCIID. You must always read a register.
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 34f8698ac3aa..34b2ec04ccd8 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2908,6 +2908,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> > #define MBUS_BBOX_CTL_S1 _MMIO(0x45040)
> > #define MBUS_BBOX_CTL_S2 _MMIO(0x45044)
> >
> > +#define HDPORT_STATE _MMIO(0x45050)
> > +#define HDPORT_DPLL_USED_MASK REG_GENMASK(14, 12)
> > +#define HDPORT_PHY_USED_DP(phy) REG_BIT(2 * (phy) + 2)
> > +#define HDPORT_PHY_USED_HDMI(phy) REG_BIT(2 * (phy) + 1)
> > +#define HDPORT_ENABLED REG_BIT(0)
> > +
> > /* Make render/texture TLB fetches lower priorty than associated data
> > * fetches. This is not turned on by default
> > */
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH v7 4/5] drm/i915/rkl: Add initial workarounds
2020-06-17 3:30 [Intel-gfx] [PATCH v7 0/5] Remaining RKL patches Matt Roper
` (2 preceding siblings ...)
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 3/5] drm/i915/rkl: Handle HTI Matt Roper
@ 2020-06-17 3:30 ` Matt Roper
2020-07-08 0:41 ` Souza, Jose
2020-06-17 3:31 ` [Intel-gfx] [PATCH v7 5/5] drm/i915/rkl: Add Wa_14011224835 for PHY B initialization Matt Roper
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Matt Roper @ 2020-06-17 3:30 UTC (permalink / raw)
To: intel-gfx
RKL and TGL share some general gen12 workarounds, but each platform also
has its own platform-specific workarounds.
v2:
- Add Wa_1604555607 for RKL. This makes RKL's ctx WA list identical to
TGL's, so we'll have both functions call the tgl_ function for now;
this workaround isn't listed for DG1 so we don't want to add it to
the general gen12_ function.
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_sprite.c | 5 +-
drivers/gpu/drm/i915/gt/intel_workarounds.c | 88 +++++++++++++--------
2 files changed, 59 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 3cd461bf9131..63ac79f88fa2 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -2842,8 +2842,9 @@ static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
static bool gen12_plane_supports_mc_ccs(struct drm_i915_private *dev_priv,
enum plane_id plane_id)
{
- /* Wa_14010477008:tgl[a0..c0] */
- if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_C0))
+ /* Wa_14010477008:tgl[a0..c0],rkl[all] */
+ if (IS_ROCKETLAKE(dev_priv) ||
+ IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_C0))
return false;
return plane_id < PLANE_SPRITE4;
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 2da366821dda..741710ca2b9a 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -596,8 +596,8 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine,
wa_masked_en(wal, GEN9_ROW_CHICKEN4, GEN11_DIS_PICK_2ND_EU);
}
-static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
- struct i915_wa_list *wal)
+static void gen12_ctx_workarounds_init(struct intel_engine_cs *engine,
+ struct i915_wa_list *wal)
{
/*
* Wa_1409142259:tgl
@@ -607,12 +607,28 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
* Wa_1409207793:tgl
* Wa_1409178076:tgl
* Wa_1408979724:tgl
+ * Wa_14010443199:rkl
+ * Wa_14010698770:rkl
*/
WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3,
GEN12_DISABLE_CPS_AWARE_COLOR_PIPE);
+ /* WaDisableGPGPUMidThreadPreemption:gen12 */
+ WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1,
+ GEN9_PREEMPT_GPGPU_LEVEL_MASK,
+ GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL);
+}
+
+static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
+ struct i915_wa_list *wal)
+{
+ gen12_ctx_workarounds_init(engine, wal);
+
/*
- * Wa_1604555607:gen12 and Wa_1608008084:gen12
+ * Wa_1604555607:tgl,rkl
+ *
+ * Note that the implementation of this workaround is further modified
+ * according to the FF_MODE2 guidance given by Wa_1608008084:gen12.
* FF_MODE2 register will return the wrong value when read. The default
* value for this register is zero for all fields and there are no bit
* masks. So instead of doing a RMW we should just write the GS Timer
@@ -623,11 +639,6 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
FF_MODE2_GS_TIMER_MASK | FF_MODE2_TDS_TIMER_MASK,
FF_MODE2_GS_TIMER_224 | FF_MODE2_TDS_TIMER_128,
0);
-
- /* WaDisableGPGPUMidThreadPreemption:tgl */
- WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1,
- GEN9_PREEMPT_GPGPU_LEVEL_MASK,
- GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL);
}
static void
@@ -642,8 +653,10 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
wa_init_start(wal, name, engine->name);
- if (IS_GEN(i915, 12))
+ if (IS_ROCKETLAKE(i915) || IS_TIGERLAKE(i915))
tgl_ctx_workarounds_init(engine, wal);
+ else if (IS_GEN(i915, 12))
+ gen12_ctx_workarounds_init(engine, wal);
else if (IS_GEN(i915, 11))
icl_ctx_workarounds_init(engine, wal);
else if (IS_CANNONLAKE(i915))
@@ -1176,9 +1189,16 @@ icl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
}
static void
-tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
+gen12_gt_workarounds_init(struct drm_i915_private *i915,
+ struct i915_wa_list *wal)
{
wa_init_mcr(i915, wal);
+}
+
+static void
+tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
+{
+ gen12_gt_workarounds_init(i915, wal);
/* Wa_1409420604:tgl */
if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
@@ -1196,8 +1216,10 @@ tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
static void
gt_init_workarounds(struct drm_i915_private *i915, struct i915_wa_list *wal)
{
- if (IS_GEN(i915, 12))
+ if (IS_TIGERLAKE(i915))
tgl_gt_workarounds_init(i915, wal);
+ else if (IS_GEN(i915, 12))
+ gen12_gt_workarounds_init(i915, wal);
else if (IS_GEN(i915, 11))
icl_gt_workarounds_init(i915, wal);
else if (IS_CANNONLAKE(i915))
@@ -1629,18 +1651,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
GEN9_CTX_PREEMPT_REG,
GEN12_DISABLE_POSH_BUSY_FF_DOP_CG);
- /*
- * Wa_1607030317:tgl
- * Wa_1607186500:tgl
- * Wa_1607297627:tgl there is 3 entries for this WA on BSpec, 2
- * of then says it is fixed on B0 the other one says it is
- * permanent
- */
- wa_masked_en(wal,
- GEN6_RC_SLEEP_PSMI_CONTROL,
- GEN12_WAIT_FOR_EVENT_POWER_DOWN_DISABLE |
- GEN8_RC_SEMA_IDLE_MSG_DISABLE);
-
/*
* Wa_1606679103:tgl
* (see also Wa_1606682166:icl)
@@ -1659,24 +1669,38 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
VSUNIT_CLKGATE_DIS_TGL);
}
- if (IS_TIGERLAKE(i915)) {
- /* Wa_1606931601:tgl */
+ if (IS_ROCKETLAKE(i915) || IS_TIGERLAKE(i915)) {
+ /* Wa_1606931601:tgl,rkl */
wa_masked_en(wal, GEN7_ROW_CHICKEN2, GEN12_DISABLE_EARLY_READ);
- /* Wa_1409804808:tgl */
+ /* Wa_1409804808:tgl,rkl */
wa_masked_en(wal, GEN7_ROW_CHICKEN2,
GEN12_PUSH_CONST_DEREF_HOLD_DIS);
- /* Wa_1606700617:tgl */
- wa_masked_en(wal,
- GEN9_CS_DEBUG_MODE1,
- FF_DOP_CLOCK_GATE_DISABLE);
-
/*
* Wa_1409085225:tgl
- * Wa_14010229206:tgl
+ * Wa_14010229206:tgl,rkl
*/
wa_masked_en(wal, GEN9_ROW_CHICKEN4, GEN12_DISABLE_TDL_PUSH);
+
+ /*
+ * Wa_1607030317:tgl
+ * Wa_1607186500:tgl
+ * Wa_1607297627:tgl,rkl there are multiple entries for this
+ * WA in the BSpec; some indicate this is an A0-only WA,
+ * others indicate it applies to all steppings.
+ */
+ wa_masked_en(wal,
+ GEN6_RC_SLEEP_PSMI_CONTROL,
+ GEN12_WAIT_FOR_EVENT_POWER_DOWN_DISABLE |
+ GEN8_RC_SEMA_IDLE_MSG_DISABLE);
+ }
+
+ if (IS_TIGERLAKE(i915)) {
+ /* Wa_1606700617:tgl */
+ wa_masked_en(wal,
+ GEN9_CS_DEBUG_MODE1,
+ FF_DOP_CLOCK_GATE_DISABLE);
}
if (IS_GEN(i915, 11)) {
--
2.24.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 4/5] drm/i915/rkl: Add initial workarounds
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 4/5] drm/i915/rkl: Add initial workarounds Matt Roper
@ 2020-07-08 0:41 ` Souza, Jose
0 siblings, 0 replies; 19+ messages in thread
From: Souza, Jose @ 2020-07-08 0:41 UTC (permalink / raw)
To: Roper, Matthew D, intel-gfx
On Tue, 2020-06-16 at 20:30 -0700, Matt Roper wrote:
> RKL and TGL share some general gen12 workarounds, but each platform also
> has its own platform-specific workarounds.
>
> v2:
> - Add Wa_1604555607 for RKL. This makes RKL's ctx WA list identical to
> TGL's, so we'll have both functions call the tgl_ function for now;
> this workaround isn't listed for DG1 so we don't want to add it to
> the general gen12_ function.
>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_sprite.c | 5 +-
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 88 +++++++++++++--------
> 2 files changed, 59 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 3cd461bf9131..63ac79f88fa2 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -2842,8 +2842,9 @@ static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
> static bool gen12_plane_supports_mc_ccs(struct drm_i915_private *dev_priv,
> enum plane_id plane_id)
> {
> - /* Wa_14010477008:tgl[a0..c0] */
> - if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_C0))
> + /* Wa_14010477008:tgl[a0..c0],rkl[all] */
> + if (IS_ROCKETLAKE(dev_priv) ||
> + IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_C0))
> return false;
>
> return plane_id < PLANE_SPRITE4;
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 2da366821dda..741710ca2b9a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -596,8 +596,8 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine,
> wa_masked_en(wal, GEN9_ROW_CHICKEN4, GEN11_DIS_PICK_2ND_EU);
> }
>
> -static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
> - struct i915_wa_list *wal)
> +static void gen12_ctx_workarounds_init(struct intel_engine_cs *engine,
> + struct i915_wa_list *wal)
> {
> /*
> * Wa_1409142259:tgl
> @@ -607,12 +607,28 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
> * Wa_1409207793:tgl
> * Wa_1409178076:tgl
> * Wa_1408979724:tgl
> + * Wa_14010443199:rkl
> + * Wa_14010698770:rkl
> */
> WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3,
> GEN12_DISABLE_CPS_AWARE_COLOR_PIPE);
>
> + /* WaDisableGPGPUMidThreadPreemption:gen12 */
> + WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1,
> + GEN9_PREEMPT_GPGPU_LEVEL_MASK,
> + GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL);
> +}
> +
> +static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
> + struct i915_wa_list *wal)
> +{
> + gen12_ctx_workarounds_init(engine, wal);
> +
> /*
> - * Wa_1604555607:gen12 and Wa_1608008084:gen12
> + * Wa_1604555607:tgl,rkl
> + *
> + * Note that the implementation of this workaround is further modified
> + * according to the FF_MODE2 guidance given by Wa_1608008084:gen12.
> * FF_MODE2 register will return the wrong value when read. The default
> * value for this register is zero for all fields and there are no bit
> * masks. So instead of doing a RMW we should just write the GS Timer
> @@ -623,11 +639,6 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
> FF_MODE2_GS_TIMER_MASK | FF_MODE2_TDS_TIMER_MASK,
> FF_MODE2_GS_TIMER_224 | FF_MODE2_TDS_TIMER_128,
> 0);
> -
> - /* WaDisableGPGPUMidThreadPreemption:tgl */
> - WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1,
> - GEN9_PREEMPT_GPGPU_LEVEL_MASK,
> - GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL);
> }
>
> static void
> @@ -642,8 +653,10 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
>
> wa_init_start(wal, name, engine->name);
>
> - if (IS_GEN(i915, 12))
> + if (IS_ROCKETLAKE(i915) || IS_TIGERLAKE(i915))
> tgl_ctx_workarounds_init(engine, wal);
> + else if (IS_GEN(i915, 12))
> + gen12_ctx_workarounds_init(engine, wal);
> else if (IS_GEN(i915, 11))
> icl_ctx_workarounds_init(engine, wal);
> else if (IS_CANNONLAKE(i915))
> @@ -1176,9 +1189,16 @@ icl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
> }
>
> static void
> -tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
> +gen12_gt_workarounds_init(struct drm_i915_private *i915,
> + struct i915_wa_list *wal)
> {
> wa_init_mcr(i915, wal);
> +}
> +
> +static void
> +tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
> +{
> + gen12_gt_workarounds_init(i915, wal);
>
> /* Wa_1409420604:tgl */
> if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> @@ -1196,8 +1216,10 @@ tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
> static void
> gt_init_workarounds(struct drm_i915_private *i915, struct i915_wa_list *wal)
> {
> - if (IS_GEN(i915, 12))
> + if (IS_TIGERLAKE(i915))
> tgl_gt_workarounds_init(i915, wal);
> + else if (IS_GEN(i915, 12))
> + gen12_gt_workarounds_init(i915, wal);
> else if (IS_GEN(i915, 11))
> icl_gt_workarounds_init(i915, wal);
> else if (IS_CANNONLAKE(i915))
> @@ -1629,18 +1651,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> GEN9_CTX_PREEMPT_REG,
> GEN12_DISABLE_POSH_BUSY_FF_DOP_CG);
>
> - /*
> - * Wa_1607030317:tgl
> - * Wa_1607186500:tgl
> - * Wa_1607297627:tgl there is 3 entries for this WA on BSpec, 2
> - * of then says it is fixed on B0 the other one says it is
> - * permanent
> - */
> - wa_masked_en(wal,
> - GEN6_RC_SLEEP_PSMI_CONTROL,
> - GEN12_WAIT_FOR_EVENT_POWER_DOWN_DISABLE |
> - GEN8_RC_SEMA_IDLE_MSG_DISABLE);
> -
> /*
> * Wa_1606679103:tgl
> * (see also Wa_1606682166:icl)
> @@ -1659,24 +1669,38 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> VSUNIT_CLKGATE_DIS_TGL);
> }
>
> - if (IS_TIGERLAKE(i915)) {
> - /* Wa_1606931601:tgl */
> + if (IS_ROCKETLAKE(i915) || IS_TIGERLAKE(i915)) {
> + /* Wa_1606931601:tgl,rkl */
> wa_masked_en(wal, GEN7_ROW_CHICKEN2, GEN12_DISABLE_EARLY_READ);
>
> - /* Wa_1409804808:tgl */
> + /* Wa_1409804808:tgl,rkl */
> wa_masked_en(wal, GEN7_ROW_CHICKEN2,
> GEN12_PUSH_CONST_DEREF_HOLD_DIS);
>
> - /* Wa_1606700617:tgl */
> - wa_masked_en(wal,
> - GEN9_CS_DEBUG_MODE1,
> - FF_DOP_CLOCK_GATE_DISABLE);
> -
> /*
> * Wa_1409085225:tgl
> - * Wa_14010229206:tgl
> + * Wa_14010229206:tgl,rkl
> */
> wa_masked_en(wal, GEN9_ROW_CHICKEN4, GEN12_DISABLE_TDL_PUSH);
> +
> + /*
> + * Wa_1607030317:tgl
> + * Wa_1607186500:tgl
> + * Wa_1607297627:tgl,rkl there are multiple entries for this
> + * WA in the BSpec; some indicate this is an A0-only WA,
> + * others indicate it applies to all steppings.
> + */
> + wa_masked_en(wal,
> + GEN6_RC_SLEEP_PSMI_CONTROL,
> + GEN12_WAIT_FOR_EVENT_POWER_DOWN_DISABLE |
> + GEN8_RC_SEMA_IDLE_MSG_DISABLE);
> + }
> +
> + if (IS_TIGERLAKE(i915)) {
> + /* Wa_1606700617:tgl */
> + wa_masked_en(wal,
> + GEN9_CS_DEBUG_MODE1,
> + FF_DOP_CLOCK_GATE_DISABLE);
RKL might need this one too under the number 14010230801 but is still pending.
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> }
>
> if (IS_GEN(i915, 11)) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH v7 5/5] drm/i915/rkl: Add Wa_14011224835 for PHY B initialization
2020-06-17 3:30 [Intel-gfx] [PATCH v7 0/5] Remaining RKL patches Matt Roper
` (3 preceding siblings ...)
2020-06-17 3:30 ` [Intel-gfx] [PATCH v7 4/5] drm/i915/rkl: Add initial workarounds Matt Roper
@ 2020-06-17 3:31 ` Matt Roper
2020-07-08 0:42 ` Souza, Jose
2020-07-08 0:43 ` Souza, Jose
2020-06-17 4:30 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for Remaining RKL patches (rev6) Patchwork
2020-06-17 9:14 ` [Intel-gfx] ✓ Fi.CI.IGT: success " Patchwork
6 siblings, 2 replies; 19+ messages in thread
From: Matt Roper @ 2020-06-17 3:31 UTC (permalink / raw)
To: intel-gfx
After doing normal PHY-B initialization on Rocket Lake, we need to
manually copy some additional PHY-A register values into PHY-B
registers.
Note that the bspec's combo phy page doesn't specify that this
workaround is restricted to specific platform steppings (and doesn't
even do a very good job of specifying that RKL is the only platform this
is needed on), but the RKL workaround page lists this as relevant only
for A and B steppings, so I'm trusting that information for now.
v2: Make rkl_combo_phy_b_init_wa() static
Bspec: 49291
Bspec: 53273
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
.../gpu/drm/i915/display/intel_combo_phy.c | 26 +++++++++++++++++++
drivers/gpu/drm/i915/i915_reg.h | 13 +++++++++-
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c b/drivers/gpu/drm/i915/display/intel_combo_phy.c
index 77b04bb3ec62..d5d95e2746c2 100644
--- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
@@ -338,6 +338,27 @@ void intel_combo_phy_power_up_lanes(struct drm_i915_private *dev_priv,
intel_de_write(dev_priv, ICL_PORT_CL_DW10(phy), val);
}
+static void rkl_combo_phy_b_init_wa(struct drm_i915_private *i915)
+{
+ u32 grccode, grccode_ldo;
+ u32 iref_rcal_ord, rcompcode_ld_cap_ov;
+
+ intel_de_wait_for_register(i915, ICL_PORT_COMP_DW3(PHY_A),
+ FIRST_COMP_DONE, FIRST_COMP_DONE, 100);
+
+ grccode = REG_FIELD_GET(GRCCODE,
+ intel_de_read(i915, ICL_PORT_COMP_DW6(PHY_A)));
+ iref_rcal_ord = REG_FIELD_PREP(IREF_RCAL_ORD, grccode);
+ intel_de_rmw(i915, ICL_PORT_COMP_DW2(PHY_B), IREF_RCAL_ORD,
+ iref_rcal_ord | IREF_RCAL_ORD_EN);
+
+ grccode_ldo = REG_FIELD_GET(GRCCODE_LDO,
+ intel_de_read(i915, ICL_PORT_COMP_DW0(PHY_A)));
+ rcompcode_ld_cap_ov = REG_FIELD_PREP(RCOMPCODE_LD_CAP_OV, grccode_ldo);
+ intel_de_rmw(i915, ICL_PORT_COMP_DW6(PHY_B), RCOMPCODE_LD_CAP_OV,
+ rcompcode_ld_cap_ov | RCOMPCODEOVEN_LDO_SYNC);
+}
+
static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
{
enum phy phy;
@@ -390,6 +411,11 @@ static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
val = intel_de_read(dev_priv, ICL_PORT_CL_DW5(phy));
val |= CL_POWER_DOWN_ENABLE;
intel_de_write(dev_priv, ICL_PORT_CL_DW5(phy), val);
+
+ if (IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_B0) &&
+ phy == PHY_B)
+ /* Wa_14011224835:rkl[a0..c0] */
+ rkl_combo_phy_b_init_wa(dev_priv);
}
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 34b2ec04ccd8..10f6e46523b6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1908,11 +1908,16 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define CNL_PORT_COMP_DW0 _MMIO(0x162100)
#define ICL_PORT_COMP_DW0(phy) _MMIO(_ICL_PORT_COMP_DW(0, phy))
-#define COMP_INIT (1 << 31)
+#define COMP_INIT REG_BIT(31)
+#define GRCCODE_LDO REG_GENMASK(7, 0)
#define CNL_PORT_COMP_DW1 _MMIO(0x162104)
#define ICL_PORT_COMP_DW1(phy) _MMIO(_ICL_PORT_COMP_DW(1, phy))
+#define ICL_PORT_COMP_DW2(phy) _MMIO(_ICL_PORT_COMP_DW(2, phy))
+#define IREF_RCAL_ORD_EN REG_BIT(7)
+#define IREF_RCAL_ORD REG_GENMASK(6, 0)
+
#define CNL_PORT_COMP_DW3 _MMIO(0x16210c)
#define ICL_PORT_COMP_DW3(phy) _MMIO(_ICL_PORT_COMP_DW(3, phy))
#define PROCESS_INFO_DOT_0 (0 << 26)
@@ -1925,6 +1930,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define VOLTAGE_INFO_1_05V (2 << 24)
#define VOLTAGE_INFO_MASK (3 << 24)
#define VOLTAGE_INFO_SHIFT 24
+#define FIRST_COMP_DONE REG_BIT(22)
+
+#define ICL_PORT_COMP_DW6(phy) _MMIO(_ICL_PORT_COMP_DW(6, phy))
+#define GRCCODE REG_GENMASK(30, 24)
+#define RCOMPCODEOVEN_LDO_SYNC REG_BIT(23)
+#define RCOMPCODE_LD_CAP_OV REG_GENMASK(22, 16)
#define ICL_PORT_COMP_DW8(phy) _MMIO(_ICL_PORT_COMP_DW(8, phy))
#define IREFGEN (1 << 24)
--
2.24.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 5/5] drm/i915/rkl: Add Wa_14011224835 for PHY B initialization
2020-06-17 3:31 ` [Intel-gfx] [PATCH v7 5/5] drm/i915/rkl: Add Wa_14011224835 for PHY B initialization Matt Roper
@ 2020-07-08 0:42 ` Souza, Jose
2020-07-08 0:43 ` Souza, Jose
1 sibling, 0 replies; 19+ messages in thread
From: Souza, Jose @ 2020-07-08 0:42 UTC (permalink / raw)
To: Roper, Matthew D, intel-gfx
On Tue, 2020-06-16 at 20:31 -0700, Matt Roper wrote:
> After doing normal PHY-B initialization on Rocket Lake, we need to
> manually copy some additional PHY-A register values into PHY-B
> registers.
>
> Note that the bspec's combo phy page doesn't specify that this
> workaround is restricted to specific platform steppings (and doesn't
> even do a very good job of specifying that RKL is the only platform this
> is needed on), but the RKL workaround page lists this as relevant only
> for A and B steppings, so I'm trusting that information for now.
>
> v2: Make rkl_combo_phy_b_init_wa() static
>
> Bspec: 49291
> Bspec: 53273
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> .../gpu/drm/i915/display/intel_combo_phy.c | 26 +++++++++++++++++++
> drivers/gpu/drm/i915/i915_reg.h | 13 +++++++++-
> 2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> index 77b04bb3ec62..d5d95e2746c2 100644
> --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> @@ -338,6 +338,27 @@ void intel_combo_phy_power_up_lanes(struct drm_i915_private *dev_priv,
> intel_de_write(dev_priv, ICL_PORT_CL_DW10(phy), val);
> }
>
> +static void rkl_combo_phy_b_init_wa(struct drm_i915_private *i915)
> +{
> + u32 grccode, grccode_ldo;
> + u32 iref_rcal_ord, rcompcode_ld_cap_ov;
> +
> + intel_de_wait_for_register(i915, ICL_PORT_COMP_DW3(PHY_A),
> + FIRST_COMP_DONE, FIRST_COMP_DONE, 100);
> +
> + grccode = REG_FIELD_GET(GRCCODE,
> + intel_de_read(i915, ICL_PORT_COMP_DW6(PHY_A)));
> + iref_rcal_ord = REG_FIELD_PREP(IREF_RCAL_ORD, grccode);
> + intel_de_rmw(i915, ICL_PORT_COMP_DW2(PHY_B), IREF_RCAL_ORD,
> + iref_rcal_ord | IREF_RCAL_ORD_EN);
> +
> + grccode_ldo = REG_FIELD_GET(GRCCODE_LDO,
> + intel_de_read(i915, ICL_PORT_COMP_DW0(PHY_A)));
> + rcompcode_ld_cap_ov = REG_FIELD_PREP(RCOMPCODE_LD_CAP_OV, grccode_ldo);
> + intel_de_rmw(i915, ICL_PORT_COMP_DW6(PHY_B), RCOMPCODE_LD_CAP_OV,
> + rcompcode_ld_cap_ov | RCOMPCODEOVEN_LDO_SYNC);
> +}
> +
> static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> {
> enum phy phy;
> @@ -390,6 +411,11 @@ static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> val = intel_de_read(dev_priv, ICL_PORT_CL_DW5(phy));
> val |= CL_POWER_DOWN_ENABLE;
> intel_de_write(dev_priv, ICL_PORT_CL_DW5(phy), val);
> +
> + if (IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_B0) &&
> + phy == PHY_B)
> + /* Wa_14011224835:rkl[a0..c0] */
> + rkl_combo_phy_b_init_wa(dev_priv);
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 34b2ec04ccd8..10f6e46523b6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1908,11 +1908,16 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define CNL_PORT_COMP_DW0 _MMIO(0x162100)
> #define ICL_PORT_COMP_DW0(phy) _MMIO(_ICL_PORT_COMP_DW(0, phy))
> -#define COMP_INIT (1 << 31)
> +#define COMP_INIT REG_BIT(31)
> +#define GRCCODE_LDO REG_GENMASK(7, 0)
>
> #define CNL_PORT_COMP_DW1 _MMIO(0x162104)
> #define ICL_PORT_COMP_DW1(phy) _MMIO(_ICL_PORT_COMP_DW(1, phy))
>
> +#define ICL_PORT_COMP_DW2(phy) _MMIO(_ICL_PORT_COMP_DW(2, phy))
> +#define IREF_RCAL_ORD_EN REG_BIT(7)
> +#define IREF_RCAL_ORD REG_GENMASK(6, 0)
> +
> #define CNL_PORT_COMP_DW3 _MMIO(0x16210c)
> #define ICL_PORT_COMP_DW3(phy) _MMIO(_ICL_PORT_COMP_DW(3, phy))
> #define PROCESS_INFO_DOT_0 (0 << 26)
> @@ -1925,6 +1930,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define VOLTAGE_INFO_1_05V (2 << 24)
> #define VOLTAGE_INFO_MASK (3 << 24)
> #define VOLTAGE_INFO_SHIFT 24
> +#define FIRST_COMP_DONE REG_BIT(22)
> +
> +#define ICL_PORT_COMP_DW6(phy) _MMIO(_ICL_PORT_COMP_DW(6, phy))
> +#define GRCCODE REG_GENMASK(30, 24)
> +#define RCOMPCODEOVEN_LDO_SYNC REG_BIT(23)
> +#define RCOMPCODE_LD_CAP_OV REG_GENMASK(22, 16)
>
> #define ICL_PORT_COMP_DW8(phy) _MMIO(_ICL_PORT_COMP_DW(8, phy))
> #define IREFGEN (1 << 24)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Intel-gfx] [PATCH v7 5/5] drm/i915/rkl: Add Wa_14011224835 for PHY B initialization
2020-06-17 3:31 ` [Intel-gfx] [PATCH v7 5/5] drm/i915/rkl: Add Wa_14011224835 for PHY B initialization Matt Roper
2020-07-08 0:42 ` Souza, Jose
@ 2020-07-08 0:43 ` Souza, Jose
1 sibling, 0 replies; 19+ messages in thread
From: Souza, Jose @ 2020-07-08 0:43 UTC (permalink / raw)
To: Roper, Matthew D, intel-gfx
On Tue, 2020-06-16 at 20:31 -0700, Matt Roper wrote:
> After doing normal PHY-B initialization on Rocket Lake, we need to
> manually copy some additional PHY-A register values into PHY-B
> registers.
>
> Note that the bspec's combo phy page doesn't specify that this
> workaround is restricted to specific platform steppings (and doesn't
> even do a very good job of specifying that RKL is the only platform this
> is needed on), but the RKL workaround page lists this as relevant only
> for A and B steppings, so I'm trusting that information for now.
>
> v2: Make rkl_combo_phy_b_init_wa() static
>
> Bspec: 49291
> Bspec: 53273
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> .../gpu/drm/i915/display/intel_combo_phy.c | 26 +++++++++++++++++++
> drivers/gpu/drm/i915/i915_reg.h | 13 +++++++++-
> 2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> index 77b04bb3ec62..d5d95e2746c2 100644
> --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> @@ -338,6 +338,27 @@ void intel_combo_phy_power_up_lanes(struct drm_i915_private *dev_priv,
> intel_de_write(dev_priv, ICL_PORT_CL_DW10(phy), val);
> }
>
> +static void rkl_combo_phy_b_init_wa(struct drm_i915_private *i915)
> +{
> + u32 grccode, grccode_ldo;
> + u32 iref_rcal_ord, rcompcode_ld_cap_ov;
Nitpick: you could do all the bellow with just 2 u32(val and grccode).
> +
> + intel_de_wait_for_register(i915, ICL_PORT_COMP_DW3(PHY_A),
> + FIRST_COMP_DONE, FIRST_COMP_DONE, 100);
The timeout parameter here is in ms not usec
> +
> + grccode = REG_FIELD_GET(GRCCODE,
> + intel_de_read(i915, ICL_PORT_COMP_DW6(PHY_A)));
> + iref_rcal_ord = REG_FIELD_PREP(IREF_RCAL_ORD, grccode);
> + intel_de_rmw(i915, ICL_PORT_COMP_DW2(PHY_B), IREF_RCAL_ORD,
> + iref_rcal_ord | IREF_RCAL_ORD_EN);
> +
> + grccode_ldo = REG_FIELD_GET(GRCCODE_LDO,
> + intel_de_read(i915, ICL_PORT_COMP_DW0(PHY_A)));
> + rcompcode_ld_cap_ov = REG_FIELD_PREP(RCOMPCODE_LD_CAP_OV, grccode_ldo);
> + intel_de_rmw(i915, ICL_PORT_COMP_DW6(PHY_B), RCOMPCODE_LD_CAP_OV,
> + rcompcode_ld_cap_ov | RCOMPCODEOVEN_LDO_SYNC);
> +}
> +
> static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> {
> enum phy phy;
> @@ -390,6 +411,11 @@ static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> val = intel_de_read(dev_priv, ICL_PORT_CL_DW5(phy));
> val |= CL_POWER_DOWN_ENABLE;
> intel_de_write(dev_priv, ICL_PORT_CL_DW5(phy), val);
> +
> + if (IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_B0) &&
> + phy == PHY_B)
> + /* Wa_14011224835:rkl[a0..c0] */
> + rkl_combo_phy_b_init_wa(dev_priv);
Missing the icl_combo_phy_verify_state() counter part.
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 34b2ec04ccd8..10f6e46523b6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1908,11 +1908,16 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define CNL_PORT_COMP_DW0 _MMIO(0x162100)
> #define ICL_PORT_COMP_DW0(phy) _MMIO(_ICL_PORT_COMP_DW(0, phy))
> -#define COMP_INIT (1 << 31)
> +#define COMP_INIT REG_BIT(31)
> +#define GRCCODE_LDO REG_GENMASK(7, 0)
>
> #define CNL_PORT_COMP_DW1 _MMIO(0x162104)
> #define ICL_PORT_COMP_DW1(phy) _MMIO(_ICL_PORT_COMP_DW(1, phy))
>
> +#define ICL_PORT_COMP_DW2(phy) _MMIO(_ICL_PORT_COMP_DW(2, phy))
> +#define IREF_RCAL_ORD_EN REG_BIT(7)
> +#define IREF_RCAL_ORD REG_GENMASK(6, 0)
> +
> #define CNL_PORT_COMP_DW3 _MMIO(0x16210c)
> #define ICL_PORT_COMP_DW3(phy) _MMIO(_ICL_PORT_COMP_DW(3, phy))
> #define PROCESS_INFO_DOT_0 (0 << 26)
> @@ -1925,6 +1930,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define VOLTAGE_INFO_1_05V (2 << 24)
> #define VOLTAGE_INFO_MASK (3 << 24)
> #define VOLTAGE_INFO_SHIFT 24
> +#define FIRST_COMP_DONE REG_BIT(22)
> +
> +#define ICL_PORT_COMP_DW6(phy) _MMIO(_ICL_PORT_COMP_DW(6, phy))
> +#define GRCCODE REG_GENMASK(30, 24)
> +#define RCOMPCODEOVEN_LDO_SYNC REG_BIT(23)
> +#define RCOMPCODE_LD_CAP_OV REG_GENMASK(22, 16)
>
Register definition matches.
> #define ICL_PORT_COMP_DW8(phy) _MMIO(_ICL_PORT_COMP_DW(8, phy))
> #define IREFGEN (1 << 24)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for Remaining RKL patches (rev6)
2020-06-17 3:30 [Intel-gfx] [PATCH v7 0/5] Remaining RKL patches Matt Roper
` (4 preceding siblings ...)
2020-06-17 3:31 ` [Intel-gfx] [PATCH v7 5/5] drm/i915/rkl: Add Wa_14011224835 for PHY B initialization Matt Roper
@ 2020-06-17 4:30 ` Patchwork
2020-06-17 9:14 ` [Intel-gfx] ✓ Fi.CI.IGT: success " Patchwork
6 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2020-06-17 4:30 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Series Details ==
Series: Remaining RKL patches (rev6)
URL : https://patchwork.freedesktop.org/series/77971/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_8638 -> Patchwork_17975
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_17975 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_17975, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17975/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_17975:
### IGT changes ###
#### Possible regressions ####
* igt@runner@aborted:
- fi-tgl-u2: NOTRUN -> [FAIL][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17975/fi-tgl-u2/igt@runner@aborted.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@runner@aborted:
- {fi-tgl-dsi}: NOTRUN -> [FAIL][2]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17975/fi-tgl-dsi/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_17975 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_suspend@basic-s0:
- fi-apl-guc: [PASS][3] -> [INCOMPLETE][4] ([i915#1242])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8638/fi-apl-guc/igt@gem_exec_suspend@basic-s0.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17975/fi-apl-guc/igt@gem_exec_suspend@basic-s0.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#1242]: https://gitlab.freedesktop.org/drm/intel/issues/1242
[i915#1569]: https://gitlab.freedesktop.org/drm/intel/issues/1569
[i915#192]: https://gitlab.freedesktop.org/drm/intel/issues/192
[i915#193]: https://gitlab.freedesktop.org/drm/intel/issues/193
[i915#194]: https://gitlab.freedesktop.org/drm/intel/issues/194
[i915#2029]: https://gitlab.freedesktop.org/drm/intel/issues/2029
Participating hosts (47 -> 10)
------------------------------
ERROR: It appears as if the changes made in Patchwork_17975 prevented too many machines from booting.
Missing (37): fi-icl-u2 fi-skl-lmem fi-blb-e6850 fi-byt-n2820 fi-skl-6600u fi-snb-2600 fi-cml-u2 fi-bxt-dsi fi-bdw-5557u fi-cml-s fi-bsw-n3050 fi-byt-j1900 fi-glk-dsi fi-bwr-2160 fi-ilk-650 fi-kbl-7500u fi-hsw-4770 fi-gdg-551 fi-ivb-3770 fi-elk-e7500 fi-bsw-nick fi-skl-6700k2 fi-kbl-r fi-ilk-m540 fi-ehl-1 fi-skl-guc fi-cfl-8700k fi-byt-squawks fi-bsw-cyan fi-cfl-guc fi-kbl-guc fi-whl-u fi-kbl-x1275 fi-cfl-8109u fi-bsw-kefka fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_8638 -> Patchwork_17975
CI-20190529: 20190529
CI_DRM_8638: 83818e4910cac8b84d8f915c773ab3f55fa30365 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5711: 90611a0c90afa4a46496c78a4faf9638a1538ac3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_17975: a03fa5993eeac9c31bbf8ab3b6c79ac7f4b6e4d7 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
a03fa5993eea drm/i915/rkl: Add Wa_14011224835 for PHY B initialization
0b4911dd9a47 drm/i915/rkl: Add initial workarounds
1f13cd3654fb drm/i915/rkl: Handle HTI
c8d0aab47fb8 drm/i915/rkl: Add DPLL4 support
bf41347d51b5 drm/i915/rkl: Handle new DPCLKA_CFGCR0 layout
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17975/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* [Intel-gfx] ✓ Fi.CI.IGT: success for Remaining RKL patches (rev6)
2020-06-17 3:30 [Intel-gfx] [PATCH v7 0/5] Remaining RKL patches Matt Roper
` (5 preceding siblings ...)
2020-06-17 4:30 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for Remaining RKL patches (rev6) Patchwork
@ 2020-06-17 9:14 ` Patchwork
6 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2020-06-17 9:14 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Series Details ==
Series: Remaining RKL patches (rev6)
URL : https://patchwork.freedesktop.org/series/77971/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_8638_full -> Patchwork_17975_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17975/index.html
Changes
-------
No changes found
Participating hosts (11 -> 8)
------------------------------
Missing (3): pig-skl-6260u pig-glk-j5005 pig-icl-1065g7
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_8638 -> Patchwork_17975
* Piglit: piglit_4509 -> None
CI-20190529: 20190529
CI_DRM_8638: 83818e4910cac8b84d8f915c773ab3f55fa30365 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5711: 90611a0c90afa4a46496c78a4faf9638a1538ac3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_17975: a03fa5993eeac9c31bbf8ab3b6c79ac7f4b6e4d7 @ 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_17975/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread