From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 71/89] drm/i915/skl: Implementation of SKL display power well support Date: Tue, 16 Sep 2014 16:56:29 +0300 Message-ID: <1410875789.19704.41.camel@intelbox> References: <1409830075-11139-1-git-send-email-damien.lespiau@intel.com> <1409830075-11139-72-git-send-email-damien.lespiau@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0511523297==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id B532E6E10E for ; Tue, 16 Sep 2014 06:59:29 -0700 (PDT) In-Reply-To: <1409830075-11139-72-git-send-email-damien.lespiau@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Damien Lespiau Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0511523297== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-NaiNDYIJ1s9/k3AeLrq0" --=-NaiNDYIJ1s9/k3AeLrq0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2014-09-04 at 12:27 +0100, Damien Lespiau wrote: > From: Satheeshakrishna M >=20 > This patch implements core logic of SKL display power well. >=20 > FIXME: hsw_pwr needs to go. The audio guys promised us that they'll do a = proper > implementation for skl+. >=20 > v2: Addressed Imre's comments > - Added respective DDIs under power well #1 and #2 > - Simplified repetitive code in power well programming >=20 > v3: Implemented Imre's comments > - Further simplified power well programming > - Made sure that PW 1 is enabled prior to PW 2 >=20 > v4: Fix minor conflict with the the cherryview support (Damien) >=20 > v5: Add the PLL power domain to the always on power well (Damien) >=20 > 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) >=20 > Signed-off-by: Satheeshakrishna M (v3,v6) > Signed-off-by: Damien Lespiau > --- > drivers/gpu/drm/i915/i915_reg.h | 13 +++ > drivers/gpu/drm/i915/intel_pm.c | 207 ++++++++++++++++++++++++++++++++++= ++++++ > 2 files changed, 220 insertions(+) >=20 > 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, > }; > =20 > +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 =3D 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_pr= ivate *dev_priv, > } > } > =20 > +#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 =3D false; > + > + tmp =3D I915_READ(HSW_PWR_WELL_DRIVER); > + fuse_status =3D 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 =3D SKL_POWER_WELL_REQ(power_well->data); > + state_mask =3D 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 > + if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) & > + state_mask), 20)) > + DRM_ERROR("%s enable timeout\n", > + power_well->name); > + check_fuse_status =3D 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 =3D=3D 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 =3D=3D 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); > } > =20 > +static bool skl_power_well_enabled(struct drm_i915_private *dev_priv, > + struct i915_power_well *power_well) > +{ > + uint32_t mask =3D SKL_POWER_WELL_REQ(power_well->data) | > + SKL_POWER_WELL_STATE(power_well->data); > + > + return (I915_READ(HSW_PWR_WELL_DRIVER) & mask) =3D=3D 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 =3D { > .is_enabled =3D hsw_power_well_enabled, > }; > =20 > +static const struct i915_power_well_ops skl_power_well_ops =3D { > + .sync_hw =3D skl_power_well_sync_hw, > + .enable =3D skl_power_well_enable, > + .disable =3D skl_power_well_disable, > + .is_enabled =3D skl_power_well_enabled, > +}; > + > static struct i915_power_well hsw_power_wells[] =3D { > { > .name =3D "always-on", > @@ -8009,6 +8168,51 @@ static struct i915_power_well *lookup_power_well(s= truct drm_i915_private *dev_pr > return NULL; > } > =20 > +static struct i915_power_well skl_power_wells[] =3D { > + { > + .name =3D "always-on", > + .always_on =3D 1, > + .domains =3D SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS, > + .ops =3D &i9xx_always_on_power_well_ops, > + }, > + { > + .name =3D "power well 1", > + .domains =3D SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS, > + .ops =3D &skl_power_well_ops, > + .data =3D SKL_DISP_PW_1, > + }, > + { > + .name =3D "power well 2", > + .domains =3D SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS, > + .ops =3D &skl_power_well_ops, > + .data =3D SKL_DISP_PW_2, > + }, > + { > + .name =3D "DDI A/E power well", > + .domains =3D SKL_DISPLAY_DDI_A_E_POWER_DOMAINS, > + .ops =3D &skl_power_well_ops, > + .data =3D SKL_DISP_PW_DDI_A_E, > + }, > + { > + .name =3D "DDI B power well", > + .domains =3D SKL_DISPLAY_DDI_B_POWER_DOMAINS, > + .ops =3D &skl_power_well_ops, > + .data =3D SKL_DISP_PW_DDI_B, > + }, > + { > + .name =3D "DDI C power well", > + .domains =3D SKL_DISPLAY_DDI_C_POWER_DOMAINS, > + .ops =3D &skl_power_well_ops, > + .data =3D SKL_DISP_PW_DDI_C, > + }, > + { > + .name =3D "DDI D power well", > + .domains =3D SKL_DISPLAY_DDI_D_POWER_DOMAINS, > + .ops =3D &skl_power_well_ops, > + .data =3D SKL_DISP_PW_DDI_D, > + }, > +}; > + > #define set_power_wells(power_domains, __power_wells) ({ \ > (power_domains)->power_wells =3D (__power_wells); \ > (power_domains)->power_well_count =3D ARRAY_SIZE(__power_wells); \ > @@ -8030,6 +8234,9 @@ int intel_power_domains_init(struct drm_i915_privat= e *dev_priv) > } else if (IS_BROADWELL(dev_priv->dev)) { > set_power_wells(power_domains, bdw_power_wells); > hsw_pwr =3D power_domains; > + } else if (IS_SKYLAKE(dev_priv->dev)) { > + set_power_wells(power_domains, skl_power_wells); > + hsw_pwr =3D power_domains; > } else if (IS_CHERRYVIEW(dev_priv->dev)) { > set_power_wells(power_domains, chv_power_wells); > } else if (IS_VALLEYVIEW(dev_priv->dev)) { --=-NaiNDYIJ1s9/k3AeLrq0 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQEcBAABAgAGBQJUGEGNAAoJEORIIAnNuWDFRd0H/0xdMjfCNqZODrOhCbAy9E2X 3pfPkJcxf31jbCvqXf/8StshK42iwdr53w1L80xXUsVkd9Qk/zydqlgxFPxegln/ mVS3g3HCvG6IPjzMssxFjtJhR6bb49CLobI0hMsBEgoVrg1PUZrHptBR0kGfBcfe BDfjrz/9WsrKLXbW3ZKRdqxpFKTwN0dVrnRE1QBRZ/ZQEowmEN0MsYzTMfy+E1a9 ygOSCV8tSkIGCecKpzImv0nrYbbKF+2/xbPMI9PmkGR/RcHSsVU5jEu0dEyu2LbY zDTSHQ/+I6L9DnvApYlNoTfqwJ96Af3HcrORCFsLHDzP8zgmdQ0Ebs4gUTzK7/0= =g54h -----END PGP SIGNATURE----- --=-NaiNDYIJ1s9/k3AeLrq0-- --===============0511523297== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0511523297==--