On Thu, 2014-09-04 at 12:27 +0100, Damien Lespiau wrote: > From: Satheeshakrishna M <[email protected]> > > This patch implements core logic of SKL display power well. > > FIXME: hsw_pwr needs to go. The audio guys promised us that they'll do a > proper > implementation for skl+. > > 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) > > Signed-off-by: Satheeshakrishna M <[email protected]> (v3,v6) > Signed-off-by: Damien Lespiau <[email protected]> > --- > drivers/gpu/drm/i915/i915_reg.h | 13 +++ > drivers/gpu/drm/i915/intel_pm.c | 207 > ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 220 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 4d072a8..84a0de6 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -541,6 +541,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)) > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index ec849db..853b596 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -7160,6 +7160,128 @@ 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)) > +#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_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)) | \ > + 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), 5)) {
The spec says 5 us not 5 ms, so we could just wait for 1 ms. The same
applies to similar places below.
> + 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:
> + break;
> + default:
This would be a driver bug, so it needs a WARN().
> + 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);
> +
> + if (!(tmp & state_mask)) {
> + DRM_DEBUG_KMS("Enabling DDI power well\n");
Could be just "Enabling %s", power_well->name. Also we could miss the
message depending on timing, so it needs to go above where the request
bit is set.
With the above changes, this looks ok:
Reviewed-by: Imre Deak <[email protected]>
> + if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
> + state_mask), 20))
> + 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), 5))
> + 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)
> {
> @@ -7185,6 +7307,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)
> {
> @@ -7787,6 +7939,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",
> @@ -8009,6 +8168,51 @@ 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,
> + },
> +};
> +
> #define set_power_wells(power_domains, __power_wells) ({ \
> (power_domains)->power_wells = (__power_wells); \
> (power_domains)->power_well_count = ARRAY_SIZE(__power_wells); \
> @@ -8030,6 +8234,9 @@ int intel_power_domains_init(struct drm_i915_private
> *dev_priv)
> } else if (IS_BROADWELL(dev_priv->dev)) {
> set_power_wells(power_domains, bdw_power_wells);
> hsw_pwr = power_domains;
> + } else if (IS_SKYLAKE(dev_priv->dev)) {
> + set_power_wells(power_domains, skl_power_wells);
> + hsw_pwr = power_domains;
> } else if (IS_CHERRYVIEW(dev_priv->dev)) {
> set_power_wells(power_domains, chv_power_wells);
> } else if (IS_VALLEYVIEW(dev_priv->dev)) {
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
