| Message ID | 20190703230353.24059-1-vivek.kasireddy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/ehl: Add support for DPLL4 (v10) | expand |
On Fri, Jul 05, 2019 at 03:11:05AM -0000, Patchwork wrote: > == Series Details == > > Series: drm/i915/ehl: Add support for DPLL4 (v10) > URL : https://patchwork.freedesktop.org/series/63171/ > State : failure > > == Summary == > > CI Bug Log - changes from CI_DRM_6405_full -> Patchwork_13517_full > ==================================================== > > Summary > ------- > > **FAILURE** > > Serious unknown changes coming with Patchwork_13517_full absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_13517_full, please notify your bug team to allow them > to document this new failure mode, which will reduce false positives in CI. > > > > Possible new issues > ------------------- > > Here are the unknown changes that may have been introduced in Patchwork_13517_full: > > ### IGT changes ### > > #### Possible regressions #### > > * igt@gem_busy@close-race: > - shard-snb: [PASS][1] -> [DMESG-FAIL][2] > [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-snb4/igt@gem_busy@close-race.html > [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13517/shard-snb6/igt@gem_busy@close-race.html Some kind of gem fail. Nothing to do with this patch. <4> [602.084517] general protection fault: 0000 [#1] PREEMPT SMP PTI <4> [602.084530] CPU: 1 PID: 2824 Comm: gem_busy Tainted: G U 5.2.0-rc7-CI-Patchwork_13517+ #1 <4> [602.084542] Hardware name: Dell Inc. XPS 8300 /0Y2MRG, BIOS A06 10/17/2011 <4> [602.084611] RIP: 0010:i915_gem_busy_ioctl+0x136/0x5d0 [i915]
On Wed, Jul 03, 2019 at 04:03:53PM -0700, Vivek Kasireddy wrote: > This patch adds support for DPLL4 on EHL that include the > following restrictions: > > - DPLL4 cannot be used with DDIA (combo port A internal eDP usage). > DPLL4 can be used with other DDIs, including DDID > (combo port A external usage). > > - DPLL4 cannot be enabled when DC5 or DC6 are enabled. > > - The DPLL4 enable, lock, power enabled, and power state are connected > to the MGPLL1_ENABLE register. > > v2: (suggestions from Bob Paauwe) > - Rework ehl_get_dpll() function to call intel_find_shared_dpll() and > iterate twice: once for Combo plls and once for MG plls. > > - Use MG pll funcs for DPLL4 instead of creating new ones and modify > mg_pll_enable to include the restrictions for EHL. > > v3: Fix compilation error > > v4: (suggestions from Lucas and Ville) > - Treat DPLL4 as a combo phy PLL and not as MG PLL > - Disable DC states when this DPLL is being enabled > - Reuse icl_get_dpll instead of creating a separate one for EHL > > v5: (suggestion from Ville) > - Refcount the DC OFF power domains during the enabling and disabling > of this DPLL. > > v6: rebase > > v7: (suggestion from Imre) > - Add a new power domain instead of iterating over the domains > assoicated with DC OFF power well. > > v8: (Ville and Imre) > - Rename POWER_DOMAIN_DPLL4 TO POWER_DOMAIN_DPLL_DC_OFF > - Grab a reference in intel_modeset_setup_hw_state() if this > DPLL was already enabled perhaps by BIOS. > - Check for the port type instead of the encoder > > v9: (Ville) > - Move the block of code that grabs a reference to the power domain > POWER_DOMAIN_DPLL_DC_OFF to intel_modeset_readout_hw_state() to ensure > that there is a reference present before this DPLL might get disabled. > > v10: rebase > > Cc: José Roberto de Souza <jose.souza@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> lgtm. Picked up José's r-b from an earlier posting (in the future plese collect those yourself so they don't get lost), and pushed to dinq. Thanks for the patch.
On Wed, Jul 03, 2019 at 04:03:53PM -0700, Vivek Kasireddy wrote: > This patch adds support for DPLL4 on EHL that include the > following restrictions: > > - DPLL4 cannot be used with DDIA (combo port A internal eDP usage). > DPLL4 can be used with other DDIs, including DDID > (combo port A external usage). > > - DPLL4 cannot be enabled when DC5 or DC6 are enabled. > > - The DPLL4 enable, lock, power enabled, and power state are connected > to the MGPLL1_ENABLE register. > > v2: (suggestions from Bob Paauwe) > - Rework ehl_get_dpll() function to call intel_find_shared_dpll() and > iterate twice: once for Combo plls and once for MG plls. > > - Use MG pll funcs for DPLL4 instead of creating new ones and modify > mg_pll_enable to include the restrictions for EHL. > > v3: Fix compilation error > > v4: (suggestions from Lucas and Ville) > - Treat DPLL4 as a combo phy PLL and not as MG PLL > - Disable DC states when this DPLL is being enabled > - Reuse icl_get_dpll instead of creating a separate one for EHL > > v5: (suggestion from Ville) > - Refcount the DC OFF power domains during the enabling and disabling > of this DPLL. > > v6: rebase > > v7: (suggestion from Imre) > - Add a new power domain instead of iterating over the domains > assoicated with DC OFF power well. > > v8: (Ville and Imre) > - Rename POWER_DOMAIN_DPLL4 TO POWER_DOMAIN_DPLL_DC_OFF > - Grab a reference in intel_modeset_setup_hw_state() if this > DPLL was already enabled perhaps by BIOS. > - Check for the port type instead of the encoder > > v9: (Ville) > - Move the block of code that grabs a reference to the power domain > POWER_DOMAIN_DPLL_DC_OFF to intel_modeset_readout_hw_state() to ensure > that there is a reference present before this DPLL might get disabled. > > v10: rebase > > Cc: José Roberto de Souza <jose.souza@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 7 +++ > .../drm/i915/display/intel_display_power.c | 3 ++ > .../drm/i915/display/intel_display_power.h | 1 + > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 47 +++++++++++++++++-- > drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 6 +++ > 5 files changed, 60 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 919f5ac844c8..557462208462 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -16653,6 +16653,13 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > > pll->on = pll->info->funcs->get_hw_state(dev_priv, pll, > &pll->state.hw_state); > + > + if (IS_ELKHARTLAKE(dev_priv) && pll->on && > + pll->info->id == DPLL_ID_EHL_DPLL4) { > + pll->wakeref = intel_display_power_get(dev_priv, > + POWER_DOMAIN_DPLL_DC_OFF); > + } > + > pll->state.crtc_mask = 0; > for_each_intel_crtc(dev, crtc) { > struct intel_crtc_state *crtc_state = > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index c19b958461ca..7437fc71d289 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -118,6 +118,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > return "MODESET"; > case POWER_DOMAIN_GT_IRQ: > return "GT_IRQ"; > + case POWER_DOMAIN_DPLL_DC_OFF: > + return "DPLL_DC_OFF"; > default: > MISSING_CASE(domain); > return "?"; > @@ -2455,6 +2457,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > ICL_PW_2_POWER_DOMAINS | \ > BIT_ULL(POWER_DOMAIN_MODESET) | \ > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > + BIT_ULL(POWER_DOMAIN_DPLL_DC_OFF) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > > #define ICL_DDI_IO_A_POWER_DOMAINS ( \ > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > index ff57b0a7fe59..8f43f7051a16 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -59,6 +59,7 @@ enum intel_display_power_domain { > POWER_DOMAIN_GMBUS, > POWER_DOMAIN_MODESET, > POWER_DOMAIN_GT_IRQ, > + POWER_DOMAIN_DPLL_DC_OFF, > POWER_DOMAIN_INIT, > > POWER_DOMAIN_NUM, > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > index f953971e7c3b..67cfe836286e 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > @@ -2905,6 +2905,9 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state, > intel_atomic_get_new_crtc_state(state, crtc); > struct icl_port_dpll *port_dpll = > &crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT]; > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + enum port port = encoder->port; > + bool has_dpll4 = false; > > if (!icl_calc_dpll_state(crtc_state, encoder, &port_dpll->hw_state)) { > DRM_DEBUG_KMS("Could not calculate combo PHY PLL state.\n"); > @@ -2912,10 +2915,14 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state, > return false; > } > > + if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) > + has_dpll4 = true; > + > port_dpll->pll = intel_find_shared_dpll(state, crtc, > &port_dpll->hw_state, > DPLL_ID_ICL_DPLL0, > - DPLL_ID_ICL_DPLL1); > + has_dpll4 ? DPLL_ID_EHL_DPLL4 > + : DPLL_ID_ICL_DPLL1); > if (!port_dpll->pll) { > DRM_DEBUG_KMS("No combo PHY PLL found for port %c\n", > port_name(encoder->port)); > @@ -3119,8 +3126,14 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv, > struct intel_shared_dpll *pll, > struct intel_dpll_hw_state *hw_state) > { > - return icl_pll_get_hw_state(dev_priv, pll, hw_state, > - CNL_DPLL_ENABLE(pll->info->id)); > + i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > + > + if (IS_ELKHARTLAKE(dev_priv) && > + pll->info->id == DPLL_ID_EHL_DPLL4) { > + enable_reg = MG_PLL_ENABLE(0); > + } > + > + return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg); > } > > static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv, > @@ -3231,6 +3244,19 @@ static void combo_pll_enable(struct drm_i915_private *dev_priv, > { > i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > > + if (IS_ELKHARTLAKE(dev_priv) && > + pll->info->id == DPLL_ID_EHL_DPLL4) { > + enable_reg = MG_PLL_ENABLE(0); > + > + /* > + * We need to disable DC states when this DPLL is enabled. > + * This can be done by taking a reference on DPLL4 power > + * domain. > + */ > + pll->wakeref = intel_display_power_get(dev_priv, > + POWER_DOMAIN_DPLL_DC_OFF); > + } > + > icl_pll_power_enable(dev_priv, pll, enable_reg); > > icl_dpll_write(dev_priv, pll); > @@ -3326,7 +3352,19 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv, > static void combo_pll_disable(struct drm_i915_private *dev_priv, > struct intel_shared_dpll *pll) > { > - icl_pll_disable(dev_priv, pll, CNL_DPLL_ENABLE(pll->info->id)); > + i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > + > + if (IS_ELKHARTLAKE(dev_priv) && > + pll->info->id == DPLL_ID_EHL_DPLL4) { > + enable_reg = MG_PLL_ENABLE(0); > + icl_pll_disable(dev_priv, pll, enable_reg); > + > + intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF, > + pll->wakeref); > + return; > + } > + > + icl_pll_disable(dev_priv, pll, enable_reg); > } > > static void tbt_pll_disable(struct drm_i915_private *dev_priv, > @@ -3406,6 +3444,7 @@ static const struct intel_dpll_mgr icl_pll_mgr = { > static const struct dpll_info ehl_plls[] = { > { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 }, > { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 }, > + { "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 }, > { }, > }; > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h > index 1668f8116908..4c2c5e93aff3 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h > @@ -28,6 +28,7 @@ > #include <linux/types.h> > > #include "intel_display.h" > +#include "intel_wakeref.h" > > /*FIXME: Move this to a more appropriate place. */ > #define abs_diff(a, b) ({ \ > @@ -118,6 +119,10 @@ enum intel_dpll_id { > * @DPLL_ID_ICL_DPLL1: ICL combo PHY DPLL1 > */ > DPLL_ID_ICL_DPLL1 = 1, > + /** > + * @DPLL_ID_EHL_DPLL4: EHL combo PHY DPLL4 > + */ > + DPLL_ID_EHL_DPLL4 = 2, While looking at the TGL stuff I noticed that this ID seems to be wrong. We use this to generate the register offsets but the docs show the following: DPLL0_CFGCR0 0x164000 DPLL1_CFGCR0 0x164080 TBTPLL_CFGCR0 0x164100 DPLL4_CFGCR0 0x164200 So the DPLL4 accesses will now land on TBTPLL. The ID if DPLL4 should really be 4 I guess. > /** > * @DPLL_ID_ICL_TBTPLL: ICL TBT PLL > */ > @@ -320,6 +325,7 @@ struct intel_shared_dpll { > * @info: platform specific info > */ > const struct dpll_info *info; > + intel_wakeref_t wakeref; > }; > > #define SKL_DPLL0 0 > -- > 2.21.0
On Wed, 10 Jul 2019 21:47:52 +0300 Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: Hi Ville, > On Wed, Jul 03, 2019 at 04:03:53PM -0700, Vivek Kasireddy wrote: > > This patch adds support for DPLL4 on EHL that include the > > following restrictions: > > > > - DPLL4 cannot be used with DDIA (combo port A internal eDP usage). > > DPLL4 can be used with other DDIs, including DDID > > (combo port A external usage). > > > > - DPLL4 cannot be enabled when DC5 or DC6 are enabled. > > > > - The DPLL4 enable, lock, power enabled, and power state are > > connected to the MGPLL1_ENABLE register. > > > > v2: (suggestions from Bob Paauwe) > > - Rework ehl_get_dpll() function to call intel_find_shared_dpll() > > and iterate twice: once for Combo plls and once for MG plls. > > > > - Use MG pll funcs for DPLL4 instead of creating new ones and modify > > mg_pll_enable to include the restrictions for EHL. > > > > v3: Fix compilation error > > > > v4: (suggestions from Lucas and Ville) > > - Treat DPLL4 as a combo phy PLL and not as MG PLL > > - Disable DC states when this DPLL is being enabled > > - Reuse icl_get_dpll instead of creating a separate one for EHL > > > > v5: (suggestion from Ville) > > - Refcount the DC OFF power domains during the enabling and > > disabling of this DPLL. > > > > v6: rebase > > > > v7: (suggestion from Imre) > > - Add a new power domain instead of iterating over the domains > > assoicated with DC OFF power well. > > > > v8: (Ville and Imre) > > - Rename POWER_DOMAIN_DPLL4 TO POWER_DOMAIN_DPLL_DC_OFF > > - Grab a reference in intel_modeset_setup_hw_state() if this > > DPLL was already enabled perhaps by BIOS. > > - Check for the port type instead of the encoder > > > > v9: (Ville) > > - Move the block of code that grabs a reference to the power domain > > POWER_DOMAIN_DPLL_DC_OFF to intel_modeset_readout_hw_state() to > > ensure that there is a reference present before this DPLL might get > > disabled. > > > > v10: rebase > > > > Cc: José Roberto de Souza <jose.souza@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Matt Roper <matthew.d.roper@intel.com> > > Cc: Imre Deak <imre.deak@intel.com> > > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 7 +++ > > .../drm/i915/display/intel_display_power.c | 3 ++ > > .../drm/i915/display/intel_display_power.h | 1 + > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 47 > > +++++++++++++++++-- drivers/gpu/drm/i915/display/intel_dpll_mgr.h > > | 6 +++ 5 files changed, 60 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c index > > 919f5ac844c8..557462208462 100644 --- > > a/drivers/gpu/drm/i915/display/intel_display.c +++ > > b/drivers/gpu/drm/i915/display/intel_display.c @@ -16653,6 > > +16653,13 @@ static void intel_modeset_readout_hw_state(struct > > drm_device *dev) pll->on = pll->info->funcs->get_hw_state(dev_priv, > > pll, &pll->state.hw_state); > > + > > + if (IS_ELKHARTLAKE(dev_priv) && pll->on && > > + pll->info->id == DPLL_ID_EHL_DPLL4) { > > + pll->wakeref = > > intel_display_power_get(dev_priv, > > + > > POWER_DOMAIN_DPLL_DC_OFF); > > + } > > + > > pll->state.crtc_mask = 0; > > for_each_intel_crtc(dev, crtc) { > > struct intel_crtc_state *crtc_state = > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > b/drivers/gpu/drm/i915/display/intel_display_power.c index > > c19b958461ca..7437fc71d289 100644 --- > > a/drivers/gpu/drm/i915/display/intel_display_power.c +++ > > b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -118,6 > > +118,8 @@ intel_display_power_domain_str(enum > > intel_display_power_domain domain) return "MODESET"; case > > POWER_DOMAIN_GT_IRQ: return "GT_IRQ"; > > + case POWER_DOMAIN_DPLL_DC_OFF: > > + return "DPLL_DC_OFF"; > > default: > > MISSING_CASE(domain); > > return "?"; > > @@ -2455,6 +2457,7 @@ void intel_display_power_put(struct > > drm_i915_private *dev_priv, ICL_PW_2_POWER_DOMAINS > > | \ BIT_ULL(POWER_DOMAIN_MODESET) > > | \ BIT_ULL(POWER_DOMAIN_AUX_A) > > | \ > > + BIT_ULL(POWER_DOMAIN_DPLL_DC_OFF) | > > \ BIT_ULL(POWER_DOMAIN_INIT)) > > > > #define ICL_DDI_IO_A_POWER_DOMAINS ( \ > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h > > b/drivers/gpu/drm/i915/display/intel_display_power.h index > > ff57b0a7fe59..8f43f7051a16 100644 --- > > a/drivers/gpu/drm/i915/display/intel_display_power.h +++ > > b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -59,6 +59,7 > > @@ enum intel_display_power_domain { POWER_DOMAIN_GMBUS, > > POWER_DOMAIN_MODESET, > > POWER_DOMAIN_GT_IRQ, > > + POWER_DOMAIN_DPLL_DC_OFF, > > POWER_DOMAIN_INIT, > > > > POWER_DOMAIN_NUM, > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c index > > f953971e7c3b..67cfe836286e 100644 --- > > a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c +++ > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c @@ -2905,6 +2905,9 > > @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state > > *state, intel_atomic_get_new_crtc_state(state, crtc); struct > > icl_port_dpll *port_dpll = > > &crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT]; > > + struct drm_i915_private *dev_priv = > > to_i915(crtc->base.dev); > > + enum port port = encoder->port; > > + bool has_dpll4 = false; > > > > if (!icl_calc_dpll_state(crtc_state, encoder, > > &port_dpll->hw_state)) { DRM_DEBUG_KMS("Could not calculate combo > > PHY PLL state.\n"); @@ -2912,10 +2915,14 @@ static bool > > icl_get_combo_phy_dpll(struct intel_atomic_state *state, return > > false; } > > > > + if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) > > + has_dpll4 = true; > > + > > port_dpll->pll = intel_find_shared_dpll(state, crtc, > > &port_dpll->hw_state, > > DPLL_ID_ICL_DPLL0, > > - DPLL_ID_ICL_DPLL1); > > + has_dpll4 ? > > DPLL_ID_EHL_DPLL4 > > + : > > DPLL_ID_ICL_DPLL1); if (!port_dpll->pll) { > > DRM_DEBUG_KMS("No combo PHY PLL found for port > > %c\n", port_name(encoder->port)); > > @@ -3119,8 +3126,14 @@ static bool combo_pll_get_hw_state(struct > > drm_i915_private *dev_priv, struct intel_shared_dpll *pll, > > struct intel_dpll_hw_state > > *hw_state) { > > - return icl_pll_get_hw_state(dev_priv, pll, hw_state, > > - > > CNL_DPLL_ENABLE(pll->info->id)); > > + i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > > + > > + if (IS_ELKHARTLAKE(dev_priv) && > > + pll->info->id == DPLL_ID_EHL_DPLL4) { > > + enable_reg = MG_PLL_ENABLE(0); > > + } > > + > > + return icl_pll_get_hw_state(dev_priv, pll, hw_state, > > enable_reg); } > > > > static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv, > > @@ -3231,6 +3244,19 @@ static void combo_pll_enable(struct > > drm_i915_private *dev_priv, { > > i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > > > > + if (IS_ELKHARTLAKE(dev_priv) && > > + pll->info->id == DPLL_ID_EHL_DPLL4) { > > + enable_reg = MG_PLL_ENABLE(0); > > + > > + /* > > + * We need to disable DC states when this DPLL is > > enabled. > > + * This can be done by taking a reference on DPLL4 > > power > > + * domain. > > + */ > > + pll->wakeref = intel_display_power_get(dev_priv, > > + > > POWER_DOMAIN_DPLL_DC_OFF); > > + } > > + > > icl_pll_power_enable(dev_priv, pll, enable_reg); > > > > icl_dpll_write(dev_priv, pll); > > @@ -3326,7 +3352,19 @@ static void icl_pll_disable(struct > > drm_i915_private *dev_priv, static void combo_pll_disable(struct > > drm_i915_private *dev_priv, struct intel_shared_dpll *pll) > > { > > - icl_pll_disable(dev_priv, pll, > > CNL_DPLL_ENABLE(pll->info->id)); > > + i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > > + > > + if (IS_ELKHARTLAKE(dev_priv) && > > + pll->info->id == DPLL_ID_EHL_DPLL4) { > > + enable_reg = MG_PLL_ENABLE(0); > > + icl_pll_disable(dev_priv, pll, enable_reg); > > + > > + intel_display_power_put(dev_priv, > > POWER_DOMAIN_DPLL_DC_OFF, > > + pll->wakeref); > > + return; > > + } > > + > > + icl_pll_disable(dev_priv, pll, enable_reg); > > } > > > > static void tbt_pll_disable(struct drm_i915_private *dev_priv, > > @@ -3406,6 +3444,7 @@ static const struct intel_dpll_mgr > > icl_pll_mgr = { static const struct dpll_info ehl_plls[] = { > > { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 }, > > { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 }, > > + { "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 }, > > { }, > > }; > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h index > > 1668f8116908..4c2c5e93aff3 100644 --- > > a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h +++ > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h @@ -28,6 +28,7 @@ > > #include <linux/types.h> > > > > #include "intel_display.h" > > +#include "intel_wakeref.h" > > > > /*FIXME: Move this to a more appropriate place. */ > > #define abs_diff(a, b) ({ \ > > @@ -118,6 +119,10 @@ enum intel_dpll_id { > > * @DPLL_ID_ICL_DPLL1: ICL combo PHY DPLL1 > > */ > > DPLL_ID_ICL_DPLL1 = 1, > > + /** > > + * @DPLL_ID_EHL_DPLL4: EHL combo PHY DPLL4 > > + */ > > + DPLL_ID_EHL_DPLL4 = 2, > > While looking at the TGL stuff I noticed that this ID seems to be > wrong. We use this to generate the register offsets but the docs show > the following: > > DPLL0_CFGCR0 0x164000 > DPLL1_CFGCR0 0x164080 > TBTPLL_CFGCR0 0x164100 > DPLL4_CFGCR0 0x164200 > > So the DPLL4 accesses will now land on TBTPLL. The ID if DPLL4 should > really be 4 I guess. Yes, looks like it should be 4 indeed. However, I was wondering what is the best way to address this issue. Simply changing the id to 4 isn't going to work as there are many places in intel_dpll_mgr.c that assume the dpll id to be the index in the dev_priv->shared_dplls array as stated in the definition of dpll_info: /** * @id: unique indentifier for this DPLL; should match the index in the * dev_priv->shared_dplls array */ enum intel_dpll_id id; Should I change all these places that make this assumption and fix them? Or, should I just simply add an if condition around cr0 and cr1 accesses in icl_dpll_write and icl_pll_get_hw_state? Thanks, Vivek > > > /** > > * @DPLL_ID_ICL_TBTPLL: ICL TBT PLL > > */ > > @@ -320,6 +325,7 @@ struct intel_shared_dpll { > > * @info: platform specific info > > */ > > const struct dpll_info *info; > > + intel_wakeref_t wakeref; > > }; > > > > #define SKL_DPLL0 0 > > -- > > 2.21.0 >
On Tue, Jul 16, 2019 at 06:54:39PM -0700, Vivek Kasireddy wrote: > On Wed, 10 Jul 2019 21:47:52 +0300 > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > Hi Ville, > > > On Wed, Jul 03, 2019 at 04:03:53PM -0700, Vivek Kasireddy wrote: > > > This patch adds support for DPLL4 on EHL that include the > > > following restrictions: > > > > > > - DPLL4 cannot be used with DDIA (combo port A internal eDP usage). > > > DPLL4 can be used with other DDIs, including DDID > > > (combo port A external usage). > > > > > > - DPLL4 cannot be enabled when DC5 or DC6 are enabled. > > > > > > - The DPLL4 enable, lock, power enabled, and power state are > > > connected to the MGPLL1_ENABLE register. > > > > > > v2: (suggestions from Bob Paauwe) > > > - Rework ehl_get_dpll() function to call intel_find_shared_dpll() > > > and iterate twice: once for Combo plls and once for MG plls. > > > > > > - Use MG pll funcs for DPLL4 instead of creating new ones and modify > > > mg_pll_enable to include the restrictions for EHL. > > > > > > v3: Fix compilation error > > > > > > v4: (suggestions from Lucas and Ville) > > > - Treat DPLL4 as a combo phy PLL and not as MG PLL > > > - Disable DC states when this DPLL is being enabled > > > - Reuse icl_get_dpll instead of creating a separate one for EHL > > > > > > v5: (suggestion from Ville) > > > - Refcount the DC OFF power domains during the enabling and > > > disabling of this DPLL. > > > > > > v6: rebase > > > > > > v7: (suggestion from Imre) > > > - Add a new power domain instead of iterating over the domains > > > assoicated with DC OFF power well. > > > > > > v8: (Ville and Imre) > > > - Rename POWER_DOMAIN_DPLL4 TO POWER_DOMAIN_DPLL_DC_OFF > > > - Grab a reference in intel_modeset_setup_hw_state() if this > > > DPLL was already enabled perhaps by BIOS. > > > - Check for the port type instead of the encoder > > > > > > v9: (Ville) > > > - Move the block of code that grabs a reference to the power domain > > > POWER_DOMAIN_DPLL_DC_OFF to intel_modeset_readout_hw_state() to > > > ensure that there is a reference present before this DPLL might get > > > disabled. > > > > > > v10: rebase > > > > > > Cc: José Roberto de Souza <jose.souza@intel.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > > Cc: Imre Deak <imre.deak@intel.com> > > > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 7 +++ > > > .../drm/i915/display/intel_display_power.c | 3 ++ > > > .../drm/i915/display/intel_display_power.h | 1 + > > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 47 > > > +++++++++++++++++-- drivers/gpu/drm/i915/display/intel_dpll_mgr.h > > > | 6 +++ 5 files changed, 60 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > b/drivers/gpu/drm/i915/display/intel_display.c index > > > 919f5ac844c8..557462208462 100644 --- > > > a/drivers/gpu/drm/i915/display/intel_display.c +++ > > > b/drivers/gpu/drm/i915/display/intel_display.c @@ -16653,6 > > > +16653,13 @@ static void intel_modeset_readout_hw_state(struct > > > drm_device *dev) pll->on = pll->info->funcs->get_hw_state(dev_priv, > > > pll, &pll->state.hw_state); > > > + > > > + if (IS_ELKHARTLAKE(dev_priv) && pll->on && > > > + pll->info->id == DPLL_ID_EHL_DPLL4) { > > > + pll->wakeref = > > > intel_display_power_get(dev_priv, > > > + > > > POWER_DOMAIN_DPLL_DC_OFF); > > > + } > > > + > > > pll->state.crtc_mask = 0; > > > for_each_intel_crtc(dev, crtc) { > > > struct intel_crtc_state *crtc_state = > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > > b/drivers/gpu/drm/i915/display/intel_display_power.c index > > > c19b958461ca..7437fc71d289 100644 --- > > > a/drivers/gpu/drm/i915/display/intel_display_power.c +++ > > > b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -118,6 > > > +118,8 @@ intel_display_power_domain_str(enum > > > intel_display_power_domain domain) return "MODESET"; case > > > POWER_DOMAIN_GT_IRQ: return "GT_IRQ"; > > > + case POWER_DOMAIN_DPLL_DC_OFF: > > > + return "DPLL_DC_OFF"; > > > default: > > > MISSING_CASE(domain); > > > return "?"; > > > @@ -2455,6 +2457,7 @@ void intel_display_power_put(struct > > > drm_i915_private *dev_priv, ICL_PW_2_POWER_DOMAINS > > > | \ BIT_ULL(POWER_DOMAIN_MODESET) > > > | \ BIT_ULL(POWER_DOMAIN_AUX_A) > > > | \ > > > + BIT_ULL(POWER_DOMAIN_DPLL_DC_OFF) | > > > \ BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > #define ICL_DDI_IO_A_POWER_DOMAINS ( \ > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h > > > b/drivers/gpu/drm/i915/display/intel_display_power.h index > > > ff57b0a7fe59..8f43f7051a16 100644 --- > > > a/drivers/gpu/drm/i915/display/intel_display_power.h +++ > > > b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -59,6 +59,7 > > > @@ enum intel_display_power_domain { POWER_DOMAIN_GMBUS, > > > POWER_DOMAIN_MODESET, > > > POWER_DOMAIN_GT_IRQ, > > > + POWER_DOMAIN_DPLL_DC_OFF, > > > POWER_DOMAIN_INIT, > > > > > > POWER_DOMAIN_NUM, > > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c index > > > f953971e7c3b..67cfe836286e 100644 --- > > > a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c +++ > > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c @@ -2905,6 +2905,9 > > > @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state > > > *state, intel_atomic_get_new_crtc_state(state, crtc); struct > > > icl_port_dpll *port_dpll = > > > &crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT]; > > > + struct drm_i915_private *dev_priv = > > > to_i915(crtc->base.dev); > > > + enum port port = encoder->port; > > > + bool has_dpll4 = false; > > > > > > if (!icl_calc_dpll_state(crtc_state, encoder, > > > &port_dpll->hw_state)) { DRM_DEBUG_KMS("Could not calculate combo > > > PHY PLL state.\n"); @@ -2912,10 +2915,14 @@ static bool > > > icl_get_combo_phy_dpll(struct intel_atomic_state *state, return > > > false; } > > > > > > + if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) > > > + has_dpll4 = true; > > > + > > > port_dpll->pll = intel_find_shared_dpll(state, crtc, > > > &port_dpll->hw_state, > > > DPLL_ID_ICL_DPLL0, > > > - DPLL_ID_ICL_DPLL1); > > > + has_dpll4 ? > > > DPLL_ID_EHL_DPLL4 > > > + : > > > DPLL_ID_ICL_DPLL1); if (!port_dpll->pll) { > > > DRM_DEBUG_KMS("No combo PHY PLL found for port > > > %c\n", port_name(encoder->port)); > > > @@ -3119,8 +3126,14 @@ static bool combo_pll_get_hw_state(struct > > > drm_i915_private *dev_priv, struct intel_shared_dpll *pll, > > > struct intel_dpll_hw_state > > > *hw_state) { > > > - return icl_pll_get_hw_state(dev_priv, pll, hw_state, > > > - > > > CNL_DPLL_ENABLE(pll->info->id)); > > > + i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > > > + > > > + if (IS_ELKHARTLAKE(dev_priv) && > > > + pll->info->id == DPLL_ID_EHL_DPLL4) { > > > + enable_reg = MG_PLL_ENABLE(0); > > > + } > > > + > > > + return icl_pll_get_hw_state(dev_priv, pll, hw_state, > > > enable_reg); } > > > > > > static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv, > > > @@ -3231,6 +3244,19 @@ static void combo_pll_enable(struct > > > drm_i915_private *dev_priv, { > > > i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > > > > > > + if (IS_ELKHARTLAKE(dev_priv) && > > > + pll->info->id == DPLL_ID_EHL_DPLL4) { > > > + enable_reg = MG_PLL_ENABLE(0); > > > + > > > + /* > > > + * We need to disable DC states when this DPLL is > > > enabled. > > > + * This can be done by taking a reference on DPLL4 > > > power > > > + * domain. > > > + */ > > > + pll->wakeref = intel_display_power_get(dev_priv, > > > + > > > POWER_DOMAIN_DPLL_DC_OFF); > > > + } > > > + > > > icl_pll_power_enable(dev_priv, pll, enable_reg); > > > > > > icl_dpll_write(dev_priv, pll); > > > @@ -3326,7 +3352,19 @@ static void icl_pll_disable(struct > > > drm_i915_private *dev_priv, static void combo_pll_disable(struct > > > drm_i915_private *dev_priv, struct intel_shared_dpll *pll) > > > { > > > - icl_pll_disable(dev_priv, pll, > > > CNL_DPLL_ENABLE(pll->info->id)); > > > + i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > > > + > > > + if (IS_ELKHARTLAKE(dev_priv) && > > > + pll->info->id == DPLL_ID_EHL_DPLL4) { > > > + enable_reg = MG_PLL_ENABLE(0); > > > + icl_pll_disable(dev_priv, pll, enable_reg); > > > + > > > + intel_display_power_put(dev_priv, > > > POWER_DOMAIN_DPLL_DC_OFF, > > > + pll->wakeref); > > > + return; > > > + } > > > + > > > + icl_pll_disable(dev_priv, pll, enable_reg); > > > } > > > > > > static void tbt_pll_disable(struct drm_i915_private *dev_priv, > > > @@ -3406,6 +3444,7 @@ static const struct intel_dpll_mgr > > > icl_pll_mgr = { static const struct dpll_info ehl_plls[] = { > > > { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 }, > > > { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 }, > > > + { "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 }, > > > { }, > > > }; > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h > > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h index > > > 1668f8116908..4c2c5e93aff3 100644 --- > > > a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h +++ > > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h @@ -28,6 +28,7 @@ > > > #include <linux/types.h> > > > > > > #include "intel_display.h" > > > +#include "intel_wakeref.h" > > > > > > /*FIXME: Move this to a more appropriate place. */ > > > #define abs_diff(a, b) ({ \ > > > @@ -118,6 +119,10 @@ enum intel_dpll_id { > > > * @DPLL_ID_ICL_DPLL1: ICL combo PHY DPLL1 > > > */ > > > DPLL_ID_ICL_DPLL1 = 1, > > > + /** > > > + * @DPLL_ID_EHL_DPLL4: EHL combo PHY DPLL4 > > > + */ > > > + DPLL_ID_EHL_DPLL4 = 2, > > > > While looking at the TGL stuff I noticed that this ID seems to be > > wrong. We use this to generate the register offsets but the docs show > > the following: > > > > DPLL0_CFGCR0 0x164000 > > DPLL1_CFGCR0 0x164080 > > TBTPLL_CFGCR0 0x164100 > > DPLL4_CFGCR0 0x164200 > > > > So the DPLL4 accesses will now land on TBTPLL. The ID if DPLL4 should > > really be 4 I guess. > Yes, looks like it should be 4 indeed. However, I was wondering what is > the best way to address this issue. Simply changing the id to 4 isn't > going to work as there are many places in intel_dpll_mgr.c that assume > the dpll id to be the index in the dev_priv->shared_dplls array as > stated in the definition of dpll_info: > /** > * @id: unique indentifier for this DPLL; should match the > index in the > * dev_priv->shared_dplls array > */ > enum intel_dpll_id id; Ugh. That's a bit unfortunate. I guess it would be nice to abstrace that away and either allow the array(s) to be sparsely populated, or decouple the index from the id. > > Should I change all these places that make this assumption and fix them? > Or, should I just simply add an if condition around cr0 and cr1 accesses > in icl_dpll_write and icl_pll_get_hw_state? I guess we can go with the simple solution as a quick fix.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 919f5ac844c8..557462208462 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -16653,6 +16653,13 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) pll->on = pll->info->funcs->get_hw_state(dev_priv, pll, &pll->state.hw_state); + + if (IS_ELKHARTLAKE(dev_priv) && pll->on && + pll->info->id == DPLL_ID_EHL_DPLL4) { + pll->wakeref = intel_display_power_get(dev_priv, + POWER_DOMAIN_DPLL_DC_OFF); + } + pll->state.crtc_mask = 0; for_each_intel_crtc(dev, crtc) { struct intel_crtc_state *crtc_state = diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index c19b958461ca..7437fc71d289 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -118,6 +118,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) return "MODESET"; case POWER_DOMAIN_GT_IRQ: return "GT_IRQ"; + case POWER_DOMAIN_DPLL_DC_OFF: + return "DPLL_DC_OFF"; default: MISSING_CASE(domain); return "?"; @@ -2455,6 +2457,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, ICL_PW_2_POWER_DOMAINS | \ BIT_ULL(POWER_DOMAIN_MODESET) | \ BIT_ULL(POWER_DOMAIN_AUX_A) | \ + BIT_ULL(POWER_DOMAIN_DPLL_DC_OFF) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define ICL_DDI_IO_A_POWER_DOMAINS ( \ diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index ff57b0a7fe59..8f43f7051a16 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -59,6 +59,7 @@ enum intel_display_power_domain { POWER_DOMAIN_GMBUS, POWER_DOMAIN_MODESET, POWER_DOMAIN_GT_IRQ, + POWER_DOMAIN_DPLL_DC_OFF, POWER_DOMAIN_INIT, POWER_DOMAIN_NUM, diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c index f953971e7c3b..67cfe836286e 100644 --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c @@ -2905,6 +2905,9 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state, intel_atomic_get_new_crtc_state(state, crtc); struct icl_port_dpll *port_dpll = &crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT]; + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + enum port port = encoder->port; + bool has_dpll4 = false; if (!icl_calc_dpll_state(crtc_state, encoder, &port_dpll->hw_state)) { DRM_DEBUG_KMS("Could not calculate combo PHY PLL state.\n"); @@ -2912,10 +2915,14 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state, return false; } + if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) + has_dpll4 = true; + port_dpll->pll = intel_find_shared_dpll(state, crtc, &port_dpll->hw_state, DPLL_ID_ICL_DPLL0, - DPLL_ID_ICL_DPLL1); + has_dpll4 ? DPLL_ID_EHL_DPLL4 + : DPLL_ID_ICL_DPLL1); if (!port_dpll->pll) { DRM_DEBUG_KMS("No combo PHY PLL found for port %c\n", port_name(encoder->port)); @@ -3119,8 +3126,14 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll, struct intel_dpll_hw_state *hw_state) { - return icl_pll_get_hw_state(dev_priv, pll, hw_state, - CNL_DPLL_ENABLE(pll->info->id)); + i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); + + if (IS_ELKHARTLAKE(dev_priv) && + pll->info->id == DPLL_ID_EHL_DPLL4) { + enable_reg = MG_PLL_ENABLE(0); + } + + return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg); } static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv, @@ -3231,6 +3244,19 @@ static void combo_pll_enable(struct drm_i915_private *dev_priv, { i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); + if (IS_ELKHARTLAKE(dev_priv) && + pll->info->id == DPLL_ID_EHL_DPLL4) { + enable_reg = MG_PLL_ENABLE(0); + + /* + * We need to disable DC states when this DPLL is enabled. + * This can be done by taking a reference on DPLL4 power + * domain. + */ + pll->wakeref = intel_display_power_get(dev_priv, + POWER_DOMAIN_DPLL_DC_OFF); + } + icl_pll_power_enable(dev_priv, pll, enable_reg); icl_dpll_write(dev_priv, pll); @@ -3326,7 +3352,19 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv, static void combo_pll_disable(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll) { - icl_pll_disable(dev_priv, pll, CNL_DPLL_ENABLE(pll->info->id)); + i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); + + if (IS_ELKHARTLAKE(dev_priv) && + pll->info->id == DPLL_ID_EHL_DPLL4) { + enable_reg = MG_PLL_ENABLE(0); + icl_pll_disable(dev_priv, pll, enable_reg); + + intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF, + pll->wakeref); + return; + } + + icl_pll_disable(dev_priv, pll, enable_reg); } static void tbt_pll_disable(struct drm_i915_private *dev_priv, @@ -3406,6 +3444,7 @@ static const struct intel_dpll_mgr icl_pll_mgr = { static const struct dpll_info ehl_plls[] = { { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 }, { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 }, + { "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 }, { }, }; diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h index 1668f8116908..4c2c5e93aff3 100644 --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h @@ -28,6 +28,7 @@ #include <linux/types.h> #include "intel_display.h" +#include "intel_wakeref.h" /*FIXME: Move this to a more appropriate place. */ #define abs_diff(a, b) ({ \ @@ -118,6 +119,10 @@ enum intel_dpll_id { * @DPLL_ID_ICL_DPLL1: ICL combo PHY DPLL1 */ DPLL_ID_ICL_DPLL1 = 1, + /** + * @DPLL_ID_EHL_DPLL4: EHL combo PHY DPLL4 + */ + DPLL_ID_EHL_DPLL4 = 2, /** * @DPLL_ID_ICL_TBTPLL: ICL TBT PLL */ @@ -320,6 +325,7 @@ struct intel_shared_dpll { * @info: platform specific info */ const struct dpll_info *info; + intel_wakeref_t wakeref; }; #define SKL_DPLL0 0
This patch adds support for DPLL4 on EHL that include the following restrictions: - DPLL4 cannot be used with DDIA (combo port A internal eDP usage). DPLL4 can be used with other DDIs, including DDID (combo port A external usage). - DPLL4 cannot be enabled when DC5 or DC6 are enabled. - The DPLL4 enable, lock, power enabled, and power state are connected to the MGPLL1_ENABLE register. v2: (suggestions from Bob Paauwe) - Rework ehl_get_dpll() function to call intel_find_shared_dpll() and iterate twice: once for Combo plls and once for MG plls. - Use MG pll funcs for DPLL4 instead of creating new ones and modify mg_pll_enable to include the restrictions for EHL. v3: Fix compilation error v4: (suggestions from Lucas and Ville) - Treat DPLL4 as a combo phy PLL and not as MG PLL - Disable DC states when this DPLL is being enabled - Reuse icl_get_dpll instead of creating a separate one for EHL v5: (suggestion from Ville) - Refcount the DC OFF power domains during the enabling and disabling of this DPLL. v6: rebase v7: (suggestion from Imre) - Add a new power domain instead of iterating over the domains assoicated with DC OFF power well. v8: (Ville and Imre) - Rename POWER_DOMAIN_DPLL4 TO POWER_DOMAIN_DPLL_DC_OFF - Grab a reference in intel_modeset_setup_hw_state() if this DPLL was already enabled perhaps by BIOS. - Check for the port type instead of the encoder v9: (Ville) - Move the block of code that grabs a reference to the power domain POWER_DOMAIN_DPLL_DC_OFF to intel_modeset_readout_hw_state() to ensure that there is a reference present before this DPLL might get disabled. v10: rebase Cc: José Roberto de Souza <jose.souza@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Imre Deak <imre.deak@intel.com> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 7 +++ .../drm/i915/display/intel_display_power.c | 3 ++ .../drm/i915/display/intel_display_power.h | 1 + drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 47 +++++++++++++++++-- drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 6 +++ 5 files changed, 60 insertions(+), 4 deletions(-)