* [PATCH 0/5] icl: Misc PLL patches
@ 2019-01-17 20:21 Lucas De Marchi
2019-01-17 20:21 ` [PATCH 1/5] drm/i915/icl: use tc_port in MG_PLL macros Lucas De Marchi
` (8 more replies)
0 siblings, 9 replies; 22+ messages in thread
From: Lucas De Marchi @ 2019-01-17 20:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
Some PLL reworks on ICL. Patches are more or less independent of each
other, but touch the same part of the code.
Lucas De Marchi (5):
drm/i915/icl: use tc_port in MG_PLL macros
drm/i915: always return something
drm/i915/icl: remove dpll from clk_sel
drm/i915/icl: keep track of unused pll while looping
drm/i915: allow shared plls to be non-consecutive
drivers/gpu/drm/i915/i915_reg.h | 52 +++++-----
drivers/gpu/drm/i915/intel_ddi.c | 20 ++--
drivers/gpu/drm/i915/intel_display.c | 3 +-
drivers/gpu/drm/i915/intel_dpll_mgr.c | 137 ++++++++++++--------------
drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 +-
5 files changed, 105 insertions(+), 109 deletions(-)
--
2.20.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 1/5] drm/i915/icl: use tc_port in MG_PLL macros 2019-01-17 20:21 [PATCH 0/5] icl: Misc PLL patches Lucas De Marchi @ 2019-01-17 20:21 ` Lucas De Marchi 2019-01-24 1:15 ` Paulo Zanoni 2019-01-17 20:21 ` [PATCH 2/5] drm/i915: always return something Lucas De Marchi ` (7 subsequent siblings) 8 siblings, 1 reply; 22+ messages in thread From: Lucas De Marchi @ 2019-01-17 20:21 UTC (permalink / raw) To: intel-gfx; +Cc: Lucas De Marchi Fix the TODO leftover in the code by changing the argument in MG_PLL macros. The MG_PLL ids used to access the register values can be converted from tc_port rather than port. The helper functions were also renamed to use "tc" as prefix to make them more generic. Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 52 +++++++++--------- drivers/gpu/drm/i915/intel_ddi.c | 7 ++- drivers/gpu/drm/i915/intel_display.c | 3 +- drivers/gpu/drm/i915/intel_dpll_mgr.c | 79 +++++++++++++-------------- drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 +- 5 files changed, 72 insertions(+), 71 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9a1340cfda6c..de209e0fdc01 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9545,7 +9545,7 @@ enum skl_power_gate { #define _MG_PLL3_ENABLE 0x46038 #define _MG_PLL4_ENABLE 0x4603C /* Bits are the same as DPLL0_ENABLE */ -#define MG_PLL_ENABLE(port) _MMIO_PORT((port) - PORT_C, _MG_PLL1_ENABLE, \ +#define MG_PLL_ENABLE(tc_port) _MMIO_PORT((tc_port), _MG_PLL1_ENABLE, \ _MG_PLL2_ENABLE) #define _MG_REFCLKIN_CTL_PORT1 0x16892C @@ -9554,9 +9554,9 @@ enum skl_power_gate { #define _MG_REFCLKIN_CTL_PORT4 0x16B92C #define MG_REFCLKIN_CTL_OD_2_MUX(x) ((x) << 8) #define MG_REFCLKIN_CTL_OD_2_MUX_MASK (0x7 << 8) -#define MG_REFCLKIN_CTL(port) _MMIO_PORT((port) - PORT_C, \ - _MG_REFCLKIN_CTL_PORT1, \ - _MG_REFCLKIN_CTL_PORT2) +#define MG_REFCLKIN_CTL(tc_port) _MMIO_PORT((tc_port), \ + _MG_REFCLKIN_CTL_PORT1, \ + _MG_REFCLKIN_CTL_PORT2) #define _MG_CLKTOP2_CORECLKCTL1_PORT1 0x1688D8 #define _MG_CLKTOP2_CORECLKCTL1_PORT2 0x1698D8 @@ -9566,9 +9566,9 @@ enum skl_power_gate { #define MG_CLKTOP2_CORECLKCTL1_B_DIVRATIO_MASK (0xff << 16) #define MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO(x) ((x) << 8) #define MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK (0xff << 8) -#define MG_CLKTOP2_CORECLKCTL1(port) _MMIO_PORT((port) - PORT_C, \ - _MG_CLKTOP2_CORECLKCTL1_PORT1, \ - _MG_CLKTOP2_CORECLKCTL1_PORT2) +#define MG_CLKTOP2_CORECLKCTL1(tc_port) _MMIO_PORT((tc_port), \ + _MG_CLKTOP2_CORECLKCTL1_PORT1, \ + _MG_CLKTOP2_CORECLKCTL1_PORT2) #define _MG_CLKTOP2_HSCLKCTL_PORT1 0x1688D4 #define _MG_CLKTOP2_HSCLKCTL_PORT2 0x1698D4 @@ -9586,9 +9586,9 @@ enum skl_power_gate { #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO(x) ((x) << 8) #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_SHIFT 8 #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK (0xf << 8) -#define MG_CLKTOP2_HSCLKCTL(port) _MMIO_PORT((port) - PORT_C, \ - _MG_CLKTOP2_HSCLKCTL_PORT1, \ - _MG_CLKTOP2_HSCLKCTL_PORT2) +#define MG_CLKTOP2_HSCLKCTL(tc_port) _MMIO_PORT((tc_port), \ + _MG_CLKTOP2_HSCLKCTL_PORT1, \ + _MG_CLKTOP2_HSCLKCTL_PORT2) #define _MG_PLL_DIV0_PORT1 0x168A00 #define _MG_PLL_DIV0_PORT2 0x169A00 @@ -9600,8 +9600,8 @@ enum skl_power_gate { #define MG_PLL_DIV0_FBDIV_FRAC(x) ((x) << 8) #define MG_PLL_DIV0_FBDIV_INT_MASK (0xff << 0) #define MG_PLL_DIV0_FBDIV_INT(x) ((x) << 0) -#define MG_PLL_DIV0(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_DIV0_PORT1, \ - _MG_PLL_DIV0_PORT2) +#define MG_PLL_DIV0(tc_port) _MMIO_PORT((tc_port), _MG_PLL_DIV0_PORT1, \ + _MG_PLL_DIV0_PORT2) #define _MG_PLL_DIV1_PORT1 0x168A04 #define _MG_PLL_DIV1_PORT2 0x169A04 @@ -9615,8 +9615,8 @@ enum skl_power_gate { #define MG_PLL_DIV1_NDIVRATIO(x) ((x) << 4) #define MG_PLL_DIV1_FBPREDIV_MASK (0xf << 0) #define MG_PLL_DIV1_FBPREDIV(x) ((x) << 0) -#define MG_PLL_DIV1(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_DIV1_PORT1, \ - _MG_PLL_DIV1_PORT2) +#define MG_PLL_DIV1(tc_port) _MMIO_PORT((tc_port), _MG_PLL_DIV1_PORT1, \ + _MG_PLL_DIV1_PORT2) #define _MG_PLL_LF_PORT1 0x168A08 #define _MG_PLL_LF_PORT2 0x169A08 @@ -9628,8 +9628,8 @@ enum skl_power_gate { #define MG_PLL_LF_GAINCTRL(x) ((x) << 16) #define MG_PLL_LF_INT_COEFF(x) ((x) << 8) #define MG_PLL_LF_PROP_COEFF(x) ((x) << 0) -#define MG_PLL_LF(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_LF_PORT1, \ - _MG_PLL_LF_PORT2) +#define MG_PLL_LF(tc_port) _MMIO_PORT((tc_port), _MG_PLL_LF_PORT1, \ + _MG_PLL_LF_PORT2) #define _MG_PLL_FRAC_LOCK_PORT1 0x168A0C #define _MG_PLL_FRAC_LOCK_PORT2 0x169A0C @@ -9641,9 +9641,9 @@ enum skl_power_gate { #define MG_PLL_FRAC_LOCK_DCODITHEREN (1 << 10) #define MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN (1 << 8) #define MG_PLL_FRAC_LOCK_FEEDFWRDGAIN(x) ((x) << 0) -#define MG_PLL_FRAC_LOCK(port) _MMIO_PORT((port) - PORT_C, \ - _MG_PLL_FRAC_LOCK_PORT1, \ - _MG_PLL_FRAC_LOCK_PORT2) +#define MG_PLL_FRAC_LOCK(tc_port) _MMIO_PORT((tc_port), \ + _MG_PLL_FRAC_LOCK_PORT1, \ + _MG_PLL_FRAC_LOCK_PORT2) #define _MG_PLL_SSC_PORT1 0x168A10 #define _MG_PLL_SSC_PORT2 0x169A10 @@ -9655,8 +9655,8 @@ enum skl_power_gate { #define MG_PLL_SSC_STEPNUM(x) ((x) << 10) #define MG_PLL_SSC_FLLEN (1 << 9) #define MG_PLL_SSC_STEPSIZE(x) ((x) << 0) -#define MG_PLL_SSC(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_SSC_PORT1, \ - _MG_PLL_SSC_PORT2) +#define MG_PLL_SSC(tc_port) _MMIO_PORT((tc_port), _MG_PLL_SSC_PORT1, \ + _MG_PLL_SSC_PORT2) #define _MG_PLL_BIAS_PORT1 0x168A14 #define _MG_PLL_BIAS_PORT2 0x169A14 @@ -9675,8 +9675,8 @@ enum skl_power_gate { #define MG_PLL_BIAS_VREF_RDAC_MASK (0x7 << 5) #define MG_PLL_BIAS_IREFTRIM(x) ((x) << 0) #define MG_PLL_BIAS_IREFTRIM_MASK (0x1f << 0) -#define MG_PLL_BIAS(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_BIAS_PORT1, \ - _MG_PLL_BIAS_PORT2) +#define MG_PLL_BIAS(tc_port) _MMIO_PORT((tc_port), _MG_PLL_BIAS_PORT1, \ + _MG_PLL_BIAS_PORT2) #define _MG_PLL_TDC_COLDST_BIAS_PORT1 0x168A18 #define _MG_PLL_TDC_COLDST_BIAS_PORT2 0x169A18 @@ -9687,9 +9687,9 @@ enum skl_power_gate { #define MG_PLL_TDC_COLDST_COLDSTART (1 << 16) #define MG_PLL_TDC_TDCOVCCORR_EN (1 << 2) #define MG_PLL_TDC_TDCSEL(x) ((x) << 0) -#define MG_PLL_TDC_COLDST_BIAS(port) _MMIO_PORT((port) - PORT_C, \ - _MG_PLL_TDC_COLDST_BIAS_PORT1, \ - _MG_PLL_TDC_COLDST_BIAS_PORT2) +#define MG_PLL_TDC_COLDST_BIAS(tc_port) _MMIO_PORT((tc_port), \ + _MG_PLL_TDC_COLDST_BIAS_PORT1, \ + _MG_PLL_TDC_COLDST_BIAS_PORT2) #define _CNL_DPLL0_CFGCR0 0x6C000 #define _CNL_DPLL1_CFGCR0 0x6C080 diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index ce44744a5f9d..8dbf6c9e22fb 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1391,16 +1391,17 @@ static int icl_calc_tbt_pll_link(struct drm_i915_private *dev_priv, static int icl_calc_mg_pll_link(struct drm_i915_private *dev_priv, enum port port) { + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); u32 mg_pll_div0, mg_clktop_hsclkctl; u32 m1, m2_int, m2_frac, div1, div2, refclk; u64 tmp; refclk = dev_priv->cdclk.hw.ref; - mg_pll_div0 = I915_READ(MG_PLL_DIV0(port)); - mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(port)); + mg_pll_div0 = I915_READ(MG_PLL_DIV0(tc_port)); + mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); - m1 = I915_READ(MG_PLL_DIV1(port)) & MG_PLL_DIV1_FBPREDIV_MASK; + m1 = I915_READ(MG_PLL_DIV1(tc_port)) & MG_PLL_DIV1_FBPREDIV_MASK; m2_int = mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK; m2_frac = (mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) ? (mg_pll_div0 & MG_PLL_DIV0_FBDIV_FRAC_MASK) >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 62d61fcad89c..a5de70e6bf59 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9415,7 +9415,8 @@ static void icelake_get_ddi_pll(struct drm_i915_private *dev_priv, if (WARN_ON(!intel_dpll_is_combophy(id))) return; } else if (intel_port_is_tc(dev_priv, port)) { - id = icl_port_to_mg_pll_id(port); + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); + id = icl_tc_port_to_pll_id(tc_port); } else { WARN(1, "Invalid port %x\n", port); return; diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 606f54dde086..211b3ffa5bed 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2639,14 +2639,14 @@ int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv, return link_clock; } -static enum port icl_mg_pll_id_to_port(enum intel_dpll_id id) +static enum tc_port icl_pll_id_to_tc_port(enum intel_dpll_id id) { - return id - DPLL_ID_ICL_MGPLL1 + PORT_C; + return id - DPLL_ID_ICL_MGPLL1; } -enum intel_dpll_id icl_port_to_mg_pll_id(enum port port) +enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port) { - return port - PORT_C + DPLL_ID_ICL_MGPLL1; + return tc_port + DPLL_ID_ICL_MGPLL1; } bool intel_dpll_is_combophy(enum intel_dpll_id id) @@ -2925,7 +2925,10 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, ret = icl_calc_dpll_state(crtc_state, encoder, clock, &pll_state); } else { - min = icl_port_to_mg_pll_id(port); + enum tc_port tc_port; + + tc_port = intel_port_to_tc(dev_priv, port); + min = icl_tc_port_to_pll_id(tc_port); max = min; ret = icl_calc_mg_pll_state(crtc_state, encoder, clock, &pll_state); @@ -2959,12 +2962,8 @@ static i915_reg_t icl_pll_id_to_enable_reg(enum intel_dpll_id id) return CNL_DPLL_ENABLE(id); else if (id == DPLL_ID_ICL_TBTPLL) return TBT_PLL_ENABLE; - else - /* - * TODO: Make MG_PLL macros use - * tc port id instead of port id - */ - return MG_PLL_ENABLE(icl_mg_pll_id_to_port(id)); + + return MG_PLL_ENABLE(icl_pll_id_to_tc_port(id)); } static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, @@ -2974,7 +2973,6 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, const enum intel_dpll_id id = pll->info->id; intel_wakeref_t wakeref; bool ret = false; - enum port port; u32 val; wakeref = intel_display_power_get_if_enabled(dev_priv, @@ -2991,32 +2989,33 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id)); hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id)); } else { - port = icl_mg_pll_id_to_port(id); - hw_state->mg_refclkin_ctl = I915_READ(MG_REFCLKIN_CTL(port)); + enum tc_port tc_port = icl_pll_id_to_tc_port(id); + + hw_state->mg_refclkin_ctl = I915_READ(MG_REFCLKIN_CTL(tc_port)); hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK; hw_state->mg_clktop2_coreclkctl1 = - I915_READ(MG_CLKTOP2_CORECLKCTL1(port)); + I915_READ(MG_CLKTOP2_CORECLKCTL1(tc_port)); hw_state->mg_clktop2_coreclkctl1 &= MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK; hw_state->mg_clktop2_hsclkctl = - I915_READ(MG_CLKTOP2_HSCLKCTL(port)); + I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); hw_state->mg_clktop2_hsclkctl &= MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK | MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK | MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK | MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK; - hw_state->mg_pll_div0 = I915_READ(MG_PLL_DIV0(port)); - hw_state->mg_pll_div1 = I915_READ(MG_PLL_DIV1(port)); - hw_state->mg_pll_lf = I915_READ(MG_PLL_LF(port)); - hw_state->mg_pll_frac_lock = I915_READ(MG_PLL_FRAC_LOCK(port)); - hw_state->mg_pll_ssc = I915_READ(MG_PLL_SSC(port)); + hw_state->mg_pll_div0 = I915_READ(MG_PLL_DIV0(tc_port)); + hw_state->mg_pll_div1 = I915_READ(MG_PLL_DIV1(tc_port)); + hw_state->mg_pll_lf = I915_READ(MG_PLL_LF(tc_port)); + hw_state->mg_pll_frac_lock = I915_READ(MG_PLL_FRAC_LOCK(tc_port)); + hw_state->mg_pll_ssc = I915_READ(MG_PLL_SSC(tc_port)); - hw_state->mg_pll_bias = I915_READ(MG_PLL_BIAS(port)); + hw_state->mg_pll_bias = I915_READ(MG_PLL_BIAS(tc_port)); hw_state->mg_pll_tdc_coldst_bias = - I915_READ(MG_PLL_TDC_COLDST_BIAS(port)); + I915_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); if (dev_priv->cdclk.hw.ref == 38400) { hw_state->mg_pll_tdc_coldst_bias_mask = MG_PLL_TDC_COLDST_COLDSTART; @@ -3051,7 +3050,7 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll) { struct intel_dpll_hw_state *hw_state = &pll->state.hw_state; - enum port port = icl_mg_pll_id_to_port(pll->info->id); + enum tc_port tc_port = icl_pll_id_to_tc_port(pll->info->id); u32 val; /* @@ -3060,41 +3059,41 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv, * during the calc/readout phase if the mask depends on some other HW * state like refclk, see icl_calc_mg_pll_state(). */ - val = I915_READ(MG_REFCLKIN_CTL(port)); + val = I915_READ(MG_REFCLKIN_CTL(tc_port)); val &= ~MG_REFCLKIN_CTL_OD_2_MUX_MASK; val |= hw_state->mg_refclkin_ctl; - I915_WRITE(MG_REFCLKIN_CTL(port), val); + I915_WRITE(MG_REFCLKIN_CTL(tc_port), val); - val = I915_READ(MG_CLKTOP2_CORECLKCTL1(port)); + val = I915_READ(MG_CLKTOP2_CORECLKCTL1(tc_port)); val &= ~MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK; val |= hw_state->mg_clktop2_coreclkctl1; - I915_WRITE(MG_CLKTOP2_CORECLKCTL1(port), val); + I915_WRITE(MG_CLKTOP2_CORECLKCTL1(tc_port), val); - val = I915_READ(MG_CLKTOP2_HSCLKCTL(port)); + val = I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); val &= ~(MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK | MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK | MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK | MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK); val |= hw_state->mg_clktop2_hsclkctl; - I915_WRITE(MG_CLKTOP2_HSCLKCTL(port), val); + I915_WRITE(MG_CLKTOP2_HSCLKCTL(tc_port), val); - I915_WRITE(MG_PLL_DIV0(port), hw_state->mg_pll_div0); - I915_WRITE(MG_PLL_DIV1(port), hw_state->mg_pll_div1); - I915_WRITE(MG_PLL_LF(port), hw_state->mg_pll_lf); - I915_WRITE(MG_PLL_FRAC_LOCK(port), hw_state->mg_pll_frac_lock); - I915_WRITE(MG_PLL_SSC(port), hw_state->mg_pll_ssc); + I915_WRITE(MG_PLL_DIV0(tc_port), hw_state->mg_pll_div0); + I915_WRITE(MG_PLL_DIV1(tc_port), hw_state->mg_pll_div1); + I915_WRITE(MG_PLL_LF(tc_port), hw_state->mg_pll_lf); + I915_WRITE(MG_PLL_FRAC_LOCK(tc_port), hw_state->mg_pll_frac_lock); + I915_WRITE(MG_PLL_SSC(tc_port), hw_state->mg_pll_ssc); - val = I915_READ(MG_PLL_BIAS(port)); + val = I915_READ(MG_PLL_BIAS(tc_port)); val &= ~hw_state->mg_pll_bias_mask; val |= hw_state->mg_pll_bias; - I915_WRITE(MG_PLL_BIAS(port), val); + I915_WRITE(MG_PLL_BIAS(tc_port), val); - val = I915_READ(MG_PLL_TDC_COLDST_BIAS(port)); + val = I915_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); val &= ~hw_state->mg_pll_tdc_coldst_bias_mask; val |= hw_state->mg_pll_tdc_coldst_bias; - I915_WRITE(MG_PLL_TDC_COLDST_BIAS(port), val); + I915_WRITE(MG_PLL_TDC_COLDST_BIAS(tc_port), val); - POSTING_READ(MG_PLL_TDC_COLDST_BIAS(port)); + POSTING_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); } static void icl_pll_enable(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h index e96e79413b54..40e8391a92f2 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h @@ -344,7 +344,7 @@ void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv, int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv, u32 pll_id); int cnl_hdmi_pll_ref_clock(struct drm_i915_private *dev_priv); -enum intel_dpll_id icl_port_to_mg_pll_id(enum port port); +enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port); bool intel_dpll_is_combophy(enum intel_dpll_id id); #endif /* _INTEL_DPLL_MGR_H_ */ -- 2.20.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] drm/i915/icl: use tc_port in MG_PLL macros 2019-01-17 20:21 ` [PATCH 1/5] drm/i915/icl: use tc_port in MG_PLL macros Lucas De Marchi @ 2019-01-24 1:15 ` Paulo Zanoni 2019-01-24 18:52 ` Lucas De Marchi 0 siblings, 1 reply; 22+ messages in thread From: Paulo Zanoni @ 2019-01-24 1:15 UTC (permalink / raw) To: Lucas De Marchi, intel-gfx Em qui, 2019-01-17 às 12:21 -0800, Lucas De Marchi escreveu: > Fix the TODO leftover in the code by changing the argument in MG_PLL > macros. The MG_PLL ids used to access the register values can be > converted from tc_port rather than port. > An explanation on why the new model is better would be amazing. It may be obvious to you, but it's not to other people. > The helper functions were also renamed to use "tc" as prefix to make > them more generic. > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 52 +++++++++--------- > drivers/gpu/drm/i915/intel_ddi.c | 7 ++- > drivers/gpu/drm/i915/intel_display.c | 3 +- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 79 +++++++++++++-------------- > drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 +- > 5 files changed, 72 insertions(+), 71 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 9a1340cfda6c..de209e0fdc01 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -9545,7 +9545,7 @@ enum skl_power_gate { > #define _MG_PLL3_ENABLE 0x46038 > #define _MG_PLL4_ENABLE 0x4603C > /* Bits are the same as DPLL0_ENABLE */ > -#define MG_PLL_ENABLE(port) _MMIO_PORT((port) - PORT_C, _MG_PLL1_ENABLE, \ > +#define MG_PLL_ENABLE(tc_port) _MMIO_PORT((tc_port), _MG_PLL1_ENABLE, \ > _MG_PLL2_ENABLE) > > #define _MG_REFCLKIN_CTL_PORT1 0x16892C > @@ -9554,9 +9554,9 @@ enum skl_power_gate { > #define _MG_REFCLKIN_CTL_PORT4 0x16B92C > #define MG_REFCLKIN_CTL_OD_2_MUX(x) ((x) << 8) > #define MG_REFCLKIN_CTL_OD_2_MUX_MASK (0x7 << 8) > -#define MG_REFCLKIN_CTL(port) _MMIO_PORT((port) - PORT_C, \ > - _MG_REFCLKIN_CTL_PORT1, \ > - _MG_REFCLKIN_CTL_PORT2) > +#define MG_REFCLKIN_CTL(tc_port) _MMIO_PORT((tc_port), \ > + _MG_REFCLKIN_CTL_PORT1, \ > + _MG_REFCLKIN_CTL_PORT2) > > #define _MG_CLKTOP2_CORECLKCTL1_PORT1 0x1688D8 > #define _MG_CLKTOP2_CORECLKCTL1_PORT2 0x1698D8 > @@ -9566,9 +9566,9 @@ enum skl_power_gate { > #define MG_CLKTOP2_CORECLKCTL1_B_DIVRATIO_MASK (0xff << 16) > #define MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO(x) ((x) << 8) > #define MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK (0xff << 8) > -#define MG_CLKTOP2_CORECLKCTL1(port) _MMIO_PORT((port) - PORT_C, \ > - _MG_CLKTOP2_CORECLKCTL1_PORT1, \ > - _MG_CLKTOP2_CORECLKCTL1_PORT2) > +#define MG_CLKTOP2_CORECLKCTL1(tc_port) _MMIO_PORT((tc_port), \ > + _MG_CLKTOP2_CORECLKCTL1_PORT1, \ > + _MG_CLKTOP2_CORECLKCTL1_PORT2) > > #define _MG_CLKTOP2_HSCLKCTL_PORT1 0x1688D4 > #define _MG_CLKTOP2_HSCLKCTL_PORT2 0x1698D4 > @@ -9586,9 +9586,9 @@ enum skl_power_gate { > #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO(x) ((x) << 8) > #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_SHIFT 8 > #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK (0xf << 8) > -#define MG_CLKTOP2_HSCLKCTL(port) _MMIO_PORT((port) - PORT_C, \ > - _MG_CLKTOP2_HSCLKCTL_PORT1, \ > - _MG_CLKTOP2_HSCLKCTL_PORT2) > +#define MG_CLKTOP2_HSCLKCTL(tc_port) _MMIO_PORT((tc_port), \ > + _MG_CLKTOP2_HSCLKCTL_PORT1, \ > + _MG_CLKTOP2_HSCLKCTL_PORT2) > > #define _MG_PLL_DIV0_PORT1 0x168A00 > #define _MG_PLL_DIV0_PORT2 0x169A00 > @@ -9600,8 +9600,8 @@ enum skl_power_gate { > #define MG_PLL_DIV0_FBDIV_FRAC(x) ((x) << 8) > #define MG_PLL_DIV0_FBDIV_INT_MASK (0xff << 0) > #define MG_PLL_DIV0_FBDIV_INT(x) ((x) << 0) > -#define MG_PLL_DIV0(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_DIV0_PORT1, \ > - _MG_PLL_DIV0_PORT2) > +#define MG_PLL_DIV0(tc_port) _MMIO_PORT((tc_port), _MG_PLL_DIV0_PORT1, \ > + _MG_PLL_DIV0_PORT2) > > #define _MG_PLL_DIV1_PORT1 0x168A04 > #define _MG_PLL_DIV1_PORT2 0x169A04 > @@ -9615,8 +9615,8 @@ enum skl_power_gate { > #define MG_PLL_DIV1_NDIVRATIO(x) ((x) << 4) > #define MG_PLL_DIV1_FBPREDIV_MASK (0xf << 0) > #define MG_PLL_DIV1_FBPREDIV(x) ((x) << 0) > -#define MG_PLL_DIV1(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_DIV1_PORT1, \ > - _MG_PLL_DIV1_PORT2) > +#define MG_PLL_DIV1(tc_port) _MMIO_PORT((tc_port), _MG_PLL_DIV1_PORT1, \ > + _MG_PLL_DIV1_PORT2) > > #define _MG_PLL_LF_PORT1 0x168A08 > #define _MG_PLL_LF_PORT2 0x169A08 > @@ -9628,8 +9628,8 @@ enum skl_power_gate { > #define MG_PLL_LF_GAINCTRL(x) ((x) << 16) > #define MG_PLL_LF_INT_COEFF(x) ((x) << 8) > #define MG_PLL_LF_PROP_COEFF(x) ((x) << 0) > -#define MG_PLL_LF(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_LF_PORT1, \ > - _MG_PLL_LF_PORT2) > +#define MG_PLL_LF(tc_port) _MMIO_PORT((tc_port), _MG_PLL_LF_PORT1, \ > + _MG_PLL_LF_PORT2) > > #define _MG_PLL_FRAC_LOCK_PORT1 0x168A0C > #define _MG_PLL_FRAC_LOCK_PORT2 0x169A0C > @@ -9641,9 +9641,9 @@ enum skl_power_gate { > #define MG_PLL_FRAC_LOCK_DCODITHEREN (1 << 10) > #define MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN (1 << 8) > #define MG_PLL_FRAC_LOCK_FEEDFWRDGAIN(x) ((x) << 0) > -#define MG_PLL_FRAC_LOCK(port) _MMIO_PORT((port) - PORT_C, \ > - _MG_PLL_FRAC_LOCK_PORT1, \ > - _MG_PLL_FRAC_LOCK_PORT2) > +#define MG_PLL_FRAC_LOCK(tc_port) _MMIO_PORT((tc_port), \ > + _MG_PLL_FRAC_LOCK_PORT1, \ > + _MG_PLL_FRAC_LOCK_PORT2) > > #define _MG_PLL_SSC_PORT1 0x168A10 > #define _MG_PLL_SSC_PORT2 0x169A10 > @@ -9655,8 +9655,8 @@ enum skl_power_gate { > #define MG_PLL_SSC_STEPNUM(x) ((x) << 10) > #define MG_PLL_SSC_FLLEN (1 << 9) > #define MG_PLL_SSC_STEPSIZE(x) ((x) << 0) > -#define MG_PLL_SSC(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_SSC_PORT1, \ > - _MG_PLL_SSC_PORT2) > +#define MG_PLL_SSC(tc_port) _MMIO_PORT((tc_port), _MG_PLL_SSC_PORT1, \ > + _MG_PLL_SSC_PORT2) > > #define _MG_PLL_BIAS_PORT1 0x168A14 > #define _MG_PLL_BIAS_PORT2 0x169A14 > @@ -9675,8 +9675,8 @@ enum skl_power_gate { > #define MG_PLL_BIAS_VREF_RDAC_MASK (0x7 << 5) > #define MG_PLL_BIAS_IREFTRIM(x) ((x) << 0) > #define MG_PLL_BIAS_IREFTRIM_MASK (0x1f << 0) > -#define MG_PLL_BIAS(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_BIAS_PORT1, \ > - _MG_PLL_BIAS_PORT2) > +#define MG_PLL_BIAS(tc_port) _MMIO_PORT((tc_port), _MG_PLL_BIAS_PORT1, \ > + _MG_PLL_BIAS_PORT2) > > #define _MG_PLL_TDC_COLDST_BIAS_PORT1 0x168A18 > #define _MG_PLL_TDC_COLDST_BIAS_PORT2 0x169A18 > @@ -9687,9 +9687,9 @@ enum skl_power_gate { > #define MG_PLL_TDC_COLDST_COLDSTART (1 << 16) > #define MG_PLL_TDC_TDCOVCCORR_EN (1 << 2) > #define MG_PLL_TDC_TDCSEL(x) ((x) << 0) > -#define MG_PLL_TDC_COLDST_BIAS(port) _MMIO_PORT((port) - PORT_C, \ > - _MG_PLL_TDC_COLDST_BIAS_PORT1, \ > - _MG_PLL_TDC_COLDST_BIAS_PORT2) > +#define MG_PLL_TDC_COLDST_BIAS(tc_port) _MMIO_PORT((tc_port), \ > + _MG_PLL_TDC_COLDST_BIAS_PORT1, \ > + _MG_PLL_TDC_COLDST_BIAS_PORT2) > > #define _CNL_DPLL0_CFGCR0 0x6C000 > #define _CNL_DPLL1_CFGCR0 0x6C080 > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index ce44744a5f9d..8dbf6c9e22fb 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1391,16 +1391,17 @@ static int icl_calc_tbt_pll_link(struct drm_i915_private *dev_priv, > static int icl_calc_mg_pll_link(struct drm_i915_private *dev_priv, > enum port port) > { > + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > u32 mg_pll_div0, mg_clktop_hsclkctl; > u32 m1, m2_int, m2_frac, div1, div2, refclk; > u64 tmp; > > refclk = dev_priv->cdclk.hw.ref; > > - mg_pll_div0 = I915_READ(MG_PLL_DIV0(port)); > - mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(port)); > + mg_pll_div0 = I915_READ(MG_PLL_DIV0(tc_port)); > + mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); > > - m1 = I915_READ(MG_PLL_DIV1(port)) & MG_PLL_DIV1_FBPREDIV_MASK; > + m1 = I915_READ(MG_PLL_DIV1(tc_port)) & MG_PLL_DIV1_FBPREDIV_MASK; > m2_int = mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK; > m2_frac = (mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) ? > (mg_pll_div0 & MG_PLL_DIV0_FBDIV_FRAC_MASK) >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 62d61fcad89c..a5de70e6bf59 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9415,7 +9415,8 @@ static void icelake_get_ddi_pll(struct drm_i915_private *dev_priv, > if (WARN_ON(!intel_dpll_is_combophy(id))) > return; > } else if (intel_port_is_tc(dev_priv, port)) { > - id = icl_port_to_mg_pll_id(port); > + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > + id = icl_tc_port_to_pll_id(tc_port); Checkpatch's complaint makes sense here. You could also opt to simply to: id = icl_tc_port_to_pll_id(intel_port_to_tc(dev_priv, port)); > } else { > WARN(1, "Invalid port %x\n", port); > return; > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index 606f54dde086..211b3ffa5bed 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -2639,14 +2639,14 @@ int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv, > return link_clock; > } > > -static enum port icl_mg_pll_id_to_port(enum intel_dpll_id id) > +static enum tc_port icl_pll_id_to_tc_port(enum intel_dpll_id id) > { > - return id - DPLL_ID_ICL_MGPLL1 + PORT_C; > + return id - DPLL_ID_ICL_MGPLL1; > } > > -enum intel_dpll_id icl_port_to_mg_pll_id(enum port port) > +enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port) > { > - return port - PORT_C + DPLL_ID_ICL_MGPLL1; > + return tc_port + DPLL_ID_ICL_MGPLL1; > } The "_mg_" in the name was supposed to help callers easily realize that these functions don't make sense without mg plls. I kinda liked them, especially in the first case where you can pass any id, which would result in wrong code. So, one of my fears is that there may be other patches in-flight which touch these registers and will silently make our code use port again. The cherry on top of this patch would be to put those enums inside single-element structs in a way that would make gcc complain when you accessed one instead of the other (including inside macros). Feel free to implement this if you like the idea. With the checkpatch error addressed and the improved commit message: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > bool intel_dpll_is_combophy(enum intel_dpll_id id) > @@ -2925,7 +2925,10 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > ret = icl_calc_dpll_state(crtc_state, encoder, clock, > &pll_state); > } else { > - min = icl_port_to_mg_pll_id(port); > + enum tc_port tc_port; > + > + tc_port = intel_port_to_tc(dev_priv, port); > + min = icl_tc_port_to_pll_id(tc_port); > max = min; > ret = icl_calc_mg_pll_state(crtc_state, encoder, clock, > &pll_state); > @@ -2959,12 +2962,8 @@ static i915_reg_t icl_pll_id_to_enable_reg(enum intel_dpll_id id) > return CNL_DPLL_ENABLE(id); > else if (id == DPLL_ID_ICL_TBTPLL) > return TBT_PLL_ENABLE; > - else > - /* > - * TODO: Make MG_PLL macros use > - * tc port id instead of port id > - */ > - return MG_PLL_ENABLE(icl_mg_pll_id_to_port(id)); > + > + return MG_PLL_ENABLE(icl_pll_id_to_tc_port(id)); > } > > static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, > @@ -2974,7 +2973,6 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, > const enum intel_dpll_id id = pll->info->id; > intel_wakeref_t wakeref; > bool ret = false; > - enum port port; > u32 val; > > wakeref = intel_display_power_get_if_enabled(dev_priv, > @@ -2991,32 +2989,33 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, > hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id)); > hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id)); > } else { > - port = icl_mg_pll_id_to_port(id); > - hw_state->mg_refclkin_ctl = I915_READ(MG_REFCLKIN_CTL(port)); > + enum tc_port tc_port = icl_pll_id_to_tc_port(id); > + > + hw_state->mg_refclkin_ctl = I915_READ(MG_REFCLKIN_CTL(tc_port)); > hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK; > > hw_state->mg_clktop2_coreclkctl1 = > - I915_READ(MG_CLKTOP2_CORECLKCTL1(port)); > + I915_READ(MG_CLKTOP2_CORECLKCTL1(tc_port)); > hw_state->mg_clktop2_coreclkctl1 &= > MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK; > > hw_state->mg_clktop2_hsclkctl = > - I915_READ(MG_CLKTOP2_HSCLKCTL(port)); > + I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); > hw_state->mg_clktop2_hsclkctl &= > MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK | > MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK | > MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK | > MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK; > > - hw_state->mg_pll_div0 = I915_READ(MG_PLL_DIV0(port)); > - hw_state->mg_pll_div1 = I915_READ(MG_PLL_DIV1(port)); > - hw_state->mg_pll_lf = I915_READ(MG_PLL_LF(port)); > - hw_state->mg_pll_frac_lock = I915_READ(MG_PLL_FRAC_LOCK(port)); > - hw_state->mg_pll_ssc = I915_READ(MG_PLL_SSC(port)); > + hw_state->mg_pll_div0 = I915_READ(MG_PLL_DIV0(tc_port)); > + hw_state->mg_pll_div1 = I915_READ(MG_PLL_DIV1(tc_port)); > + hw_state->mg_pll_lf = I915_READ(MG_PLL_LF(tc_port)); > + hw_state->mg_pll_frac_lock = I915_READ(MG_PLL_FRAC_LOCK(tc_port)); > + hw_state->mg_pll_ssc = I915_READ(MG_PLL_SSC(tc_port)); > > - hw_state->mg_pll_bias = I915_READ(MG_PLL_BIAS(port)); > + hw_state->mg_pll_bias = I915_READ(MG_PLL_BIAS(tc_port)); > hw_state->mg_pll_tdc_coldst_bias = > - I915_READ(MG_PLL_TDC_COLDST_BIAS(port)); > + I915_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); > > if (dev_priv->cdclk.hw.ref == 38400) { > hw_state->mg_pll_tdc_coldst_bias_mask = MG_PLL_TDC_COLDST_COLDSTART; > @@ -3051,7 +3050,7 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv, > struct intel_shared_dpll *pll) > { > struct intel_dpll_hw_state *hw_state = &pll->state.hw_state; > - enum port port = icl_mg_pll_id_to_port(pll->info->id); > + enum tc_port tc_port = icl_pll_id_to_tc_port(pll->info->id); > u32 val; > > /* > @@ -3060,41 +3059,41 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv, > * during the calc/readout phase if the mask depends on some other HW > * state like refclk, see icl_calc_mg_pll_state(). > */ > - val = I915_READ(MG_REFCLKIN_CTL(port)); > + val = I915_READ(MG_REFCLKIN_CTL(tc_port)); > val &= ~MG_REFCLKIN_CTL_OD_2_MUX_MASK; > val |= hw_state->mg_refclkin_ctl; > - I915_WRITE(MG_REFCLKIN_CTL(port), val); > + I915_WRITE(MG_REFCLKIN_CTL(tc_port), val); > > - val = I915_READ(MG_CLKTOP2_CORECLKCTL1(port)); > + val = I915_READ(MG_CLKTOP2_CORECLKCTL1(tc_port)); > val &= ~MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK; > val |= hw_state->mg_clktop2_coreclkctl1; > - I915_WRITE(MG_CLKTOP2_CORECLKCTL1(port), val); > + I915_WRITE(MG_CLKTOP2_CORECLKCTL1(tc_port), val); > > - val = I915_READ(MG_CLKTOP2_HSCLKCTL(port)); > + val = I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); > val &= ~(MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK | > MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK | > MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK | > MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK); > val |= hw_state->mg_clktop2_hsclkctl; > - I915_WRITE(MG_CLKTOP2_HSCLKCTL(port), val); > + I915_WRITE(MG_CLKTOP2_HSCLKCTL(tc_port), val); > > - I915_WRITE(MG_PLL_DIV0(port), hw_state->mg_pll_div0); > - I915_WRITE(MG_PLL_DIV1(port), hw_state->mg_pll_div1); > - I915_WRITE(MG_PLL_LF(port), hw_state->mg_pll_lf); > - I915_WRITE(MG_PLL_FRAC_LOCK(port), hw_state->mg_pll_frac_lock); > - I915_WRITE(MG_PLL_SSC(port), hw_state->mg_pll_ssc); > + I915_WRITE(MG_PLL_DIV0(tc_port), hw_state->mg_pll_div0); > + I915_WRITE(MG_PLL_DIV1(tc_port), hw_state->mg_pll_div1); > + I915_WRITE(MG_PLL_LF(tc_port), hw_state->mg_pll_lf); > + I915_WRITE(MG_PLL_FRAC_LOCK(tc_port), hw_state->mg_pll_frac_lock); > + I915_WRITE(MG_PLL_SSC(tc_port), hw_state->mg_pll_ssc); > > - val = I915_READ(MG_PLL_BIAS(port)); > + val = I915_READ(MG_PLL_BIAS(tc_port)); > val &= ~hw_state->mg_pll_bias_mask; > val |= hw_state->mg_pll_bias; > - I915_WRITE(MG_PLL_BIAS(port), val); > + I915_WRITE(MG_PLL_BIAS(tc_port), val); > > - val = I915_READ(MG_PLL_TDC_COLDST_BIAS(port)); > + val = I915_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); > val &= ~hw_state->mg_pll_tdc_coldst_bias_mask; > val |= hw_state->mg_pll_tdc_coldst_bias; > - I915_WRITE(MG_PLL_TDC_COLDST_BIAS(port), val); > + I915_WRITE(MG_PLL_TDC_COLDST_BIAS(tc_port), val); > > - POSTING_READ(MG_PLL_TDC_COLDST_BIAS(port)); > + POSTING_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); > } > > static void icl_pll_enable(struct drm_i915_private *dev_priv, > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h > index e96e79413b54..40e8391a92f2 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h > @@ -344,7 +344,7 @@ void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv, > int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv, > u32 pll_id); > int cnl_hdmi_pll_ref_clock(struct drm_i915_private *dev_priv); > -enum intel_dpll_id icl_port_to_mg_pll_id(enum port port); > +enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port); > bool intel_dpll_is_combophy(enum intel_dpll_id id); > > #endif /* _INTEL_DPLL_MGR_H_ */ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] drm/i915/icl: use tc_port in MG_PLL macros 2019-01-24 1:15 ` Paulo Zanoni @ 2019-01-24 18:52 ` Lucas De Marchi 2019-01-24 20:40 ` Paulo Zanoni 0 siblings, 1 reply; 22+ messages in thread From: Lucas De Marchi @ 2019-01-24 18:52 UTC (permalink / raw) To: Paulo Zanoni; +Cc: intel-gfx On Wed, Jan 23, 2019 at 05:15:26PM -0800, Paulo Zanoni wrote: >Em qui, 2019-01-17 às 12:21 -0800, Lucas De Marchi escreveu: >> Fix the TODO leftover in the code by changing the argument in MG_PLL >> macros. The MG_PLL ids used to access the register values can be >> converted from tc_port rather than port. >> > >An explanation on why the new model is better would be amazing. It may >be obvious to you, but it's not to other people. What about: All these registers can use the TC port to calculate the right offsets because they are only available for TC ports. The range (PORT_C onwards) may not be stable and change from platform to platform. So by using the TC id directly we avoid having to check for the platform in the "leaf functions" and thus passing dev_priv around. > > >> The helper functions were also renamed to use "tc" as prefix to make >> them more generic. >> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 52 +++++++++--------- >> drivers/gpu/drm/i915/intel_ddi.c | 7 ++- >> drivers/gpu/drm/i915/intel_display.c | 3 +- >> drivers/gpu/drm/i915/intel_dpll_mgr.c | 79 +++++++++++++-------------- >> drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 +- >> 5 files changed, 72 insertions(+), 71 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index 9a1340cfda6c..de209e0fdc01 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -9545,7 +9545,7 @@ enum skl_power_gate { >> #define _MG_PLL3_ENABLE 0x46038 >> #define _MG_PLL4_ENABLE 0x4603C >> /* Bits are the same as DPLL0_ENABLE */ >> -#define MG_PLL_ENABLE(port) _MMIO_PORT((port) - PORT_C, _MG_PLL1_ENABLE, \ >> +#define MG_PLL_ENABLE(tc_port) _MMIO_PORT((tc_port), _MG_PLL1_ENABLE, \ >> _MG_PLL2_ENABLE) >> >> #define _MG_REFCLKIN_CTL_PORT1 0x16892C >> @@ -9554,9 +9554,9 @@ enum skl_power_gate { >> #define _MG_REFCLKIN_CTL_PORT4 0x16B92C >> #define MG_REFCLKIN_CTL_OD_2_MUX(x) ((x) << 8) >> #define MG_REFCLKIN_CTL_OD_2_MUX_MASK (0x7 << 8) >> -#define MG_REFCLKIN_CTL(port) _MMIO_PORT((port) - PORT_C, \ >> - _MG_REFCLKIN_CTL_PORT1, \ >> - _MG_REFCLKIN_CTL_PORT2) >> +#define MG_REFCLKIN_CTL(tc_port) _MMIO_PORT((tc_port), \ >> + _MG_REFCLKIN_CTL_PORT1, \ >> + _MG_REFCLKIN_CTL_PORT2) >> >> #define _MG_CLKTOP2_CORECLKCTL1_PORT1 0x1688D8 >> #define _MG_CLKTOP2_CORECLKCTL1_PORT2 0x1698D8 >> @@ -9566,9 +9566,9 @@ enum skl_power_gate { >> #define MG_CLKTOP2_CORECLKCTL1_B_DIVRATIO_MASK (0xff << 16) >> #define MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO(x) ((x) << 8) >> #define MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK (0xff << 8) >> -#define MG_CLKTOP2_CORECLKCTL1(port) _MMIO_PORT((port) - PORT_C, \ >> - _MG_CLKTOP2_CORECLKCTL1_PORT1, \ >> - _MG_CLKTOP2_CORECLKCTL1_PORT2) >> +#define MG_CLKTOP2_CORECLKCTL1(tc_port) _MMIO_PORT((tc_port), \ >> + _MG_CLKTOP2_CORECLKCTL1_PORT1, \ >> + _MG_CLKTOP2_CORECLKCTL1_PORT2) >> >> #define _MG_CLKTOP2_HSCLKCTL_PORT1 0x1688D4 >> #define _MG_CLKTOP2_HSCLKCTL_PORT2 0x1698D4 >> @@ -9586,9 +9586,9 @@ enum skl_power_gate { >> #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO(x) ((x) << 8) >> #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_SHIFT 8 >> #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK (0xf << 8) >> -#define MG_CLKTOP2_HSCLKCTL(port) _MMIO_PORT((port) - PORT_C, \ >> - _MG_CLKTOP2_HSCLKCTL_PORT1, \ >> - _MG_CLKTOP2_HSCLKCTL_PORT2) >> +#define MG_CLKTOP2_HSCLKCTL(tc_port) _MMIO_PORT((tc_port), \ >> + _MG_CLKTOP2_HSCLKCTL_PORT1, \ >> + _MG_CLKTOP2_HSCLKCTL_PORT2) >> >> #define _MG_PLL_DIV0_PORT1 0x168A00 >> #define _MG_PLL_DIV0_PORT2 0x169A00 >> @@ -9600,8 +9600,8 @@ enum skl_power_gate { >> #define MG_PLL_DIV0_FBDIV_FRAC(x) ((x) << 8) >> #define MG_PLL_DIV0_FBDIV_INT_MASK (0xff << 0) >> #define MG_PLL_DIV0_FBDIV_INT(x) ((x) << 0) >> -#define MG_PLL_DIV0(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_DIV0_PORT1, \ >> - _MG_PLL_DIV0_PORT2) >> +#define MG_PLL_DIV0(tc_port) _MMIO_PORT((tc_port), _MG_PLL_DIV0_PORT1, \ >> + _MG_PLL_DIV0_PORT2) >> >> #define _MG_PLL_DIV1_PORT1 0x168A04 >> #define _MG_PLL_DIV1_PORT2 0x169A04 >> @@ -9615,8 +9615,8 @@ enum skl_power_gate { >> #define MG_PLL_DIV1_NDIVRATIO(x) ((x) << 4) >> #define MG_PLL_DIV1_FBPREDIV_MASK (0xf << 0) >> #define MG_PLL_DIV1_FBPREDIV(x) ((x) << 0) >> -#define MG_PLL_DIV1(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_DIV1_PORT1, \ >> - _MG_PLL_DIV1_PORT2) >> +#define MG_PLL_DIV1(tc_port) _MMIO_PORT((tc_port), _MG_PLL_DIV1_PORT1, \ >> + _MG_PLL_DIV1_PORT2) >> >> #define _MG_PLL_LF_PORT1 0x168A08 >> #define _MG_PLL_LF_PORT2 0x169A08 >> @@ -9628,8 +9628,8 @@ enum skl_power_gate { >> #define MG_PLL_LF_GAINCTRL(x) ((x) << 16) >> #define MG_PLL_LF_INT_COEFF(x) ((x) << 8) >> #define MG_PLL_LF_PROP_COEFF(x) ((x) << 0) >> -#define MG_PLL_LF(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_LF_PORT1, \ >> - _MG_PLL_LF_PORT2) >> +#define MG_PLL_LF(tc_port) _MMIO_PORT((tc_port), _MG_PLL_LF_PORT1, \ >> + _MG_PLL_LF_PORT2) >> >> #define _MG_PLL_FRAC_LOCK_PORT1 0x168A0C >> #define _MG_PLL_FRAC_LOCK_PORT2 0x169A0C >> @@ -9641,9 +9641,9 @@ enum skl_power_gate { >> #define MG_PLL_FRAC_LOCK_DCODITHEREN (1 << 10) >> #define MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN (1 << 8) >> #define MG_PLL_FRAC_LOCK_FEEDFWRDGAIN(x) ((x) << 0) >> -#define MG_PLL_FRAC_LOCK(port) _MMIO_PORT((port) - PORT_C, \ >> - _MG_PLL_FRAC_LOCK_PORT1, \ >> - _MG_PLL_FRAC_LOCK_PORT2) >> +#define MG_PLL_FRAC_LOCK(tc_port) _MMIO_PORT((tc_port), \ >> + _MG_PLL_FRAC_LOCK_PORT1, \ >> + _MG_PLL_FRAC_LOCK_PORT2) >> >> #define _MG_PLL_SSC_PORT1 0x168A10 >> #define _MG_PLL_SSC_PORT2 0x169A10 >> @@ -9655,8 +9655,8 @@ enum skl_power_gate { >> #define MG_PLL_SSC_STEPNUM(x) ((x) << 10) >> #define MG_PLL_SSC_FLLEN (1 << 9) >> #define MG_PLL_SSC_STEPSIZE(x) ((x) << 0) >> -#define MG_PLL_SSC(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_SSC_PORT1, \ >> - _MG_PLL_SSC_PORT2) >> +#define MG_PLL_SSC(tc_port) _MMIO_PORT((tc_port), _MG_PLL_SSC_PORT1, \ >> + _MG_PLL_SSC_PORT2) >> >> #define _MG_PLL_BIAS_PORT1 0x168A14 >> #define _MG_PLL_BIAS_PORT2 0x169A14 >> @@ -9675,8 +9675,8 @@ enum skl_power_gate { >> #define MG_PLL_BIAS_VREF_RDAC_MASK (0x7 << 5) >> #define MG_PLL_BIAS_IREFTRIM(x) ((x) << 0) >> #define MG_PLL_BIAS_IREFTRIM_MASK (0x1f << 0) >> -#define MG_PLL_BIAS(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_BIAS_PORT1, \ >> - _MG_PLL_BIAS_PORT2) >> +#define MG_PLL_BIAS(tc_port) _MMIO_PORT((tc_port), _MG_PLL_BIAS_PORT1, \ >> + _MG_PLL_BIAS_PORT2) >> >> #define _MG_PLL_TDC_COLDST_BIAS_PORT1 0x168A18 >> #define _MG_PLL_TDC_COLDST_BIAS_PORT2 0x169A18 >> @@ -9687,9 +9687,9 @@ enum skl_power_gate { >> #define MG_PLL_TDC_COLDST_COLDSTART (1 << 16) >> #define MG_PLL_TDC_TDCOVCCORR_EN (1 << 2) >> #define MG_PLL_TDC_TDCSEL(x) ((x) << 0) >> -#define MG_PLL_TDC_COLDST_BIAS(port) _MMIO_PORT((port) - PORT_C, \ >> - _MG_PLL_TDC_COLDST_BIAS_PORT1, \ >> - _MG_PLL_TDC_COLDST_BIAS_PORT2) >> +#define MG_PLL_TDC_COLDST_BIAS(tc_port) _MMIO_PORT((tc_port), \ >> + _MG_PLL_TDC_COLDST_BIAS_PORT1, \ >> + _MG_PLL_TDC_COLDST_BIAS_PORT2) >> >> #define _CNL_DPLL0_CFGCR0 0x6C000 >> #define _CNL_DPLL1_CFGCR0 0x6C080 >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c >> index ce44744a5f9d..8dbf6c9e22fb 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -1391,16 +1391,17 @@ static int icl_calc_tbt_pll_link(struct drm_i915_private *dev_priv, >> static int icl_calc_mg_pll_link(struct drm_i915_private *dev_priv, >> enum port port) >> { >> + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); >> u32 mg_pll_div0, mg_clktop_hsclkctl; >> u32 m1, m2_int, m2_frac, div1, div2, refclk; >> u64 tmp; >> >> refclk = dev_priv->cdclk.hw.ref; >> >> - mg_pll_div0 = I915_READ(MG_PLL_DIV0(port)); >> - mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(port)); >> + mg_pll_div0 = I915_READ(MG_PLL_DIV0(tc_port)); >> + mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); >> >> - m1 = I915_READ(MG_PLL_DIV1(port)) & MG_PLL_DIV1_FBPREDIV_MASK; >> + m1 = I915_READ(MG_PLL_DIV1(tc_port)) & MG_PLL_DIV1_FBPREDIV_MASK; >> m2_int = mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK; >> m2_frac = (mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) ? >> (mg_pll_div0 & MG_PLL_DIV0_FBDIV_FRAC_MASK) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 62d61fcad89c..a5de70e6bf59 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -9415,7 +9415,8 @@ static void icelake_get_ddi_pll(struct drm_i915_private *dev_priv, >> if (WARN_ON(!intel_dpll_is_combophy(id))) >> return; >> } else if (intel_port_is_tc(dev_priv, port)) { >> - id = icl_port_to_mg_pll_id(port); >> + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); >> + id = icl_tc_port_to_pll_id(tc_port); > >Checkpatch's complaint makes sense here. You could also opt to simply >to: > >id = icl_tc_port_to_pll_id(intel_port_to_tc(dev_priv, port)); yeah, I will resend, but just adding a blank line. > >> } else { >> WARN(1, "Invalid port %x\n", port); >> return; >> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c >> index 606f54dde086..211b3ffa5bed 100644 >> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c >> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c >> @@ -2639,14 +2639,14 @@ int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv, >> return link_clock; >> } >> >> -static enum port icl_mg_pll_id_to_port(enum intel_dpll_id id) >> +static enum tc_port icl_pll_id_to_tc_port(enum intel_dpll_id id) >> { >> - return id - DPLL_ID_ICL_MGPLL1 + PORT_C; >> + return id - DPLL_ID_ICL_MGPLL1; >> } >> >> -enum intel_dpll_id icl_port_to_mg_pll_id(enum port port) >> +enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port) >> { >> - return port - PORT_C + DPLL_ID_ICL_MGPLL1; >> + return tc_port + DPLL_ID_ICL_MGPLL1; >> } > >The "_mg_" in the name was supposed to help callers easily realize that >these functions don't make sense without mg plls. I kinda liked them, >especially in the first case where you can pass any id, which would >result in wrong code. well, yeah, but because the argument was *port*. Now I removed the "mg" from the name and added "tc", changing the argument accordingly: it's implicit that mg plls only make sense for TC ports. >So, one of my fears is that there may be other patches in-flight which >touch these registers and will silently make our code use port again. >The cherry on top of this patch would be to put those enums inside >single-element structs in a way that would make gcc complain when you >accessed one instead of the other (including inside macros). Feel free >to implement this if you like the idea. yeah, that would be a nice way to prevent callers passing in the wrong argument. Kind of what "enum class" does for C++. But as you noted, it's a cherry on *top* :) > >With the checkpatch error addressed and the improved commit message: >Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> thanks Lucas De Marchi > >> >> bool intel_dpll_is_combophy(enum intel_dpll_id id) >> @@ -2925,7 +2925,10 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, >> ret = icl_calc_dpll_state(crtc_state, encoder, clock, >> &pll_state); >> } else { >> - min = icl_port_to_mg_pll_id(port); >> + enum tc_port tc_port; >> + >> + tc_port = intel_port_to_tc(dev_priv, port); >> + min = icl_tc_port_to_pll_id(tc_port); >> max = min; >> ret = icl_calc_mg_pll_state(crtc_state, encoder, clock, >> &pll_state); >> @@ -2959,12 +2962,8 @@ static i915_reg_t icl_pll_id_to_enable_reg(enum intel_dpll_id id) >> return CNL_DPLL_ENABLE(id); >> else if (id == DPLL_ID_ICL_TBTPLL) >> return TBT_PLL_ENABLE; >> - else >> - /* >> - * TODO: Make MG_PLL macros use >> - * tc port id instead of port id >> - */ >> - return MG_PLL_ENABLE(icl_mg_pll_id_to_port(id)); >> + >> + return MG_PLL_ENABLE(icl_pll_id_to_tc_port(id)); >> } >> >> static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, >> @@ -2974,7 +2973,6 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, >> const enum intel_dpll_id id = pll->info->id; >> intel_wakeref_t wakeref; >> bool ret = false; >> - enum port port; >> u32 val; >> >> wakeref = intel_display_power_get_if_enabled(dev_priv, >> @@ -2991,32 +2989,33 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, >> hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id)); >> hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id)); >> } else { >> - port = icl_mg_pll_id_to_port(id); >> - hw_state->mg_refclkin_ctl = I915_READ(MG_REFCLKIN_CTL(port)); >> + enum tc_port tc_port = icl_pll_id_to_tc_port(id); >> + >> + hw_state->mg_refclkin_ctl = I915_READ(MG_REFCLKIN_CTL(tc_port)); >> hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK; >> >> hw_state->mg_clktop2_coreclkctl1 = >> - I915_READ(MG_CLKTOP2_CORECLKCTL1(port)); >> + I915_READ(MG_CLKTOP2_CORECLKCTL1(tc_port)); >> hw_state->mg_clktop2_coreclkctl1 &= >> MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK; >> >> hw_state->mg_clktop2_hsclkctl = >> - I915_READ(MG_CLKTOP2_HSCLKCTL(port)); >> + I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); >> hw_state->mg_clktop2_hsclkctl &= >> MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK | >> MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK | >> MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK | >> MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK; >> >> - hw_state->mg_pll_div0 = I915_READ(MG_PLL_DIV0(port)); >> - hw_state->mg_pll_div1 = I915_READ(MG_PLL_DIV1(port)); >> - hw_state->mg_pll_lf = I915_READ(MG_PLL_LF(port)); >> - hw_state->mg_pll_frac_lock = I915_READ(MG_PLL_FRAC_LOCK(port)); >> - hw_state->mg_pll_ssc = I915_READ(MG_PLL_SSC(port)); >> + hw_state->mg_pll_div0 = I915_READ(MG_PLL_DIV0(tc_port)); >> + hw_state->mg_pll_div1 = I915_READ(MG_PLL_DIV1(tc_port)); >> + hw_state->mg_pll_lf = I915_READ(MG_PLL_LF(tc_port)); >> + hw_state->mg_pll_frac_lock = I915_READ(MG_PLL_FRAC_LOCK(tc_port)); >> + hw_state->mg_pll_ssc = I915_READ(MG_PLL_SSC(tc_port)); >> >> - hw_state->mg_pll_bias = I915_READ(MG_PLL_BIAS(port)); >> + hw_state->mg_pll_bias = I915_READ(MG_PLL_BIAS(tc_port)); >> hw_state->mg_pll_tdc_coldst_bias = >> - I915_READ(MG_PLL_TDC_COLDST_BIAS(port)); >> + I915_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); >> >> if (dev_priv->cdclk.hw.ref == 38400) { >> hw_state->mg_pll_tdc_coldst_bias_mask = MG_PLL_TDC_COLDST_COLDSTART; >> @@ -3051,7 +3050,7 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv, >> struct intel_shared_dpll *pll) >> { >> struct intel_dpll_hw_state *hw_state = &pll->state.hw_state; >> - enum port port = icl_mg_pll_id_to_port(pll->info->id); >> + enum tc_port tc_port = icl_pll_id_to_tc_port(pll->info->id); >> u32 val; >> >> /* >> @@ -3060,41 +3059,41 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv, >> * during the calc/readout phase if the mask depends on some other HW >> * state like refclk, see icl_calc_mg_pll_state(). >> */ >> - val = I915_READ(MG_REFCLKIN_CTL(port)); >> + val = I915_READ(MG_REFCLKIN_CTL(tc_port)); >> val &= ~MG_REFCLKIN_CTL_OD_2_MUX_MASK; >> val |= hw_state->mg_refclkin_ctl; >> - I915_WRITE(MG_REFCLKIN_CTL(port), val); >> + I915_WRITE(MG_REFCLKIN_CTL(tc_port), val); >> >> - val = I915_READ(MG_CLKTOP2_CORECLKCTL1(port)); >> + val = I915_READ(MG_CLKTOP2_CORECLKCTL1(tc_port)); >> val &= ~MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK; >> val |= hw_state->mg_clktop2_coreclkctl1; >> - I915_WRITE(MG_CLKTOP2_CORECLKCTL1(port), val); >> + I915_WRITE(MG_CLKTOP2_CORECLKCTL1(tc_port), val); >> >> - val = I915_READ(MG_CLKTOP2_HSCLKCTL(port)); >> + val = I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); >> val &= ~(MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK | >> MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK | >> MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK | >> MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK); >> val |= hw_state->mg_clktop2_hsclkctl; >> - I915_WRITE(MG_CLKTOP2_HSCLKCTL(port), val); >> + I915_WRITE(MG_CLKTOP2_HSCLKCTL(tc_port), val); >> >> - I915_WRITE(MG_PLL_DIV0(port), hw_state->mg_pll_div0); >> - I915_WRITE(MG_PLL_DIV1(port), hw_state->mg_pll_div1); >> - I915_WRITE(MG_PLL_LF(port), hw_state->mg_pll_lf); >> - I915_WRITE(MG_PLL_FRAC_LOCK(port), hw_state->mg_pll_frac_lock); >> - I915_WRITE(MG_PLL_SSC(port), hw_state->mg_pll_ssc); >> + I915_WRITE(MG_PLL_DIV0(tc_port), hw_state->mg_pll_div0); >> + I915_WRITE(MG_PLL_DIV1(tc_port), hw_state->mg_pll_div1); >> + I915_WRITE(MG_PLL_LF(tc_port), hw_state->mg_pll_lf); >> + I915_WRITE(MG_PLL_FRAC_LOCK(tc_port), hw_state->mg_pll_frac_lock); >> + I915_WRITE(MG_PLL_SSC(tc_port), hw_state->mg_pll_ssc); >> >> - val = I915_READ(MG_PLL_BIAS(port)); >> + val = I915_READ(MG_PLL_BIAS(tc_port)); >> val &= ~hw_state->mg_pll_bias_mask; >> val |= hw_state->mg_pll_bias; >> - I915_WRITE(MG_PLL_BIAS(port), val); >> + I915_WRITE(MG_PLL_BIAS(tc_port), val); >> >> - val = I915_READ(MG_PLL_TDC_COLDST_BIAS(port)); >> + val = I915_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); >> val &= ~hw_state->mg_pll_tdc_coldst_bias_mask; >> val |= hw_state->mg_pll_tdc_coldst_bias; >> - I915_WRITE(MG_PLL_TDC_COLDST_BIAS(port), val); >> + I915_WRITE(MG_PLL_TDC_COLDST_BIAS(tc_port), val); >> >> - POSTING_READ(MG_PLL_TDC_COLDST_BIAS(port)); >> + POSTING_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); >> } >> >> static void icl_pll_enable(struct drm_i915_private *dev_priv, >> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h >> index e96e79413b54..40e8391a92f2 100644 >> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h >> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h >> @@ -344,7 +344,7 @@ void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv, >> int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv, >> u32 pll_id); >> int cnl_hdmi_pll_ref_clock(struct drm_i915_private *dev_priv); >> -enum intel_dpll_id icl_port_to_mg_pll_id(enum port port); >> +enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port); >> bool intel_dpll_is_combophy(enum intel_dpll_id id); >> >> #endif /* _INTEL_DPLL_MGR_H_ */ > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] drm/i915/icl: use tc_port in MG_PLL macros 2019-01-24 18:52 ` Lucas De Marchi @ 2019-01-24 20:40 ` Paulo Zanoni 0 siblings, 0 replies; 22+ messages in thread From: Paulo Zanoni @ 2019-01-24 20:40 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx Em qui, 2019-01-24 às 10:52 -0800, Lucas De Marchi escreveu: > On Wed, Jan 23, 2019 at 05:15:26PM -0800, Paulo Zanoni wrote: > > Em qui, 2019-01-17 às 12:21 -0800, Lucas De Marchi escreveu: > > > Fix the TODO leftover in the code by changing the argument in MG_PLL > > > macros. The MG_PLL ids used to access the register values can be > > > converted from tc_port rather than port. > > > > > > > An explanation on why the new model is better would be amazing. It may > > be obvious to you, but it's not to other people. > > What about: > > All these registers can use the TC port to calculate the right offsets > because they are only available for TC ports. The range (PORT_C onwards) > may not be stable and change from platform to platform. So by using the > TC id directly we avoid having to check for the platform in the "leaf > functions" and thus passing dev_priv around. Works for me. Thanks. > > > > > > > The helper functions were also renamed to use "tc" as prefix to make > > > them more generic. > > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 52 +++++++++--------- > > > drivers/gpu/drm/i915/intel_ddi.c | 7 ++- > > > drivers/gpu/drm/i915/intel_display.c | 3 +- > > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 79 +++++++++++++-------------- > > > drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 +- > > > 5 files changed, 72 insertions(+), 71 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index 9a1340cfda6c..de209e0fdc01 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -9545,7 +9545,7 @@ enum skl_power_gate { > > > #define _MG_PLL3_ENABLE 0x46038 > > > #define _MG_PLL4_ENABLE 0x4603C > > > /* Bits are the same as DPLL0_ENABLE */ > > > -#define MG_PLL_ENABLE(port) _MMIO_PORT((port) - PORT_C, _MG_PLL1_ENABLE, \ > > > +#define MG_PLL_ENABLE(tc_port) _MMIO_PORT((tc_port), _MG_PLL1_ENABLE, \ > > > _MG_PLL2_ENABLE) > > > > > > #define _MG_REFCLKIN_CTL_PORT1 0x16892C > > > @@ -9554,9 +9554,9 @@ enum skl_power_gate { > > > #define _MG_REFCLKIN_CTL_PORT4 0x16B92C > > > #define MG_REFCLKIN_CTL_OD_2_MUX(x) ((x) << 8) > > > #define MG_REFCLKIN_CTL_OD_2_MUX_MASK (0x7 << 8) > > > -#define MG_REFCLKIN_CTL(port) _MMIO_PORT((port) - PORT_C, \ > > > - _MG_REFCLKIN_CTL_PORT1, \ > > > - _MG_REFCLKIN_CTL_PORT2) > > > +#define MG_REFCLKIN_CTL(tc_port) _MMIO_PORT((tc_port), \ > > > + _MG_REFCLKIN_CTL_PORT1, \ > > > + _MG_REFCLKIN_CTL_PORT2) > > > > > > #define _MG_CLKTOP2_CORECLKCTL1_PORT1 0x1688D8 > > > #define _MG_CLKTOP2_CORECLKCTL1_PORT2 0x1698D8 > > > @@ -9566,9 +9566,9 @@ enum skl_power_gate { > > > #define MG_CLKTOP2_CORECLKCTL1_B_DIVRATIO_MASK (0xff << 16) > > > #define MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO(x) ((x) << 8) > > > #define MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK (0xff << 8) > > > -#define MG_CLKTOP2_CORECLKCTL1(port) _MMIO_PORT((port) - PORT_C, \ > > > - _MG_CLKTOP2_CORECLKCTL1_PORT1, \ > > > - _MG_CLKTOP2_CORECLKCTL1_PORT2) > > > +#define MG_CLKTOP2_CORECLKCTL1(tc_port) _MMIO_PORT((tc_port), \ > > > + _MG_CLKTOP2_CORECLKCTL1_PORT1, \ > > > + _MG_CLKTOP2_CORECLKCTL1_PORT2) > > > > > > #define _MG_CLKTOP2_HSCLKCTL_PORT1 0x1688D4 > > > #define _MG_CLKTOP2_HSCLKCTL_PORT2 0x1698D4 > > > @@ -9586,9 +9586,9 @@ enum skl_power_gate { > > > #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO(x) ((x) << 8) > > > #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_SHIFT 8 > > > #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK (0xf << 8) > > > -#define MG_CLKTOP2_HSCLKCTL(port) _MMIO_PORT((port) - PORT_C, \ > > > - _MG_CLKTOP2_HSCLKCTL_PORT1, \ > > > - _MG_CLKTOP2_HSCLKCTL_PORT2) > > > +#define MG_CLKTOP2_HSCLKCTL(tc_port) _MMIO_PORT((tc_port), \ > > > + _MG_CLKTOP2_HSCLKCTL_PORT1, \ > > > + _MG_CLKTOP2_HSCLKCTL_PORT2) > > > > > > #define _MG_PLL_DIV0_PORT1 0x168A00 > > > #define _MG_PLL_DIV0_PORT2 0x169A00 > > > @@ -9600,8 +9600,8 @@ enum skl_power_gate { > > > #define MG_PLL_DIV0_FBDIV_FRAC(x) ((x) << 8) > > > #define MG_PLL_DIV0_FBDIV_INT_MASK (0xff << 0) > > > #define MG_PLL_DIV0_FBDIV_INT(x) ((x) << 0) > > > -#define MG_PLL_DIV0(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_DIV0_PORT1, \ > > > - _MG_PLL_DIV0_PORT2) > > > +#define MG_PLL_DIV0(tc_port) _MMIO_PORT((tc_port), _MG_PLL_DIV0_PORT1, \ > > > + _MG_PLL_DIV0_PORT2) > > > > > > #define _MG_PLL_DIV1_PORT1 0x168A04 > > > #define _MG_PLL_DIV1_PORT2 0x169A04 > > > @@ -9615,8 +9615,8 @@ enum skl_power_gate { > > > #define MG_PLL_DIV1_NDIVRATIO(x) ((x) << 4) > > > #define MG_PLL_DIV1_FBPREDIV_MASK (0xf << 0) > > > #define MG_PLL_DIV1_FBPREDIV(x) ((x) << 0) > > > -#define MG_PLL_DIV1(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_DIV1_PORT1, \ > > > - _MG_PLL_DIV1_PORT2) > > > +#define MG_PLL_DIV1(tc_port) _MMIO_PORT((tc_port), _MG_PLL_DIV1_PORT1, \ > > > + _MG_PLL_DIV1_PORT2) > > > > > > #define _MG_PLL_LF_PORT1 0x168A08 > > > #define _MG_PLL_LF_PORT2 0x169A08 > > > @@ -9628,8 +9628,8 @@ enum skl_power_gate { > > > #define MG_PLL_LF_GAINCTRL(x) ((x) << 16) > > > #define MG_PLL_LF_INT_COEFF(x) ((x) << 8) > > > #define MG_PLL_LF_PROP_COEFF(x) ((x) << 0) > > > -#define MG_PLL_LF(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_LF_PORT1, \ > > > - _MG_PLL_LF_PORT2) > > > +#define MG_PLL_LF(tc_port) _MMIO_PORT((tc_port), _MG_PLL_LF_PORT1, \ > > > + _MG_PLL_LF_PORT2) > > > > > > #define _MG_PLL_FRAC_LOCK_PORT1 0x168A0C > > > #define _MG_PLL_FRAC_LOCK_PORT2 0x169A0C > > > @@ -9641,9 +9641,9 @@ enum skl_power_gate { > > > #define MG_PLL_FRAC_LOCK_DCODITHEREN (1 << 10) > > > #define MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN (1 << 8) > > > #define MG_PLL_FRAC_LOCK_FEEDFWRDGAIN(x) ((x) << 0) > > > -#define MG_PLL_FRAC_LOCK(port) _MMIO_PORT((port) - PORT_C, \ > > > - _MG_PLL_FRAC_LOCK_PORT1, \ > > > - _MG_PLL_FRAC_LOCK_PORT2) > > > +#define MG_PLL_FRAC_LOCK(tc_port) _MMIO_PORT((tc_port), \ > > > + _MG_PLL_FRAC_LOCK_PORT1, \ > > > + _MG_PLL_FRAC_LOCK_PORT2) > > > > > > #define _MG_PLL_SSC_PORT1 0x168A10 > > > #define _MG_PLL_SSC_PORT2 0x169A10 > > > @@ -9655,8 +9655,8 @@ enum skl_power_gate { > > > #define MG_PLL_SSC_STEPNUM(x) ((x) << 10) > > > #define MG_PLL_SSC_FLLEN (1 << 9) > > > #define MG_PLL_SSC_STEPSIZE(x) ((x) << 0) > > > -#define MG_PLL_SSC(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_SSC_PORT1, \ > > > - _MG_PLL_SSC_PORT2) > > > +#define MG_PLL_SSC(tc_port) _MMIO_PORT((tc_port), _MG_PLL_SSC_PORT1, \ > > > + _MG_PLL_SSC_PORT2) > > > > > > #define _MG_PLL_BIAS_PORT1 0x168A14 > > > #define _MG_PLL_BIAS_PORT2 0x169A14 > > > @@ -9675,8 +9675,8 @@ enum skl_power_gate { > > > #define MG_PLL_BIAS_VREF_RDAC_MASK (0x7 << 5) > > > #define MG_PLL_BIAS_IREFTRIM(x) ((x) << 0) > > > #define MG_PLL_BIAS_IREFTRIM_MASK (0x1f << 0) > > > -#define MG_PLL_BIAS(port) _MMIO_PORT((port) - PORT_C, _MG_PLL_BIAS_PORT1, \ > > > - _MG_PLL_BIAS_PORT2) > > > +#define MG_PLL_BIAS(tc_port) _MMIO_PORT((tc_port), _MG_PLL_BIAS_PORT1, \ > > > + _MG_PLL_BIAS_PORT2) > > > > > > #define _MG_PLL_TDC_COLDST_BIAS_PORT1 0x168A18 > > > #define _MG_PLL_TDC_COLDST_BIAS_PORT2 0x169A18 > > > @@ -9687,9 +9687,9 @@ enum skl_power_gate { > > > #define MG_PLL_TDC_COLDST_COLDSTART (1 << 16) > > > #define MG_PLL_TDC_TDCOVCCORR_EN (1 << 2) > > > #define MG_PLL_TDC_TDCSEL(x) ((x) << 0) > > > -#define MG_PLL_TDC_COLDST_BIAS(port) _MMIO_PORT((port) - PORT_C, \ > > > - _MG_PLL_TDC_COLDST_BIAS_PORT1, \ > > > - _MG_PLL_TDC_COLDST_BIAS_PORT2) > > > +#define MG_PLL_TDC_COLDST_BIAS(tc_port) _MMIO_PORT((tc_port), \ > > > + _MG_PLL_TDC_COLDST_BIAS_PORT1, \ > > > + _MG_PLL_TDC_COLDST_BIAS_PORT2) > > > > > > #define _CNL_DPLL0_CFGCR0 0x6C000 > > > #define _CNL_DPLL1_CFGCR0 0x6C080 > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > > index ce44744a5f9d..8dbf6c9e22fb 100644 > > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > > @@ -1391,16 +1391,17 @@ static int icl_calc_tbt_pll_link(struct drm_i915_private *dev_priv, > > > static int icl_calc_mg_pll_link(struct drm_i915_private *dev_priv, > > > enum port port) > > > { > > > + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > > > u32 mg_pll_div0, mg_clktop_hsclkctl; > > > u32 m1, m2_int, m2_frac, div1, div2, refclk; > > > u64 tmp; > > > > > > refclk = dev_priv->cdclk.hw.ref; > > > > > > - mg_pll_div0 = I915_READ(MG_PLL_DIV0(port)); > > > - mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(port)); > > > + mg_pll_div0 = I915_READ(MG_PLL_DIV0(tc_port)); > > > + mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); > > > > > > - m1 = I915_READ(MG_PLL_DIV1(port)) & MG_PLL_DIV1_FBPREDIV_MASK; > > > + m1 = I915_READ(MG_PLL_DIV1(tc_port)) & MG_PLL_DIV1_FBPREDIV_MASK; > > > m2_int = mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK; > > > m2_frac = (mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) ? > > > (mg_pll_div0 & MG_PLL_DIV0_FBDIV_FRAC_MASK) >> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index 62d61fcad89c..a5de70e6bf59 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -9415,7 +9415,8 @@ static void icelake_get_ddi_pll(struct drm_i915_private *dev_priv, > > > if (WARN_ON(!intel_dpll_is_combophy(id))) > > > return; > > > } else if (intel_port_is_tc(dev_priv, port)) { > > > - id = icl_port_to_mg_pll_id(port); > > > + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > > > + id = icl_tc_port_to_pll_id(tc_port); > > > > Checkpatch's complaint makes sense here. You could also opt to simply > > to: > > > > id = icl_tc_port_to_pll_id(intel_port_to_tc(dev_priv, port)); > > yeah, I will resend, but just adding a blank line. > > > > } else { > > > WARN(1, "Invalid port %x\n", port); > > > return; > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > > index 606f54dde086..211b3ffa5bed 100644 > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > > @@ -2639,14 +2639,14 @@ int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv, > > > return link_clock; > > > } > > > > > > -static enum port icl_mg_pll_id_to_port(enum intel_dpll_id id) > > > +static enum tc_port icl_pll_id_to_tc_port(enum intel_dpll_id id) > > > { > > > - return id - DPLL_ID_ICL_MGPLL1 + PORT_C; > > > + return id - DPLL_ID_ICL_MGPLL1; > > > } > > > > > > -enum intel_dpll_id icl_port_to_mg_pll_id(enum port port) > > > +enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port) > > > { > > > - return port - PORT_C + DPLL_ID_ICL_MGPLL1; > > > + return tc_port + DPLL_ID_ICL_MGPLL1; > > > } > > > > The "_mg_" in the name was supposed to help callers easily realize that > > these functions don't make sense without mg plls. I kinda liked them, > > especially in the first case where you can pass any id, which would > > result in wrong code. > > well, yeah, but because the argument was *port*. Now I removed the "mg" > from the name and added "tc", changing the argument accordingly: it's > implicit that mg plls only make sense for TC ports. Adding _mg_ back was not a requirement for the R-B, so it's fine to keep it this way if you prefer. > > > So, one of my fears is that there may be other patches in-flight which > > touch these registers and will silently make our code use port again. > > The cherry on top of this patch would be to put those enums inside > > single-element structs in a way that would make gcc complain when you > > accessed one instead of the other (including inside macros). Feel free > > to implement this if you like the idea. > > yeah, that would be a nice way to prevent callers passing in the wrong > argument. Kind of what "enum class" does for C++. But as you noted, it's > a cherry on *top* :) Yes, and we even already have the precedent from i915_reg_t. > > > > With the checkpatch error addressed and the improved commit message: > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > thanks > Lucas De Marchi > > > > bool intel_dpll_is_combophy(enum intel_dpll_id id) > > > @@ -2925,7 +2925,10 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > > > ret = icl_calc_dpll_state(crtc_state, encoder, clock, > > > &pll_state); > > > } else { > > > - min = icl_port_to_mg_pll_id(port); > > > + enum tc_port tc_port; > > > + > > > + tc_port = intel_port_to_tc(dev_priv, port); > > > + min = icl_tc_port_to_pll_id(tc_port); > > > max = min; > > > ret = icl_calc_mg_pll_state(crtc_state, encoder, clock, > > > &pll_state); > > > @@ -2959,12 +2962,8 @@ static i915_reg_t icl_pll_id_to_enable_reg(enum intel_dpll_id id) > > > return CNL_DPLL_ENABLE(id); > > > else if (id == DPLL_ID_ICL_TBTPLL) > > > return TBT_PLL_ENABLE; > > > - else > > > - /* > > > - * TODO: Make MG_PLL macros use > > > - * tc port id instead of port id > > > - */ > > > - return MG_PLL_ENABLE(icl_mg_pll_id_to_port(id)); > > > + > > > + return MG_PLL_ENABLE(icl_pll_id_to_tc_port(id)); > > > } > > > > > > static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, > > > @@ -2974,7 +2973,6 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, > > > const enum intel_dpll_id id = pll->info->id; > > > intel_wakeref_t wakeref; > > > bool ret = false; > > > - enum port port; > > > u32 val; > > > > > > wakeref = intel_display_power_get_if_enabled(dev_priv, > > > @@ -2991,32 +2989,33 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv, > > > hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id)); > > > hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id)); > > > } else { > > > - port = icl_mg_pll_id_to_port(id); > > > - hw_state->mg_refclkin_ctl = I915_READ(MG_REFCLKIN_CTL(port)); > > > + enum tc_port tc_port = icl_pll_id_to_tc_port(id); > > > + > > > + hw_state->mg_refclkin_ctl = I915_READ(MG_REFCLKIN_CTL(tc_port)); > > > hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK; > > > > > > hw_state->mg_clktop2_coreclkctl1 = > > > - I915_READ(MG_CLKTOP2_CORECLKCTL1(port)); > > > + I915_READ(MG_CLKTOP2_CORECLKCTL1(tc_port)); > > > hw_state->mg_clktop2_coreclkctl1 &= > > > MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK; > > > > > > hw_state->mg_clktop2_hsclkctl = > > > - I915_READ(MG_CLKTOP2_HSCLKCTL(port)); > > > + I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); > > > hw_state->mg_clktop2_hsclkctl &= > > > MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK | > > > MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK | > > > MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK | > > > MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK; > > > > > > - hw_state->mg_pll_div0 = I915_READ(MG_PLL_DIV0(port)); > > > - hw_state->mg_pll_div1 = I915_READ(MG_PLL_DIV1(port)); > > > - hw_state->mg_pll_lf = I915_READ(MG_PLL_LF(port)); > > > - hw_state->mg_pll_frac_lock = I915_READ(MG_PLL_FRAC_LOCK(port)); > > > - hw_state->mg_pll_ssc = I915_READ(MG_PLL_SSC(port)); > > > + hw_state->mg_pll_div0 = I915_READ(MG_PLL_DIV0(tc_port)); > > > + hw_state->mg_pll_div1 = I915_READ(MG_PLL_DIV1(tc_port)); > > > + hw_state->mg_pll_lf = I915_READ(MG_PLL_LF(tc_port)); > > > + hw_state->mg_pll_frac_lock = I915_READ(MG_PLL_FRAC_LOCK(tc_port)); > > > + hw_state->mg_pll_ssc = I915_READ(MG_PLL_SSC(tc_port)); > > > > > > - hw_state->mg_pll_bias = I915_READ(MG_PLL_BIAS(port)); > > > + hw_state->mg_pll_bias = I915_READ(MG_PLL_BIAS(tc_port)); > > > hw_state->mg_pll_tdc_coldst_bias = > > > - I915_READ(MG_PLL_TDC_COLDST_BIAS(port)); > > > + I915_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); > > > > > > if (dev_priv->cdclk.hw.ref == 38400) { > > > hw_state->mg_pll_tdc_coldst_bias_mask = MG_PLL_TDC_COLDST_COLDSTART; > > > @@ -3051,7 +3050,7 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv, > > > struct intel_shared_dpll *pll) > > > { > > > struct intel_dpll_hw_state *hw_state = &pll->state.hw_state; > > > - enum port port = icl_mg_pll_id_to_port(pll->info->id); > > > + enum tc_port tc_port = icl_pll_id_to_tc_port(pll->info->id); > > > u32 val; > > > > > > /* > > > @@ -3060,41 +3059,41 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv, > > > * during the calc/readout phase if the mask depends on some other HW > > > * state like refclk, see icl_calc_mg_pll_state(). > > > */ > > > - val = I915_READ(MG_REFCLKIN_CTL(port)); > > > + val = I915_READ(MG_REFCLKIN_CTL(tc_port)); > > > val &= ~MG_REFCLKIN_CTL_OD_2_MUX_MASK; > > > val |= hw_state->mg_refclkin_ctl; > > > - I915_WRITE(MG_REFCLKIN_CTL(port), val); > > > + I915_WRITE(MG_REFCLKIN_CTL(tc_port), val); > > > > > > - val = I915_READ(MG_CLKTOP2_CORECLKCTL1(port)); > > > + val = I915_READ(MG_CLKTOP2_CORECLKCTL1(tc_port)); > > > val &= ~MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK; > > > val |= hw_state->mg_clktop2_coreclkctl1; > > > - I915_WRITE(MG_CLKTOP2_CORECLKCTL1(port), val); > > > + I915_WRITE(MG_CLKTOP2_CORECLKCTL1(tc_port), val); > > > > > > - val = I915_READ(MG_CLKTOP2_HSCLKCTL(port)); > > > + val = I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port)); > > > val &= ~(MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK | > > > MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK | > > > MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK | > > > MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK); > > > val |= hw_state->mg_clktop2_hsclkctl; > > > - I915_WRITE(MG_CLKTOP2_HSCLKCTL(port), val); > > > + I915_WRITE(MG_CLKTOP2_HSCLKCTL(tc_port), val); > > > > > > - I915_WRITE(MG_PLL_DIV0(port), hw_state->mg_pll_div0); > > > - I915_WRITE(MG_PLL_DIV1(port), hw_state->mg_pll_div1); > > > - I915_WRITE(MG_PLL_LF(port), hw_state->mg_pll_lf); > > > - I915_WRITE(MG_PLL_FRAC_LOCK(port), hw_state->mg_pll_frac_lock); > > > - I915_WRITE(MG_PLL_SSC(port), hw_state->mg_pll_ssc); > > > + I915_WRITE(MG_PLL_DIV0(tc_port), hw_state->mg_pll_div0); > > > + I915_WRITE(MG_PLL_DIV1(tc_port), hw_state->mg_pll_div1); > > > + I915_WRITE(MG_PLL_LF(tc_port), hw_state->mg_pll_lf); > > > + I915_WRITE(MG_PLL_FRAC_LOCK(tc_port), hw_state->mg_pll_frac_lock); > > > + I915_WRITE(MG_PLL_SSC(tc_port), hw_state->mg_pll_ssc); > > > > > > - val = I915_READ(MG_PLL_BIAS(port)); > > > + val = I915_READ(MG_PLL_BIAS(tc_port)); > > > val &= ~hw_state->mg_pll_bias_mask; > > > val |= hw_state->mg_pll_bias; > > > - I915_WRITE(MG_PLL_BIAS(port), val); > > > + I915_WRITE(MG_PLL_BIAS(tc_port), val); > > > > > > - val = I915_READ(MG_PLL_TDC_COLDST_BIAS(port)); > > > + val = I915_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); > > > val &= ~hw_state->mg_pll_tdc_coldst_bias_mask; > > > val |= hw_state->mg_pll_tdc_coldst_bias; > > > - I915_WRITE(MG_PLL_TDC_COLDST_BIAS(port), val); > > > + I915_WRITE(MG_PLL_TDC_COLDST_BIAS(tc_port), val); > > > > > > - POSTING_READ(MG_PLL_TDC_COLDST_BIAS(port)); > > > + POSTING_READ(MG_PLL_TDC_COLDST_BIAS(tc_port)); > > > } > > > > > > static void icl_pll_enable(struct drm_i915_private *dev_priv, > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h > > > index e96e79413b54..40e8391a92f2 100644 > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h > > > @@ -344,7 +344,7 @@ void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv, > > > int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv, > > > u32 pll_id); > > > int cnl_hdmi_pll_ref_clock(struct drm_i915_private *dev_priv); > > > -enum intel_dpll_id icl_port_to_mg_pll_id(enum port port); > > > +enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port); > > > bool intel_dpll_is_combophy(enum intel_dpll_id id); > > > > > > #endif /* _INTEL_DPLL_MGR_H_ */ > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] drm/i915: always return something 2019-01-17 20:21 [PATCH 0/5] icl: Misc PLL patches Lucas De Marchi 2019-01-17 20:21 ` [PATCH 1/5] drm/i915/icl: use tc_port in MG_PLL macros Lucas De Marchi @ 2019-01-17 20:21 ` Lucas De Marchi 2019-01-22 12:01 ` Kahola, Mika 2019-01-23 14:19 ` Joonas Lahtinen 2019-01-17 20:21 ` [PATCH 3/5] drm/i915/icl: remove dpll from clk_sel Lucas De Marchi ` (6 subsequent siblings) 8 siblings, 2 replies; 22+ messages in thread From: Lucas De Marchi @ 2019-01-17 20:21 UTC (permalink / raw) To: intel-gfx; +Cc: Lucas De Marchi, Paulo Zanoni, stable Even if we don't have the correct clock and get a warning, we should not skip the return. Fixes: 1fa11ee2d9d0 ("drm/i915/icl: start adding the TBT pll") Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: <stable@vger.kernel.org> # v4.19+ Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 8dbf6c9e22fb..4dc03e8c6c10 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1021,7 +1021,7 @@ static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder, return DDI_CLK_SEL_TBT_810; default: MISSING_CASE(clock); - break; + return DDI_CLK_SEL_NONE; } case DPLL_ID_ICL_MGPLL1: case DPLL_ID_ICL_MGPLL2: -- 2.20.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Intel-gfx] [PATCH 2/5] drm/i915: always return something 2019-01-17 20:21 ` [PATCH 2/5] drm/i915: always return something Lucas De Marchi @ 2019-01-22 12:01 ` Kahola, Mika 2019-01-23 14:19 ` Joonas Lahtinen 1 sibling, 0 replies; 22+ messages in thread From: Kahola, Mika @ 2019-01-22 12:01 UTC (permalink / raw) To: intel-gfx, De Marchi, Lucas; +Cc: stable, Zanoni, Paulo R Patch look ok to me. On Thu, 2019-01-17 at 12:21 -0800, Lucas De Marchi wrote: > Even if we don't have the correct clock and get a warning, we should > not > skip the return. > > Fixes: 1fa11ee2d9d0 ("drm/i915/icl: start adding the TBT pll") > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: <stable@vger.kernel.org> # v4.19+ Reviewed-by: Mika Kahola <mika.kahola@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 8dbf6c9e22fb..4dc03e8c6c10 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1021,7 +1021,7 @@ static uint32_t icl_pll_to_ddi_pll_sel(struct > intel_encoder *encoder, > return DDI_CLK_SEL_TBT_810; > default: > MISSING_CASE(clock); > - break; > + return DDI_CLK_SEL_NONE; > } > case DPLL_ID_ICL_MGPLL1: > case DPLL_ID_ICL_MGPLL2: ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/5] drm/i915: always return something @ 2019-01-22 12:01 ` Kahola, Mika 0 siblings, 0 replies; 22+ messages in thread From: Kahola, Mika @ 2019-01-22 12:01 UTC (permalink / raw) To: intel-gfx, De Marchi, Lucas; +Cc: Zanoni, Paulo R, stable Patch look ok to me. On Thu, 2019-01-17 at 12:21 -0800, Lucas De Marchi wrote: > Even if we don't have the correct clock and get a warning, we should > not > skip the return. > > Fixes: 1fa11ee2d9d0 ("drm/i915/icl: start adding the TBT pll") > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: <stable@vger.kernel.org> # v4.19+ Reviewed-by: Mika Kahola <mika.kahola@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 8dbf6c9e22fb..4dc03e8c6c10 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1021,7 +1021,7 @@ static uint32_t icl_pll_to_ddi_pll_sel(struct > intel_encoder *encoder, > return DDI_CLK_SEL_TBT_810; > default: > MISSING_CASE(clock); > - break; > + return DDI_CLK_SEL_NONE; > } > case DPLL_ID_ICL_MGPLL1: > case DPLL_ID_ICL_MGPLL2: _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Intel-gfx] [PATCH 2/5] drm/i915: always return something 2019-01-17 20:21 ` [PATCH 2/5] drm/i915: always return something Lucas De Marchi @ 2019-01-23 14:19 ` Joonas Lahtinen 2019-01-23 14:19 ` Joonas Lahtinen 1 sibling, 0 replies; 22+ messages in thread From: Joonas Lahtinen @ 2019-01-23 14:19 UTC (permalink / raw) To: Lucas De Marchi, intel-gfx; +Cc: Lucas De Marchi, Paulo Zanoni, stable The subject of this patch could really be bit more specific "... on DDI clock selection". Regards, Joonas Quoting Lucas De Marchi (2019-01-17 22:21:10) > Even if we don't have the correct clock and get a warning, we should not > skip the return. > > Fixes: 1fa11ee2d9d0 ("drm/i915/icl: start adding the TBT pll") > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: <stable@vger.kernel.org> # v4.19+ > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 8dbf6c9e22fb..4dc03e8c6c10 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1021,7 +1021,7 @@ static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder, > return DDI_CLK_SEL_TBT_810; > default: > MISSING_CASE(clock); > - break; > + return DDI_CLK_SEL_NONE; > } > case DPLL_ID_ICL_MGPLL1: > case DPLL_ID_ICL_MGPLL2: > -- > 2.20.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Intel-gfx] [PATCH 2/5] drm/i915: always return something @ 2019-01-23 14:19 ` Joonas Lahtinen 0 siblings, 0 replies; 22+ messages in thread From: Joonas Lahtinen @ 2019-01-23 14:19 UTC (permalink / raw) To: intel-gfx; +Cc: Lucas De Marchi, Paulo Zanoni, stable The subject of this patch could really be bit more specific "... on DDI clock selection". Regards, Joonas Quoting Lucas De Marchi (2019-01-17 22:21:10) > Even if we don't have the correct clock and get a warning, we should not > skip the return. > > Fixes: 1fa11ee2d9d0 ("drm/i915/icl: start adding the TBT pll") > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: <stable@vger.kernel.org> # v4.19+ > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 8dbf6c9e22fb..4dc03e8c6c10 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1021,7 +1021,7 @@ static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder, > return DDI_CLK_SEL_TBT_810; > default: > MISSING_CASE(clock); > - break; > + return DDI_CLK_SEL_NONE; > } > case DPLL_ID_ICL_MGPLL1: > case DPLL_ID_ICL_MGPLL2: > -- > 2.20.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Intel-gfx] [PATCH 2/5] drm/i915: always return something 2019-01-23 14:19 ` Joonas Lahtinen (?) @ 2019-01-23 18:01 ` Lucas De Marchi -1 siblings, 0 replies; 22+ messages in thread From: Lucas De Marchi @ 2019-01-23 18:01 UTC (permalink / raw) To: Joonas Lahtinen; +Cc: intel-gfx, Paulo Zanoni, stable On Wed, Jan 23, 2019 at 04:19:30PM +0200, Joonas Lahtinen wrote: >The subject of this patch could really be bit more specific "... on DDI clock >selection". Fixed. I'll wait for reviews on other patches to re-spin this series as it's already conflicting. thanks Lucas De Marchi > >Regards, Joonas > >Quoting Lucas De Marchi (2019-01-17 22:21:10) >> Even if we don't have the correct clock and get a warning, we should not >> skip the return. >> >> Fixes: 1fa11ee2d9d0 ("drm/i915/icl: start adding the TBT pll") >> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> Cc: <stable@vger.kernel.org> # v4.19+ >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> --- >> drivers/gpu/drm/i915/intel_ddi.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c >> index 8dbf6c9e22fb..4dc03e8c6c10 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -1021,7 +1021,7 @@ static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder, >> return DDI_CLK_SEL_TBT_810; >> default: >> MISSING_CASE(clock); >> - break; >> + return DDI_CLK_SEL_NONE; >> } >> case DPLL_ID_ICL_MGPLL1: >> case DPLL_ID_ICL_MGPLL2: >> -- >> 2.20.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/5] drm/i915/icl: remove dpll from clk_sel 2019-01-17 20:21 [PATCH 0/5] icl: Misc PLL patches Lucas De Marchi 2019-01-17 20:21 ` [PATCH 1/5] drm/i915/icl: use tc_port in MG_PLL macros Lucas De Marchi 2019-01-17 20:21 ` [PATCH 2/5] drm/i915: always return something Lucas De Marchi @ 2019-01-17 20:21 ` Lucas De Marchi 2019-01-24 1:31 ` Paulo Zanoni 2019-01-17 20:21 ` [PATCH 4/5] drm/i915/icl: keep track of unused pll while looping Lucas De Marchi ` (5 subsequent siblings) 8 siblings, 1 reply; 22+ messages in thread From: Lucas De Marchi @ 2019-01-17 20:21 UTC (permalink / raw) To: intel-gfx; +Cc: Lucas De Marchi We should not pass DPLL_ID_ICL_DPLL0 or DPLL_ID_ICL_DPLL1 to this function because the path is only taken for non-combophy ports. Let the warning trigger if improper value is given. While at it, rename the function to match the register name we are trying to program. Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 4dc03e8c6c10..94d0fdc14b42 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -995,7 +995,7 @@ static uint32_t hsw_pll_to_ddi_pll_sel(const struct intel_shared_dpll *pll) } } -static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder, +static uint32_t icl_pll_to_ddi_clk_sel(struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state) { const struct intel_shared_dpll *pll = crtc_state->shared_dpll; @@ -1004,10 +1004,11 @@ static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder, switch (id) { default: + /* + * DPLL_ID_ICL_DPLL0 and DPLL_ID_ICL_DPLL1 should not be use + * here, so do warn if this get passed in + */ MISSING_CASE(id); - /* fall through */ - case DPLL_ID_ICL_DPLL0: - case DPLL_ID_ICL_DPLL1: return DDI_CLK_SEL_NONE; case DPLL_ID_ICL_TBTPLL: switch (clock) { @@ -2869,7 +2870,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder, if (IS_ICELAKE(dev_priv)) { if (!intel_port_is_combophy(dev_priv, port)) I915_WRITE(DDI_CLK_SEL(port), - icl_pll_to_ddi_pll_sel(encoder, crtc_state)); + icl_pll_to_ddi_clk_sel(encoder, crtc_state)); } else if (IS_CANNONLAKE(dev_priv)) { /* Configure DPCLKA_CFGCR0 to map the DPLL to the DDI. */ val = I915_READ(DPCLKA_CFGCR0); -- 2.20.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 3/5] drm/i915/icl: remove dpll from clk_sel 2019-01-17 20:21 ` [PATCH 3/5] drm/i915/icl: remove dpll from clk_sel Lucas De Marchi @ 2019-01-24 1:31 ` Paulo Zanoni 0 siblings, 0 replies; 22+ messages in thread From: Paulo Zanoni @ 2019-01-24 1:31 UTC (permalink / raw) To: Lucas De Marchi, intel-gfx Em qui, 2019-01-17 às 12:21 -0800, Lucas De Marchi escreveu: > We should not pass DPLL_ID_ICL_DPLL0 or DPLL_ID_ICL_DPLL1 to this > function because the path is only taken for non-combophy ports. Let the > warning trigger if improper value is given. > > While at it, rename the function to match the register name we are > trying to program. > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 4dc03e8c6c10..94d0fdc14b42 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -995,7 +995,7 @@ static uint32_t hsw_pll_to_ddi_pll_sel(const struct intel_shared_dpll *pll) > } > } > > -static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder, > +static uint32_t icl_pll_to_ddi_clk_sel(struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state) > { > const struct intel_shared_dpll *pll = crtc_state->shared_dpll; > @@ -1004,10 +1004,11 @@ static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder, > > switch (id) { > default: > + /* > + * DPLL_ID_ICL_DPLL0 and DPLL_ID_ICL_DPLL1 should not be use s/use/used/ With that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > + * here, so do warn if this get passed in > + */ > MISSING_CASE(id); > - /* fall through */ > - case DPLL_ID_ICL_DPLL0: > - case DPLL_ID_ICL_DPLL1: > return DDI_CLK_SEL_NONE; > case DPLL_ID_ICL_TBTPLL: > switch (clock) { > @@ -2869,7 +2870,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder, > if (IS_ICELAKE(dev_priv)) { > if (!intel_port_is_combophy(dev_priv, port)) > I915_WRITE(DDI_CLK_SEL(port), > - icl_pll_to_ddi_pll_sel(encoder, crtc_state)); > + icl_pll_to_ddi_clk_sel(encoder, crtc_state)); > } else if (IS_CANNONLAKE(dev_priv)) { > /* Configure DPCLKA_CFGCR0 to map the DPLL to the DDI. */ > val = I915_READ(DPCLKA_CFGCR0); _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] drm/i915/icl: keep track of unused pll while looping 2019-01-17 20:21 [PATCH 0/5] icl: Misc PLL patches Lucas De Marchi ` (2 preceding siblings ...) 2019-01-17 20:21 ` [PATCH 3/5] drm/i915/icl: remove dpll from clk_sel Lucas De Marchi @ 2019-01-17 20:21 ` Lucas De Marchi 2019-01-24 1:38 ` Paulo Zanoni 2019-01-17 20:21 ` [PATCH 5/5] drm/i915: allow shared plls to be non-consecutive Lucas De Marchi ` (4 subsequent siblings) 8 siblings, 1 reply; 22+ messages in thread From: Lucas De Marchi @ 2019-01-17 20:21 UTC (permalink / raw) To: intel-gfx; +Cc: Lucas De Marchi Instead of looping again on the range of plls, just keep track of one unused one and use it later. Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/intel_dpll_mgr.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 211b3ffa5bed..8f70838ac7d8 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -247,7 +247,7 @@ intel_find_shared_dpll(struct intel_crtc *crtc, enum intel_dpll_id range_max) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - struct intel_shared_dpll *pll; + struct intel_shared_dpll *pll, *unused_pll = NULL; struct intel_shared_dpll_state *shared_dpll; enum intel_dpll_id i; @@ -257,8 +257,10 @@ intel_find_shared_dpll(struct intel_crtc *crtc, pll = &dev_priv->shared_dplls[i]; /* Only want to check enabled timings first */ - if (shared_dpll[i].crtc_mask == 0) + if (shared_dpll[i].crtc_mask == 0) { + unused_pll = pll; continue; + } if (memcmp(&crtc_state->dpll_hw_state, &shared_dpll[i].hw_state, @@ -273,14 +275,11 @@ intel_find_shared_dpll(struct intel_crtc *crtc, } /* Ok no matching timings, maybe there's a free one? */ - for (i = range_min; i <= range_max; i++) { - pll = &dev_priv->shared_dplls[i]; - if (shared_dpll[i].crtc_mask == 0) { - DRM_DEBUG_KMS("[CRTC:%d:%s] allocated %s\n", - crtc->base.base.id, crtc->base.name, - pll->info->name); - return pll; - } + if (unused_pll) { + DRM_DEBUG_KMS("[CRTC:%d:%s] allocated %s\n", + crtc->base.base.id, crtc->base.name, + unused_pll->info->name); + return unused_pll; } return NULL; -- 2.20.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 4/5] drm/i915/icl: keep track of unused pll while looping 2019-01-17 20:21 ` [PATCH 4/5] drm/i915/icl: keep track of unused pll while looping Lucas De Marchi @ 2019-01-24 1:38 ` Paulo Zanoni 0 siblings, 0 replies; 22+ messages in thread From: Paulo Zanoni @ 2019-01-24 1:38 UTC (permalink / raw) To: Lucas De Marchi, intel-gfx Em qui, 2019-01-17 às 12:21 -0800, Lucas De Marchi escreveu: > Instead of looping again on the range of plls, just keep track of one > unused one and use it later. > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index 211b3ffa5bed..8f70838ac7d8 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -247,7 +247,7 @@ intel_find_shared_dpll(struct intel_crtc *crtc, > enum intel_dpll_id range_max) > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > - struct intel_shared_dpll *pll; > + struct intel_shared_dpll *pll, *unused_pll = NULL; > struct intel_shared_dpll_state *shared_dpll; > enum intel_dpll_id i; > > @@ -257,8 +257,10 @@ intel_find_shared_dpll(struct intel_crtc *crtc, > pll = &dev_priv->shared_dplls[i]; > > /* Only want to check enabled timings first */ > - if (shared_dpll[i].crtc_mask == 0) > + if (shared_dpll[i].crtc_mask == 0) { > + unused_pll = pll; > continue; > + } > > if (memcmp(&crtc_state->dpll_hw_state, > &shared_dpll[i].hw_state, > @@ -273,14 +275,11 @@ intel_find_shared_dpll(struct intel_crtc *crtc, > } > > /* Ok no matching timings, maybe there's a free one? */ > - for (i = range_min; i <= range_max; i++) { > - pll = &dev_priv->shared_dplls[i]; > - if (shared_dpll[i].crtc_mask == 0) { > - DRM_DEBUG_KMS("[CRTC:%d:%s] allocated %s\n", > - crtc->base.base.id, crtc->base.name, > - pll->info->name); > - return pll; > - } > + if (unused_pll) { > + DRM_DEBUG_KMS("[CRTC:%d:%s] allocated %s\n", > + crtc->base.base.id, crtc->base.name, > + unused_pll->info->name); > + return unused_pll; > } > > return NULL; _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 5/5] drm/i915: allow shared plls to be non-consecutive 2019-01-17 20:21 [PATCH 0/5] icl: Misc PLL patches Lucas De Marchi ` (3 preceding siblings ...) 2019-01-17 20:21 ` [PATCH 4/5] drm/i915/icl: keep track of unused pll while looping Lucas De Marchi @ 2019-01-17 20:21 ` Lucas De Marchi 2019-01-17 22:06 ` Lucas De Marchi 2019-01-17 20:38 ` ✗ Fi.CI.CHECKPATCH: warning for icl: Misc PLL patches Patchwork ` (3 subsequent siblings) 8 siblings, 1 reply; 22+ messages in thread From: Lucas De Marchi @ 2019-01-17 20:21 UTC (permalink / raw) To: intel-gfx; +Cc: Lucas De Marchi Right now when searching for shared plls we mandate that they have consecutive IDs since we just pass the min and max id and loop over the range. However the IDs can't be arbitrarily defined since they are used as index to the MMIO address, hence dependent on what the hardware implements. This allows us to use PLLs that are not consecutive (although we don't currently have any case) while clarifying the code paths in which only one PLL is supposed to be used. Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/intel_dpll_mgr.c | 41 ++++++++++++--------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 8f70838ac7d8..103e42cfa2e3 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -243,8 +243,7 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state) static struct intel_shared_dpll * intel_find_shared_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, - enum intel_dpll_id range_min, - enum intel_dpll_id range_max) + unsigned long pll_mask) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); struct intel_shared_dpll *pll, *unused_pll = NULL; @@ -253,7 +252,7 @@ intel_find_shared_dpll(struct intel_crtc *crtc, shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state); - for (i = range_min; i <= range_max; i++) { + for_each_set_bit(i, &pll_mask, I915_NUM_PLLS) { pll = &dev_priv->shared_dplls[i]; /* Only want to check enabled timings first */ @@ -436,8 +435,8 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, pll->info->name); } else { pll = intel_find_shared_dpll(crtc, crtc_state, - DPLL_ID_PCH_PLL_A, - DPLL_ID_PCH_PLL_B); + GENMASK(DPLL_ID_PCH_PLL_B, + DPLL_ID_PCH_PLL_A)); } if (!pll) @@ -780,7 +779,7 @@ static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(int clock, crtc_state->dpll_hw_state.wrpll = val; pll = intel_find_shared_dpll(crtc, crtc_state, - DPLL_ID_WRPLL1, DPLL_ID_WRPLL2); + GENMASK(DPLL_ID_WRPLL2, DPLL_ID_WRPLL1)); if (!pll) return NULL; @@ -840,7 +839,7 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC; pll = intel_find_shared_dpll(crtc, crtc_state, - DPLL_ID_SPLL, DPLL_ID_SPLL); + BIT(DPLL_ID_SPLL)); } else { return NULL; } @@ -1389,6 +1388,7 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, int clock = crtc_state->port_clock; bool bret; struct intel_dpll_hw_state dpll_hw_state; + unsigned long pll_mask; memset(&dpll_hw_state, 0, sizeof(dpll_hw_state)); @@ -1410,13 +1410,11 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, } if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) - pll = intel_find_shared_dpll(crtc, crtc_state, - DPLL_ID_SKL_DPLL0, - DPLL_ID_SKL_DPLL0); + pll_mask = BIT(DPLL_ID_SKL_DPLL0); else - pll = intel_find_shared_dpll(crtc, crtc_state, - DPLL_ID_SKL_DPLL1, - DPLL_ID_SKL_DPLL3); + pll_mask = GENMASK(DPLL_ID_SKL_DPLL3, DPLL_ID_SKL_DPLL1); + + pll = intel_find_shared_dpll(crtc, crtc_state, pll_mask); if (!pll) return NULL; @@ -2390,8 +2388,8 @@ cnl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, } pll = intel_find_shared_dpll(crtc, crtc_state, - DPLL_ID_SKL_DPLL0, - DPLL_ID_SKL_DPLL2); + GENMASK(DPLL_ID_SKL_DPLL2, + DPLL_ID_SKL_DPLL0)); if (!pll) { DRM_DEBUG_KMS("No PLL selected\n"); return NULL; @@ -2899,13 +2897,12 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, struct intel_shared_dpll *pll; struct intel_dpll_hw_state pll_state = {}; enum port port = encoder->port; - enum intel_dpll_id min, max; + unsigned long pll_mask; int clock = crtc_state->port_clock; bool ret; if (intel_port_is_combophy(dev_priv, port)) { - min = DPLL_ID_ICL_DPLL0; - max = DPLL_ID_ICL_DPLL1; + pll_mask = GENMASK(DPLL_ID_ICL_DPLL1, DPLL_ID_ICL_DPLL0); ret = icl_calc_dpll_state(crtc_state, encoder, clock, &pll_state); } else if (intel_port_is_tc(dev_priv, port)) { @@ -2919,16 +2916,14 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, } if (intel_dig_port->tc_type == TC_PORT_TBT) { - min = DPLL_ID_ICL_TBTPLL; - max = min; + pll_mask = BIT(DPLL_ID_ICL_TBTPLL); ret = icl_calc_dpll_state(crtc_state, encoder, clock, &pll_state); } else { enum tc_port tc_port; tc_port = intel_port_to_tc(dev_priv, port); - min = icl_tc_port_to_pll_id(tc_port); - max = min; + pll_mask = BIT(icl_tc_port_to_pll_id(tc_port)); ret = icl_calc_mg_pll_state(crtc_state, encoder, clock, &pll_state); } @@ -2944,7 +2939,7 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, crtc_state->dpll_hw_state = pll_state; - pll = intel_find_shared_dpll(crtc, crtc_state, min, max); + pll = intel_find_shared_dpll(crtc, crtc_state, pll_mask); if (!pll) { DRM_DEBUG_KMS("No PLL selected\n"); return NULL; -- 2.20.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] drm/i915: allow shared plls to be non-consecutive 2019-01-17 20:21 ` [PATCH 5/5] drm/i915: allow shared plls to be non-consecutive Lucas De Marchi @ 2019-01-17 22:06 ` Lucas De Marchi 0 siblings, 0 replies; 22+ messages in thread From: Lucas De Marchi @ 2019-01-17 22:06 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx On Thu, Jan 17, 2019 at 12:21 PM Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > Right now when searching for shared plls we mandate that they have > consecutive IDs since we just pass the min and max id and loop over the > range. However the IDs can't be arbitrarily defined since they are used > as index to the MMIO address, hence dependent on what the hardware > implements. > > This allows us to use PLLs that are not consecutive (although we don't > currently have any case) while clarifying the code paths in which only > one PLL is supposed to be used. Other possible approach for if/when we need this would be to use a different macro that doesn't use the id as the index for the registers... Not sure which one is better. Lucas De Marchi > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 41 ++++++++++++--------------- > 1 file changed, 18 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index 8f70838ac7d8..103e42cfa2e3 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -243,8 +243,7 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state) > static struct intel_shared_dpll * > intel_find_shared_dpll(struct intel_crtc *crtc, > struct intel_crtc_state *crtc_state, > - enum intel_dpll_id range_min, > - enum intel_dpll_id range_max) > + unsigned long pll_mask) > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > struct intel_shared_dpll *pll, *unused_pll = NULL; > @@ -253,7 +252,7 @@ intel_find_shared_dpll(struct intel_crtc *crtc, > > shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state); > > - for (i = range_min; i <= range_max; i++) { > + for_each_set_bit(i, &pll_mask, I915_NUM_PLLS) { > pll = &dev_priv->shared_dplls[i]; > > /* Only want to check enabled timings first */ > @@ -436,8 +435,8 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > pll->info->name); > } else { > pll = intel_find_shared_dpll(crtc, crtc_state, > - DPLL_ID_PCH_PLL_A, > - DPLL_ID_PCH_PLL_B); > + GENMASK(DPLL_ID_PCH_PLL_B, > + DPLL_ID_PCH_PLL_A)); > } > > if (!pll) > @@ -780,7 +779,7 @@ static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(int clock, > crtc_state->dpll_hw_state.wrpll = val; > > pll = intel_find_shared_dpll(crtc, crtc_state, > - DPLL_ID_WRPLL1, DPLL_ID_WRPLL2); > + GENMASK(DPLL_ID_WRPLL2, DPLL_ID_WRPLL1)); > > if (!pll) > return NULL; > @@ -840,7 +839,7 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC; > > pll = intel_find_shared_dpll(crtc, crtc_state, > - DPLL_ID_SPLL, DPLL_ID_SPLL); > + BIT(DPLL_ID_SPLL)); > } else { > return NULL; > } > @@ -1389,6 +1388,7 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > int clock = crtc_state->port_clock; > bool bret; > struct intel_dpll_hw_state dpll_hw_state; > + unsigned long pll_mask; > > memset(&dpll_hw_state, 0, sizeof(dpll_hw_state)); > > @@ -1410,13 +1410,11 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > } > > if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) > - pll = intel_find_shared_dpll(crtc, crtc_state, > - DPLL_ID_SKL_DPLL0, > - DPLL_ID_SKL_DPLL0); > + pll_mask = BIT(DPLL_ID_SKL_DPLL0); > else > - pll = intel_find_shared_dpll(crtc, crtc_state, > - DPLL_ID_SKL_DPLL1, > - DPLL_ID_SKL_DPLL3); > + pll_mask = GENMASK(DPLL_ID_SKL_DPLL3, DPLL_ID_SKL_DPLL1); > + > + pll = intel_find_shared_dpll(crtc, crtc_state, pll_mask); > if (!pll) > return NULL; > > @@ -2390,8 +2388,8 @@ cnl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > } > > pll = intel_find_shared_dpll(crtc, crtc_state, > - DPLL_ID_SKL_DPLL0, > - DPLL_ID_SKL_DPLL2); > + GENMASK(DPLL_ID_SKL_DPLL2, > + DPLL_ID_SKL_DPLL0)); > if (!pll) { > DRM_DEBUG_KMS("No PLL selected\n"); > return NULL; > @@ -2899,13 +2897,12 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > struct intel_shared_dpll *pll; > struct intel_dpll_hw_state pll_state = {}; > enum port port = encoder->port; > - enum intel_dpll_id min, max; > + unsigned long pll_mask; > int clock = crtc_state->port_clock; > bool ret; > > if (intel_port_is_combophy(dev_priv, port)) { > - min = DPLL_ID_ICL_DPLL0; > - max = DPLL_ID_ICL_DPLL1; > + pll_mask = GENMASK(DPLL_ID_ICL_DPLL1, DPLL_ID_ICL_DPLL0); > ret = icl_calc_dpll_state(crtc_state, encoder, clock, > &pll_state); > } else if (intel_port_is_tc(dev_priv, port)) { > @@ -2919,16 +2916,14 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > } > > if (intel_dig_port->tc_type == TC_PORT_TBT) { > - min = DPLL_ID_ICL_TBTPLL; > - max = min; > + pll_mask = BIT(DPLL_ID_ICL_TBTPLL); > ret = icl_calc_dpll_state(crtc_state, encoder, clock, > &pll_state); > } else { > enum tc_port tc_port; > > tc_port = intel_port_to_tc(dev_priv, port); > - min = icl_tc_port_to_pll_id(tc_port); > - max = min; > + pll_mask = BIT(icl_tc_port_to_pll_id(tc_port)); > ret = icl_calc_mg_pll_state(crtc_state, encoder, clock, > &pll_state); > } > @@ -2944,7 +2939,7 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > > crtc_state->dpll_hw_state = pll_state; > > - pll = intel_find_shared_dpll(crtc, crtc_state, min, max); > + pll = intel_find_shared_dpll(crtc, crtc_state, pll_mask); > if (!pll) { > DRM_DEBUG_KMS("No PLL selected\n"); > return NULL; > -- > 2.20.0 > > _______________________________________________ > 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] 22+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for icl: Misc PLL patches 2019-01-17 20:21 [PATCH 0/5] icl: Misc PLL patches Lucas De Marchi ` (4 preceding siblings ...) 2019-01-17 20:21 ` [PATCH 5/5] drm/i915: allow shared plls to be non-consecutive Lucas De Marchi @ 2019-01-17 20:38 ` Patchwork 2019-01-17 21:02 ` ✓ Fi.CI.BAT: success " Patchwork ` (2 subsequent siblings) 8 siblings, 0 replies; 22+ messages in thread From: Patchwork @ 2019-01-17 20:38 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx == Series Details == Series: icl: Misc PLL patches URL : https://patchwork.freedesktop.org/series/55378/ State : warning == Summary == $ dim checkpatch origin/drm-tip bc90321eb832 drm/i915/icl: use tc_port in MG_PLL macros -:183: WARNING:LINE_SPACING: Missing a blank line after declarations #183: FILE: drivers/gpu/drm/i915/intel_display.c:9419: + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); + id = icl_tc_port_to_pll_id(tc_port); total: 0 errors, 1 warnings, 0 checks, 314 lines checked 6246a3763ba7 drm/i915: always return something 29f565fdc104 drm/i915/icl: remove dpll from clk_sel 27401a448f20 drm/i915/icl: keep track of unused pll while looping 12401b81e6c3 drm/i915: allow shared plls to be non-consecutive _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* ✓ Fi.CI.BAT: success for icl: Misc PLL patches 2019-01-17 20:21 [PATCH 0/5] icl: Misc PLL patches Lucas De Marchi ` (5 preceding siblings ...) 2019-01-17 20:38 ` ✗ Fi.CI.CHECKPATCH: warning for icl: Misc PLL patches Patchwork @ 2019-01-17 21:02 ` Patchwork 2019-01-18 6:05 ` ✓ Fi.CI.IGT: " Patchwork 2019-01-23 18:00 ` [PATCH 0/5] " Lucas De Marchi 8 siblings, 0 replies; 22+ messages in thread From: Patchwork @ 2019-01-17 21:02 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx == Series Details == Series: icl: Misc PLL patches URL : https://patchwork.freedesktop.org/series/55378/ State : success == Summary == CI Bug Log - changes from CI_DRM_5440 -> Patchwork_11972 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/55378/revisions/1/mbox/ Known issues ------------ Here are the changes found in Patchwork_11972 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@i915_selftest@live_hangcheck: - fi-skl-gvtdvm: PASS -> INCOMPLETE [fdo#105600] / [fdo#108744] * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence: - fi-byt-clapper: NOTRUN -> FAIL [fdo#103191] / [fdo#107362] #### Possible fixes #### * igt@kms_frontbuffer_tracking@basic: - fi-icl-u3: FAIL [fdo#103167] -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#105600]: https://bugs.freedesktop.org/show_bug.cgi?id=105600 [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362 [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278 Participating hosts (45 -> 42) ------------------------------ Additional (3): fi-kbl-7567u fi-glk-j4005 fi-byt-clapper Missing (6): fi-kbl-soraka fi-ilk-m540 fi-bsw-n3050 fi-byt-squawks fi-bsw-cyan fi-icl-y Build changes ------------- * Linux: CI_DRM_5440 -> Patchwork_11972 CI_DRM_5440: b36a89b5ab74fd49a4369e6df0d2c02bc464a474 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4777: 8614d5eb114a660c3bd7ff77eab8bed53424cd30 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_11972: 12401b81e6c363f85249d4e0d0060d94a27106f6 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 12401b81e6c3 drm/i915: allow shared plls to be non-consecutive 27401a448f20 drm/i915/icl: keep track of unused pll while looping 29f565fdc104 drm/i915/icl: remove dpll from clk_sel 6246a3763ba7 drm/i915: always return something bc90321eb832 drm/i915/icl: use tc_port in MG_PLL macros == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11972/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* ✓ Fi.CI.IGT: success for icl: Misc PLL patches 2019-01-17 20:21 [PATCH 0/5] icl: Misc PLL patches Lucas De Marchi ` (6 preceding siblings ...) 2019-01-17 21:02 ` ✓ Fi.CI.BAT: success " Patchwork @ 2019-01-18 6:05 ` Patchwork 2019-01-23 18:00 ` [PATCH 0/5] " Lucas De Marchi 8 siblings, 0 replies; 22+ messages in thread From: Patchwork @ 2019-01-18 6:05 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx == Series Details == Series: icl: Misc PLL patches URL : https://patchwork.freedesktop.org/series/55378/ State : success == Summary == CI Bug Log - changes from CI_DRM_5440_full -> Patchwork_11972_full ==================================================== Summary ------- **SUCCESS** No regressions found. Known issues ------------ Here are the changes found in Patchwork_11972_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_exec_schedule@pi-ringfull-vebox: - shard-skl: NOTRUN -> FAIL [fdo#103158] * igt@i915_suspend@shrink: - shard-iclb: NOTRUN -> DMESG-WARN [fdo#107886] / [fdo#109244] * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a: - shard-iclb: NOTRUN -> DMESG-WARN [fdo#107956] * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b: - shard-skl: NOTRUN -> DMESG-WARN [fdo#107956] +1 * igt@kms_cursor_crc@cursor-64x21-sliding: - shard-apl: PASS -> FAIL [fdo#103232] +6 * igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic: - shard-glk: PASS -> FAIL [fdo#104873] * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size: - shard-iclb: PASS -> FAIL [fdo#103355] * igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled: - shard-skl: PASS -> FAIL [fdo#103232] / [fdo#108472] * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-ytiled: - shard-skl: PASS -> FAIL [fdo#108222] * igt@kms_fbcon_fbt@fbc-suspend: - shard-skl: PASS -> INCOMPLETE [fdo#104108] / [fdo#107773] * igt@kms_flip@flip-vs-expired-vblank: - shard-skl: PASS -> FAIL [fdo#105363] * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc: - shard-apl: PASS -> FAIL [fdo#103167] * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-move: - shard-glk: PASS -> FAIL [fdo#103167] * igt@kms_frontbuffer_tracking@fbc-stridechange: - shard-skl: NOTRUN -> FAIL [fdo#105683] * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc: - shard-skl: NOTRUN -> FAIL [fdo#105682] +3 * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-gtt: - shard-iclb: PASS -> FAIL [fdo#103167] +4 * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen: - shard-iclb: NOTRUN -> FAIL [fdo#103167] * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-blt: - shard-skl: NOTRUN -> FAIL [fdo#103167] +1 * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: - shard-skl: NOTRUN -> FAIL [fdo#103191] / [fdo#107362] * igt@kms_plane@pixel-format-pipe-a-planes: - shard-glk: PASS -> FAIL [fdo#103166] * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping: - shard-skl: NOTRUN -> DMESG-WARN [fdo#106885] * igt@kms_plane@plane-panning-bottom-right-pipe-a-planes: - shard-skl: NOTRUN -> FAIL [fdo#103166] * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb: - shard-skl: NOTRUN -> FAIL [fdo#108145] +2 * igt@kms_plane_multiple@atomic-pipe-a-tiling-y: - shard-iclb: PASS -> FAIL [fdo#103166] +1 * igt@kms_plane_multiple@atomic-pipe-c-tiling-x: - shard-apl: PASS -> FAIL [fdo#103166] * igt@kms_setmode@basic: - shard-kbl: PASS -> FAIL [fdo#99912] * igt@pm_rpm@dpms-lpsp: - shard-skl: PASS -> INCOMPLETE [fdo#107807] #### Possible fixes #### * igt@debugfs_test@read_all_entries_display_off: - shard-skl: INCOMPLETE [fdo#104108] -> PASS * igt@kms_atomic_transition@plane-all-modeset-transition-internal-panels: - shard-iclb: DMESG-WARN [fdo#107724] -> PASS +2 * igt@kms_busy@extended-modeset-hang-newfb-render-c: - shard-iclb: DMESG-WARN [fdo#107956] -> PASS * igt@kms_color@pipe-a-gamma: - shard-skl: FAIL [fdo#104782] -> PASS * igt@kms_color@pipe-a-legacy-gamma: - shard-apl: FAIL [fdo#104782] / [fdo#108145] -> PASS * igt@kms_cursor_crc@cursor-128x128-suspend: - shard-apl: FAIL [fdo#103191] / [fdo#103232] -> PASS * igt@kms_cursor_crc@cursor-128x42-sliding: - shard-glk: FAIL [fdo#103232] -> PASS +2 * igt@kms_cursor_crc@cursor-64x21-random: - shard-apl: FAIL [fdo#103232] -> PASS +2 * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic: - shard-hsw: FAIL [fdo#105767] -> PASS * igt@kms_flip_event_leak: - shard-kbl: DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +4 * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite: - shard-iclb: FAIL [fdo#103167] -> PASS - shard-apl: FAIL [fdo#103167] -> PASS +1 * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen: - shard-glk: FAIL [fdo#103167] -> PASS * igt@kms_frontbuffer_tracking@fbc-2p-rte: - shard-glk: FAIL [fdo#103167] / [fdo#105682] -> PASS * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping: - shard-apl: FAIL [fdo#108948] -> PASS * igt@kms_plane@plane-panning-bottom-right-pipe-a-planes: - shard-iclb: DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +2 * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max: - shard-glk: FAIL [fdo#108145] -> PASS * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf: - shard-apl: FAIL [fdo#103166] -> PASS +2 * igt@pm_rc6_residency@rc6-accuracy: - shard-kbl: {SKIP} [fdo#109271] -> PASS #### Warnings #### * igt@kms_rotation_crc@multiplane-rotation-cropping-top: - shard-iclb: FAIL -> DMESG-FAIL [fdo#107724] {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158 [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166 [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232 [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355 [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558 [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108 [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782 [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873 [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363 [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602 [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682 [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683 [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767 [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885 [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362 [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724 [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773 [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807 [fdo#107886]: https://bugs.freedesktop.org/show_bug.cgi?id=107886 [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956 [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145 [fdo#108222]: https://bugs.freedesktop.org/show_bug.cgi?id=108222 [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336 [fdo#108472]: https://bugs.freedesktop.org/show_bug.cgi?id=108472 [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948 [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274 [fdo#109275]: https://bugs.freedesktop.org/show_bug.cgi?id=109275 [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276 [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278 [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279 [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280 [fdo#109281]: https://bugs.freedesktop.org/show_bug.cgi?id=109281 [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284 [fdo#109287]: https://bugs.freedesktop.org/show_bug.cgi?id=109287 [fdo#109290]: https://bugs.freedesktop.org/show_bug.cgi?id=109290 [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912 Participating hosts (7 -> 7) ------------------------------ No changes in participating hosts Build changes ------------- * Linux: CI_DRM_5440 -> Patchwork_11972 CI_DRM_5440: b36a89b5ab74fd49a4369e6df0d2c02bc464a474 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4777: 8614d5eb114a660c3bd7ff77eab8bed53424cd30 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_11972: 12401b81e6c363f85249d4e0d0060d94a27106f6 @ 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_11972/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/5] icl: Misc PLL patches 2019-01-17 20:21 [PATCH 0/5] icl: Misc PLL patches Lucas De Marchi ` (7 preceding siblings ...) 2019-01-18 6:05 ` ✓ Fi.CI.IGT: " Patchwork @ 2019-01-23 18:00 ` Lucas De Marchi 8 siblings, 0 replies; 22+ messages in thread From: Lucas De Marchi @ 2019-01-23 18:00 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni CC'ing people who have commits related to this series. Could you take a look on it? thanks Lucas De Marchi On Thu, Jan 17, 2019 at 12:21:08PM -0800, Lucas De Marchi wrote: >Some PLL reworks on ICL. Patches are more or less independent of each >other, but touch the same part of the code. > >Lucas De Marchi (5): > drm/i915/icl: use tc_port in MG_PLL macros > drm/i915: always return something > drm/i915/icl: remove dpll from clk_sel > drm/i915/icl: keep track of unused pll while looping > drm/i915: allow shared plls to be non-consecutive > > drivers/gpu/drm/i915/i915_reg.h | 52 +++++----- > drivers/gpu/drm/i915/intel_ddi.c | 20 ++-- > drivers/gpu/drm/i915/intel_display.c | 3 +- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 137 ++++++++++++-------------- > drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 +- > 5 files changed, 105 insertions(+), 109 deletions(-) > >-- >2.20.0 > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20190116235600.10687-1-lucas.demarchi@intel.com>]
* [PATCH 2/5] drm/i915: always return something [not found] <20190116235600.10687-1-lucas.demarchi@intel.com> @ 2019-01-16 23:55 ` Lucas De Marchi 0 siblings, 0 replies; 22+ messages in thread From: Lucas De Marchi @ 2019-01-16 23:55 UTC (permalink / raw) To: intel-gfx-trybot; +Cc: Paulo Zanoni, stable Even if we don't have the correct clock and get a warning, we should not skip the return. Fixes: 1fa11ee2d9d0 ("drm/i915/icl: start adding the TBT pll") Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: <stable@vger.kernel.org> # v4.19+ Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 8dbf6c9e22fb..4dc03e8c6c10 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1021,7 +1021,7 @@ static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder, return DDI_CLK_SEL_TBT_810; default: MISSING_CASE(clock); - break; + return DDI_CLK_SEL_NONE; } case DPLL_ID_ICL_MGPLL1: case DPLL_ID_ICL_MGPLL2: -- 2.20.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2019-01-24 20:40 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-17 20:21 [PATCH 0/5] icl: Misc PLL patches Lucas De Marchi
2019-01-17 20:21 ` [PATCH 1/5] drm/i915/icl: use tc_port in MG_PLL macros Lucas De Marchi
2019-01-24 1:15 ` Paulo Zanoni
2019-01-24 18:52 ` Lucas De Marchi
2019-01-24 20:40 ` Paulo Zanoni
2019-01-17 20:21 ` [PATCH 2/5] drm/i915: always return something Lucas De Marchi
2019-01-22 12:01 ` [Intel-gfx] " Kahola, Mika
2019-01-22 12:01 ` Kahola, Mika
2019-01-23 14:19 ` [Intel-gfx] " Joonas Lahtinen
2019-01-23 14:19 ` Joonas Lahtinen
2019-01-23 18:01 ` Lucas De Marchi
2019-01-17 20:21 ` [PATCH 3/5] drm/i915/icl: remove dpll from clk_sel Lucas De Marchi
2019-01-24 1:31 ` Paulo Zanoni
2019-01-17 20:21 ` [PATCH 4/5] drm/i915/icl: keep track of unused pll while looping Lucas De Marchi
2019-01-24 1:38 ` Paulo Zanoni
2019-01-17 20:21 ` [PATCH 5/5] drm/i915: allow shared plls to be non-consecutive Lucas De Marchi
2019-01-17 22:06 ` Lucas De Marchi
2019-01-17 20:38 ` ✗ Fi.CI.CHECKPATCH: warning for icl: Misc PLL patches Patchwork
2019-01-17 21:02 ` ✓ Fi.CI.BAT: success " Patchwork
2019-01-18 6:05 ` ✓ Fi.CI.IGT: " Patchwork
2019-01-23 18:00 ` [PATCH 0/5] " Lucas De Marchi
[not found] <20190116235600.10687-1-lucas.demarchi@intel.com>
2019-01-16 23:55 ` [PATCH 2/5] drm/i915: always return something Lucas De Marchi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.