| Message ID | 1421423872-15059-3-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5595
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV -1 353/353 352/353
ILK 353/353 353/353
SNB +1 400/422 401/422
IVB 487/487 487/487
BYT 296/296 296/296
HSW +21-1 487/508 507/508
BDW 401/402 401/402
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*PNV igt_gen3_render_linear_blits PASS(4, M25M23) CRASH(1, M23)
*SNB igt_kms_flip_event_leak NSPT(2, M35) PASS(1, M35)
HSW igt_kms_cursor_crc_cursor-size-change NSPT(1, M19)TIMEOUT(1, M40)PASS(2, M20) PASS(1, M20)
HSW igt_kms_fence_pin_leak NSPT(1, M19)DMESG_WARN(1, M40)PASS(2, M20) PASS(1, M20)
HSW igt_kms_flip_dpms-vs-vblank-race DMESG_WARN(3, M20M40)PASS(1, M19) DMESG_WARN(1, M20)
HSW igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M19)TIMEOUT(1, M40)PASS(2, M20) PASS(1, M20)
HSW igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M19)TIMEOUT(1, M40)PASS(2, M20) PASS(1, M20)
HSW igt_pm_lpsp_non-edp NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_cursor NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_cursor-dpms NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_dpms-mode-unset-non-lpsp NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_dpms-non-lpsp NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_drm-resources-equal NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_fences NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_fences-dpms NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_gem-execbuf NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_gem-mmap-cpu NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_gem-mmap-gtt NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_gem-pread NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_i2c NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_modeset-non-lpsp NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_modeset-non-lpsp-stress-no-wait NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_pci-d3-state NSPT(1, M19)PASS(2, M20) PASS(1, M20)
HSW igt_pm_rpm_rte NSPT(1, M19)PASS(2, M20) PASS(1, M20)
Note: You need to pay more attention to line start with '*'
On Fri, 2015-01-16 at 15:57 +0000, Damien Lespiau wrote: > From: Satheeshakrishna M <satheeshakrishna.m@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@intel.com> (v3,v6,v7) > Signed-off-by: Damien Lespiau <damien.lespiau@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@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)) {
On Tue, Feb 03, 2015 at 01:06:31AM +0200, Imre Deak wrote: > > +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, > > + }, snip > > + { > > + .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. So this one was causing problems. When I try to enabled MISC IO before PW1, the request times out. Enabling MISC IO just right after PW1 seems to work fine though.
On ke, 2015-02-04 at 13:53 +0000, Damien Lespiau wrote: > On Tue, Feb 03, 2015 at 01:06:31AM +0200, Imre Deak wrote: > > > +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, > > > + }, > > snip > > > > + { > > > + .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. > > So this one was causing problems. When I try to enabled MISC IO before > PW1, the request times out. Enabling MISC IO just right after PW1 seems > to work fine though. Ok. Bspec doesn't say anything about the ordering between PW1 and MISC IO, just that you have to enable them together and wait for PG1 fuse afterwards. How about then moving the MISC IO power well right after PW1 in the list and wait for the PG1 fuse after enabling MISC IO?
On ke, 2015-02-04 at 16:20 +0200, Imre Deak wrote: > On ke, 2015-02-04 at 13:53 +0000, Damien Lespiau wrote: > > On Tue, Feb 03, 2015 at 01:06:31AM +0200, Imre Deak wrote: > > > > +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, > > > > + }, > > > > snip > > > > > > + { > > > > + .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. > > > > So this one was causing problems. When I try to enabled MISC IO before > > PW1, the request times out. Enabling MISC IO just right after PW1 seems > > to work fine though. > > Ok. Bspec doesn't say anything about the ordering between PW1 and MISC > IO, just that you have to enable them together and wait for PG1 fuse > afterwards. How about then moving the MISC IO power well right after PW1 > in the list and wait for the PG1 fuse after enabling MISC IO? Ah, haven't noticed v10, it looks ok to me. --Imre
On Wed, Feb 04, 2015 at 04:20:28PM +0200, Imre Deak wrote: > On ke, 2015-02-04 at 13:53 +0000, Damien Lespiau wrote: > > On Tue, Feb 03, 2015 at 01:06:31AM +0200, Imre Deak wrote: > > > > +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, > > > > + }, > > > > snip > > > > > > + { > > > > + .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. > > > > So this one was causing problems. When I try to enabled MISC IO before > > PW1, the request times out. Enabling MISC IO just right after PW1 seems > > to work fine though. > > Ok. Bspec doesn't say anything about the ordering between PW1 and MISC > IO, just that you have to enable them together and wait for PG1 fuse > afterwards. How about then moving the MISC IO power well right after PW1 > in the list and wait for the PG1 fuse after enabling MISC IO? I think we can even set the 2 requests in the same write and it should do the right thing (and so merge the two power wells). That's really a detail though and as the current code it seems to work, I'll leave such refinements for later/if needed.
On ke, 2015-02-04 at 14:24 +0000, Damien Lespiau wrote: > On Wed, Feb 04, 2015 at 04:20:28PM +0200, Imre Deak wrote: > > On ke, 2015-02-04 at 13:53 +0000, Damien Lespiau wrote: > > > On Tue, Feb 03, 2015 at 01:06:31AM +0200, Imre Deak wrote: > > > > > +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, > > > > > + }, > > > > > > snip > > > > > > > > + { > > > > > + .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. > > > > > > So this one was causing problems. When I try to enabled MISC IO before > > > PW1, the request times out. Enabling MISC IO just right after PW1 seems > > > to work fine though. > > > > Ok. Bspec doesn't say anything about the ordering between PW1 and MISC > > IO, just that you have to enable them together and wait for PG1 fuse > > afterwards. How about then moving the MISC IO power well right after PW1 > > in the list and wait for the PG1 fuse after enabling MISC IO? > > I think we can even set the 2 requests in the same write and it should > do the right thing (and so merge the two power wells). That's really a > detail though and as the current code it seems to work, I'll leave such > refinements for later/if needed. Yes, I had the same thought, agreed that it could be done as a follow-up. --Imre
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)) +#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)) +#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), 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, + } +}; + #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)) {