[Intel-gfx] [PATCH v2 6/8] drm/i915/tc/tgl: Implement TC cold sequences
Souza, Jose
jose.souza at intel.com
Tue Apr 7 21:20:06 UTC 2020
On Tue, 2020-04-07 at 19:02 +0300, Imre Deak wrote:
> On Mon, Apr 06, 2020 at 06:11:55PM -0700, José Roberto de Souza
> wrote:
> > TC ports can enter in TCCOLD to save power and is required to
> > request
> > to PCODE to exit this state before use or read to TC registers.
> >
> > For TGL there is a new MBOX command to do that with a parameter to
> > ask
> > PCODE to exit and block TCCOLD entry or unblock TCCOLD entry.
> >
> > So adding a new power domain to reuse the refcount and only allow
> > TC cold when all TC ports are not in use.
> >
> > v2:
> > - fixed missing case in intel_display_power_domain_str()
> > - moved tgl_tc_cold_request to intel_display_power.c
> > - renamed TGL_TC_COLD_OFF to TGL_TC_COLD_OFF_POWER_DOMAINS
> > - added all TC and TBT aux power domains to
> > TGL_TC_COLD_OFF_POWER_DOMAINS
> >
> > BSpec: 49294
> > Cc: Imre Deak <imre.deak at intel.com>
> > Cc: Cooper Chiou <cooper.chiou at intel.com>
> > Cc: Kai-Heng Feng <kai.heng.feng at canonical.com>
> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > ---
> > .../drm/i915/display/intel_display_power.c | 98
> > +++++++++++++++++++
> > .../drm/i915/display/intel_display_power.h | 1 +
> > drivers/gpu/drm/i915/display/intel_tc.c | 17 +++-
> > drivers/gpu/drm/i915/i915_reg.h | 3 +
> > 4 files changed, 116 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 0383801a9acc..5d33929f3724 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -151,6 +151,8 @@ intel_display_power_domain_str(enum
> > intel_display_power_domain domain)
> > return "GT_IRQ";
> > case POWER_DOMAIN_DPLL_DC_OFF:
> > return "DPLL_DC_OFF";
> > + case POWER_DOMAIN_TC_COLD_OFF:
> > + return "TC_COLD_OFF";
> > default:
> > MISSING_CASE(domain);
> > return "?";
> > @@ -2858,6 +2860,21 @@ void intel_display_power_put(struct
> > drm_i915_private *dev_priv,
> > #define TGL_AUX_I_TBT6_IO_POWER_DOMAINS ( \
> > BIT_ULL(POWER_DOMAIN_AUX_I_TBT))
> >
> > +#define TGL_TC_COLD_OFF_POWER_DOMAINS ( \
> > + BIT_ULL(POWER_DOMAIN_AUX_D) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_E) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_F) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_G) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_H) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_I) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_D_TBT) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_E_TBT) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_F_TBT) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_G_TBT) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_H_TBT) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_I_TBT) | \
> > + BIT_ULL(POWER_DOMAIN_TC_COLD_OFF))
> > +
> > static const struct i915_power_well_ops
> > i9xx_always_on_power_well_ops = {
> > .sync_hw = i9xx_power_well_sync_hw_noop,
> > .enable = i9xx_always_on_power_well_noop,
> > @@ -3960,6 +3977,81 @@ static const struct i915_power_well_desc
> > ehl_power_wells[] = {
> > },
> > };
> >
> > +static void
> > +tgl_tc_cold_request(struct drm_i915_private *i915, bool block)
> > +{
> > + u32 low_val, high_val;
>
> Can be moved to their scope.
Done
>
> > + u8 tries = 0;
> > + int ret;
> > +
> > + do {
> > + low_val = 0;
> > + high_val = block ? 0 :
> > TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ;
> > +
> > + /*
> > + * Spec states that we should timeout the request after
> > 200us
> > + * but the function below will timeout after 500us
> > + */
> > + ret = sandybridge_pcode_read(i915, TGL_PCODE_TCCOLD,
> > &low_val,
> > + &high_val);
> > + if (ret == 0) {
> > + if (block &&
> > + (low_val &
> > TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED))
> > + ret = -EIO;
> > + else
> > + break;
> > + }
> > +
> > + if (ret != -EAGAIN)
> > + tries++;
>
> -EAGAIN means that the PCODE run/busy flag didn't get cleared in the
> previous iteration, and BSpec says to bail out and not to use the
> port
> in that case. But we can't really do that, so let's give some slack
> to
> PUNIT (and CPU), msleep(1) so the next iteration doesn't return
> immediately and still protect against an endless loop?
Done
>
> > + } while (tries < 3);
> > +
> > + drm_dbg_kms(&i915->drm, "TC cold %sblock %s\n", block ? "" :
> > "un",
> > + ret == 0 ? "succeeded" : "failed");
>
> Isn't the fail always a true error? (also on ICL)
Done
>
> > +}
> > +
> > +static void
> > +tgl_tc_cold_off_power_well_enable(struct drm_i915_private *i915,
> > + struct i915_power_well *power_well)
> > +{
> > + tgl_tc_cold_request(i915, true);
> > +}
> > +
> > +static void
> > +tgl_tc_cold_off_power_well_disable(struct drm_i915_private *i915,
> > + struct i915_power_well *power_well)
> > +{
> > + tgl_tc_cold_request(i915, false);
> > +}
> > +
> > +static void
> > +tgl_tc_cold_off_power_well_sync_hw(struct drm_i915_private *i915,
> > + struct i915_power_well *power_well)
> > +{
> > + if (power_well->count > 0)
> > + tgl_tc_cold_off_power_well_enable(i915, power_well);
> > + else
> > + tgl_tc_cold_off_power_well_disable(i915, power_well);
> > +}
> > +
> > +static bool
> > +tgl_tc_cold_off_power_well_is_enabled(struct drm_i915_private
> > *dev_priv,
> > + struct i915_power_well
> > *power_well)
> > +{
> > + /*
> > + * Not the correctly implementation but there is no way to just
> > read it
> > + * from PCODE, so returning count to avoid state mismatch
> > errors
> > + */
> > + return power_well->count;
> > +}
> > +
> > +static const struct i915_power_well_ops tgl_tc_cold_off_ops = {
> > + .sync_hw = tgl_tc_cold_off_power_well_sync_hw,
> > + .enable = tgl_tc_cold_off_power_well_enable,
> > + .disable = tgl_tc_cold_off_power_well_disable,
> > + .is_enabled = tgl_tc_cold_off_power_well_is_enabled,
> > +};
> > +
> > static const struct i915_power_well_desc tgl_power_wells[] = {
> > {
> > .name = "always-on",
> > @@ -4287,6 +4379,12 @@ static const struct i915_power_well_desc
> > tgl_power_wells[] = {
> > .hsw.irq_pipe_mask = BIT(PIPE_D),
> > },
> > },
> > + {
> > + .name = "TC cold off",
> > + .domains = TGL_TC_COLD_OFF_POWER_DOMAINS,
> > + .ops = &tgl_tc_cold_off_ops,
> > + .id = DISP_PW_ID_NONE,
> > + },
> > };
> >
> > static int
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h
> > b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index da64a5edae7a..070457e7b948 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -76,6 +76,7 @@ enum intel_display_power_domain {
> > POWER_DOMAIN_MODESET,
> > POWER_DOMAIN_GT_IRQ,
> > POWER_DOMAIN_DPLL_DC_OFF,
> > + POWER_DOMAIN_TC_COLD_OFF,
> > POWER_DOMAIN_INIT,
> >
> > POWER_DOMAIN_NUM,
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > b/drivers/gpu/drm/i915/display/intel_tc.c
> > index 7564259d677e..83861653768d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -53,16 +53,27 @@ tc_port_load_fia_params(struct drm_i915_private
> > *i915,
> > }
> > }
> >
> > +static enum intel_display_power_domain
> > +tc_cold_get_power_domain(struct intel_digital_port *dig_port)
> > +{
> > + struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > +
> > + if (INTEL_GEN(i915) == 11)
> > + return intel_legacy_aux_to_power_domain(dig_port-
> > >aux_ch);
> > + else
> > + return POWER_DOMAIN_TC_COLD_OFF;
> > +}
> > +
> > static intel_wakeref_t
> > tc_cold_block(struct intel_digital_port *dig_port)
> > {
> > struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > enum intel_display_power_domain domain;
> >
> > - if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port)
> > + if (INTEL_GEN(i915) == 11 && !dig_port->tc_legacy_port)
> > return 0;
> >
> > - domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> > + domain = tc_cold_get_power_domain(dig_port);
> > return intel_display_power_get(i915, domain);
> > }
> >
> > @@ -80,7 +91,7 @@ tc_cold_unblock(struct intel_digital_port
> > *dig_port, intel_wakeref_t wakeref)
> > if (wakeref == 0)
> > return;
> >
> > - domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> > + domain = tc_cold_get_power_domain(dig_port);
> > intel_display_power_put_async(i915, domain, wakeref);
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 5cbcd01ac3d5..e04eec003d4b 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -9110,6 +9110,9 @@ enum {
> > #define ICL_PCODE_EXIT_TCCOLD 0x12
> > #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
> > #define DISPLAY_IPS_CONTROL 0x19
> > +#define TGL_PCODE_TCCOLD 0x26
> > +#define TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED REG_BIT(0)
> > +#define TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ REG_BIT(0)
> > /* See also IPS_CTL */
> > #define IPS_PCODE_CONTROL (1 << 30)
> > #define HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A
> > --
> > 2.26.0
> >
More information about the Intel-gfx
mailing list