From: Imre Deak <imre.deak@intel.com>
To: "José Roberto de Souza" <jose.souza@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>,
intel-gfx@lists.freedesktop.org,
Kai-Heng Feng <kai.heng.feng@canonical.com>
Subject: Re: [Intel-gfx] [PATCH 6/6] drm/i915/tc/tgl: Implement TC cold sequences
Date: Wed, 1 Apr 2020 15:55:57 +0300 [thread overview]
Message-ID: <20200401125557.GE24964@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <20200401004120.408586-6-jose.souza@intel.com>
On Tue, Mar 31, 2020 at 05:41:20PM -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.
>
> BSpec: 49294
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> .../drm/i915/display/intel_display_power.c | 46 ++++++++++++++
> .../drm/i915/display/intel_display_power.h | 1 +
> drivers/gpu/drm/i915/display/intel_tc.c | 63 +++++++++++++++----
> drivers/gpu/drm/i915/display/intel_tc.h | 1 +
> drivers/gpu/drm/i915/i915_reg.h | 3 +
> 5 files changed, 103 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 1ccd57d645c7..5de115583146 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -2842,6 +2842,8 @@ 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 (BIT_ULL(POWER_DOMAIN_TC_COLD_OFF))
TGL_TC_COLD_OFF_POWER_DOMAINS
and should also include all the AUX power domains.
> +
> 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,
> @@ -3944,6 +3946,44 @@ static const struct i915_power_well_desc ehl_power_wells[] = {
> },
> };
>
> +static void
> +tgl_tc_cold_off_power_well_enable(struct drm_i915_private *i915,
> + struct i915_power_well *power_well)
> +{
> + intel_tc_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)
> +{
> + intel_tc_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)
> +{
> + /* There is no way to just read it from PCODE */
> + return false;
> +}
> +
> +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",
> @@ -4271,6 +4311,12 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
> .hsw.irq_pipe_mask = BIT(PIPE_D),
> },
> },
> + {
> + .name = "TC cold off",
> + .domains = POWER_DOMAIN_TC_COLD_OFF,
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 b6d67f069ef7..58f19037411a 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -507,11 +507,16 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port,
>
> mutex_lock(&dig_port->tc_lock);
>
> - if (INTEL_GEN(i915) == 11 && dig_port->tc_link_refcount == 0) {
> - enum intel_display_power_domain aux_domain;
> + if (dig_port->tc_link_refcount == 0) {
> + enum intel_display_power_domain domain;
>
> - aux_domain = intel_aux_ch_to_power_domain(dig_port->aux_ch);
> - dig_port->tc_cold_wakeref = intel_display_power_get(i915, aux_domain);
> + if (INTEL_GEN(i915) == 11)
> + domain = intel_aux_ch_to_power_domain(dig_port->aux_ch);
> + else
> + domain = POWER_DOMAIN_TC_COLD_OFF;
> +
> + dig_port->tc_cold_wakeref = intel_display_power_get(i915,
> + domain);
> }
>
> if (!dig_port->tc_link_refcount &&
> @@ -527,18 +532,23 @@ void intel_tc_port_lock(struct intel_digital_port *dig_port)
> __intel_tc_port_lock(dig_port, 1);
> }
>
> -static void icl_tc_cold_unblock(struct intel_digital_port *dig_port)
> +static void tc_cold_unblock(struct intel_digital_port *dig_port)
> {
> struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> - enum intel_display_power_domain aux_domain;
> + enum intel_display_power_domain domain;
> intel_wakeref_t tc_cold_wakeref;
>
> - if (INTEL_GEN(i915) != 11 || dig_port->tc_link_refcount > 0)
> + if (dig_port->tc_link_refcount > 0)
You could drop the ref whenever wakeref passed to this function is not NULL.
> return;
>
> tc_cold_wakeref = fetch_and_zero(&dig_port->tc_cold_wakeref);
> - aux_domain = intel_aux_ch_to_power_domain(dig_port->aux_ch);
> - intel_display_power_put_async(i915, aux_domain, tc_cold_wakeref);
> +
> + if (INTEL_GEN(i915) == 11)
> + domain = intel_aux_ch_to_power_domain(dig_port->aux_ch);
> + else
> + domain = POWER_DOMAIN_TC_COLD_OFF;
> +
> + intel_display_power_put_async(i915, domain, tc_cold_wakeref);
> }
>
> void intel_tc_port_unlock(struct intel_digital_port *dig_port)
> @@ -546,7 +556,7 @@ void intel_tc_port_unlock(struct intel_digital_port *dig_port)
> struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> intel_wakeref_t wakeref = fetch_and_zero(&dig_port->tc_lock_wakeref);
>
> - icl_tc_cold_unblock(dig_port);
> + tc_cold_unblock(dig_port);
>
> mutex_unlock(&dig_port->tc_lock);
>
> @@ -571,7 +581,7 @@ void intel_tc_port_put_link(struct intel_digital_port *dig_port)
> {
> mutex_lock(&dig_port->tc_lock);
> dig_port->tc_link_refcount--;
> - icl_tc_cold_unblock(dig_port);
> + tc_cold_unblock(dig_port);
> mutex_unlock(&dig_port->tc_lock);
> }
>
> @@ -611,3 +621,34 @@ void intel_tc_icl_tc_cold_exit(struct drm_i915_private *i915)
> drm_dbg_kms(&i915->drm, "TC cold block %s\n",
> (ret == 0 ? "succeeded" : "failed"));
> }
> +
> +void
> +intel_tc_tgl_tc_cold_request(struct drm_i915_private *i915, bool block)
> +{
> + u32 low_val, high_val;
> + u8 tries = 0;
> + int ret;
> +
> + do {
> + low_val = 0;
> + high_val = block ? 0 : TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ;
> +
> + 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++;
> + } while (tries < 3);
> +
> + if (ret)
> + drm_dbg_kms(&i915->drm, "TC cold %sblock %s\n",
> + (block ? "" : "un"),
> + (ret == 0 ? "succeeded" : "failed"));
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
> index 168d8896fcfd..8bb358cc8f15 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.h
> +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> @@ -31,5 +31,6 @@ void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);
>
> u32 intel_tc_port_live_status_mask(struct intel_digital_port *dig_port);
> void intel_tc_icl_tc_cold_exit(struct drm_i915_private *i915);
> +void intel_tc_tgl_tc_cold_request(struct drm_i915_private *i915, bool block);
>
> #endif /* __INTEL_TC_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b111815d6596..5548f3b56c0b 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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-04-01 12:56 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-01 0:41 [Intel-gfx] [PATCH 1/6] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
2020-04-01 0:41 ` [Intel-gfx] [PATCH 2/6] drm/i915/tc: Export tc_port_live_status_mask() José Roberto de Souza
2020-04-01 0:41 ` [Intel-gfx] [PATCH 3/6] drm/i915/display: Add intel_aux_ch_to_power_domain() José Roberto de Souza
2020-04-01 12:09 ` Imre Deak
2020-04-01 0:41 ` [Intel-gfx] [PATCH 4/6] drm/i915/display: Split hsw_power_well_enable() into two José Roberto de Souza
2020-04-01 0:41 ` [Intel-gfx] [PATCH 5/6] drm/i915/tc/icl: Implement TC cold sequences José Roberto de Souza
2020-04-01 12:43 ` Imre Deak
2020-04-01 22:35 ` Souza, Jose
2020-04-02 1:02 ` Imre Deak
2020-04-03 1:18 ` Souza, Jose
2020-04-06 12:57 ` Imre Deak
2020-04-01 0:41 ` [Intel-gfx] [PATCH 6/6] drm/i915/tc/tgl: " José Roberto de Souza
2020-04-01 12:55 ` Imre Deak [this message]
2020-04-01 23:36 ` Souza, Jose
2020-04-02 1:08 ` Imre Deak
2020-04-01 1:31 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] drm/i915/display: Move out code to return the digital_port of the aux ch Patchwork
2020-04-01 1:43 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-04-01 2:34 ` [Intel-gfx] [PATCH 1/6] " kbuild test robot
2020-04-01 5:06 ` kbuild test robot
2020-04-01 7:18 ` You-Sheng Yang
2020-04-01 18:18 ` Souza, Jose
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=20200401125557.GE24964@ideak-desk.fi.intel.com \
--to=imre.deak@intel.com \
--cc=cooper.chiou@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jose.souza@intel.com \
--cc=kai.heng.feng@canonical.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox