On Mon, Jun 05, 2017 at 07:38:45PM +0300, Vivi, Rodrigo wrote: > On Mon, 2017-06-05 at 18:07 +0300, Imre Deak wrote: > > On Thu, Apr 13, 2017 at 09:13:02AM -0700, Rodrigo Vivi wrote: > > > From: Ville Syrjälä <[email protected]> > > > > > > Implement the CNL display init/uninit sequence as outlined in Bspec. > > > > > > Quite similar to SKL/BXT. The main complicaiton is probably the extra > > > procmon setup we must do based on the process/voltage information we > > > can read out from some register. > > > > > > For now we assume DMC will handle the AUX wells, and we'll just enable > > > all of them during the init sequence. Even if DMC will handle them, we > > > should perhaps trim the set of enabled wells based on which DDI ports > > > are actually present. > > > > The above needs to be aligned with the current version of the code. > > > > Yes, looks like DMC saves/restores the state of both AUX and DDI power > > wells. The spec says to enable the AUX wells during init, but I think > > it's ok to do this on-demand. > > Indeed. I updated the code during some rebase but forgot to update the > commit message. I will remove this chuck on next rebase. > > > > > > > > > v2: s/skl_dbuf/gen9_dbuf/ to follow upstream > > > bxt needed a cdclk sanitize step, so let's add it for cnl too > > > v3: s/CHICKEN_MISC_1/CHICKEN_MISC_2/ (Ander) > > > v4: Rebased by Rodrigo after Ville's cdclk rework > > > v5: Removed unecessary Aux IO forced enable/disable, Fix DW10 setup > > > Fix procpon Mask. (Credits-to Paulo and Clint) > > > Remove A0 workaround. > > > v6: Rebased on top of recent code (Rodrigo). > > > v7: Respect the order of sanitize_ after set_ > > > (Done by Rodrigo, Requested by Ville) > > > v8: (Rodrigo) Remove CHICKEN_MISC_2 double definition. > > > > > > Cc: Paulo Zanoni <[email protected]> > > > Cc: Clint Taylor <[email protected]> > > > Signed-off-by: Ville Syrjälä <[email protected]> > > > Signed-off-by: Rodrigo Vivi <[email protected]> > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 23 +++++++ > > > drivers/gpu/drm/i915/intel_cdclk.c | 108 > > > ++++++++++++++++++++++++++++++ > > > drivers/gpu/drm/i915/intel_drv.h | 2 + > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 113 > > > +++++++++++++++++++++++++++++++- > > > 4 files changed, 244 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index 8353892..9b2d8c0 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -1655,6 +1655,9 @@ enum skl_disp_power_wells { > > > #define PHY_RESERVED (1 << 7) > > > #define BXT_PORT_CL1CM_DW0(phy) _BXT_PHY((phy), > > > _PORT_CL1CM_DW0_BC) > > > > > > +#define CNL_PORT_CL1CM_DW5 _MMIO(0x162014) > > > +#define CL_POWER_DOWN_ENABLE (1 << 4) > > > + > > > #define _PORT_CL1CM_DW9_A 0x162024 > > > #define _PORT_CL1CM_DW9_BC 0x6C024 > > > #define IREF0RC_OFFSET_SHIFT 8 > > > @@ -1687,6 +1690,25 @@ enum skl_disp_power_wells { > > > #define BXT_PORT_CL2CM_DW6(phy) _BXT_PHY((phy), > > > _PORT_CL2CM_DW6_BC) > > > #define DW6_OLDO_DYN_PWR_DOWN_EN (1 << 28) > > > > > > +#define CNL_PORT_COMP_DW0 _MMIO(0x162100) > > > +#define COMP_INIT (1 << 31) > > > +#define CNL_PORT_COMP_DW1 _MMIO(0x162104) > > > +#define CNL_PORT_COMP_DW3 _MMIO(0x16210c) > > > +#define PROCESS_INFO_DOT_0 (0 << 26) > > > +#define PROCESS_INFO_DOT_1 (1 << 26) > > > +#define PROCESS_INFO_DOT_4 (2 << 26) > > > +#define PROCESS_INFO_MASK (7 << 26) > > > +#define PROCESS_INFO_SHIFT 26 > > > +#define VOLTAGE_INFO_0_85V (0 << 24) > > > +#define VOLTAGE_INFO_0_95V (1 << 24) > > > +#define VOLTAGE_INFO_1_05V (2 << 24) > > > +#define VOLTAGE_INFO_MASK (3 << 24) > > > +#define VOLTAGE_INFO_SHIFT 24 > > > +#define CNL_PORT_COMP_DW8 _MMIO(0x162120) > > > +#define PRDIC_ICOMP_DIS (1 << 14) > > > > DW8 looks to be unused. > > I believe an early w/a for A0 was using that, but that got remove. > > I will also remove this. > > > > > > +#define CNL_PORT_COMP_DW9 _MMIO(0x162124) > > > +#define CNL_PORT_COMP_DW10 _MMIO(0x162128) > > > + > > > /* BXT PHY Ref registers */ > > > #define _PORT_REF_DW3_A 0x16218C > > > #define _PORT_REF_DW3_BC 0x6C18C > > > @@ -6513,6 +6535,7 @@ enum { > > > #define KVM_CONFIG_CHANGE_NOTIFICATION_SELECT (1 << 14) > > > > > > #define CHICKEN_MISC_2 _MMIO(0x42084) > > > +#define COMP_PWR_DOWN (1 << 23) > > > #define GLK_CL0_PWR_DOWN (1 << 10) > > > #define GLK_CL1_PWR_DOWN (1 << 11) > > > #define GLK_CL2_PWR_DOWN (1 << 12) > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c > > > b/drivers/gpu/drm/i915/intel_cdclk.c > > > index bee4394..f9ba1e7 100644 > > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > > @@ -1555,6 +1555,114 @@ static void cnl_set_cdclk(struct drm_i915_private > > > *dev_priv, > > > intel_update_cdclk(dev_priv); > > > } > > > > > > +static int cnl_cdclk_pll_vco(struct drm_i915_private *dev_priv, int > > > cdclk) > > > +{ > > > + int ratio; > > > + > > > + if (cdclk == dev_priv->cdclk.hw.ref) > > > + return 0; > > > + > > > + switch (cdclk) { > > > + default: > > > + MISSING_CASE(cdclk); > > > + case 168000: > > > + case 336000: > > > + ratio = dev_priv->cdclk.hw.ref == 19200 ? 35 : 28; > > > + break; > > > + case 528000: > > > + ratio = dev_priv->cdclk.hw.ref == 19200 ? 55 : 44; > > > + break; > > > + } > > > + > > > + return dev_priv->cdclk.hw.ref * ratio; > > > +} > > > + > > > +static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv) > > > +{ > > > + u32 cdctl, expected; > > > + > > > + intel_update_cdclk(dev_priv); > > > + > > > + if (dev_priv->cdclk.hw.vco == 0 || > > > + dev_priv->cdclk.hw.cdclk == dev_priv->cdclk.hw.ref) > > > + goto sanitize; > > > + > > > + /* DPLL okay; verify the cdclock > > > + * > > > + * Some BIOS versions leave an incorrect decimal frequency value and > > > + * set reserved MBZ bits in CDCLK_CTL at least during exiting from S4, > > > + * so sanitize this register. > > > + */ > > > + cdctl = I915_READ(CDCLK_CTL); > > > + /* > > > + * Let's ignore the pipe field, since BIOS could have configured the > > > + * dividers both synching to an active pipe, or asynchronously > > > + * (PIPE_NONE). > > > + */ > > > + cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE; > > > + > > > + expected = (cdctl & BXT_CDCLK_CD2X_DIV_SEL_MASK) | > > > + skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk); > > > + > > > + if (cdctl == expected) > > > + /* All well; nothing to sanitize */ > > > + return; > > > + > > > +sanitize: > > > + DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n"); > > > + > > > + /* force cdclk programming */ > > > + dev_priv->cdclk.hw.cdclk = 0; > > > + > > > + /* force full PLL disable + enable */ > > > + dev_priv->cdclk.hw.vco = -1; > > > +} > > > + > > > +/** > > > + * cnl_init_cdclk - Initialize CDCLK on CNL > > > + * @dev_priv: i915 device > > > + * > > > + * Initialize CDCLK for CNL. This is generally > > > + * done only during the display core initialization sequence, > > > + * after which the DMC will take care of turning CDCLK off/on > > > + * as needed. > > > + */ > > > + > > > > Extra w/s. > > what do you mean here?
The blank line between the docbook part and function definition. > > > > > Looks ok: > > Reviewed-by: Imre Deak <[email protected]> > > thanks > > > > > > +void cnl_init_cdclk(struct drm_i915_private *dev_priv) > > > +{ > > > + struct intel_cdclk_state cdclk_state; > > > + > > > + cnl_sanitize_cdclk(dev_priv); > > > + > > > + if (dev_priv->cdclk.hw.cdclk != 0 && > > > + dev_priv->cdclk.hw.vco != 0) > > > + return; > > > + > > > + cdclk_state = dev_priv->cdclk.hw; > > > + > > > + cdclk_state.cdclk = 168000; > > > + cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk); > > > + > > > + cnl_set_cdclk(dev_priv, &cdclk_state); > > > +} > > > + > > > +/** > > > + * cnl_uninit_cdclk - Uninitialize CDCLK on CNL > > > + * @dev_priv: i915 device > > > + * > > > + * Uninitialize CDCLK for CNL. This is done only > > > + * during the display core uninitialization sequence. > > > + */ > > > +void cnl_uninit_cdclk(struct drm_i915_private *dev_priv) > > > +{ > > > + struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw; > > > + > > > + cdclk_state.cdclk = cdclk_state.ref; > > > + cdclk_state.vco = 0; > > > + > > > + cnl_set_cdclk(dev_priv, &cdclk_state); > > > +} > > > + > > > /** > > > * intel_cdclk_state_compare - Determine if two CDCLK states differ > > > * @a: first CDCLK state > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > > b/drivers/gpu/drm/i915/intel_drv.h > > > index 7bc0c25..a526e6e 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -1280,6 +1280,8 @@ void intel_audio_codec_enable(struct intel_encoder > > > *encoder, > > > /* intel_cdclk.c */ > > > void skl_init_cdclk(struct drm_i915_private *dev_priv); > > > void skl_uninit_cdclk(struct drm_i915_private *dev_priv); > > > +void cnl_init_cdclk(struct drm_i915_private *dev_priv); > > > +void cnl_uninit_cdclk(struct drm_i915_private *dev_priv); > > > void bxt_init_cdclk(struct drm_i915_private *dev_priv); > > > void bxt_uninit_cdclk(struct drm_i915_private *dev_priv); > > > void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv); > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > index 1797c91..5c3c6ec 100644 > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > @@ -2698,6 +2698,111 @@ void bxt_display_core_uninit(struct > > > drm_i915_private *dev_priv) > > > mutex_unlock(&power_domains->lock); > > > } > > > > > > +#define CNL_PROCMON_IDX(val) \ > > > + (((val) & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) >> > > > VOLTAGE_INFO_SHIFT) > > > +#define NUM_CNL_PROCMON \ > > > + (CNL_PROCMON_IDX(VOLTAGE_INFO_MASK | PROCESS_INFO_MASK) + 1) > > > + > > > +static const struct cnl_procmon { > > > + u32 dw1, dw9, dw10; > > > +} cnl_procmon_values[NUM_CNL_PROCMON] = { > > > + [CNL_PROCMON_IDX(VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0)] = > > > + { .dw1 = 0x00 << 16, .dw9 = 0x62AB67BB, .dw10 = 0x51914F96, }, > > > + [CNL_PROCMON_IDX(VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0)] = > > > + { .dw1 = 0x00 << 16, .dw9 = 0x86E172C7, .dw10 = 0x77CA5EAB, }, > > > + [CNL_PROCMON_IDX(VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1)] = > > > + { .dw1 = 0x00 << 16, .dw9 = 0x93F87FE1, .dw10 = 0x8AE871C5, }, > > > + [CNL_PROCMON_IDX(VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0)] = > > > + { .dw1 = 0x00 << 16, .dw9 = 0x98FA82DD, .dw10 = 0x89E46DC1, }, > > > + [CNL_PROCMON_IDX(VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1)] = > > > + { .dw1 = 0x44 << 16, .dw9 = 0x9A00AB25, .dw10 = 0x8AE38FF1, }, > > > +}; > > > + > > > +static void cnl_display_core_init(struct drm_i915_private *dev_priv, > > > bool resume) > > > +{ > > > + struct i915_power_domains *power_domains = &dev_priv->power_domains; > > > + const struct cnl_procmon *procmon; > > > + struct i915_power_well *well; > > > + u32 val; > > > + > > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > > + > > > + /* 1. Enable PCH Reset Handshake */ > > > + val = I915_READ(HSW_NDE_RSTWRN_OPT); > > > + val |= RESET_PCH_HANDSHAKE_ENABLE; > > > + I915_WRITE(HSW_NDE_RSTWRN_OPT, val); > > > + > > > + /* 2. Enable Comp */ > > > + val = I915_READ(CHICKEN_MISC_2); > > > + val &= ~COMP_PWR_DOWN; > > > + I915_WRITE(CHICKEN_MISC_2, val); > > > + > > > + val = I915_READ(CNL_PORT_COMP_DW3); > > > + procmon = &cnl_procmon_values[CNL_PROCMON_IDX(val)]; > > > + > > > + WARN_ON(procmon->dw10 == 0); > > > + > > > + val = I915_READ(CNL_PORT_COMP_DW1); > > > + val &= ~((0xff << 16) | 0xff); > > > + val |= procmon->dw1; > > > + I915_WRITE(CNL_PORT_COMP_DW1, val); > > > + > > > + I915_WRITE(CNL_PORT_COMP_DW9, procmon->dw9); > > > + I915_WRITE(CNL_PORT_COMP_DW10, procmon->dw10); > > > + > > > + val = I915_READ(CNL_PORT_COMP_DW0); > > > + val |= COMP_INIT; > > > + I915_WRITE(CNL_PORT_COMP_DW0, val); > > > + > > > + /* 3. */ > > > + val = I915_READ(CNL_PORT_CL1CM_DW5); > > > + val |= CL_POWER_DOWN_ENABLE; > > > + I915_WRITE(CNL_PORT_CL1CM_DW5, val); > > > + > > > + /* 4. Enable Power Well 1 (PG1) and Aux IO Power */ > > > + mutex_lock(&power_domains->lock); > > > + well = lookup_power_well(dev_priv, SKL_DISP_PW_1); > > > + intel_power_well_enable(dev_priv, well); > > > + mutex_unlock(&power_domains->lock); > > > + > > > + /* 5. Enable CD clock */ > > > + cnl_init_cdclk(dev_priv); > > > + > > > + /* 6. Enable DBUF */ > > > + gen9_dbuf_enable(dev_priv); > > > +} > > > + > > > +#undef CNL_PROCMON_IDX > > > +#undef NUM_CNL_PROCMON > > > + > > > +static void cnl_display_core_uninit(struct drm_i915_private *dev_priv) > > > +{ > > > + struct i915_power_domains *power_domains = &dev_priv->power_domains; > > > + struct i915_power_well *well; > > > + u32 val; > > > + > > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > > + > > > + /* 1. Disable all display engine functions -> aready done */ > > > + > > > + /* 2. Disable DBUF */ > > > + gen9_dbuf_disable(dev_priv); > > > + > > > + /* 3. Disable CD clock */ > > > + cnl_uninit_cdclk(dev_priv); > > > + > > > + /* 4. Disable Power Well 1 (PG1) and Aux IO Power */ > > > + mutex_lock(&power_domains->lock); > > > + well = lookup_power_well(dev_priv, SKL_DISP_PW_1); > > > + intel_power_well_disable(dev_priv, well); > > > + mutex_unlock(&power_domains->lock); > > > + > > > + /* 5. Disable Comp */ > > > + val = I915_READ(CHICKEN_MISC_2); > > > + val |= COMP_PWR_DOWN; > > > + I915_WRITE(CHICKEN_MISC_2, val); > > > +} > > > + > > > static void chv_phy_control_init(struct drm_i915_private *dev_priv) > > > { > > > struct i915_power_well *cmn_bc = > > > @@ -2830,7 +2935,9 @@ void intel_power_domains_init_hw(struct > > > drm_i915_private *dev_priv, bool resume) > > > > > > power_domains->initializing = true; > > > > > > - if (IS_GEN9_BC(dev_priv)) { > > > + if (IS_CANNONLAKE(dev_priv)) { > > > + cnl_display_core_init(dev_priv, resume); > > > + } else if (IS_GEN9_BC(dev_priv)) { > > > skl_display_core_init(dev_priv, resume); > > > } else if (IS_GEN9_LP(dev_priv)) { > > > bxt_display_core_init(dev_priv, resume); > > > @@ -2869,7 +2976,9 @@ void intel_power_domains_suspend(struct > > > drm_i915_private *dev_priv) > > > if (!i915.disable_power_well) > > > intel_display_power_put(dev_priv, POWER_DOMAIN_INIT); > > > > > > - if (IS_GEN9_BC(dev_priv)) > > > + if (IS_CANNONLAKE(dev_priv)) > > > + cnl_display_core_uninit(dev_priv); > > > + else if (IS_GEN9_BC(dev_priv)) > > > skl_display_core_uninit(dev_priv); > > > else if (IS_GEN9_LP(dev_priv)) > > > bxt_display_core_uninit(dev_priv); > > > -- > > > 1.9.1 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > [email protected] > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
