* [Intel-gfx] [PATCH 2/6] drm/i915/tc: Export tc_port_live_status_mask()
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 ` 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
` (8 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-01 0:41 UTC (permalink / raw)
To: intel-gfx
It will be used by ICL TC cold exit sequence outside of intel_tc.
No functional change here.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_tc.c | 10 +++++-----
drivers/gpu/drm/i915/display/intel_tc.h | 2 ++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 9b850c11aa78..d944be935423 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -170,7 +170,7 @@ static void tc_port_fixup_legacy_flag(struct intel_digital_port *dig_port,
dig_port->tc_legacy_port = !dig_port->tc_legacy_port;
}
-static u32 tc_port_live_status_mask(struct intel_digital_port *dig_port)
+u32 intel_tc_port_live_status_mask(struct intel_digital_port *dig_port)
{
struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
@@ -310,7 +310,7 @@ static void icl_tc_phy_connect(struct intel_digital_port *dig_port,
* Now we have to re-check the live state, in case the port recently
* became disconnected. Not necessary for legacy mode.
*/
- if (!(tc_port_live_status_mask(dig_port) & BIT(TC_PORT_DP_ALT))) {
+ if (!(intel_tc_port_live_status_mask(dig_port) & BIT(TC_PORT_DP_ALT))) {
DRM_DEBUG_KMS("Port %s: PHY sudden disconnect\n",
dig_port->tc_port_name);
goto out_set_safe_mode;
@@ -377,7 +377,7 @@ static bool icl_tc_phy_is_connected(struct intel_digital_port *dig_port)
static enum tc_port_mode
intel_tc_port_get_current_mode(struct intel_digital_port *dig_port)
{
- u32 live_status_mask = tc_port_live_status_mask(dig_port);
+ u32 live_status_mask = intel_tc_port_live_status_mask(dig_port);
bool in_safe_mode = icl_tc_phy_is_in_safe_mode(dig_port);
enum tc_port_mode mode;
@@ -398,7 +398,7 @@ intel_tc_port_get_current_mode(struct intel_digital_port *dig_port)
static enum tc_port_mode
intel_tc_port_get_target_mode(struct intel_digital_port *dig_port)
{
- u32 live_status_mask = tc_port_live_status_mask(dig_port);
+ u32 live_status_mask = intel_tc_port_live_status_mask(dig_port);
if (live_status_mask)
return fls(live_status_mask) - 1;
@@ -489,7 +489,7 @@ bool intel_tc_port_connected(struct intel_digital_port *dig_port)
bool is_connected;
intel_tc_port_lock(dig_port);
- is_connected = tc_port_live_status_mask(dig_port) &
+ is_connected = intel_tc_port_live_status_mask(dig_port) &
BIT(dig_port->tc_mode);
intel_tc_port_unlock(dig_port);
diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index 463f1b3c836f..a1afcee48818 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -28,4 +28,6 @@ bool intel_tc_port_ref_held(struct intel_digital_port *dig_port);
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);
+
#endif /* __INTEL_TC_H__ */
--
2.26.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Intel-gfx] [PATCH 3/6] drm/i915/display: Add intel_aux_ch_to_power_domain()
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 ` 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
` (7 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-01 0:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Cooper Chiou, Kai-Heng Feng
This is a similar function to intel_aux_power_domain() but it do not
care about TBT ports, this will be needed by GEN11 TC sequences.
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>
---
drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++++++++--
drivers/gpu/drm/i915/display/intel_display.h | 2 ++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e09a11b1e509..7e06d2306dcd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7278,7 +7278,17 @@ intel_aux_power_domain(struct intel_digital_port *dig_port)
}
}
- switch (dig_port->aux_ch) {
+ return intel_aux_ch_to_power_domain(dig_port->aux_ch);
+}
+
+/*
+ * Converts aux_ch to power_domain without caring about TBT ports for that use
+ * intel_aux_power_domain()
+ */
+enum intel_display_power_domain
+intel_aux_ch_to_power_domain(enum aux_ch aux_ch)
+{
+ switch (aux_ch) {
case AUX_CH_A:
return POWER_DOMAIN_AUX_A;
case AUX_CH_B:
@@ -7294,7 +7304,7 @@ intel_aux_power_domain(struct intel_digital_port *dig_port)
case AUX_CH_G:
return POWER_DOMAIN_AUX_G;
default:
- MISSING_CASE(dig_port->aux_ch);
+ MISSING_CASE(aux_ch);
return POWER_DOMAIN_AUX_A;
}
}
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index adb1225a3480..ad50119c0453 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -579,6 +579,8 @@ void hsw_disable_ips(const struct intel_crtc_state *crtc_state);
enum intel_display_power_domain intel_port_to_power_domain(enum port port);
enum intel_display_power_domain
intel_aux_power_domain(struct intel_digital_port *dig_port);
+enum intel_display_power_domain
+intel_aux_ch_to_power_domain(enum aux_ch aux_ch);
void intel_mode_from_pipe_config(struct drm_display_mode *mode,
struct intel_crtc_state *pipe_config);
void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
--
2.26.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 3/6] drm/i915/display: Add intel_aux_ch_to_power_domain()
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
0 siblings, 0 replies; 21+ messages in thread
From: Imre Deak @ 2020-04-01 12:09 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: Cooper Chiou, intel-gfx, Kai-Heng Feng
On Tue, Mar 31, 2020 at 05:41:17PM -0700, José Roberto de Souza wrote:
> This is a similar function to intel_aux_power_domain() but it do not
> care about TBT ports, this will be needed by GEN11 TC sequences.
>
> 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>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++++++++--
> drivers/gpu/drm/i915/display/intel_display.h | 2 ++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e09a11b1e509..7e06d2306dcd 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7278,7 +7278,17 @@ intel_aux_power_domain(struct intel_digital_port *dig_port)
> }
> }
>
> - switch (dig_port->aux_ch) {
> + return intel_aux_ch_to_power_domain(dig_port->aux_ch);
> +}
> +
> +/*
> + * Converts aux_ch to power_domain without caring about TBT ports for that use
> + * intel_aux_power_domain()
> + */
> +enum intel_display_power_domain
> +intel_aux_ch_to_power_domain(enum aux_ch aux_ch)
Maybe intel_legacy_aux_to_power_domain()?
> +{
> + switch (aux_ch) {
> case AUX_CH_A:
> return POWER_DOMAIN_AUX_A;
> case AUX_CH_B:
> @@ -7294,7 +7304,7 @@ intel_aux_power_domain(struct intel_digital_port *dig_port)
> case AUX_CH_G:
> return POWER_DOMAIN_AUX_G;
> default:
> - MISSING_CASE(dig_port->aux_ch);
> + MISSING_CASE(aux_ch);
> return POWER_DOMAIN_AUX_A;
> }
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index adb1225a3480..ad50119c0453 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -579,6 +579,8 @@ void hsw_disable_ips(const struct intel_crtc_state *crtc_state);
> enum intel_display_power_domain intel_port_to_power_domain(enum port port);
> enum intel_display_power_domain
> intel_aux_power_domain(struct intel_digital_port *dig_port);
> +enum intel_display_power_domain
> +intel_aux_ch_to_power_domain(enum aux_ch aux_ch);
> void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> struct intel_crtc_state *pipe_config);
> void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
> --
> 2.26.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Intel-gfx] [PATCH 4/6] drm/i915/display: Split hsw_power_well_enable() into two
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 0:41 ` 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
` (6 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-01 0:41 UTC (permalink / raw)
To: intel-gfx
This is a preparation for ICL TC cold exit sequences.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
.../drm/i915/display/intel_display_power.c | 39 +++++++++++++++----
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 02a07aa710e4..dbd61517ba63 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -353,16 +353,16 @@ static void gen9_wait_for_power_well_fuses(struct drm_i915_private *dev_priv,
SKL_FUSE_PG_DIST_STATUS(pg), 1));
}
-static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
- struct i915_power_well *power_well)
+static void _hsw_power_well_enable(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
{
const struct i915_power_well_regs *regs = power_well->desc->hsw.regs;
int pw_idx = power_well->desc->hsw.idx;
- bool wait_fuses = power_well->desc->hsw.has_fuses;
- enum skl_power_gate uninitialized_var(pg);
u32 val;
- if (wait_fuses) {
+ if (power_well->desc->hsw.has_fuses) {
+ enum skl_power_gate pg;
+
pg = INTEL_GEN(dev_priv) >= 11 ? ICL_PW_CTL_IDX_TO_PG(pw_idx) :
SKL_PW_CTL_IDX_TO_PG(pw_idx);
/*
@@ -379,25 +379,46 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
val = intel_de_read(dev_priv, regs->driver);
intel_de_write(dev_priv, regs->driver,
val | HSW_PWR_WELL_CTL_REQ(pw_idx));
+}
+
+static void _hsw_power_well_continue_enable(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+ int pw_idx = power_well->desc->hsw.idx;
+
hsw_wait_for_power_well_enable(dev_priv, power_well);
/* Display WA #1178: cnl */
if (IS_CANNONLAKE(dev_priv) &&
pw_idx >= GLK_PW_CTL_IDX_AUX_B &&
pw_idx <= CNL_PW_CTL_IDX_AUX_F) {
+ u32 val;
+
val = intel_de_read(dev_priv, CNL_AUX_ANAOVRD1(pw_idx));
val |= CNL_AUX_ANAOVRD1_ENABLE | CNL_AUX_ANAOVRD1_LDO_BYPASS;
intel_de_write(dev_priv, CNL_AUX_ANAOVRD1(pw_idx), val);
}
- if (wait_fuses)
+ if (power_well->desc->hsw.has_fuses) {
+ enum skl_power_gate pg;
+
+ pg = INTEL_GEN(dev_priv) >= 11 ? ICL_PW_CTL_IDX_TO_PG(pw_idx) :
+ SKL_PW_CTL_IDX_TO_PG(pw_idx);
gen9_wait_for_power_well_fuses(dev_priv, pg);
+ }
hsw_power_well_post_enable(dev_priv,
power_well->desc->hsw.irq_pipe_mask,
power_well->desc->hsw.has_vga);
}
+static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+ _hsw_power_well_enable(dev_priv, power_well);
+ _hsw_power_well_continue_enable(dev_priv, power_well);
+}
+
static void hsw_power_well_disable(struct drm_i915_private *dev_priv,
struct i915_power_well *power_well)
{
@@ -569,7 +590,11 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
val |= DP_AUX_CH_CTL_TBT_IO;
intel_de_write(dev_priv, DP_AUX_CH_CTL(aux_ch), val);
- hsw_power_well_enable(dev_priv, power_well);
+ _hsw_power_well_enable(dev_priv, power_well);
+
+ /* TODO ICL TC cold handling */
+
+ _hsw_power_well_continue_enable(dev_priv, power_well);
if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) {
enum tc_port tc_port;
--
2.26.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Intel-gfx] [PATCH 5/6] drm/i915/tc/icl: Implement TC cold sequences
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
` (2 preceding siblings ...)
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 ` José Roberto de Souza
2020-04-01 12:43 ` Imre Deak
2020-04-01 0:41 ` [Intel-gfx] [PATCH 6/6] drm/i915/tc/tgl: " José Roberto de Souza
` (5 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-01 0:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Cooper Chiou, Kai-Heng Feng
This is required for legacy/static TC ports as IOM is not aware of
the connection and will not trigger the TC cold exit.
Just request PCODE to exit TCCOLD is not enough as it could enter
again be driver makes use of the port, to prevent it BSpec states that
aux powerwell should be held.
So here embedding the TC cold exit sequence into ICL aux enable,
it will enable aux, request tc cold exit and depending in the TC live
state continue with the regular aux enable sequence.
And then turning on aux power well during tc lock and turning off
during unlock both depending into the TC port refcount.
BSpec: 21750
Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1296
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>
---
Will run some tests in the office with TBT dockstation to check if
it will be a issue keep both aux enabled. Otherwise more changes will
be required here.
.../drm/i915/display/intel_display_power.c | 12 ++++-
.../drm/i915/display/intel_display_types.h | 1 +
drivers/gpu/drm/i915/display/intel_tc.c | 47 ++++++++++++++++++-
drivers/gpu/drm/i915/display/intel_tc.h | 2 +
drivers/gpu/drm/i915/i915_reg.h | 1 +
5 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index dbd61517ba63..1ccd57d645c7 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -592,9 +592,17 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
_hsw_power_well_enable(dev_priv, power_well);
- /* TODO ICL TC cold handling */
+ if (INTEL_GEN(dev_priv) == 11)
+ intel_tc_icl_tc_cold_exit(dev_priv);
- _hsw_power_well_continue_enable(dev_priv, power_well);
+ /*
+ * To avoid power well enable timeouts when disconnected or in TBT mode
+ * when doing the TC cold exit sequence for GEN11
+ */
+ if (INTEL_GEN(dev_priv) != 11 ||
+ (intel_tc_port_live_status_mask(dig_port) &
+ (TC_PORT_LEGACY | TC_PORT_DP_ALT)))
+ _hsw_power_well_continue_enable(dev_priv, power_well);
if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) {
enum tc_port tc_port;
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 176ab5f1e867..a9a4a3c1b4d7 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1391,6 +1391,7 @@ struct intel_digital_port {
enum intel_display_power_domain ddi_io_power_domain;
struct mutex tc_lock; /* protects the TypeC port mode */
intel_wakeref_t tc_lock_wakeref;
+ intel_wakeref_t tc_cold_wakeref;
int tc_link_refcount;
bool tc_legacy_port:1;
char tc_port_name[8];
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index d944be935423..b6d67f069ef7 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)
@@ -506,6 +507,13 @@ 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;
+
+ 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 (!dig_port->tc_link_refcount &&
intel_tc_port_needs_reset(dig_port))
intel_tc_port_reset_mode(dig_port, required_lanes);
@@ -519,15 +527,30 @@ 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)
+{
+ struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+ enum intel_display_power_domain aux_domain;
+ intel_wakeref_t tc_cold_wakeref;
+
+ if (INTEL_GEN(i915) != 11 || dig_port->tc_link_refcount > 0)
+ 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);
+}
+
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);
+
mutex_unlock(&dig_port->tc_lock);
- intel_display_power_put_async(i915, POWER_DOMAIN_DISPLAY_CORE,
- wakeref);
+ intel_display_power_put_async(i915, POWER_DOMAIN_DISPLAY_CORE, wakeref);
}
bool intel_tc_port_ref_held(struct intel_digital_port *dig_port)
@@ -548,6 +571,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);
mutex_unlock(&dig_port->tc_lock);
}
@@ -568,3 +592,22 @@ void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy)
dig_port->tc_link_refcount = 0;
tc_port_load_fia_params(i915, dig_port);
}
+
+void intel_tc_icl_tc_cold_exit(struct drm_i915_private *i915)
+{
+ int ret;
+
+ do {
+ ret = sandybridge_pcode_write_timeout(i915,
+ ICL_PCODE_EXIT_TCCOLD,
+ 0, 250, 1);
+
+ } while (ret == -EAGAIN);
+
+ if (!ret)
+ msleep(1);
+
+ if (ret)
+ drm_dbg_kms(&i915->drm, "TC cold block %s\n",
+ (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 a1afcee48818..168d8896fcfd 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -9,6 +9,7 @@
#include <linux/mutex.h>
#include <linux/types.h>
+struct drm_i915_private;
struct intel_digital_port;
bool intel_tc_port_connected(struct intel_digital_port *dig_port);
@@ -29,5 +30,6 @@ bool intel_tc_port_ref_held(struct intel_digital_port *dig_port);
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);
#endif /* __INTEL_TC_H__ */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 17484345cb80..b111815d6596 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9107,6 +9107,7 @@ enum {
#define ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point) (((point) << 16) | (0x1 << 8))
#define GEN6_PCODE_READ_D_COMP 0x10
#define GEN6_PCODE_WRITE_D_COMP 0x11
+#define ICL_PCODE_EXIT_TCCOLD 0x12
#define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
#define DISPLAY_IPS_CONTROL 0x19
/* See also IPS_CTL */
--
2.26.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 5/6] drm/i915/tc/icl: Implement TC cold sequences
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
0 siblings, 1 reply; 21+ messages in thread
From: Imre Deak @ 2020-04-01 12:43 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: Cooper Chiou, intel-gfx, Kai-Heng Feng
On Tue, Mar 31, 2020 at 05:41:19PM -0700, José Roberto de Souza wrote:
> This is required for legacy/static TC ports as IOM is not aware of
> the connection and will not trigger the TC cold exit.
>
> Just request PCODE to exit TCCOLD is not enough as it could enter
> again be driver makes use of the port, to prevent it BSpec states that
> aux powerwell should be held.
>
> So here embedding the TC cold exit sequence into ICL aux enable,
> it will enable aux, request tc cold exit and depending in the TC live
> state continue with the regular aux enable sequence.
>
> And then turning on aux power well during tc lock and turning off
> during unlock both depending into the TC port refcount.
>
> BSpec: 21750
> Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1296
> 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>
> ---
>
> Will run some tests in the office with TBT dockstation to check if
> it will be a issue keep both aux enabled. Otherwise more changes will
> be required here.
>
> .../drm/i915/display/intel_display_power.c | 12 ++++-
> .../drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/display/intel_tc.c | 47 ++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_tc.h | 2 +
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 5 files changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index dbd61517ba63..1ccd57d645c7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -592,9 +592,17 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
>
> _hsw_power_well_enable(dev_priv, power_well);
>
> - /* TODO ICL TC cold handling */
> + if (INTEL_GEN(dev_priv) == 11)
Should be if (ICL && dig_port->tc_legacy_port)
> + intel_tc_icl_tc_cold_exit(dev_priv);
>
> - _hsw_power_well_continue_enable(dev_priv, power_well);
> + /*
> + * To avoid power well enable timeouts when disconnected or in TBT mode
> + * when doing the TC cold exit sequence for GEN11
> + */
> + if (INTEL_GEN(dev_priv) != 11 ||
> + (intel_tc_port_live_status_mask(dig_port) &
> + (TC_PORT_LEGACY | TC_PORT_DP_ALT)))
> + _hsw_power_well_continue_enable(dev_priv, power_well);
Why can't we call this unconditionally?
>
> if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) {
> enum tc_port tc_port;
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 176ab5f1e867..a9a4a3c1b4d7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1391,6 +1391,7 @@ struct intel_digital_port {
> enum intel_display_power_domain ddi_io_power_domain;
> struct mutex tc_lock; /* protects the TypeC port mode */
> intel_wakeref_t tc_lock_wakeref;
> + intel_wakeref_t tc_cold_wakeref;
> int tc_link_refcount;
> bool tc_legacy_port:1;
> char tc_port_name[8];
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index d944be935423..b6d67f069ef7 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)
> @@ -506,6 +507,13 @@ 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;
> +
> + aux_domain = intel_aux_ch_to_power_domain(dig_port->aux_ch);
> + dig_port->tc_cold_wakeref = intel_display_power_get(i915, aux_domain);
> + }
> +
It would be enough to hold this ref only for the time we access FIA
regs. Anything else later will hold its own AUX reference, which takes
care of blocking tc-cold. So here something like:
tc_cold_wakeref = block_tc_cold(dig_port);
where block_tc_cold() would return a non-NULL wakeref only for
ICL/dig_port->tc_legacy_port and TGL.
> if (!dig_port->tc_link_refcount &&
> intel_tc_port_needs_reset(dig_port))
> intel_tc_port_reset_mode(dig_port, required_lanes);
unblock_tc_cold(tc_cold_wakeref);
We need to call block/unblock_tc_cold() also in intel_tc_port_sanitize()
and intel_tc_port_connected().
> @@ -519,15 +527,30 @@ 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)
> +{
> + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> + enum intel_display_power_domain aux_domain;
> + intel_wakeref_t tc_cold_wakeref;
> +
> + if (INTEL_GEN(i915) != 11 || dig_port->tc_link_refcount > 0)
> + 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);
> +}
> +
> 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);
> +
> mutex_unlock(&dig_port->tc_lock);
>
> - intel_display_power_put_async(i915, POWER_DOMAIN_DISPLAY_CORE,
> - wakeref);
> + intel_display_power_put_async(i915, POWER_DOMAIN_DISPLAY_CORE, wakeref);
> }
>
> bool intel_tc_port_ref_held(struct intel_digital_port *dig_port)
> @@ -548,6 +571,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);
> mutex_unlock(&dig_port->tc_lock);
> }
>
> @@ -568,3 +592,22 @@ void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy)
> dig_port->tc_link_refcount = 0;
> tc_port_load_fia_params(i915, dig_port);
> }
> +
> +void intel_tc_icl_tc_cold_exit(struct drm_i915_private *i915)
This could be in intel_display_power.c now, so we don't need to export
it.
> +{
> + int ret;
> +
> + do {
> + ret = sandybridge_pcode_write_timeout(i915,
> + ICL_PCODE_EXIT_TCCOLD,
> + 0, 250, 1);
> +
> + } while (ret == -EAGAIN);
> +
> + if (!ret)
> + msleep(1);
Could you add a comment explaining that we need the sleep, since
according to BSpec the above request may not have completed even though
it returned success?
> +
> + if (ret)
> + drm_dbg_kms(&i915->drm, "TC cold block %s\n",
> + (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 a1afcee48818..168d8896fcfd 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.h
> +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> @@ -9,6 +9,7 @@
> #include <linux/mutex.h>
> #include <linux/types.h>
>
> +struct drm_i915_private;
> struct intel_digital_port;
>
> bool intel_tc_port_connected(struct intel_digital_port *dig_port);
> @@ -29,5 +30,6 @@ bool intel_tc_port_ref_held(struct intel_digital_port *dig_port);
> 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);
>
> #endif /* __INTEL_TC_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 17484345cb80..b111815d6596 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9107,6 +9107,7 @@ enum {
> #define ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point) (((point) << 16) | (0x1 << 8))
> #define GEN6_PCODE_READ_D_COMP 0x10
> #define GEN6_PCODE_WRITE_D_COMP 0x11
> +#define ICL_PCODE_EXIT_TCCOLD 0x12
> #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
> #define DISPLAY_IPS_CONTROL 0x19
> /* See also IPS_CTL */
> --
> 2.26.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 5/6] drm/i915/tc/icl: Implement TC cold sequences
2020-04-01 12:43 ` Imre Deak
@ 2020-04-01 22:35 ` Souza, Jose
2020-04-02 1:02 ` Imre Deak
0 siblings, 1 reply; 21+ messages in thread
From: Souza, Jose @ 2020-04-01 22:35 UTC (permalink / raw)
To: Deak, Imre
Cc: Chiou, Cooper, intel-gfx@lists.freedesktop.org,
kai.heng.feng@canonical.com
On Wed, 2020-04-01 at 15:43 +0300, Imre Deak wrote:
> On Tue, Mar 31, 2020 at 05:41:19PM -0700, José Roberto de Souza
> wrote:
> > This is required for legacy/static TC ports as IOM is not aware of
> > the connection and will not trigger the TC cold exit.
> >
> > Just request PCODE to exit TCCOLD is not enough as it could enter
> > again be driver makes use of the port, to prevent it BSpec states
> > that
> > aux powerwell should be held.
> >
> > So here embedding the TC cold exit sequence into ICL aux enable,
> > it will enable aux, request tc cold exit and depending in the TC
> > live
> > state continue with the regular aux enable sequence.
> >
> > And then turning on aux power well during tc lock and turning off
> > during unlock both depending into the TC port refcount.
> >
> > BSpec: 21750
> > Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1296
> > 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>
> > ---
> >
> > Will run some tests in the office with TBT dockstation to check if
> > it will be a issue keep both aux enabled. Otherwise more changes
> > will
> > be required here.
> >
> > .../drm/i915/display/intel_display_power.c | 12 ++++-
> > .../drm/i915/display/intel_display_types.h | 1 +
> > drivers/gpu/drm/i915/display/intel_tc.c | 47
> > ++++++++++++++++++-
> > drivers/gpu/drm/i915/display/intel_tc.h | 2 +
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > 5 files changed, 59 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index dbd61517ba63..1ccd57d645c7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -592,9 +592,17 @@ icl_tc_phy_aux_power_well_enable(struct
> > drm_i915_private *dev_priv,
> >
> > _hsw_power_well_enable(dev_priv, power_well);
> >
> > - /* TODO ICL TC cold handling */
> > + if (INTEL_GEN(dev_priv) == 11)
>
> Should be if (ICL && dig_port->tc_legacy_port)
Makes sence.
Oh so we could use it on __intel_tc_port_lock() too and
don't do this stuff for non legacy ports. Until we get a report of a
system with a wrong VBT :P
>
> > + intel_tc_icl_tc_cold_exit(dev_priv);
> >
> > - _hsw_power_well_continue_enable(dev_priv, power_well);
> > + /*
> > + * To avoid power well enable timeouts when disconnected or in
> > TBT mode
> > + * when doing the TC cold exit sequence for GEN11
> > + */
> > + if (INTEL_GEN(dev_priv) != 11 ||
> > + (intel_tc_port_live_status_mask(dig_port) &
> > + (TC_PORT_LEGACY | TC_PORT_DP_ALT)))
> > + _hsw_power_well_continue_enable(dev_priv, power_well);
>
> Why can't we call this unconditionally?
Because we are requesting aux power of regular TC ports as part of tc
cold exit sequence, if the port is disconnected it will timeout in
hsw_wait_for_power_well_enable().
Anyways it is wrong as it is not
taking care of TBT ports, so changing to: if (INTEL_GEN(dev_priv) != 11
|| !dig_port->tc_legacy_port || intel_tc_port_live_status_mask())
>
> >
> > if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc-
> > >hsw.is_tc_tbt) {
> > enum tc_port tc_port;
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 176ab5f1e867..a9a4a3c1b4d7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1391,6 +1391,7 @@ struct intel_digital_port {
> > enum intel_display_power_domain ddi_io_power_domain;
> > struct mutex tc_lock; /* protects the TypeC port mode */
> > intel_wakeref_t tc_lock_wakeref;
> > + intel_wakeref_t tc_cold_wakeref;
> > int tc_link_refcount;
> > bool tc_legacy_port:1;
> > char tc_port_name[8];
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > b/drivers/gpu/drm/i915/display/intel_tc.c
> > index d944be935423..b6d67f069ef7 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)
> > @@ -506,6 +507,13 @@ 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;
> > +
> > + aux_domain = intel_aux_ch_to_power_domain(dig_port-
> > >aux_ch);
> > + dig_port->tc_cold_wakeref =
> > intel_display_power_get(i915, aux_domain);
> > + }
> > +
>
> It would be enough to hold this ref only for the time we access FIA
> regs. Anything else later will hold its own AUX reference, which
> takes
> care of blocking tc-cold. So here something like:
>
According to BSpec we need to keep TC cold blocked while accessing TC
PHY registers too.
> tc_cold_wakeref = block_tc_cold(dig_port);
>
> where block_tc_cold() would return a non-NULL wakeref only for
> ICL/dig_port->tc_legacy_port and TGL.
>
> > if (!dig_port->tc_link_refcount &&
> > intel_tc_port_needs_reset(dig_port))
> > intel_tc_port_reset_mode(dig_port, required_lanes);
>
> unblock_tc_cold(tc_cold_wakeref);
>
> We need to call block/unblock_tc_cold() also in
> intel_tc_port_sanitize()
> and intel_tc_port_connected().
>
> > @@ -519,15 +527,30 @@ 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)
> > +{
> > + struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > + enum intel_display_power_domain aux_domain;
> > + intel_wakeref_t tc_cold_wakeref;
> > +
> > + if (INTEL_GEN(i915) != 11 || dig_port->tc_link_refcount > 0)
> > + 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);
> > +}
> > +
> > 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);
> > +
> > mutex_unlock(&dig_port->tc_lock);
> >
> > - intel_display_power_put_async(i915, POWER_DOMAIN_DISPLAY_CORE,
> > - wakeref);
> > + intel_display_power_put_async(i915, POWER_DOMAIN_DISPLAY_CORE,
> > wakeref);
> > }
> >
> > bool intel_tc_port_ref_held(struct intel_digital_port *dig_port)
> > @@ -548,6 +571,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);
> > mutex_unlock(&dig_port->tc_lock);
> > }
> >
> > @@ -568,3 +592,22 @@ void intel_tc_port_init(struct
> > intel_digital_port *dig_port, bool is_legacy)
> > dig_port->tc_link_refcount = 0;
> > tc_port_load_fia_params(i915, dig_port);
> > }
> > +
> > +void intel_tc_icl_tc_cold_exit(struct drm_i915_private *i915)
>
> This could be in intel_display_power.c now, so we don't need to
> export
> it.
Okay.
>
> > +{
> > + int ret;
> > +
> > + do {
> > + ret = sandybridge_pcode_write_timeout(i915,
> > + ICL_PCODE_EXIT_TC
> > COLD,
> > + 0, 250, 1);
> > +
> > + } while (ret == -EAGAIN);
> > +
> > + if (!ret)
> > + msleep(1);
>
> Could you add a comment explaining that we need the sleep, since
> according to BSpec the above request may not have completed even
> though
> it returned success?
Sure
>
> > +
> > + if (ret)
> > + drm_dbg_kms(&i915->drm, "TC cold block %s\n",
> > + (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 a1afcee48818..168d8896fcfd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> > @@ -9,6 +9,7 @@
> > #include <linux/mutex.h>
> > #include <linux/types.h>
> >
> > +struct drm_i915_private;
> > struct intel_digital_port;
> >
> > bool intel_tc_port_connected(struct intel_digital_port *dig_port);
> > @@ -29,5 +30,6 @@ bool intel_tc_port_ref_held(struct
> > intel_digital_port *dig_port);
> > 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);
> >
> > #endif /* __INTEL_TC_H__ */
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 17484345cb80..b111815d6596 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -9107,6 +9107,7 @@ enum {
> > #define ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point) (((poin
> > t) << 16) | (0x1 << 8))
> > #define GEN6_PCODE_READ_D_COMP 0x10
> > #define GEN6_PCODE_WRITE_D_COMP 0x11
> > +#define ICL_PCODE_EXIT_TCCOLD 0x12
> > #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
> > #define DISPLAY_IPS_CONTROL 0x19
> > /* See also IPS_CTL */
> > --
> > 2.26.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 5/6] drm/i915/tc/icl: Implement TC cold sequences
2020-04-01 22:35 ` Souza, Jose
@ 2020-04-02 1:02 ` Imre Deak
2020-04-03 1:18 ` Souza, Jose
0 siblings, 1 reply; 21+ messages in thread
From: Imre Deak @ 2020-04-02 1:02 UTC (permalink / raw)
To: Souza, Jose
Cc: Chiou, Cooper, intel-gfx@lists.freedesktop.org,
kai.heng.feng@canonical.com
On Thu, Apr 02, 2020 at 01:35:30AM +0300, Souza, Jose wrote:
> On Wed, 2020-04-01 at 15:43 +0300, Imre Deak wrote:
> > On Tue, Mar 31, 2020 at 05:41:19PM -0700, José Roberto de Souza
> > wrote:
> > > This is required for legacy/static TC ports as IOM is not aware of
> > > the connection and will not trigger the TC cold exit.
> > >
> > > Just request PCODE to exit TCCOLD is not enough as it could enter
> > > again be driver makes use of the port, to prevent it BSpec states
> > > that
> > > aux powerwell should be held.
> > >
> > > So here embedding the TC cold exit sequence into ICL aux enable,
> > > it will enable aux, request tc cold exit and depending in the TC
> > > live
> > > state continue with the regular aux enable sequence.
> > >
> > > And then turning on aux power well during tc lock and turning off
> > > during unlock both depending into the TC port refcount.
> > >
> > > BSpec: 21750
> > > Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1296
> > > 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>
> > > ---
> > >
> > > Will run some tests in the office with TBT dockstation to check if
> > > it will be a issue keep both aux enabled. Otherwise more changes
> > > will
> > > be required here.
> > >
> > > .../drm/i915/display/intel_display_power.c | 12 ++++-
> > > .../drm/i915/display/intel_display_types.h | 1 +
> > > drivers/gpu/drm/i915/display/intel_tc.c | 47
> > > ++++++++++++++++++-
> > > drivers/gpu/drm/i915/display/intel_tc.h | 2 +
> > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > 5 files changed, 59 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > index dbd61517ba63..1ccd57d645c7 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > @@ -592,9 +592,17 @@ icl_tc_phy_aux_power_well_enable(struct
> > > drm_i915_private *dev_priv,
> > >
> > > _hsw_power_well_enable(dev_priv, power_well);
> > >
> > > - /* TODO ICL TC cold handling */
> > > + if (INTEL_GEN(dev_priv) == 11)
> >
> > Should be if (ICL && dig_port->tc_legacy_port)
>
> Makes sence.
> Oh so we could use it on __intel_tc_port_lock() too and
> don't do this stuff for non legacy ports. Until we get a report of a
> system with a wrong VBT :P
Yes, it's a loss of the vendor shipping a corrupt VBT. But we'll print
an error and fix up the flag.
> > > + intel_tc_icl_tc_cold_exit(dev_priv);
> > >
> > > - _hsw_power_well_continue_enable(dev_priv, power_well);
> > > + /*
> > > + * To avoid power well enable timeouts when disconnected or in
> > > TBT mode
> > > + * when doing the TC cold exit sequence for GEN11
> > > + */
> > > + if (INTEL_GEN(dev_priv) != 11 ||
> > > + (intel_tc_port_live_status_mask(dig_port) &
> > > + (TC_PORT_LEGACY | TC_PORT_DP_ALT)))
> > > + _hsw_power_well_continue_enable(dev_priv, power_well);
> >
> > Why can't we call this unconditionally?
>
> Because we are requesting aux power of regular TC ports as part of tc
> cold exit sequence, if the port is disconnected it will timeout in
> hsw_wait_for_power_well_enable().
>
> Anyways it is wrong as it is not
> taking care of TBT ports, so changing to: if (INTEL_GEN(dev_priv) != 11
> || !dig_port->tc_legacy_port || intel_tc_port_live_status_mask())
What I thought is that the legacy AUX power request will ack after the
PCODE request completes, regardless of whether the sink is connected or
not. If that is not the case then let's just suppress the timeout for
legacy AUX power wells the same way we do that for TBT AUX. I don't like
to make the above call conditional on the live status flag, as that can
change at any moment. So in either case let's make the above call
unconditional.
> > >
> > > if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc-
> > > >hsw.is_tc_tbt) {
Could you check your email client, so that it doesn't wrap lines?
> > > enum tc_port tc_port;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 176ab5f1e867..a9a4a3c1b4d7 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -1391,6 +1391,7 @@ struct intel_digital_port {
> > > enum intel_display_power_domain ddi_io_power_domain;
> > > struct mutex tc_lock; /* protects the TypeC port mode */
> > > intel_wakeref_t tc_lock_wakeref;
> > > + intel_wakeref_t tc_cold_wakeref;
> > > int tc_link_refcount;
> > > bool tc_legacy_port:1;
> > > char tc_port_name[8];
> > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > > b/drivers/gpu/drm/i915/display/intel_tc.c
> > > index d944be935423..b6d67f069ef7 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)
> > > @@ -506,6 +507,13 @@ 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;
> > > +
> > > + aux_domain = intel_aux_ch_to_power_domain(dig_port-
> > > >aux_ch);
> > > + dig_port->tc_cold_wakeref =
> > > intel_display_power_get(i915, aux_domain);
> > > + }
> > > +
> >
> > It would be enough to hold this ref only for the time we access FIA
> > regs. Anything else later will hold its own AUX reference, which
> > takes
> > care of blocking tc-cold. So here something like:
> >
>
> According to BSpec we need to keep TC cold blocked while accessing TC
> PHY registers too.
Yes, hence blocking it here whenever accessing the HW and elsewhere (AUX
transfers, modeset) whenever holding an AUX reference. See my comment
below about the other two functions in intel_tc that needs the
block/unblock.
>
> > tc_cold_wakeref = block_tc_cold(dig_port);
> >
> > where block_tc_cold() would return a non-NULL wakeref only for
> > ICL/dig_port->tc_legacy_port and TGL.
> >
> > > if (!dig_port->tc_link_refcount &&
> > > intel_tc_port_needs_reset(dig_port))
> > > intel_tc_port_reset_mode(dig_port, required_lanes);
> >
> > unblock_tc_cold(tc_cold_wakeref);
> >
> > We need to call block/unblock_tc_cold() also in
> > intel_tc_port_sanitize() and intel_tc_port_connected().
> >
> > > @@ -519,15 +527,30 @@ 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)
> > > +{
> > > + struct drm_i915_private *i915 = to_i915(dig_port-
> > > >base.base.dev);
> > > + enum intel_display_power_domain aux_domain;
> > > + intel_wakeref_t tc_cold_wakeref;
> > > +
> > > + if (INTEL_GEN(i915) != 11 || dig_port->tc_link_refcount > 0)
> > > + 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);
> > > +}
> > > +
> > > 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);
> > > +
> > > mutex_unlock(&dig_port->tc_lock);
> > >
> > > - intel_display_power_put_async(i915, POWER_DOMAIN_DISPLAY_CORE,
> > > - wakeref);
> > > + intel_display_power_put_async(i915, POWER_DOMAIN_DISPLAY_CORE,
> > > wakeref);
> > > }
> > >
> > > bool intel_tc_port_ref_held(struct intel_digital_port *dig_port)
> > > @@ -548,6 +571,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);
> > > mutex_unlock(&dig_port->tc_lock);
> > > }
> > >
> > > @@ -568,3 +592,22 @@ void intel_tc_port_init(struct
> > > intel_digital_port *dig_port, bool is_legacy)
> > > dig_port->tc_link_refcount = 0;
> > > tc_port_load_fia_params(i915, dig_port);
> > > }
> > > +
> > > +void intel_tc_icl_tc_cold_exit(struct drm_i915_private *i915)
> >
> > This could be in intel_display_power.c now, so we don't need to
> > export
> > it.
>
> Okay.
>
> >
> > > +{
> > > + int ret;
> > > +
> > > + do {
> > > + ret = sandybridge_pcode_write_timeout(i915,
> > > + ICL_PCODE_EXIT_TC
> > > COLD,
> > > + 0, 250, 1);
> > > +
> > > + } while (ret == -EAGAIN);
> > > +
> > > + if (!ret)
> > > + msleep(1);
> >
> > Could you add a comment explaining that we need the sleep, since
> > according to BSpec the above request may not have completed even
> > though
> > it returned success?
>
> Sure
>
> >
> > > +
> > > + if (ret)
> > > + drm_dbg_kms(&i915->drm, "TC cold block %s\n",
> > > + (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 a1afcee48818..168d8896fcfd 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_tc.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> > > @@ -9,6 +9,7 @@
> > > #include <linux/mutex.h>
> > > #include <linux/types.h>
> > >
> > > +struct drm_i915_private;
> > > struct intel_digital_port;
> > >
> > > bool intel_tc_port_connected(struct intel_digital_port *dig_port);
> > > @@ -29,5 +30,6 @@ bool intel_tc_port_ref_held(struct
> > > intel_digital_port *dig_port);
> > > 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);
> > >
> > > #endif /* __INTEL_TC_H__ */
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 17484345cb80..b111815d6596 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -9107,6 +9107,7 @@ enum {
> > > #define ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point) (((poin
> > > t) << 16) | (0x1 << 8))
> > > #define GEN6_PCODE_READ_D_COMP 0x10
> > > #define GEN6_PCODE_WRITE_D_COMP 0x11
> > > +#define ICL_PCODE_EXIT_TCCOLD 0x12
> > > #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
> > > #define DISPLAY_IPS_CONTROL 0x19
> > > /* See also IPS_CTL */
> > > --
> > > 2.26.0
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 5/6] drm/i915/tc/icl: Implement TC cold sequences
2020-04-02 1:02 ` Imre Deak
@ 2020-04-03 1:18 ` Souza, Jose
2020-04-06 12:57 ` Imre Deak
0 siblings, 1 reply; 21+ messages in thread
From: Souza, Jose @ 2020-04-03 1:18 UTC (permalink / raw)
To: Deak, Imre; +Cc: intel-gfx@lists.freedesktop.org
Hi Imre
I guess I did all the requested changes but trybot got some
warnings that I will need more time to understand and fix it.
If you have time please check if is this way that you are asking:
https://github.com/zehortigoza/linux/tree/tccold-v3
Trybot warnings:
https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_5784/bat-all.html
Thanks
On Thu, 2020-04-02 at 04:02 +0300, Imre Deak wrote:
> On Thu, Apr 02, 2020 at 01:35:30AM +0300, Souza, Jose wrote:
> > On Wed, 2020-04-01 at 15:43 +0300, Imre Deak wrote:
> > > On Tue, Mar 31, 2020 at 05:41:19PM -0700, José Roberto de Souza
> > > wrote:
> > > > This is required for legacy/static TC ports as IOM is not aware
> > > > of
> > > > the connection and will not trigger the TC cold exit.
> > > >
> > > > Just request PCODE to exit TCCOLD is not enough as it could
> > > > enter
> > > > again be driver makes use of the port, to prevent it BSpec
> > > > states
> > > > that
> > > > aux powerwell should be held.
> > > >
> > > > So here embedding the TC cold exit sequence into ICL aux
> > > > enable,
> > > > it will enable aux, request tc cold exit and depending in the
> > > > TC
> > > > live
> > > > state continue with the regular aux enable sequence.
> > > >
> > > > And then turning on aux power well during tc lock and turning
> > > > off
> > > > during unlock both depending into the TC port refcount.
> > > >
> > > > BSpec: 21750
> > > > Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1296
> > > > 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>
> > > > ---
> > > >
> > > > Will run some tests in the office with TBT dockstation to check
> > > > if
> > > > it will be a issue keep both aux enabled. Otherwise more
> > > > changes
> > > > will
> > > > be required here.
> > > >
> > > > .../drm/i915/display/intel_display_power.c | 12 ++++-
> > > > .../drm/i915/display/intel_display_types.h | 1 +
> > > > drivers/gpu/drm/i915/display/intel_tc.c | 47
> > > > ++++++++++++++++++-
> > > > drivers/gpu/drm/i915/display/intel_tc.h | 2 +
> > > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > > 5 files changed, 59 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > index dbd61517ba63..1ccd57d645c7 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > @@ -592,9 +592,17 @@ icl_tc_phy_aux_power_well_enable(struct
> > > > drm_i915_private *dev_priv,
> > > >
> > > > _hsw_power_well_enable(dev_priv, power_well);
> > > >
> > > > - /* TODO ICL TC cold handling */
> > > > + if (INTEL_GEN(dev_priv) == 11)
> > >
> > > Should be if (ICL && dig_port->tc_legacy_port)
> >
> > Makes sence.
> > Oh so we could use it on __intel_tc_port_lock() too and
> > don't do this stuff for non legacy ports. Until we get a report of
> > a
> > system with a wrong VBT :P
>
> Yes, it's a loss of the vendor shipping a corrupt VBT. But we'll
> print
> an error and fix up the flag.
>
> > > > + intel_tc_icl_tc_cold_exit(dev_priv);
> > > >
> > > > - _hsw_power_well_continue_enable(dev_priv, power_well);
> > > > + /*
> > > > + * To avoid power well enable timeouts when
> > > > disconnected or in
> > > > TBT mode
> > > > + * when doing the TC cold exit sequence for GEN11
> > > > + */
> > > > + if (INTEL_GEN(dev_priv) != 11 ||
> > > > + (intel_tc_port_live_status_mask(dig_port) &
> > > > + (TC_PORT_LEGACY | TC_PORT_DP_ALT)))
> > > > + _hsw_power_well_continue_enable(dev_priv,
> > > > power_well);
> > >
> > > Why can't we call this unconditionally?
> >
> > Because we are requesting aux power of regular TC ports as part of
> > tc
> > cold exit sequence, if the port is disconnected it will timeout in
> > hsw_wait_for_power_well_enable().
> >
> > Anyways it is wrong as it is not
> > taking care of TBT ports, so changing to: if (INTEL_GEN(dev_priv)
> > != 11
> > > > !dig_port->tc_legacy_port || intel_tc_port_live_status_mask())
>
> What I thought is that the legacy AUX power request will ack after
> the
> PCODE request completes, regardless of whether the sink is connected
> or
> not. If that is not the case then let's just suppress the timeout for
> legacy AUX power wells the same way we do that for TBT AUX. I don't
> like
> to make the above call conditional on the live status flag, as that
> can
> change at any moment. So in either case let's make the above call
> unconditional.
>
> > > >
> > > > if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc-
> > > > > hsw.is_tc_tbt) {
>
> Could you check your email client, so that it doesn't wrap lines?
>
> > > > enum tc_port tc_port;
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > index 176ab5f1e867..a9a4a3c1b4d7 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > @@ -1391,6 +1391,7 @@ struct intel_digital_port {
> > > > enum intel_display_power_domain ddi_io_power_domain;
> > > > struct mutex tc_lock; /* protects the TypeC port mode
> > > > */
> > > > intel_wakeref_t tc_lock_wakeref;
> > > > + intel_wakeref_t tc_cold_wakeref;
> > > > int tc_link_refcount;
> > > > bool tc_legacy_port:1;
> > > > char tc_port_name[8];
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > > > b/drivers/gpu/drm/i915/display/intel_tc.c
> > > > index d944be935423..b6d67f069ef7 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)
> > > > @@ -506,6 +507,13 @@ 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;
> > > > +
> > > > + aux_domain =
> > > > intel_aux_ch_to_power_domain(dig_port-
> > > > > aux_ch);
> > > > + dig_port->tc_cold_wakeref =
> > > > intel_display_power_get(i915, aux_domain);
> > > > + }
> > > > +
> > >
> > > It would be enough to hold this ref only for the time we access
> > > FIA
> > > regs. Anything else later will hold its own AUX reference, which
> > > takes
> > > care of blocking tc-cold. So here something like:
> > >
> >
> > According to BSpec we need to keep TC cold blocked while accessing
> > TC
> > PHY registers too.
>
> Yes, hence blocking it here whenever accessing the HW and elsewhere
> (AUX
> transfers, modeset) whenever holding an AUX reference. See my comment
> below about the other two functions in intel_tc that needs the
> block/unblock.
>
> > > tc_cold_wakeref = block_tc_cold(dig_port);
> > >
> > > where block_tc_cold() would return a non-NULL wakeref only for
> > > ICL/dig_port->tc_legacy_port and TGL.
> > >
> > > > if (!dig_port->tc_link_refcount &&
> > > > intel_tc_port_needs_reset(dig_port))
> > > > intel_tc_port_reset_mode(dig_port,
> > > > required_lanes);
> > >
> > > unblock_tc_cold(tc_cold_wakeref);
> > >
> > > We need to call block/unblock_tc_cold() also in
> > > intel_tc_port_sanitize() and intel_tc_port_connected().
> > >
> > > > @@ -519,15 +527,30 @@ 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)
> > > > +{
> > > > + struct drm_i915_private *i915 = to_i915(dig_port-
> > > > > base.base.dev);
> > > > + enum intel_display_power_domain aux_domain;
> > > > + intel_wakeref_t tc_cold_wakeref;
> > > > +
> > > > + if (INTEL_GEN(i915) != 11 || dig_port->tc_link_refcount
> > > > > 0)
> > > > + 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);
> > > > +}
> > > > +
> > > > 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);
> > > > +
> > > > mutex_unlock(&dig_port->tc_lock);
> > > >
> > > > - intel_display_power_put_async(i915,
> > > > POWER_DOMAIN_DISPLAY_CORE,
> > > > - wakeref);
> > > > + intel_display_power_put_async(i915,
> > > > POWER_DOMAIN_DISPLAY_CORE,
> > > > wakeref);
> > > > }
> > > >
> > > > bool intel_tc_port_ref_held(struct intel_digital_port
> > > > *dig_port)
> > > > @@ -548,6 +571,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);
> > > > mutex_unlock(&dig_port->tc_lock);
> > > > }
> > > >
> > > > @@ -568,3 +592,22 @@ void intel_tc_port_init(struct
> > > > intel_digital_port *dig_port, bool is_legacy)
> > > > dig_port->tc_link_refcount = 0;
> > > > tc_port_load_fia_params(i915, dig_port);
> > > > }
> > > > +
> > > > +void intel_tc_icl_tc_cold_exit(struct drm_i915_private *i915)
> > >
> > > This could be in intel_display_power.c now, so we don't need to
> > > export
> > > it.
> >
> > Okay.
> >
> > > > +{
> > > > + int ret;
> > > > +
> > > > + do {
> > > > + ret = sandybridge_pcode_write_timeout(i915,
> > > > + ICL_PCODE
> > > > _EXIT_TC
> > > > COLD,
> > > > + 0, 250,
> > > > 1);
> > > > +
> > > > + } while (ret == -EAGAIN);
> > > > +
> > > > + if (!ret)
> > > > + msleep(1);
> > >
> > > Could you add a comment explaining that we need the sleep, since
> > > according to BSpec the above request may not have completed even
> > > though
> > > it returned success?
> >
> > Sure
> >
> > > > +
> > > > + if (ret)
> > > > + drm_dbg_kms(&i915->drm, "TC cold block %s\n",
> > > > + (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 a1afcee48818..168d8896fcfd 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_tc.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> > > > @@ -9,6 +9,7 @@
> > > > #include <linux/mutex.h>
> > > > #include <linux/types.h>
> > > >
> > > > +struct drm_i915_private;
> > > > struct intel_digital_port;
> > > >
> > > > bool intel_tc_port_connected(struct intel_digital_port
> > > > *dig_port);
> > > > @@ -29,5 +30,6 @@ bool intel_tc_port_ref_held(struct
> > > > intel_digital_port *dig_port);
> > > > 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);
> > > >
> > > > #endif /* __INTEL_TC_H__ */
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > index 17484345cb80..b111815d6596 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -9107,6 +9107,7 @@ enum {
> > > > #define ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point)
> > > > (((poin
> > > > t) << 16) | (0x1 << 8))
> > > > #define GEN6_PCODE_READ_D_COMP 0x10
> > > > #define GEN6_PCODE_WRITE_D_COMP 0x11
> > > > +#define ICL_PCODE_EXIT_TCCOLD 0x12
> > > > #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
> > > > #define DISPLAY_IPS_CONTROL 0x19
> > > > /* See also IPS_CTL */
> > > > --
> > > > 2.26.0
> > > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 5/6] drm/i915/tc/icl: Implement TC cold sequences
2020-04-03 1:18 ` Souza, Jose
@ 2020-04-06 12:57 ` Imre Deak
0 siblings, 0 replies; 21+ messages in thread
From: Imre Deak @ 2020-04-06 12:57 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org
On Fri, Apr 03, 2020 at 04:18:54AM +0300, Souza, Jose wrote:
> Hi Imre
>
> I guess I did all the requested changes but trybot got some
> warnings that I will need more time to understand and fix it.
>
> If you have time please check if is this way that you are asking:
> https://github.com/zehortigoza/linux/tree/tccold-v3
Yes looks ok overall, I added a few comments on the trybot patches.
> Trybot warnings:
> https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_5784/bat-all.html
I see here
<3> [428.836598] [drm:tc_port_live_status_mask [i915]] *ERROR* Port D/TC#2: live status 00000004 mismatches the legacy port flag, fix flag
which is a known buggy VBT issue and
i915 0000:00:02.0: drm_WARN_ON(!power_well->desc->hsw.is_tc_tbt || !(((&(dev_priv)->__info)->gen) == 11 && dig_port->tc_legacy_port))
which is just a typo.
> Thanks
>
> On Thu, 2020-04-02 at 04:02 +0300, Imre Deak wrote:
> > On Thu, Apr 02, 2020 at 01:35:30AM +0300, Souza, Jose wrote:
> > > On Wed, 2020-04-01 at 15:43 +0300, Imre Deak wrote:
> > > > On Tue, Mar 31, 2020 at 05:41:19PM -0700, José Roberto de Souza
> > > > wrote:
> > > > > This is required for legacy/static TC ports as IOM is not aware
> > > > > of
> > > > > the connection and will not trigger the TC cold exit.
> > > > >
> > > > > Just request PCODE to exit TCCOLD is not enough as it could
> > > > > enter
> > > > > again be driver makes use of the port, to prevent it BSpec
> > > > > states
> > > > > that
> > > > > aux powerwell should be held.
> > > > >
> > > > > So here embedding the TC cold exit sequence into ICL aux
> > > > > enable,
> > > > > it will enable aux, request tc cold exit and depending in the
> > > > > TC
> > > > > live
> > > > > state continue with the regular aux enable sequence.
> > > > >
> > > > > And then turning on aux power well during tc lock and turning
> > > > > off
> > > > > during unlock both depending into the TC port refcount.
> > > > >
> > > > > BSpec: 21750
> > > > > Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1296
> > > > > 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>
> > > > > ---
> > > > >
> > > > > Will run some tests in the office with TBT dockstation to check
> > > > > if
> > > > > it will be a issue keep both aux enabled. Otherwise more
> > > > > changes
> > > > > will
> > > > > be required here.
> > > > >
> > > > > .../drm/i915/display/intel_display_power.c | 12 ++++-
> > > > > .../drm/i915/display/intel_display_types.h | 1 +
> > > > > drivers/gpu/drm/i915/display/intel_tc.c | 47
> > > > > ++++++++++++++++++-
> > > > > drivers/gpu/drm/i915/display/intel_tc.h | 2 +
> > > > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > > > 5 files changed, 59 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > > index dbd61517ba63..1ccd57d645c7 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > > @@ -592,9 +592,17 @@ icl_tc_phy_aux_power_well_enable(struct
> > > > > drm_i915_private *dev_priv,
> > > > >
> > > > > _hsw_power_well_enable(dev_priv, power_well);
> > > > >
> > > > > - /* TODO ICL TC cold handling */
> > > > > + if (INTEL_GEN(dev_priv) == 11)
> > > >
> > > > Should be if (ICL && dig_port->tc_legacy_port)
> > >
> > > Makes sence.
> > > Oh so we could use it on __intel_tc_port_lock() too and
> > > don't do this stuff for non legacy ports. Until we get a report of
> > > a
> > > system with a wrong VBT :P
> >
> > Yes, it's a loss of the vendor shipping a corrupt VBT. But we'll
> > print
> > an error and fix up the flag.
> >
> > > > > + intel_tc_icl_tc_cold_exit(dev_priv);
> > > > >
> > > > > - _hsw_power_well_continue_enable(dev_priv, power_well);
> > > > > + /*
> > > > > + * To avoid power well enable timeouts when
> > > > > disconnected or in
> > > > > TBT mode
> > > > > + * when doing the TC cold exit sequence for GEN11
> > > > > + */
> > > > > + if (INTEL_GEN(dev_priv) != 11 ||
> > > > > + (intel_tc_port_live_status_mask(dig_port) &
> > > > > + (TC_PORT_LEGACY | TC_PORT_DP_ALT)))
> > > > > + _hsw_power_well_continue_enable(dev_priv,
> > > > > power_well);
> > > >
> > > > Why can't we call this unconditionally?
> > >
> > > Because we are requesting aux power of regular TC ports as part of
> > > tc
> > > cold exit sequence, if the port is disconnected it will timeout in
> > > hsw_wait_for_power_well_enable().
> > >
> > > Anyways it is wrong as it is not
> > > taking care of TBT ports, so changing to: if (INTEL_GEN(dev_priv)
> > > != 11
> > > > > !dig_port->tc_legacy_port || intel_tc_port_live_status_mask())
> >
> > What I thought is that the legacy AUX power request will ack after
> > the
> > PCODE request completes, regardless of whether the sink is connected
> > or
> > not. If that is not the case then let's just suppress the timeout for
> > legacy AUX power wells the same way we do that for TBT AUX. I don't
> > like
> > to make the above call conditional on the live status flag, as that
> > can
> > change at any moment. So in either case let's make the above call
> > unconditional.
> >
> > > > >
> > > > > if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc-
> > > > > > hsw.is_tc_tbt) {
> >
> > Could you check your email client, so that it doesn't wrap lines?
> >
> > > > > enum tc_port tc_port;
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > > index 176ab5f1e867..a9a4a3c1b4d7 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > > @@ -1391,6 +1391,7 @@ struct intel_digital_port {
> > > > > enum intel_display_power_domain ddi_io_power_domain;
> > > > > struct mutex tc_lock; /* protects the TypeC port mode
> > > > > */
> > > > > intel_wakeref_t tc_lock_wakeref;
> > > > > + intel_wakeref_t tc_cold_wakeref;
> > > > > int tc_link_refcount;
> > > > > bool tc_legacy_port:1;
> > > > > char tc_port_name[8];
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > > > > b/drivers/gpu/drm/i915/display/intel_tc.c
> > > > > index d944be935423..b6d67f069ef7 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)
> > > > > @@ -506,6 +507,13 @@ 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;
> > > > > +
> > > > > + aux_domain =
> > > > > intel_aux_ch_to_power_domain(dig_port-
> > > > > > aux_ch);
> > > > > + dig_port->tc_cold_wakeref =
> > > > > intel_display_power_get(i915, aux_domain);
> > > > > + }
> > > > > +
> > > >
> > > > It would be enough to hold this ref only for the time we access
> > > > FIA
> > > > regs. Anything else later will hold its own AUX reference, which
> > > > takes
> > > > care of blocking tc-cold. So here something like:
> > > >
> > >
> > > According to BSpec we need to keep TC cold blocked while accessing
> > > TC
> > > PHY registers too.
> >
> > Yes, hence blocking it here whenever accessing the HW and elsewhere
> > (AUX
> > transfers, modeset) whenever holding an AUX reference. See my comment
> > below about the other two functions in intel_tc that needs the
> > block/unblock.
> >
> > > > tc_cold_wakeref = block_tc_cold(dig_port);
> > > >
> > > > where block_tc_cold() would return a non-NULL wakeref only for
> > > > ICL/dig_port->tc_legacy_port and TGL.
> > > >
> > > > > if (!dig_port->tc_link_refcount &&
> > > > > intel_tc_port_needs_reset(dig_port))
> > > > > intel_tc_port_reset_mode(dig_port,
> > > > > required_lanes);
> > > >
> > > > unblock_tc_cold(tc_cold_wakeref);
> > > >
> > > > We need to call block/unblock_tc_cold() also in
> > > > intel_tc_port_sanitize() and intel_tc_port_connected().
> > > >
> > > > > @@ -519,15 +527,30 @@ 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)
> > > > > +{
> > > > > + struct drm_i915_private *i915 = to_i915(dig_port-
> > > > > > base.base.dev);
> > > > > + enum intel_display_power_domain aux_domain;
> > > > > + intel_wakeref_t tc_cold_wakeref;
> > > > > +
> > > > > + if (INTEL_GEN(i915) != 11 || dig_port->tc_link_refcount
> > > > > > 0)
> > > > > + 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);
> > > > > +}
> > > > > +
> > > > > 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);
> > > > > +
> > > > > mutex_unlock(&dig_port->tc_lock);
> > > > >
> > > > > - intel_display_power_put_async(i915,
> > > > > POWER_DOMAIN_DISPLAY_CORE,
> > > > > - wakeref);
> > > > > + intel_display_power_put_async(i915,
> > > > > POWER_DOMAIN_DISPLAY_CORE,
> > > > > wakeref);
> > > > > }
> > > > >
> > > > > bool intel_tc_port_ref_held(struct intel_digital_port
> > > > > *dig_port)
> > > > > @@ -548,6 +571,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);
> > > > > mutex_unlock(&dig_port->tc_lock);
> > > > > }
> > > > >
> > > > > @@ -568,3 +592,22 @@ void intel_tc_port_init(struct
> > > > > intel_digital_port *dig_port, bool is_legacy)
> > > > > dig_port->tc_link_refcount = 0;
> > > > > tc_port_load_fia_params(i915, dig_port);
> > > > > }
> > > > > +
> > > > > +void intel_tc_icl_tc_cold_exit(struct drm_i915_private *i915)
> > > >
> > > > This could be in intel_display_power.c now, so we don't need to
> > > > export
> > > > it.
> > >
> > > Okay.
> > >
> > > > > +{
> > > > > + int ret;
> > > > > +
> > > > > + do {
> > > > > + ret = sandybridge_pcode_write_timeout(i915,
> > > > > + ICL_PCODE
> > > > > _EXIT_TC
> > > > > COLD,
> > > > > + 0, 250,
> > > > > 1);
> > > > > +
> > > > > + } while (ret == -EAGAIN);
> > > > > +
> > > > > + if (!ret)
> > > > > + msleep(1);
> > > >
> > > > Could you add a comment explaining that we need the sleep, since
> > > > according to BSpec the above request may not have completed even
> > > > though
> > > > it returned success?
> > >
> > > Sure
> > >
> > > > > +
> > > > > + if (ret)
> > > > > + drm_dbg_kms(&i915->drm, "TC cold block %s\n",
> > > > > + (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 a1afcee48818..168d8896fcfd 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_tc.h
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> > > > > @@ -9,6 +9,7 @@
> > > > > #include <linux/mutex.h>
> > > > > #include <linux/types.h>
> > > > >
> > > > > +struct drm_i915_private;
> > > > > struct intel_digital_port;
> > > > >
> > > > > bool intel_tc_port_connected(struct intel_digital_port
> > > > > *dig_port);
> > > > > @@ -29,5 +30,6 @@ bool intel_tc_port_ref_held(struct
> > > > > intel_digital_port *dig_port);
> > > > > 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);
> > > > >
> > > > > #endif /* __INTEL_TC_H__ */
> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > index 17484345cb80..b111815d6596 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > @@ -9107,6 +9107,7 @@ enum {
> > > > > #define ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point)
> > > > > (((poin
> > > > > t) << 16) | (0x1 << 8))
> > > > > #define GEN6_PCODE_READ_D_COMP 0x10
> > > > > #define GEN6_PCODE_WRITE_D_COMP 0x11
> > > > > +#define ICL_PCODE_EXIT_TCCOLD 0x12
> > > > > #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
> > > > > #define DISPLAY_IPS_CONTROL 0x19
> > > > > /* See also IPS_CTL */
> > > > > --
> > > > > 2.26.0
> > > > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Intel-gfx] [PATCH 6/6] drm/i915/tc/tgl: Implement TC cold sequences
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
` (3 preceding siblings ...)
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 0:41 ` José Roberto de Souza
2020-04-01 12:55 ` 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
` (4 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-01 0:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Cooper Chiou, Kai-Heng Feng
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))
+
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,
+ .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)
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 6/6] drm/i915/tc/tgl: Implement TC cold sequences
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
2020-04-01 23:36 ` Souza, Jose
0 siblings, 1 reply; 21+ messages in thread
From: Imre Deak @ 2020-04-01 12:55 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: Cooper Chiou, intel-gfx, Kai-Heng Feng
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 6/6] drm/i915/tc/tgl: Implement TC cold sequences
2020-04-01 12:55 ` Imre Deak
@ 2020-04-01 23:36 ` Souza, Jose
2020-04-02 1:08 ` Imre Deak
0 siblings, 1 reply; 21+ messages in thread
From: Souza, Jose @ 2020-04-01 23:36 UTC (permalink / raw)
To: Deak, Imre
Cc: Chiou, Cooper, intel-gfx@lists.freedesktop.org,
kai.heng.feng@canonical.com
On Wed, 2020-04-01 at 15:55 +0300, Imre Deak wrote:
> 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
Okay
> and should also include all the AUX power domains.
So we would call intel_display_power_get() in intel_tc with aux domain
instead of 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,
> > @@ -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,
> > + dom
> > ain);
> > }
> >
> > 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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 6/6] drm/i915/tc/tgl: Implement TC cold sequences
2020-04-01 23:36 ` Souza, Jose
@ 2020-04-02 1:08 ` Imre Deak
0 siblings, 0 replies; 21+ messages in thread
From: Imre Deak @ 2020-04-02 1:08 UTC (permalink / raw)
To: Souza, Jose
Cc: Chiou, Cooper, intel-gfx@lists.freedesktop.org,
kai.heng.feng@canonical.com
On Thu, Apr 02, 2020 at 02:36:53AM +0300, Souza, Jose wrote:
> On Wed, 2020-04-01 at 15:55 +0300, Imre Deak wrote:
> > 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
>
> Okay
>
> > and should also include all the AUX power domains.
>
> So we would call intel_display_power_get() in intel_tc with aux domain
> instead of POWER_DOMAIN_TC_COLD_OFF?
No, but we also need to block tc-cold whenever getting an AUX power
reference.
> > > +
> > > 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,
> > > + dom
> > > ain);
> > > }
> > >
> > > 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [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
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
` (4 preceding siblings ...)
2020-04-01 0:41 ` [Intel-gfx] [PATCH 6/6] drm/i915/tc/tgl: " José Roberto de Souza
@ 2020-04-01 1:31 ` Patchwork
2020-04-01 1:43 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
` (3 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2020-04-01 1:31 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/6] drm/i915/display: Move out code to return the digital_port of the aux ch
URL : https://patchwork.freedesktop.org/series/75345/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
30bd6120eb62 drm/i915/display: Move out code to return the digital_port of the aux ch
a7b982777f87 drm/i915/tc: Export tc_port_live_status_mask()
f5268c854061 drm/i915/display: Add intel_aux_ch_to_power_domain()
353532a4fb43 drm/i915/display: Split hsw_power_well_enable() into two
98804eb04a10 drm/i915/tc/icl: Implement TC cold sequences
-:150: WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.rst
#150: FILE: drivers/gpu/drm/i915/display/intel_tc.c:608:
+ msleep(1);
total: 0 errors, 1 warnings, 0 checks, 127 lines checked
a4d30b0caff3 drm/i915/tc/tgl: Implement TC cold sequences
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/6] drm/i915/display: Move out code to return the digital_port of the aux ch
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
` (5 preceding siblings ...)
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 ` Patchwork
2020-04-01 2:34 ` [Intel-gfx] [PATCH 1/6] " kbuild test robot
` (2 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2020-04-01 1:43 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/6] drm/i915/display: Move out code to return the digital_port of the aux ch
URL : https://patchwork.freedesktop.org/series/75345/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_8230 -> Patchwork_17167
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_17167 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_17167, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_17167:
### IGT changes ###
#### Possible regressions ####
* igt@debugfs_test@read_all_entries:
- fi-skl-lmem: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-skl-lmem/igt@debugfs_test@read_all_entries.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-skl-lmem/igt@debugfs_test@read_all_entries.html
- fi-ivb-3770: [PASS][3] -> [INCOMPLETE][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-ivb-3770/igt@debugfs_test@read_all_entries.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-ivb-3770/igt@debugfs_test@read_all_entries.html
- fi-kbl-guc: [PASS][5] -> [INCOMPLETE][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-kbl-guc/igt@debugfs_test@read_all_entries.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-guc/igt@debugfs_test@read_all_entries.html
- fi-bsw-kefka: NOTRUN -> [INCOMPLETE][7]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-bsw-kefka/igt@debugfs_test@read_all_entries.html
- fi-kbl-x1275: [PASS][8] -> [INCOMPLETE][9]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-kbl-x1275/igt@debugfs_test@read_all_entries.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-x1275/igt@debugfs_test@read_all_entries.html
- fi-blb-e6850: [PASS][10] -> [INCOMPLETE][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-blb-e6850/igt@debugfs_test@read_all_entries.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-blb-e6850/igt@debugfs_test@read_all_entries.html
- fi-bwr-2160: [PASS][12] -> [INCOMPLETE][13]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-bwr-2160/igt@debugfs_test@read_all_entries.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-bwr-2160/igt@debugfs_test@read_all_entries.html
- fi-bdw-5557u: [PASS][14] -> [INCOMPLETE][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-bdw-5557u/igt@debugfs_test@read_all_entries.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-bdw-5557u/igt@debugfs_test@read_all_entries.html
- fi-kbl-r: [PASS][16] -> [INCOMPLETE][17]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-kbl-r/igt@debugfs_test@read_all_entries.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-r/igt@debugfs_test@read_all_entries.html
- fi-skl-guc: NOTRUN -> [INCOMPLETE][18]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-skl-guc/igt@debugfs_test@read_all_entries.html
- fi-apl-guc: [PASS][19] -> [INCOMPLETE][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-apl-guc/igt@debugfs_test@read_all_entries.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-apl-guc/igt@debugfs_test@read_all_entries.html
- fi-icl-y: [PASS][21] -> [INCOMPLETE][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-icl-y/igt@debugfs_test@read_all_entries.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-icl-y/igt@debugfs_test@read_all_entries.html
- fi-snb-2520m: [PASS][23] -> [DMESG-WARN][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-snb-2520m/igt@debugfs_test@read_all_entries.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-snb-2520m/igt@debugfs_test@read_all_entries.html
- fi-kbl-8809g: [PASS][25] -> [INCOMPLETE][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-kbl-8809g/igt@debugfs_test@read_all_entries.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-8809g/igt@debugfs_test@read_all_entries.html
- fi-icl-u2: [PASS][27] -> [INCOMPLETE][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-icl-u2/igt@debugfs_test@read_all_entries.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-icl-u2/igt@debugfs_test@read_all_entries.html
- fi-cfl-8109u: [PASS][29] -> [INCOMPLETE][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-cfl-8109u/igt@debugfs_test@read_all_entries.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-cfl-8109u/igt@debugfs_test@read_all_entries.html
- fi-bxt-dsi: [PASS][31] -> [INCOMPLETE][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-bxt-dsi/igt@debugfs_test@read_all_entries.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-bxt-dsi/igt@debugfs_test@read_all_entries.html
- fi-cfl-8700k: [PASS][33] -> [INCOMPLETE][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-cfl-8700k/igt@debugfs_test@read_all_entries.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-cfl-8700k/igt@debugfs_test@read_all_entries.html
- fi-ilk-650: NOTRUN -> [INCOMPLETE][35]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-ilk-650/igt@debugfs_test@read_all_entries.html
- fi-bsw-n3050: [PASS][36] -> [INCOMPLETE][37]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-bsw-n3050/igt@debugfs_test@read_all_entries.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-bsw-n3050/igt@debugfs_test@read_all_entries.html
- fi-skl-6700k2: NOTRUN -> [INCOMPLETE][38]
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-skl-6700k2/igt@debugfs_test@read_all_entries.html
- fi-hsw-4770: NOTRUN -> [INCOMPLETE][39]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-hsw-4770/igt@debugfs_test@read_all_entries.html
- fi-kbl-soraka: [PASS][40] -> [INCOMPLETE][41]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-kbl-soraka/igt@debugfs_test@read_all_entries.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-soraka/igt@debugfs_test@read_all_entries.html
- fi-cfl-guc: [PASS][42] -> [INCOMPLETE][43]
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-cfl-guc/igt@debugfs_test@read_all_entries.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-cfl-guc/igt@debugfs_test@read_all_entries.html
- fi-icl-guc: [PASS][44] -> [INCOMPLETE][45]
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-icl-guc/igt@debugfs_test@read_all_entries.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-icl-guc/igt@debugfs_test@read_all_entries.html
* igt@runner@aborted:
- fi-ilk-650: NOTRUN -> [FAIL][46]
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-ilk-650/igt@runner@aborted.html
- fi-pnv-d510: NOTRUN -> [FAIL][47]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-pnv-d510/igt@runner@aborted.html
- fi-kbl-x1275: NOTRUN -> [FAIL][48]
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-x1275/igt@runner@aborted.html
- fi-cfl-8700k: NOTRUN -> [FAIL][49]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-cfl-8700k/igt@runner@aborted.html
- fi-cfl-8109u: NOTRUN -> [FAIL][50]
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-cfl-8109u/igt@runner@aborted.html
- fi-icl-u2: NOTRUN -> [FAIL][51]
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-icl-u2/igt@runner@aborted.html
- fi-gdg-551: NOTRUN -> [FAIL][52]
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-gdg-551/igt@runner@aborted.html
- fi-snb-2520m: NOTRUN -> [FAIL][53]
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-snb-2520m/igt@runner@aborted.html
- fi-kbl-r: NOTRUN -> [FAIL][54]
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-r/igt@runner@aborted.html
- fi-bwr-2160: NOTRUN -> [FAIL][55]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-bwr-2160/igt@runner@aborted.html
- fi-byt-n2820: NOTRUN -> [FAIL][56]
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-byt-n2820/igt@runner@aborted.html
- fi-kbl-soraka: NOTRUN -> [FAIL][57]
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-soraka/igt@runner@aborted.html
- fi-hsw-4770: NOTRUN -> [FAIL][58]
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-hsw-4770/igt@runner@aborted.html
- fi-snb-2600: NOTRUN -> [FAIL][59]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-snb-2600/igt@runner@aborted.html
- fi-ivb-3770: NOTRUN -> [FAIL][60]
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-ivb-3770/igt@runner@aborted.html
- fi-bxt-dsi: NOTRUN -> [FAIL][61]
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-bxt-dsi/igt@runner@aborted.html
- fi-byt-j1900: NOTRUN -> [FAIL][62]
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-byt-j1900/igt@runner@aborted.html
- fi-elk-e7500: NOTRUN -> [FAIL][63]
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-elk-e7500/igt@runner@aborted.html
- fi-icl-y: NOTRUN -> [FAIL][64]
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-icl-y/igt@runner@aborted.html
- fi-blb-e6850: NOTRUN -> [FAIL][65]
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-blb-e6850/igt@runner@aborted.html
#### Warnings ####
* igt@runner@aborted:
- fi-kbl-8809g: [FAIL][66] ([i915#1209]) -> [FAIL][67]
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-kbl-8809g/igt@runner@aborted.html
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-8809g/igt@runner@aborted.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@debugfs_test@read_all_entries:
- {fi-kbl-7560u}: NOTRUN -> [INCOMPLETE][68]
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-7560u/igt@debugfs_test@read_all_entries.html
- {fi-ehl-1}: [PASS][69] -> [INCOMPLETE][70]
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-ehl-1/igt@debugfs_test@read_all_entries.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-ehl-1/igt@debugfs_test@read_all_entries.html
* igt@runner@aborted:
- {fi-ehl-1}: NOTRUN -> [FAIL][71]
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-ehl-1/igt@runner@aborted.html
- {fi-kbl-7560u}: NOTRUN -> [FAIL][72]
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-kbl-7560u/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_17167 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@debugfs_test@read_all_entries:
- fi-cml-s: [PASS][73] -> [INCOMPLETE][74] ([i915#283])
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-cml-s/igt@debugfs_test@read_all_entries.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-cml-s/igt@debugfs_test@read_all_entries.html
- fi-byt-n2820: [PASS][75] -> [INCOMPLETE][76] ([i915#45])
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-byt-n2820/igt@debugfs_test@read_all_entries.html
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-byt-n2820/igt@debugfs_test@read_all_entries.html
- fi-elk-e7500: [PASS][77] -> [INCOMPLETE][78] ([i915#66])
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-elk-e7500/igt@debugfs_test@read_all_entries.html
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-elk-e7500/igt@debugfs_test@read_all_entries.html
- fi-glk-dsi: [PASS][79] -> [INCOMPLETE][80] ([i915#58] / [k.org#198133])
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-glk-dsi/igt@debugfs_test@read_all_entries.html
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-glk-dsi/igt@debugfs_test@read_all_entries.html
- fi-snb-2600: [PASS][81] -> [INCOMPLETE][82] ([i915#82])
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-snb-2600/igt@debugfs_test@read_all_entries.html
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-snb-2600/igt@debugfs_test@read_all_entries.html
- fi-gdg-551: [PASS][83] -> [INCOMPLETE][84] ([i915#172])
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-gdg-551/igt@debugfs_test@read_all_entries.html
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-gdg-551/igt@debugfs_test@read_all_entries.html
- fi-cml-u2: [PASS][85] -> [INCOMPLETE][86] ([i915#283])
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-cml-u2/igt@debugfs_test@read_all_entries.html
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-cml-u2/igt@debugfs_test@read_all_entries.html
- fi-pnv-d510: [PASS][87] -> [INCOMPLETE][88] ([i915#299])
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8230/fi-pnv-d510/igt@debugfs_test@read_all_entries.html
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/fi-pnv-d510/igt@debugfs_test@read_all_entries.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#1209]: https://gitlab.freedesktop.org/drm/intel/issues/1209
[i915#172]: https://gitlab.freedesktop.org/drm/intel/issues/172
[i915#283]: https://gitlab.freedesktop.org/drm/intel/issues/283
[i915#299]: https://gitlab.freedesktop.org/drm/intel/issues/299
[i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45
[i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
[i915#66]: https://gitlab.freedesktop.org/drm/intel/issues/66
[i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (40 -> 38)
------------------------------
Additional (7): fi-byt-j1900 fi-skl-guc fi-ilk-650 fi-hsw-4770 fi-bsw-kefka fi-kbl-7560u fi-skl-6700k2
Missing (9): fi-tgl-u fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus fi-byt-clapper fi-skl-6600u
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_8230 -> Patchwork_17167
CI-20190529: 20190529
CI_DRM_8230: fa9f8453ffb88a4fc4e36d68b84a7ff9bf90f769 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5550: 98927dfde17aecaecfe67bb9853ceca326ca2b23 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_17167: a4d30b0caff30e2d6c58f3619c89efce9449873c @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
a4d30b0caff3 drm/i915/tc/tgl: Implement TC cold sequences
98804eb04a10 drm/i915/tc/icl: Implement TC cold sequences
353532a4fb43 drm/i915/display: Split hsw_power_well_enable() into two
f5268c854061 drm/i915/display: Add intel_aux_ch_to_power_domain()
a7b982777f87 drm/i915/tc: Export tc_port_live_status_mask()
30bd6120eb62 drm/i915/display: Move out code to return the digital_port of the aux ch
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17167/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 1/6] drm/i915/display: Move out code to return the digital_port of the aux ch
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
` (6 preceding siblings ...)
2020-04-01 1:43 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2020-04-01 2:34 ` kbuild test robot
2020-04-01 5:06 ` kbuild test robot
2020-04-01 7:18 ` You-Sheng Yang
9 siblings, 0 replies; 21+ messages in thread
From: kbuild test robot @ 2020-04-01 2:34 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4384 bytes --]
Hi "José,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip next-20200331]
[cannot apply to v5.6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jos-Roberto-de-Souza/drm-i915-display-Move-out-code-to-return-the-digital_port-of-the-aux-ch/20200401-094021
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-defconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-6) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/display/intel_display_power.c: In function 'icl_tc_phy_aux_power_well_enable':
>> drivers/gpu/drm/i915/display/intel_display_power.c:561:40: error: implicit declaration of function 'aux_ch_to_digital_port'; did you mean 'enc_to_dig_port'? [-Werror=implicit-function-declaration]
struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
^~~~~~~~~~~~~~~~~~~~~~
enc_to_dig_port
>> drivers/gpu/drm/i915/display/intel_display_power.c:561:40: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
>> drivers/gpu/drm/i915/display/intel_display_power.c:564:2: error: too many arguments to function 'icl_tc_port_assert_ref_held'
icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c:547:13: note: declared here
static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c: In function 'icl_tc_phy_aux_power_well_disable':
drivers/gpu/drm/i915/display/intel_display_power.c:593:40: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
^~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c:595:2: error: too many arguments to function 'icl_tc_port_assert_ref_held'
icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c:547:13: note: declared here
static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +561 drivers/gpu/drm/i915/display/intel_display_power.c
555
556 static void
557 icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
558 struct i915_power_well *power_well)
559 {
560 enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> 561 struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
562 u32 val;
563
> 564 icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
565
566 val = intel_de_read(dev_priv, DP_AUX_CH_CTL(aux_ch));
567 val &= ~DP_AUX_CH_CTL_TBT_IO;
568 if (power_well->desc->hsw.is_tc_tbt)
569 val |= DP_AUX_CH_CTL_TBT_IO;
570 intel_de_write(dev_priv, DP_AUX_CH_CTL(aux_ch), val);
571
572 hsw_power_well_enable(dev_priv, power_well);
573
574 if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) {
575 enum tc_port tc_port;
576
577 tc_port = TGL_AUX_PW_TO_TC_PORT(power_well->desc->hsw.idx);
578 intel_de_write(dev_priv, HIP_INDEX_REG(tc_port),
579 HIP_INDEX_VAL(tc_port, 0x2));
580
581 if (intel_de_wait_for_set(dev_priv, DKL_CMN_UC_DW_27(tc_port),
582 DKL_CMN_UC_DW27_UC_HEALTH, 1))
583 drm_warn(&dev_priv->drm,
584 "Timeout waiting TC uC health\n");
585 }
586 }
587
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28987 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 1/6] drm/i915/display: Move out code to return the digital_port of the aux ch
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
` (7 preceding siblings ...)
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
9 siblings, 0 replies; 21+ messages in thread
From: kbuild test robot @ 2020-04-01 5:06 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4339 bytes --]
Hi "José,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip next-20200331]
[cannot apply to v5.6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jos-Roberto-de-Souza/drm-i915-display-Move-out-code-to-return-the-digital_port-of-the-aux-ch/20200401-094021
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-s0-20200401 (attached as .config)
compiler: gcc-6 (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/display/intel_display_power.c: In function 'icl_tc_phy_aux_power_well_enable':
>> drivers/gpu/drm/i915/display/intel_display_power.c:561:40: error: implicit declaration of function 'aux_ch_to_digital_port' [-Werror=implicit-function-declaration]
struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
^~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c:561:40: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
drivers/gpu/drm/i915/display/intel_display_power.c:564:2: error: too many arguments to function 'icl_tc_port_assert_ref_held'
icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c:547:13: note: declared here
static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c: In function 'icl_tc_phy_aux_power_well_disable':
drivers/gpu/drm/i915/display/intel_display_power.c:593:40: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
^~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c:595:2: error: too many arguments to function 'icl_tc_port_assert_ref_held'
icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c:547:13: note: declared here
static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/aux_ch_to_digital_port +561 drivers/gpu/drm/i915/display/intel_display_power.c
555
556 static void
557 icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
558 struct i915_power_well *power_well)
559 {
560 enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> 561 struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
562 u32 val;
563
564 icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
565
566 val = intel_de_read(dev_priv, DP_AUX_CH_CTL(aux_ch));
567 val &= ~DP_AUX_CH_CTL_TBT_IO;
568 if (power_well->desc->hsw.is_tc_tbt)
569 val |= DP_AUX_CH_CTL_TBT_IO;
570 intel_de_write(dev_priv, DP_AUX_CH_CTL(aux_ch), val);
571
572 hsw_power_well_enable(dev_priv, power_well);
573
574 if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) {
575 enum tc_port tc_port;
576
577 tc_port = TGL_AUX_PW_TO_TC_PORT(power_well->desc->hsw.idx);
578 intel_de_write(dev_priv, HIP_INDEX_REG(tc_port),
579 HIP_INDEX_VAL(tc_port, 0x2));
580
581 if (intel_de_wait_for_set(dev_priv, DKL_CMN_UC_DW_27(tc_port),
582 DKL_CMN_UC_DW27_UC_HEALTH, 1))
583 drm_warn(&dev_priv->drm,
584 "Timeout waiting TC uC health\n");
585 }
586 }
587
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31953 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 1/6] drm/i915/display: Move out code to return the digital_port of the aux ch
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
` (8 preceding siblings ...)
2020-04-01 5:06 ` kbuild test robot
@ 2020-04-01 7:18 ` You-Sheng Yang
2020-04-01 18:18 ` Souza, Jose
9 siblings, 1 reply; 21+ messages in thread
From: You-Sheng Yang @ 2020-04-01 7:18 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
[-- Attachment #1.1.1: Type: text/plain, Size: 3844 bytes --]
On 2020-04-01 08:41, José Roberto de Souza wrote:
> Moving the code to return the digital port of the aux channel also
> removing the intel_phy_is_tc() to make it generic.
> digital_port will be needed in icl_tc_phy_aux_power_well_enable()
> so adding it as a parameter to icl_tc_port_assert_ref_held().
>
> While at at removing the duplicated call to icl_tc_phy_aux_ch() in
> icl_tc_port_assert_ref_held().
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> .../drm/i915/display/intel_display_power.c | 38 ++++++++++---------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 433e5a81dd4d..02a07aa710e4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -500,26 +500,14 @@ static int power_well_async_ref_count(struct drm_i915_private *dev_priv,
> return refs;
> }
>
> -static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
> - struct i915_power_well *power_well)
> +static struct intel_digital_port *
> +aux_ch_to_digital_port(struct drm_i915_private *dev_priv,
> + enum aux_ch aux_ch)
This fails the build because icl_tc_port_assert_ref_held was originally
guarded by CONFIG_DRM_I915_DEBUG_RUNTIME_PM, but now
aux_ch_to_digital_port maybe used outside the scope.
> {
> - enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> struct intel_digital_port *dig_port = NULL;
> struct intel_encoder *encoder;
>
> - /* Bypass the check if all references are released asynchronously */
> - if (power_well_async_ref_count(dev_priv, power_well) ==
> - power_well->count)
> - return;
> -
> - aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> -
> for_each_intel_encoder(&dev_priv->drm, encoder) {
> - enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> -
> - if (!intel_phy_is_tc(dev_priv, phy))
> - continue;
> -
> /* We'll check the MST primary port */
> if (encoder->type == INTEL_OUTPUT_DP_MST)
> continue;
> @@ -536,6 +524,18 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
> break;
> }
>
> + return dig_port;
> +}
> +
> +static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well,
> + struct intel_digital_port *dig_port)
> +{
> + /* Bypass the check if all references are released asynchronously */
> + if (power_well_async_ref_count(dev_priv, power_well) ==
> + power_well->count)
> + return;
> +
> if (drm_WARN_ON(&dev_priv->drm, !dig_port))
> return;
>
> @@ -558,9 +558,10 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> + struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
E.g. here.
> u32 val;
>
> - icl_tc_port_assert_ref_held(dev_priv, power_well);
> + icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
>
> val = intel_de_read(dev_priv, DP_AUX_CH_CTL(aux_ch));
> val &= ~DP_AUX_CH_CTL_TBT_IO;
> @@ -588,7 +589,10 @@ static void
> icl_tc_phy_aux_power_well_disable(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> - icl_tc_port_assert_ref_held(dev_priv, power_well);
> + enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> + struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
> +
> + icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
>
> hsw_power_well_disable(dev_priv, power_well);
> }
>
You-Sheng Yang
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 1/6] drm/i915/display: Move out code to return the digital_port of the aux ch
2020-04-01 7:18 ` You-Sheng Yang
@ 2020-04-01 18:18 ` Souza, Jose
0 siblings, 0 replies; 21+ messages in thread
From: Souza, Jose @ 2020-04-01 18:18 UTC (permalink / raw)
To: vicamo@gmail.com; +Cc: intel-gfx@lists.freedesktop.org
On Wed, 2020-04-01 at 15:18 +0800, You-Sheng Yang wrote:
> On 2020-04-01 08:41, José Roberto de Souza wrote:
> > Moving the code to return the digital port of the aux channel also
> > removing the intel_phy_is_tc() to make it generic.
> > digital_port will be needed in icl_tc_phy_aux_power_well_enable()
> > so adding it as a parameter to icl_tc_port_assert_ref_held().
> >
> > While at at removing the duplicated call to icl_tc_phy_aux_ch() in
> > icl_tc_port_assert_ref_held().
> >
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > .../drm/i915/display/intel_display_power.c | 38 ++++++++++-----
> > ----
> > 1 file changed, 21 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 433e5a81dd4d..02a07aa710e4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -500,26 +500,14 @@ static int power_well_async_ref_count(struct
> > drm_i915_private *dev_priv,
> > return refs;
> > }
> >
> > -static void icl_tc_port_assert_ref_held(struct drm_i915_private
> > *dev_priv,
> > - struct i915_power_well
> > *power_well)
> > +static struct intel_digital_port *
> > +aux_ch_to_digital_port(struct drm_i915_private *dev_priv,
> > + enum aux_ch aux_ch)
>
> This fails the build because icl_tc_port_assert_ref_held was
> originally
> guarded by CONFIG_DRM_I915_DEBUG_RUNTIME_PM, but now
> aux_ch_to_digital_port maybe used outside the scope.
Thanks, fixed.
>
> > {
> > - enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> > struct intel_digital_port *dig_port = NULL;
> > struct intel_encoder *encoder;
> >
> > - /* Bypass the check if all references are released
> > asynchronously */
> > - if (power_well_async_ref_count(dev_priv, power_well) ==
> > - power_well->count)
> > - return;
> > -
> > - aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> > -
> > for_each_intel_encoder(&dev_priv->drm, encoder) {
> > - enum phy phy = intel_port_to_phy(dev_priv, encoder-
> > >port);
> > -
> > - if (!intel_phy_is_tc(dev_priv, phy))
> > - continue;
> > -
> > /* We'll check the MST primary port */
> > if (encoder->type == INTEL_OUTPUT_DP_MST)
> > continue;
> > @@ -536,6 +524,18 @@ static void icl_tc_port_assert_ref_held(struct
> > drm_i915_private *dev_priv,
> > break;
> > }
> >
> > + return dig_port;
> > +}
> > +
> > +static void icl_tc_port_assert_ref_held(struct drm_i915_private
> > *dev_priv,
> > + struct i915_power_well
> > *power_well,
> > + struct intel_digital_port
> > *dig_port)
> > +{
> > + /* Bypass the check if all references are released
> > asynchronously */
> > + if (power_well_async_ref_count(dev_priv, power_well) ==
> > + power_well->count)
> > + return;
> > +
> > if (drm_WARN_ON(&dev_priv->drm, !dig_port))
> > return;
> >
> > @@ -558,9 +558,10 @@ icl_tc_phy_aux_power_well_enable(struct
> > drm_i915_private *dev_priv,
> > struct i915_power_well *power_well)
> > {
> > enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> > + struct intel_digital_port *dig_port =
> > aux_ch_to_digital_port(dev_priv, aux_ch);
>
> E.g. here.
>
> > u32 val;
> >
> > - icl_tc_port_assert_ref_held(dev_priv, power_well);
> > + icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
> >
> > val = intel_de_read(dev_priv, DP_AUX_CH_CTL(aux_ch));
> > val &= ~DP_AUX_CH_CTL_TBT_IO;
> > @@ -588,7 +589,10 @@ static void
> > icl_tc_phy_aux_power_well_disable(struct drm_i915_private
> > *dev_priv,
> > struct i915_power_well *power_well)
> > {
> > - icl_tc_port_assert_ref_held(dev_priv, power_well);
> > + enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> > + struct intel_digital_port *dig_port =
> > aux_ch_to_digital_port(dev_priv, aux_ch);
> > +
> > + icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
> >
> > hsw_power_well_disable(dev_priv, power_well);
> > }
> >
>
> You-Sheng Yang
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread