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.so...@intel.com>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Cc: Matt Roper <matthew.d.ro...@intel.com>
> Cc: Imre Deak <imre.d...@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasire...@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
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
