[Intel-gfx] [PATCH v3 1/6] drm/i915/tc/tgl: Implement TCCOLD sequences
Imre Deak
imre.deak at intel.com
Sat Mar 28 09:57:39 UTC 2020
Hi José,
On Tue, Mar 24, 2020 at 01:14:24PM -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.
> For GEN11 the sequence is more complex and will be handled in a
> separated patch.
>
> 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>
> ---
> drivers/gpu/drm/i915/display/intel_tc.c | 61 ++++++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_reg.h | 3 ++
> drivers/gpu/drm/i915/intel_sideband.c | 22 +++++++++
> drivers/gpu/drm/i915/intel_sideband.h | 4 ++
> 4 files changed, 88 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index 9b850c11aa78..e4c5de5ce874 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -7,6 +7,7 @@
> #include "intel_display.h"
> #include "intel_display_types.h"
> #include "intel_dp_mst.h"
> +#include "intel_sideband.h"
> #include "intel_tc.h"
>
> static const char *tc_port_mode_name(enum tc_port_mode mode)
> @@ -496,6 +497,55 @@ bool intel_tc_port_connected(struct intel_digital_port *dig_port)
> return is_connected;
> }
>
> +static inline int tgl_tc_cold_request(struct intel_digital_port *dig_port,
> + bool block)
> +{
> + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> + 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_write_read_timeout(i915,
> + TGL_PCODE_TCCOLD,
> + &low_val, &high_val,
> + 150, 1);
The spec says trying 3 times for 200 usec. Is there a problem if we just
reuse sandybridge_pcode_read() here which has a 500 usec timeout?
> + 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);
> +
> + return ret;
The return value isn't used and I think we can't do much about it, so
just make the function a void type and warn about a timeout?
> +}
> +
> +static int tc_cold_request(struct intel_digital_port *dig_port, bool block)
> +{
> + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> + int ret;
> +
> + if (INTEL_GEN(i915) >= 12)
> + ret = tgl_tc_cold_request(dig_port, block);
> + else
> + /* TODO: implement GEN11 TCCOLD sequences */
> + ret = 0;
> +
> + drm_dbg_kms(&i915->drm, "Port %s: TCCOLD %sblock %s\n",
> + dig_port->tc_port_name, (block ? "" : "un"),
> + (ret == 0 ? "succeeded" : "failed"));
> +
> + return ret;
> +}
> +
> static void __intel_tc_port_lock(struct intel_digital_port *dig_port,
> int required_lanes)
> {
> @@ -506,9 +556,11 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port,
>
> mutex_lock(&dig_port->tc_lock);
>
> - if (!dig_port->tc_link_refcount &&
> - intel_tc_port_needs_reset(dig_port))
> + if (dig_port->tc_link_refcount == 0) {
> + tc_cold_request(dig_port, true);
I'm not sure if PCODE really refcounts the requests what this would need
(since we submit the same request for all ports). But for other reasons,
like the overhead of the request and the AUX PW ack trickery we need on
ICL, I think we need a tc_cold_off power well/domain. The tc_cold_off
power ref would be get/put around the FIA access sequence here
(intel_tc_port_reset_mode()) and would be held whenever we hold an AUX
power ref.
ICL legacy ports would hold an AUX power ref around the FIA access here
and the AUX power well code would internally do the PCODE request and
deal with the delayed power well ack (so we don't need to create a new
interface for that).
> + intel_tc_port_needs_reset(dig_port);
> intel_tc_port_reset_mode(dig_port, required_lanes);
> + }
>
> drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref);
> dig_port->tc_lock_wakeref = wakeref;
> @@ -524,6 +576,9 @@ 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);
>
> + if (dig_port->tc_link_refcount == 0)
> + tc_cold_request(dig_port, false);
> +
> mutex_unlock(&dig_port->tc_lock);
>
> intel_display_power_put_async(i915, POWER_DOMAIN_DISPLAY_CORE,
> @@ -548,6 +603,8 @@ void intel_tc_port_put_link(struct intel_digital_port *dig_port)
> {
> mutex_lock(&dig_port->tc_lock);
> dig_port->tc_link_refcount--;
> + if (dig_port->tc_link_refcount == 0)
> + tc_cold_request(dig_port, false);
> mutex_unlock(&dig_port->tc_lock);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9c53fe918be6..7e341d9945b3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9019,6 +9019,9 @@ enum {
> #define GEN6_PCODE_WRITE_D_COMP 0x11
> #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
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index 1447e7516cb7..20a9d3970930 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -463,6 +463,28 @@ int sandybridge_pcode_write_timeout(struct drm_i915_private *i915,
> return err;
> }
>
> +int sandybridge_pcode_write_read_timeout(struct drm_i915_private *i915,
> + u32 mbox, u32 *val, u32 *val1,
> + int fast_timeout_us,
> + int slow_timeout_ms)
> +{
> + int err;
> +
> + mutex_lock(&i915->sb_lock);
> + err = __sandybridge_pcode_rw(i915, mbox, val, val1,
> + fast_timeout_us, slow_timeout_ms,
> + true);
> + mutex_unlock(&i915->sb_lock);
> +
> + if (err) {
> + drm_dbg(&i915->drm,
> + "warning: pcode (write of 0x%08x to mbox %x) mailbox access failed for %ps: %d\n",
> + *val, mbox, __builtin_return_address(0), err);
> + }
> +
> + return err;
> +}
> +
> static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
> u32 request, u32 reply_mask, u32 reply,
> u32 *status)
> diff --git a/drivers/gpu/drm/i915/intel_sideband.h b/drivers/gpu/drm/i915/intel_sideband.h
> index 7fb95745a444..1939bebb4e67 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.h
> +++ b/drivers/gpu/drm/i915/intel_sideband.h
> @@ -132,6 +132,10 @@ int sandybridge_pcode_read(struct drm_i915_private *i915, u32 mbox,
> int sandybridge_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox,
> u32 val, int fast_timeout_us,
> int slow_timeout_ms);
> +int sandybridge_pcode_write_read_timeout(struct drm_i915_private *i915,
> + u32 mbox, u32 *val, u32 *val1,
> + int fast_timeout_us,
> + int slow_timeout_ms);
> #define sandybridge_pcode_write(i915, mbox, val) \
> sandybridge_pcode_write_timeout(i915, mbox, val, 500, 0)
>
> --
> 2.26.0
>
More information about the Intel-gfx
mailing list