[Intel-gfx] [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support
Imre Deak
imre.deak at intel.com
Mon Feb 2 15:06:31 PST 2015
On Fri, 2015-01-16 at 15:57 +0000, Damien Lespiau wrote:
> From: Satheeshakrishna M <satheeshakrishna.m at intel.com>
>
> This patch implements core logic of SKL display power well.
>
> v2: Addressed Imre's comments
> - Added respective DDIs under power well #1 and #2
> - Simplified repetitive code in power well programming
>
> v3: Implemented Imre's comments
> - Further simplified power well programming
> - Made sure that PW 1 is enabled prior to PW 2
>
> v4: Fix minor conflict with the the cherryview support (Damien)
>
> v5: Add the PLL power domain to the always on power well (Damien)
>
> v6: Disable BIOS power well (Imre)
> Use power well data for comparison (Imre)
> Put the PLL power domain into PW1 as its needed for CDCLK (Satheesh,
> Damien)
>
> v7: Addressed Imre's comments
> - Lowered the time out to 1ms
> - Added parantheses in macro
> - Moved debug message and fixed wait_for interval
>
> v8:
> - Add a WARN() when swiching on an unknown power well (Imre, done by Damien)
> - Whitespace fixes (spaces instead of tabs) (Damien)
>
> v9: (Imre, done by Damien)
> - Merge the register definitions with this patch
> - Merge the MISC IO power well in this patch
>
> Signed-off-by: Satheeshakrishna M <satheeshakrishna.m at intel.com> (v3,v6,v7)
> Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 20 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 230 ++++++++++++++++++++++++++++++++
> 2 files changed, 250 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a39bb03..cb96041 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -586,6 +586,19 @@ enum punit_power_well {
> PUNIT_POWER_WELL_NUM,
> };
>
> +enum skl_disp_power_wells {
> + SKL_DISP_PW_MISC_IO,
> + SKL_DISP_PW_DDI_A_E,
> + SKL_DISP_PW_DDI_B,
> + SKL_DISP_PW_DDI_C,
> + SKL_DISP_PW_DDI_D,
> + SKL_DISP_PW_1 = 14,
> + SKL_DISP_PW_2,
> +};
> +
> +#define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2))
> +#define SKL_POWER_WELL_REQ(pw) (1 << (((pw) * 2) + 1))
> +
> #define PUNIT_REG_PWRGT_CTRL 0x60
> #define PUNIT_REG_PWRGT_STATUS 0x61
> #define PUNIT_PWRGT_MASK(power_well) (3 << ((power_well) * 2))
> @@ -6323,6 +6336,13 @@ enum punit_power_well {
> #define HSW_PWR_WELL_FORCE_ON (1<<19)
> #define HSW_PWR_WELL_CTL6 0x45414
>
> +/* SKL Fuse Status */
> +#define SKL_FUSE_STATUS 0x42000
> +#define SKL_FUSE_DOWNLOAD_STATUS (1<<31)
> +#define SKL_FUSE_PG0_DIST_STATUS (1<<27)
> +#define SKL_FUSE_PG1_DIST_STATUS (1<<26)
> +#define SKL_FUSE_PG2_DIST_STATUS (1<<25)
> +
> /* Per-pipe DDI Function Control */
> #define TRANS_DDI_FUNC_CTL_A 0x60400
> #define TRANS_DDI_FUNC_CTL_B 0x61400
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 49695d7..d72ec13 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -230,6 +230,146 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> }
> }
>
> +#define SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS ( \
> + BIT(POWER_DOMAIN_PIPE_B) | \
> + BIT(POWER_DOMAIN_TRANSCODER_B) | \
> + BIT(POWER_DOMAIN_PIPE_C) | \
> + BIT(POWER_DOMAIN_TRANSCODER_C) | \
> + BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) | \
> + BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) | \
> + BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) | \
> + BIT(POWER_DOMAIN_AUX_B) | \
> + BIT(POWER_DOMAIN_AUX_C) | \
> + BIT(POWER_DOMAIN_AUX_D) | \
> + BIT(POWER_DOMAIN_AUDIO) | \
> + BIT(POWER_DOMAIN_INIT))
What about transcoder A? It's also on power well 2, and I can't see
anything preventing us to use DP or HDMI with it, so I think it needs to
be added here.
POWER_DOMAIN_VGA needs to be added here too.
> +#define SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS ( \
> + SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \
> + BIT(POWER_DOMAIN_PLLS) | \
> + BIT(POWER_DOMAIN_PIPE_A) | \
> + BIT(POWER_DOMAIN_TRANSCODER_EDP) | \
> + BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) | \
> + BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) | \
> + BIT(POWER_DOMAIN_AUX_A) | \
> + BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS ( \
> + BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) | \
> + BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_B_POWER_DOMAINS ( \
> + BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) | \
> + BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_C_POWER_DOMAINS ( \
> + BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) | \
> + BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_D_POWER_DOMAINS ( \
> + BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) | \
> + BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_MISC_IO_POWER_DOMAINS ( \
> + BIT(POWER_DOMAIN_AUX_A) | \
> + BIT(POWER_DOMAIN_AUX_B) | \
> + BIT(POWER_DOMAIN_AUX_C) | \
> + BIT(POWER_DOMAIN_AUX_D) | \
> + BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) | \
> + BIT(POWER_DOMAIN_AUDIO) | \
> + BIT(POWER_DOMAIN_INIT))
This changed recently in bspec, so that we need the MISC IO power well
for any display functionality. This means we have to enable it whenever
power well 1 is enabled, so the above should be defined simply as
SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS.
> +#define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \
> + (POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS | \
> + SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \
> + SKL_DISPLAY_DDI_A_E_POWER_DOMAINS | \
> + SKL_DISPLAY_DDI_B_POWER_DOMAINS | \
> + SKL_DISPLAY_DDI_C_POWER_DOMAINS | \
> + SKL_DISPLAY_DDI_D_POWER_DOMAINS)) | \
For consistency I would also add SKL_DISPLAY_MISC_IO_POWER_DOMAINS to
the above.
> + BIT(POWER_DOMAIN_INIT))
> +
> +static void skl_set_power_well(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well, bool enable)
> +{
> + uint32_t tmp, fuse_status;
> + uint32_t req_mask, state_mask;
> + bool check_fuse_status = false;
> +
> + tmp = I915_READ(HSW_PWR_WELL_DRIVER);
> + fuse_status = I915_READ(SKL_FUSE_STATUS);
> +
> + switch (power_well->data) {
> + case SKL_DISP_PW_1:
> + if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> + SKL_FUSE_PG0_DIST_STATUS), 1)) {
> + DRM_ERROR("PG0 not enabled\n");
> + return;
> + }
> + break;
> + case SKL_DISP_PW_2:
> + if (!(fuse_status & SKL_FUSE_PG1_DIST_STATUS)) {
> + DRM_ERROR("PG1 in disabled state\n");
> + return;
> + }
> + break;
> + case SKL_DISP_PW_DDI_A_E:
> + case SKL_DISP_PW_DDI_B:
> + case SKL_DISP_PW_DDI_C:
> + case SKL_DISP_PW_DDI_D:
> + case SKL_DISP_PW_MISC_IO:
> + break;
> + default:
> + WARN(1, "Unknown power well %lu\n", power_well->data);
> + return;
> + }
> +
> + req_mask = SKL_POWER_WELL_REQ(power_well->data);
> + state_mask = SKL_POWER_WELL_STATE(power_well->data);
> +
> + if (enable) {
> + if (!(tmp & req_mask)) {
> + I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
> + DRM_DEBUG_KMS("Enabling %s\n", power_well->name);
> + }
> +
> + if (!(tmp & state_mask)) {
> + if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
> + state_mask), 1))
> + DRM_ERROR("%s enable timeout\n",
> + power_well->name);
> + check_fuse_status = true;
> + }
> + } else {
> + if (tmp & req_mask) {
> + I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & ~req_mask);
> + POSTING_READ(HSW_PWR_WELL_DRIVER);
> + DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
> + }
> + }
> +
> + if (check_fuse_status) {
> + if (power_well->data == SKL_DISP_PW_1) {
> + if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> + SKL_FUSE_PG1_DIST_STATUS), 1))
> + DRM_ERROR("PG1 distributing status timeout\n");
> + } else if (power_well->data == SKL_DISP_PW_2) {
> + if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> + SKL_FUSE_PG2_DIST_STATUS), 1))
> + DRM_ERROR("PG2 distributing status timeout\n");
> + }
> + }
> +}
> +
> static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> @@ -255,6 +395,36 @@ static void hsw_power_well_disable(struct drm_i915_private *dev_priv,
> hsw_set_power_well(dev_priv, power_well, false);
> }
>
> +static bool skl_power_well_enabled(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well)
> +{
> + uint32_t mask = SKL_POWER_WELL_REQ(power_well->data) |
> + SKL_POWER_WELL_STATE(power_well->data);
> +
> + return (I915_READ(HSW_PWR_WELL_DRIVER) & mask) == mask;
> +}
> +
> +static void skl_power_well_sync_hw(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well)
> +{
> + skl_set_power_well(dev_priv, power_well, power_well->count > 0);
> +
> + /* Clear any request made by BIOS as driver is taking over */
> + I915_WRITE(HSW_PWR_WELL_BIOS, 0);
> +}
> +
> +static void skl_power_well_enable(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well)
> +{
> + skl_set_power_well(dev_priv, power_well, true);
> +}
> +
> +static void skl_power_well_disable(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well)
> +{
> + skl_set_power_well(dev_priv, power_well, false);
> +}
> +
> static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> @@ -829,6 +999,13 @@ static const struct i915_power_well_ops hsw_power_well_ops = {
> .is_enabled = hsw_power_well_enabled,
> };
>
> +static const struct i915_power_well_ops skl_power_well_ops = {
> + .sync_hw = skl_power_well_sync_hw,
> + .enable = skl_power_well_enable,
> + .disable = skl_power_well_disable,
> + .is_enabled = skl_power_well_enabled,
> +};
> +
> static struct i915_power_well hsw_power_wells[] = {
> {
> .name = "always-on",
> @@ -1059,6 +1236,57 @@ static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_pr
> return NULL;
> }
>
> +static struct i915_power_well skl_power_wells[] = {
> + {
> + .name = "always-on",
> + .always_on = 1,
> + .domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> + .ops = &i9xx_always_on_power_well_ops,
> + },
> + {
> + .name = "power well 1",
> + .domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
> + .ops = &skl_power_well_ops,
> + .data = SKL_DISP_PW_1,
> + },
> + {
> + .name = "power well 2",
> + .domains = SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS,
> + .ops = &skl_power_well_ops,
> + .data = SKL_DISP_PW_2,
> + },
> + {
> + .name = "DDI A/E power well",
> + .domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS,
> + .ops = &skl_power_well_ops,
> + .data = SKL_DISP_PW_DDI_A_E,
> + },
> + {
> + .name = "DDI B power well",
> + .domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS,
> + .ops = &skl_power_well_ops,
> + .data = SKL_DISP_PW_DDI_B,
> + },
> + {
> + .name = "DDI C power well",
> + .domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS,
> + .ops = &skl_power_well_ops,
> + .data = SKL_DISP_PW_DDI_C,
> + },
> + {
> + .name = "DDI D power well",
> + .domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS,
> + .ops = &skl_power_well_ops,
> + .data = SKL_DISP_PW_DDI_D,
> + },
> + {
> + .name = "MISC IO power well",
> + .domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
> + .ops = &skl_power_well_ops,
> + .data = SKL_DISP_PW_MISC_IO,
> + }
Again, since the recent bspec change the misc IO power well should be
enabled before anything else, so it needs to be listed before "power
well 1" on the list.
With the above fixed, this looks ok:
Reviewed-by: Imre Deak <imre.deak at intel.com>
> +};
> +
> #define set_power_wells(power_domains, __power_wells) ({ \
> (power_domains)->power_wells = (__power_wells); \
> (power_domains)->power_well_count = ARRAY_SIZE(__power_wells); \
> @@ -1085,6 +1313,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> set_power_wells(power_domains, hsw_power_wells);
> } else if (IS_BROADWELL(dev_priv->dev)) {
> set_power_wells(power_domains, bdw_power_wells);
> + } else if (IS_SKYLAKE(dev_priv->dev)) {
> + set_power_wells(power_domains, skl_power_wells);
> } else if (IS_CHERRYVIEW(dev_priv->dev)) {
> set_power_wells(power_domains, chv_power_wells);
> } else if (IS_VALLEYVIEW(dev_priv->dev)) {
More information about the Intel-gfx
mailing list