From: Imre Deak <imre.deak@intel.com>
To: Anshuman Gupta <anshuman.gupta@intel.com>
Cc: jani.nikula@intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v9 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well
Date: Fri, 27 Sep 2019 16:32:48 +0300 [thread overview]
Message-ID: <20190927133248.GB29139@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <20190927110743.GA29139@ideak-desk.fi.intel.com>
On Fri, Sep 27, 2019 at 02:07:43PM +0300, Imre Deak wrote:
> On Thu, Sep 26, 2019 at 08:26:17PM +0530, Anshuman Gupta wrote:
> > Add target_dc_state and tgl_set_target_dc_state() API
> > in order to enable DC3CO state with existing DC states.
> > target_dc_state will enable/disable the desired DC state in
> > DC_STATE_EN reg when "DC Off" power well gets disable/enable.
> >
> > v2: commit log improvement.
> > v3: Used intel_wait_for_register to wait for DC3CO exit. [Imre]
> > Used gen9_set_dc_state() to allow/disallow DC3CO. [Imre]
> > Moved transcoder psr2 exit line enablement from tgl_allow_dc3co()
> > to a appropriate place haswell_crtc_enable(). [Imre]
> > Changed the DC3CO power well enabled call back logic as
> > recommended in review comments. [Imre]
> > v4: Used wait_for_us() instead of intel_wait_for_reg(). [Imre (IRC)]
> > v5: using udelay() instead of waiting for DC3CO exit status.
> > v6: Fixed minor unwanted change.
> > v7: Removed DC3CO powerwell and POWER_DOMAIN_VIDEO.
> > v8: Uniform checks by using only target_dc_state instead of allowed_dc_mask
> > in "DC off" power well callback. [Imre]
> > Adding "DC off" power well id to older platforms. [Imre]
> > Removed psr2_deep_sleep flag from tgl_set_target_dc_state. [Imre]
> > v9: Used switch case for target DC state in
> > gen9_dc_off_power_well_disable(), checking DC3CO state against
> > allowed DC mask, using WARN_ON() in
> > tgl_set_target_dc_state(). [Imre]
> >
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> > .../drm/i915/display/intel_display_power.c | 110 ++++++++++++++++--
> > .../drm/i915/display/intel_display_power.h | 2 +
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > 3 files changed, 104 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 0b685c517bcb..9f787556f80d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -785,6 +785,38 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> > dev_priv->csr.dc_state = val & mask;
> > }
> >
> > +static void
> > +allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
> > +{
> > + if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
> > + dev_priv->csr.target_dc_state = DC_STATE_EN_UPTO_DC6;
> > + else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
> > + dev_priv->csr.target_dc_state = DC_STATE_EN_UPTO_DC5;
> > + else
> > + dev_priv->csr.target_dc_state = DC_STATE_DISABLE;
> > +}
>
> The following can be used in tgl_set_target_dc_state() as well:
>
> static u32
> sanitize_target_dc_state(struct drm_i915_private *dev_priv,
> u32 target_dc_state)
> {
> u32 states[] = {
> DC_STATE_EN_UPTO_DC6,
> DC_STATE_EN_UPTO_DC5,
> DC_STATE_EN_DC3CO,
> };
> int i;
>
> for (i = 0; i < ARRAY_SIZE(states); i++)
> if (target_dc_state == states[i] &&
> (dev_priv->csr.allowed_dc_mask & states[i]))
> return states[i];
>
> return DC_STATE_DISABLE;
> }
>
> > +
> > +static void tgl_enable_dc3co(struct drm_i915_private *dev_priv)
> > +{
> > + DRM_DEBUG_KMS("Enabling DC3CO\n");
> > + gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO);
> > +}
> > +
> > +static void tgl_disable_dc3co(struct drm_i915_private *dev_priv)
> > +{
> > + u32 val;
> > +
> > + DRM_DEBUG_KMS("Disabling DC3CO\n");
> > + val = I915_READ(DC_STATE_EN);
> > + val &= ~DC_STATE_DC3CO_STATUS;
> > + I915_WRITE(DC_STATE_EN, val);
> > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > + /*
> > + * Delay of 200us DC3CO Exit time B.Spec 49196
> > + */
> > + usleep_range(200, 210);
> > +}
> > +
> > static void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > {
> > assert_can_enable_dc9(dev_priv);
> > @@ -952,7 +984,8 @@ static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv)
> > static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
> > struct i915_power_well *power_well)
> > {
> > - return (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0;
> > + return ((I915_READ(DC_STATE_EN) & DC_STATE_EN_DC3CO) == 0 &&
> > + (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0);
> > }
> >
> > static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv)
> > @@ -968,6 +1001,11 @@ static void gen9_disable_dc_states(struct drm_i915_private *dev_priv)
> > {
> > struct intel_cdclk_state cdclk_state = {};
> >
> > + if (dev_priv->csr.target_dc_state == DC_STATE_EN_DC3CO) {
> > + tgl_disable_dc3co(dev_priv);
> > + return;
> > + }
> > +
> > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >
> > dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> > @@ -1000,10 +1038,63 @@ static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> > if (!dev_priv->csr.dmc_payload)
> > return;
> >
> > - if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
> > + switch (dev_priv->csr.target_dc_state) {
> > + case DC_STATE_EN_DC3CO:
> > + tgl_enable_dc3co(dev_priv);
> > + break;
> > + case DC_STATE_EN_UPTO_DC6:
> > skl_enable_dc6(dev_priv);
> > - else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
> > + break;
> > + case DC_STATE_EN_UPTO_DC5:
> > gen9_enable_dc5(dev_priv);
> > + break;
> > + }
> > +}
> > +
> > +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state)
We need a documentation for exported functions.
> > +{
> > + struct i915_power_well *power_well;
> > + bool dc_off_enabled;
> > + struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > +
> > + mutex_lock(&power_domains->lock);
> > + power_well = lookup_power_well(dev_priv, SKL_DISP_DC_OFF);
> > +
> > + if (WARN_ON(!power_well))
> > + goto unlock;
> > +
> > + /*
> > + * Compute the adjusted state wrt to the permisisble allowed dc mask.
> > + */
> > + if (state != DC_STATE_EN_DC3CO ||
> > + !(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)) {
> > + if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
> > + state = DC_STATE_EN_UPTO_DC6;
> > + else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
> > + state = DC_STATE_EN_UPTO_DC5;
> > + else
> > + state = DC_STATE_DISABLE;
> > + }
>
> Instead of the above:
> state = sanitize_dc_target_state(dev_priv, state);
>
> > +
> > + if (state == dev_priv->csr.target_dc_state)
> > + goto unlock;
> > +
> > + dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv,
> > + power_well);
> > + /*
> > + * If DC off power well is disabled, need to enable and disable the
> > + * DC off power well to effect target DC state.
> > + */
> > + if (!dc_off_enabled)
> > + power_well->desc->ops->enable(dev_priv, power_well);
> > +
> > + dev_priv->csr.target_dc_state = state;
> > +
> > + if (!dc_off_enabled)
> > + power_well->desc->ops->disable(dev_priv, power_well);
> > +
> > +unlock:
> > + mutex_unlock(&power_domains->lock);
> > }
> >
> > static void i9xx_power_well_sync_hw_noop(struct drm_i915_private *dev_priv,
> > @@ -2951,7 +3042,7 @@ static const struct i915_power_well_desc skl_power_wells[] = {
> > .name = "DC off",
> > .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS,
> > .ops = &gen9_dc_off_power_well_ops,
> > - .id = DISP_PW_ID_NONE,
> > + .id = SKL_DISP_DC_OFF,
> > },
> > {
> > .name = "power well 2",
> > @@ -3033,7 +3124,7 @@ static const struct i915_power_well_desc bxt_power_wells[] = {
> > .name = "DC off",
> > .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS,
> > .ops = &gen9_dc_off_power_well_ops,
> > - .id = DISP_PW_ID_NONE,
> > + .id = SKL_DISP_DC_OFF,
> > },
> > {
> > .name = "power well 2",
> > @@ -3093,7 +3184,7 @@ static const struct i915_power_well_desc glk_power_wells[] = {
> > .name = "DC off",
> > .domains = GLK_DISPLAY_DC_OFF_POWER_DOMAINS,
> > .ops = &gen9_dc_off_power_well_ops,
> > - .id = DISP_PW_ID_NONE,
> > + .id = SKL_DISP_DC_OFF,
> > },
> > {
> > .name = "power well 2",
> > @@ -3262,7 +3353,7 @@ static const struct i915_power_well_desc cnl_power_wells[] = {
> > .name = "DC off",
> > .domains = CNL_DISPLAY_DC_OFF_POWER_DOMAINS,
> > .ops = &gen9_dc_off_power_well_ops,
> > - .id = DISP_PW_ID_NONE,
> > + .id = SKL_DISP_DC_OFF,
> > },
> > {
> > .name = "power well 2",
> > @@ -3390,7 +3481,7 @@ static const struct i915_power_well_desc icl_power_wells[] = {
> > .name = "DC off",
> > .domains = ICL_DISPLAY_DC_OFF_POWER_DOMAINS,
> > .ops = &gen9_dc_off_power_well_ops,
> > - .id = DISP_PW_ID_NONE,
> > + .id = SKL_DISP_DC_OFF,
> > },
> > {
> > .name = "power well 2",
> > @@ -3623,7 +3714,7 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
> > .name = "DC off",
> > .domains = TGL_DISPLAY_DC_OFF_POWER_DOMAINS,
> > .ops = &gen9_dc_off_power_well_ops,
> > - .id = DISP_PW_ID_NONE,
> > + .id = SKL_DISP_DC_OFF,
> > },
> > {
> > .name = "power well 2",
> > @@ -4056,6 +4147,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> > dev_priv->csr.allowed_dc_mask =
> > get_allowed_dc_mask(dev_priv, i915_modparams.enable_dc);
> >
> > + allowed_dc_mask_to_target_dc_state(dev_priv);
>
> and this would become:
>
> dev_priv->csr.target_dc_state =
> sanitize_dc_target_state(dev_priv, DC_STATE_EN_UPTO_DC6);
>
> > BUILD_BUG_ON(POWER_DOMAIN_NUM > 64);
> >
> > mutex_init(&power_domains->lock);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index 737b5def7fc6..13fc705799fd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -100,6 +100,7 @@ enum i915_power_well_id {
> > SKL_DISP_PW_MISC_IO,
> > SKL_DISP_PW_1,
> > SKL_DISP_PW_2,
> > + SKL_DISP_DC_OFF,
> > };
> >
> > #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
> > @@ -256,6 +257,7 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915);
> > void intel_display_power_resume_early(struct drm_i915_private *i915);
> > void intel_display_power_suspend(struct drm_i915_private *i915);
> > void intel_display_power_resume(struct drm_i915_private *i915);
> > +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state);
> >
> > const char *
> > intel_display_power_domain_str(enum intel_display_power_domain domain);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index fcf7423075ef..cddc98ea9965 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -338,6 +338,7 @@ struct intel_csr {
> > i915_reg_t mmioaddr[20];
> > u32 mmiodata[20];
> > u32 dc_state;
> > + u32 target_dc_state;
> > u32 allowed_dc_mask;
> > intel_wakeref_t wakeref;
> > };
> > --
> > 2.21.0
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-09-27 13:33 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-26 14:56 [PATCH RESEND v9 0/7] DC3CO Support for TGL Anshuman Gupta
2019-09-26 14:56 ` [PATCH v9 1/7] drm/i915/tgl: Add DC3CO required register and bits Anshuman Gupta
2019-09-26 14:56 ` [PATCH v9 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask Anshuman Gupta
2019-09-26 14:56 ` [PATCH v9 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well Anshuman Gupta
2019-09-27 11:07 ` Imre Deak
2019-09-27 13:32 ` Imre Deak [this message]
2019-09-27 13:40 ` Jani Nikula
2019-09-27 14:33 ` Imre Deak
2019-09-26 14:56 ` [PATCH v9 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
2019-09-27 15:09 ` Imre Deak
2019-09-26 14:56 ` [PATCH v9 5/7] drm/i915/tgl: DC3CO PSR2 helper Anshuman Gupta
2019-09-27 15:49 ` Imre Deak
2019-09-26 14:56 ` [PATCH v9 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness Anshuman Gupta
2019-09-27 16:18 ` Imre Deak
2019-09-26 14:56 ` [PATCH v9 7/7] drm/i915/tgl: Add DC3CO counter in i915_dmc_info Anshuman Gupta
2019-09-27 16:38 ` Imre Deak
2019-09-27 16:57 ` Anshuman Gupta
2019-09-30 11:45 ` Imre Deak
2019-09-26 16:03 ` ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev12) Patchwork
2019-09-26 16:29 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-27 13:03 ` ✓ Fi.CI.IGT: " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2019-09-25 18:10 [PATCH v9 0/7] DC3CO Support for TGL Anshuman Gupta
2019-09-25 18:10 ` [PATCH v9 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well Anshuman Gupta
2019-09-26 3:56 ` Anshuman Gupta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190927133248.GB29139@ideak-desk.fi.intel.com \
--to=imre.deak@intel.com \
--cc=anshuman.gupta@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.