* [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
@ 2018-02-22 7:28 Dhinakaran Pandiyan
2018-02-22 7:28 ` [PATCH 2/2] drm/i915/cnl: New power domain for AUX IO Dhinakaran Pandiyan
` (8 more replies)
0 siblings, 9 replies; 26+ messages in thread
From: Dhinakaran Pandiyan @ 2018-02-22 7:28 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Since we no longer have a 1:1 correspondence between ports and AUX
channels, let's give AUX channels their own enum. Makes it easier
to tell the apples from the oranges, and we get rid of the
port E AUX power domain FIXME since we now derive the power domain
from the actual AUX CH.
v2: Rebase due to AUX F
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 8 +-
drivers/gpu/drm/i915/intel_display.h | 11 ++
drivers/gpu/drm/i915/intel_dp.c | 240 +++++++++++++++++------------------
drivers/gpu/drm/i915/intel_drv.h | 1 +
4 files changed, 131 insertions(+), 129 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1412abcb27d4..39d624083a17 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5347,8 +5347,8 @@ enum {
#define _DPF_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset + 0x64520)
#define _DPF_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset + 0x64524)
-#define DP_AUX_CH_CTL(port) _MMIO_PORT(port, _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
-#define DP_AUX_CH_DATA(port, i) _MMIO(_PORT(port, _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
+#define DP_AUX_CH_CTL(aux_ch) _MMIO_PORT(aux_ch, _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
+#define DP_AUX_CH_DATA(aux_ch, i) _MMIO(_PORT(aux_ch, _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
#define DP_AUX_CH_CTL_SEND_BUSY (1 << 31)
#define DP_AUX_CH_CTL_DONE (1 << 30)
@@ -7875,8 +7875,8 @@ enum {
#define _PCH_DPD_AUX_CH_DATA4 0xe4320
#define _PCH_DPD_AUX_CH_DATA5 0xe4324
-#define PCH_DP_AUX_CH_CTL(port) _MMIO_PORT((port) - PORT_B, _PCH_DPB_AUX_CH_CTL, _PCH_DPC_AUX_CH_CTL)
-#define PCH_DP_AUX_CH_DATA(port, i) _MMIO(_PORT((port) - PORT_B, _PCH_DPB_AUX_CH_DATA1, _PCH_DPC_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
+#define PCH_DP_AUX_CH_CTL(aux_ch) _MMIO_PORT((aux_ch) - AUX_CH_B, _PCH_DPB_AUX_CH_CTL, _PCH_DPC_AUX_CH_CTL)
+#define PCH_DP_AUX_CH_DATA(aux_ch, i) _MMIO(_PORT((aux_ch) - AUX_CH_B, _PCH_DPB_AUX_CH_DATA1, _PCH_DPC_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
/* CPT */
#define PORT_TRANS_A_SEL_CPT 0
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index c4042e342f50..f5733a2576e7 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -139,6 +139,17 @@ enum dpio_phy {
#define I915_NUM_PHYS_VLV 2
+enum aux_ch {
+ AUX_CH_A,
+ AUX_CH_B,
+ AUX_CH_C,
+ AUX_CH_D,
+ _AUX_CH_E, /* does not exist */
+ AUX_CH_F,
+};
+
+#define aux_ch_name(a) ((a) + 'A')
+
enum intel_display_power_domain {
POWER_DOMAIN_PIPE_A,
POWER_DOMAIN_PIPE_B,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f20b25f98e5a..eeb8a026fd08 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1331,171 +1331,194 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
return ret;
}
-static enum port intel_aux_port(struct drm_i915_private *dev_priv,
- enum port port)
+static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
{
+ struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ enum port port = encoder->port;
const struct ddi_vbt_port_info *info =
&dev_priv->vbt.ddi_port_info[port];
- enum port aux_port;
+ enum aux_ch aux_ch;
if (!info->alternate_aux_channel) {
+ aux_ch = (enum aux_ch) port;
+
DRM_DEBUG_KMS("using AUX %c for port %c (platform default)\n",
- port_name(port), port_name(port));
- return port;
+ aux_ch_name(aux_ch), port_name(port));
+ return aux_ch;
}
switch (info->alternate_aux_channel) {
case DP_AUX_A:
- aux_port = PORT_A;
+ aux_ch = AUX_CH_A;
break;
case DP_AUX_B:
- aux_port = PORT_B;
+ aux_ch = AUX_CH_B;
break;
case DP_AUX_C:
- aux_port = PORT_C;
+ aux_ch = AUX_CH_C;
break;
case DP_AUX_D:
- aux_port = PORT_D;
+ aux_ch = AUX_CH_D;
break;
case DP_AUX_F:
- aux_port = PORT_F;
+ aux_ch = AUX_CH_F;
break;
default:
MISSING_CASE(info->alternate_aux_channel);
- aux_port = PORT_A;
+ aux_ch = AUX_CH_A;
break;
}
DRM_DEBUG_KMS("using AUX %c for port %c (VBT)\n",
- port_name(aux_port), port_name(port));
+ aux_ch_name(aux_ch), port_name(port));
- return aux_port;
+ return aux_ch;
+}
+
+static enum intel_display_power_domain
+intel_aux_power_domain(struct intel_dp *intel_dp)
+{
+ switch (intel_dp->aux_ch) {
+ case AUX_CH_A:
+ return POWER_DOMAIN_AUX_A;
+ case AUX_CH_B:
+ return POWER_DOMAIN_AUX_B;
+ case AUX_CH_C:
+ return POWER_DOMAIN_AUX_C;
+ case AUX_CH_D:
+ return POWER_DOMAIN_AUX_D;
+ case AUX_CH_F:
+ return POWER_DOMAIN_AUX_F;
+ default:
+ MISSING_CASE(intel_dp->aux_ch);
+ return POWER_DOMAIN_AUX_A;
+ }
}
static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
- enum port port)
+ enum aux_ch aux_ch)
{
- switch (port) {
- case PORT_B:
- case PORT_C:
- case PORT_D:
- return DP_AUX_CH_CTL(port);
+ switch (aux_ch) {
+ case AUX_CH_B:
+ case AUX_CH_C:
+ case AUX_CH_D:
+ return DP_AUX_CH_CTL(aux_ch);
default:
- MISSING_CASE(port);
- return DP_AUX_CH_CTL(PORT_B);
+ MISSING_CASE(aux_ch);
+ return DP_AUX_CH_CTL(AUX_CH_B);
}
}
static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
- enum port port, int index)
+ enum aux_ch aux_ch, int index)
{
- switch (port) {
- case PORT_B:
- case PORT_C:
- case PORT_D:
- return DP_AUX_CH_DATA(port, index);
+ switch (aux_ch) {
+ case AUX_CH_B:
+ case AUX_CH_C:
+ case AUX_CH_D:
+ return DP_AUX_CH_DATA(aux_ch, index);
default:
- MISSING_CASE(port);
- return DP_AUX_CH_DATA(PORT_B, index);
+ MISSING_CASE(aux_ch);
+ return DP_AUX_CH_DATA(AUX_CH_B, index);
}
}
static i915_reg_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
- enum port port)
-{
- switch (port) {
- case PORT_A:
- return DP_AUX_CH_CTL(port);
- case PORT_B:
- case PORT_C:
- case PORT_D:
- return PCH_DP_AUX_CH_CTL(port);
+ enum aux_ch aux_ch)
+{
+ switch (aux_ch) {
+ case AUX_CH_A:
+ return DP_AUX_CH_CTL(aux_ch);
+ case AUX_CH_B:
+ case AUX_CH_C:
+ case AUX_CH_D:
+ return PCH_DP_AUX_CH_CTL(aux_ch);
default:
- MISSING_CASE(port);
- return DP_AUX_CH_CTL(PORT_A);
+ MISSING_CASE(aux_ch);
+ return DP_AUX_CH_CTL(AUX_CH_A);
}
}
static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv,
- enum port port, int index)
-{
- switch (port) {
- case PORT_A:
- return DP_AUX_CH_DATA(port, index);
- case PORT_B:
- case PORT_C:
- case PORT_D:
- return PCH_DP_AUX_CH_DATA(port, index);
+ enum aux_ch aux_ch, int index)
+{
+ switch (aux_ch) {
+ case AUX_CH_A:
+ return DP_AUX_CH_DATA(aux_ch, index);
+ case AUX_CH_B:
+ case AUX_CH_C:
+ case AUX_CH_D:
+ return PCH_DP_AUX_CH_DATA(aux_ch, index);
default:
- MISSING_CASE(port);
- return DP_AUX_CH_DATA(PORT_A, index);
+ MISSING_CASE(aux_ch);
+ return DP_AUX_CH_DATA(AUX_CH_A, index);
}
}
static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv,
- enum port port)
-{
- switch (port) {
- case PORT_A:
- case PORT_B:
- case PORT_C:
- case PORT_D:
- case PORT_F:
- return DP_AUX_CH_CTL(port);
+ enum aux_ch aux_ch)
+{
+ switch (aux_ch) {
+ case AUX_CH_A:
+ case AUX_CH_B:
+ case AUX_CH_C:
+ case AUX_CH_D:
+ case AUX_CH_F:
+ return DP_AUX_CH_CTL(aux_ch);
default:
- MISSING_CASE(port);
- return DP_AUX_CH_CTL(PORT_A);
+ MISSING_CASE(aux_ch);
+ return DP_AUX_CH_CTL(AUX_CH_A);
}
}
static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv,
- enum port port, int index)
-{
- switch (port) {
- case PORT_A:
- case PORT_B:
- case PORT_C:
- case PORT_D:
- case PORT_F:
- return DP_AUX_CH_DATA(port, index);
+ enum aux_ch aux_ch, int index)
+{
+ switch (aux_ch) {
+ case AUX_CH_A:
+ case AUX_CH_B:
+ case AUX_CH_C:
+ case AUX_CH_D:
+ case AUX_CH_F:
+ return DP_AUX_CH_DATA(aux_ch, index);
default:
- MISSING_CASE(port);
- return DP_AUX_CH_DATA(PORT_A, index);
+ MISSING_CASE(aux_ch);
+ return DP_AUX_CH_DATA(AUX_CH_A, index);
}
}
static i915_reg_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv,
- enum port port)
+ enum aux_ch aux_ch)
{
if (INTEL_GEN(dev_priv) >= 9)
- return skl_aux_ctl_reg(dev_priv, port);
+ return skl_aux_ctl_reg(dev_priv, aux_ch);
else if (HAS_PCH_SPLIT(dev_priv))
- return ilk_aux_ctl_reg(dev_priv, port);
+ return ilk_aux_ctl_reg(dev_priv, aux_ch);
else
- return g4x_aux_ctl_reg(dev_priv, port);
+ return g4x_aux_ctl_reg(dev_priv, aux_ch);
}
static i915_reg_t intel_aux_data_reg(struct drm_i915_private *dev_priv,
- enum port port, int index)
+ enum aux_ch aux_ch, int index)
{
if (INTEL_GEN(dev_priv) >= 9)
- return skl_aux_data_reg(dev_priv, port, index);
+ return skl_aux_data_reg(dev_priv, aux_ch, index);
else if (HAS_PCH_SPLIT(dev_priv))
- return ilk_aux_data_reg(dev_priv, port, index);
+ return ilk_aux_data_reg(dev_priv, aux_ch, index);
else
- return g4x_aux_data_reg(dev_priv, port, index);
+ return g4x_aux_data_reg(dev_priv, aux_ch, index);
}
static void intel_aux_reg_init(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
- enum port port = intel_aux_port(dev_priv,
- dp_to_dig_port(intel_dp)->base.port);
+ enum aux_ch aux_ch = intel_dp->aux_ch;
int i;
- intel_dp->aux_ch_ctl_reg = intel_aux_ctl_reg(dev_priv, port);
+ intel_dp->aux_ch_ctl_reg = intel_aux_ctl_reg(dev_priv, aux_ch);
for (i = 0; i < ARRAY_SIZE(intel_dp->aux_ch_data_reg); i++)
- intel_dp->aux_ch_data_reg[i] = intel_aux_data_reg(dev_priv, port, i);
+ intel_dp->aux_ch_data_reg[i] = intel_aux_data_reg(dev_priv, aux_ch, i);
}
static void
@@ -1507,14 +1530,17 @@ intel_dp_aux_fini(struct intel_dp *intel_dp)
static void
intel_dp_aux_init(struct intel_dp *intel_dp)
{
- struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- enum port port = intel_dig_port->base.port;
+ struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+
+ intel_dp->aux_ch = intel_aux_ch(intel_dp);
+ intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp);
intel_aux_reg_init(intel_dp);
drm_dp_aux_init(&intel_dp->aux);
/* Failure to allocate our preferred name is not critical */
- intel_dp->aux.name = kasprintf(GFP_KERNEL, "DPDDC-%c", port_name(port));
+ intel_dp->aux.name = kasprintf(GFP_KERNEL, "DPDDC-%c",
+ port_name(encoder->port));
intel_dp->aux.transfer = intel_dp_aux_transfer;
}
@@ -6266,42 +6292,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
return false;
}
-/* Set up the hotplug pin and aux power domain. */
-static void
-intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
-{
- struct intel_encoder *encoder = &intel_dig_port->base;
- struct intel_dp *intel_dp = &intel_dig_port->dp;
- struct intel_encoder *intel_encoder = &intel_dig_port->base;
- struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
-
- encoder->hpd_pin = intel_hpd_pin_default(dev_priv, encoder->port);
-
- switch (encoder->port) {
- case PORT_A:
- intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A;
- break;
- case PORT_B:
- intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B;
- break;
- case PORT_C:
- intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C;
- break;
- case PORT_D:
- intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
- break;
- case PORT_E:
- /* FIXME: Check VBT for actual wiring of PORT E */
- intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
- break;
- case PORT_F:
- intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F;
- break;
- default:
- MISSING_CASE(encoder->port);
- }
-}
-
static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
{
struct intel_connector *intel_connector;
@@ -6407,7 +6397,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
connector->interlace_allowed = true;
connector->doublescan_allowed = 0;
- intel_dp_init_connector_port_info(intel_dig_port);
+ intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
intel_dp_aux_init(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 50874f4035cf..b70ed154c4ce 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1052,6 +1052,7 @@ struct intel_dp {
bool detect_done;
bool channel_eq_status;
bool reset_link_params;
+ enum aux_ch aux_ch;
uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/2] drm/i915/cnl: New power domain for AUX IO.
2018-02-22 7:28 [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Dhinakaran Pandiyan
@ 2018-02-22 7:28 ` Dhinakaran Pandiyan
2018-02-22 10:28 ` Imre Deak
2018-02-22 18:54 ` Ville Syrjälä
2018-02-22 7:35 ` [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Pandiyan, Dhinakaran
` (7 subsequent siblings)
8 siblings, 2 replies; 26+ messages in thread
From: Dhinakaran Pandiyan @ 2018-02-22 7:28 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
for AUX-A enables DC_OFF well too. This is not required, so add a new
AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
channels re-use the existing AUX domains as they do need power well 2.
v2: Add AUX IO domain only for AUX-A
Rebased on top of Ville's AUX series.
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Suggested-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/intel_display.h | 1 +
drivers/gpu/drm/i915/intel_dp.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_psr.c | 37 +++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
5 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index f5733a2576e7..4e7418b345bc 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -186,6 +186,7 @@ enum intel_display_power_domain {
POWER_DOMAIN_AUX_C,
POWER_DOMAIN_AUX_D,
POWER_DOMAIN_AUX_F,
+ POWER_DOMAIN_AUX_IO_A,
POWER_DOMAIN_GMBUS,
POWER_DOMAIN_MODESET,
POWER_DOMAIN_GT_IRQ,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index eeb8a026fd08..777682a925c9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
return ret;
}
-static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
+enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
{
struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b70ed154c4ce..725a5b8ab611 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1684,6 +1684,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
int intel_dp_link_required(int pixel_clock, int bpp);
int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
bool intel_digital_port_connected(struct intel_encoder *encoder);
+enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
/* intel_dp_aux_backlight.c */
int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2ef374f936b9..ff77b505534c 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,6 +56,39 @@
#include "intel_drv.h"
#include "i915_drv.h"
+static void psr_power_get(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
+ enum intel_display_power_domain aux_domain;
+
+ if (INTEL_GEN(dev_priv) < 10)
+ return;
+
+ /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
+ * does not require the driver to disable DC states, but the rest do.
+ * Although PSR is enabled only on Port A currently, let's do this
+ * correctly for other ports too.
+ */
+ aux_domain = intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
+ intel_dp->aux_power_domain;
+ intel_display_power_get(dev_priv, aux_domain);
+}
+
+static void psr_power_put(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
+ enum intel_display_power_domain aux_domain;
+
+ if (INTEL_GEN(dev_priv) < 10)
+ return;
+
+ aux_domain = intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
+ intel_dp->aux_power_domain;
+ intel_display_power_put(dev_priv, aux_domain);
+}
+
static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
{
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -459,6 +492,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
u32 chicken;
+ psr_power_get(intel_dp);
+
if (dev_priv->psr.psr2_support) {
chicken = PSR2_VSC_ENABLE_PROG_HEADER;
if (dev_priv->psr.y_cord_support)
@@ -617,6 +652,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
else
WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
}
+
+ psr_power_put(intel_dp);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index b7924feb9f27..53ea564f971e 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
return "AUX_D";
case POWER_DOMAIN_AUX_F:
return "AUX_F";
+ case POWER_DOMAIN_AUX_IO_A:
+ return "AUX_IO_A";
case POWER_DOMAIN_GMBUS:
return "GMBUS";
case POWER_DOMAIN_INIT:
@@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
BIT_ULL(POWER_DOMAIN_INIT))
#define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
BIT_ULL(POWER_DOMAIN_AUX_A) | \
+ BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
BIT_ULL(POWER_DOMAIN_INIT))
#define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
BIT_ULL(POWER_DOMAIN_AUX_B) | \
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
2018-02-22 7:28 [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Dhinakaran Pandiyan
2018-02-22 7:28 ` [PATCH 2/2] drm/i915/cnl: New power domain for AUX IO Dhinakaran Pandiyan
@ 2018-02-22 7:35 ` Pandiyan, Dhinakaran
2018-02-22 7:54 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] " Patchwork
` (6 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-02-22 7:35 UTC (permalink / raw)
To: intel-gfx
On Wed, 2018-02-21 at 23:28 -0800, Dhinakaran Pandiyan wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Since we no longer have a 1:1 correspondence between ports and AUX
> channels, let's give AUX channels their own enum. Makes it easier
> to tell the apples from the oranges, and we get rid of the
> port E AUX power domain FIXME since we now derive the power domain
> from the actual AUX CH.
>
> v2: Rebase due to AUX F
Patch 2/2 has a dependency on this patch from Ville, so I have included
this patch for CI to test them together.
-DK
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
2018-02-22 7:28 [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Dhinakaran Pandiyan
2018-02-22 7:28 ` [PATCH 2/2] drm/i915/cnl: New power domain for AUX IO Dhinakaran Pandiyan
2018-02-22 7:35 ` [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Pandiyan, Dhinakaran
@ 2018-02-22 7:54 ` Patchwork
2018-02-22 8:12 ` ✓ Fi.CI.BAT: success " Patchwork
` (5 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2018-02-22 7:54 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
URL : https://patchwork.freedesktop.org/series/38744/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
b28837aa5cbe drm/i915: Add enum aux_ch and clean up the aux init to use it
-:31: WARNING: line over 80 characters
#31: FILE: drivers/gpu/drm/i915/i915_reg.h:5350:
+#define DP_AUX_CH_CTL(aux_ch) _MMIO_PORT(aux_ch, _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
-:32: WARNING: line over 80 characters
#32: FILE: drivers/gpu/drm/i915/i915_reg.h:5351:
+#define DP_AUX_CH_DATA(aux_ch, i) _MMIO(_PORT(aux_ch, _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
-:42: WARNING: line over 80 characters
#42: FILE: drivers/gpu/drm/i915/i915_reg.h:7878:
+#define PCH_DP_AUX_CH_CTL(aux_ch) _MMIO_PORT((aux_ch) - AUX_CH_B, _PCH_DPB_AUX_CH_CTL, _PCH_DPC_AUX_CH_CTL)
-:43: WARNING: line over 80 characters
#43: FILE: drivers/gpu/drm/i915/i915_reg.h:7879:
+#define PCH_DP_AUX_CH_DATA(aux_ch, i) _MMIO(_PORT((aux_ch) - AUX_CH_B, _PCH_DPB_AUX_CH_DATA1, _PCH_DPC_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
-:90: CHECK: No space is necessary after a cast
#90: FILE: drivers/gpu/drm/i915/intel_dp.c:1344:
+ aux_ch = (enum aux_ch) port;
-:349: WARNING: line over 80 characters
#349: FILE: drivers/gpu/drm/i915/intel_dp.c:1521:
+ intel_dp->aux_ch_data_reg[i] = intel_aux_data_reg(dev_priv, aux_ch, i);
total: 0 errors, 5 warnings, 1 checks, 393 lines checked
daf9018b0155 drm/i915/cnl: New power domain for AUX IO.
-:71: WARNING: line over 80 characters
#71: FILE: drivers/gpu/drm/i915/intel_psr.c:62:
+ struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
-:90: WARNING: line over 80 characters
#90: FILE: drivers/gpu/drm/i915/intel_psr.c:81:
+ struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
total: 0 errors, 2 warnings, 0 checks, 92 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
2018-02-22 7:28 [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Dhinakaran Pandiyan
` (2 preceding siblings ...)
2018-02-22 7:54 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] " Patchwork
@ 2018-02-22 8:12 ` Patchwork
2018-02-22 10:12 ` ✗ Fi.CI.IGT: warning " Patchwork
` (4 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2018-02-22 8:12 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
URL : https://patchwork.freedesktop.org/series/38744/
State : success
== Summary ==
Series 38744v1 series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
https://patchwork.freedesktop.org/api/1.0/series/38744/revisions/1/mbox/
Test gem_mmap_gtt:
Subgroup basic-small-bo-tiledx:
fail -> PASS (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-c:
pass -> INCOMPLETE (fi-skl-6260u) fdo#104108
incomplete -> PASS (fi-bxt-dsi) fdo#103927
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> FAIL (fi-ilk-650) fdo#104008
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:420s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:426s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:374s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:489s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:286s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:480s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:482s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:467s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:465s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:560s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:416s
fi-gdg-551 total:288 pass:180 dwarn:0 dfail:0 fail:0 skip:108 time:284s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:508s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:388s
fi-ilk-650 total:288 pass:227 dwarn:0 dfail:0 fail:1 skip:60 time:415s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:447s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:411s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:457s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:494s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:450s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:495s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:584s
fi-skl-6260u total:246 pass:229 dwarn:0 dfail:0 fail:0 skip:16
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:501s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:520s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:488s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:481s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:432s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:518s
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:394s
42016703e66b7b572d4ab651946b715cdbff3050 drm-tip: 2018y-02m-21d-21h-26m-53s UTC integration manifest
daf9018b0155 drm/i915/cnl: New power domain for AUX IO.
b28837aa5cbe drm/i915: Add enum aux_ch and clean up the aux init to use it
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8119/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✗ Fi.CI.IGT: warning for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
2018-02-22 7:28 [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Dhinakaran Pandiyan
` (3 preceding siblings ...)
2018-02-22 8:12 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-02-22 10:12 ` Patchwork
2018-02-22 21:22 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev2) Patchwork
` (3 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2018-02-22 10:12 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
URL : https://patchwork.freedesktop.org/series/38744/
State : warning
== Summary ==
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-c-frame-sequence:
pass -> FAIL (shard-apl) fdo#103481
Test kms_chv_cursor_fail:
Subgroup pipe-b-128x128-top-edge:
dmesg-warn -> PASS (shard-snb) fdo#105185
Subgroup pipe-b-64x64-left-edge:
pass -> DMESG-WARN (shard-snb)
Test perf:
Subgroup oa-exponents:
incomplete -> PASS (shard-apl) fdo#102254
Subgroup buffer-fill:
pass -> FAIL (shard-apl) fdo#103755
Test kms_flip:
Subgroup modeset-vs-vblank-race-interruptible:
fail -> PASS (shard-apl) fdo#103060 +1
Subgroup 2x-plain-flip-ts-check-interruptible:
pass -> FAIL (shard-hsw) fdo#100368
fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#103755 https://bugs.freedesktop.org/show_bug.cgi?id=103755
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
shard-apl total:3369 pass:1764 dwarn:1 dfail:0 fail:14 skip:1589 time:12069s
shard-hsw total:3464 pass:1766 dwarn:1 dfail:0 fail:3 skip:1693 time:11798s
shard-snb total:3464 pass:1355 dwarn:2 dfail:0 fail:3 skip:2104 time:6633s
Blacklisted hosts:
shard-kbl total:3464 pass:1960 dwarn:1 dfail:0 fail:15 skip:1488 time:9698s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8119/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] drm/i915/cnl: New power domain for AUX IO.
2018-02-22 7:28 ` [PATCH 2/2] drm/i915/cnl: New power domain for AUX IO Dhinakaran Pandiyan
@ 2018-02-22 10:28 ` Imre Deak
2018-02-22 18:32 ` Imre Deak
2018-02-22 18:54 ` Ville Syrjälä
1 sibling, 1 reply; 26+ messages in thread
From: Imre Deak @ 2018-02-22 10:28 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx, Rodrigo Vivi
On Wed, Feb 21, 2018 at 11:28:56PM -0800, Dhinakaran Pandiyan wrote:
> PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> for AUX-A enables DC_OFF well too. This is not required, so add a new
> AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> channels re-use the existing AUX domains as they do need power well 2.
>
> v2: Add AUX IO domain only for AUX-A
> Rebased on top of Ville's AUX series.
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Suggested-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.h | 1 +
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_psr.c | 37 +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> 5 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index f5733a2576e7..4e7418b345bc 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> POWER_DOMAIN_AUX_C,
> POWER_DOMAIN_AUX_D,
> POWER_DOMAIN_AUX_F,
> + POWER_DOMAIN_AUX_IO_A,
> POWER_DOMAIN_GMBUS,
> POWER_DOMAIN_MODESET,
> POWER_DOMAIN_GT_IRQ,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index eeb8a026fd08..777682a925c9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> return ret;
> }
>
> -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> {
> struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b70ed154c4ce..725a5b8ab611 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1684,6 +1684,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> int intel_dp_link_required(int pixel_clock, int bpp);
> int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> bool intel_digital_port_connected(struct intel_encoder *encoder);
> +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
>
> /* intel_dp_aux_backlight.c */
> int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 2ef374f936b9..ff77b505534c 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -56,6 +56,39 @@
> #include "intel_drv.h"
> #include "i915_drv.h"
>
> +static void psr_power_get(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> + enum intel_display_power_domain aux_domain;
> +
> + if (INTEL_GEN(dev_priv) < 10)
> + return;
> +
> + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> + * does not require the driver to disable DC states, but the rest do.
> + * Although PSR is enabled only on Port A currently, let's do this
> + * correctly for other ports too.
> + */
> + aux_domain = intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
The AUX power well is port specific, while there could be an alternative
port->aux-channel mapping. So we should check the port here.
> + intel_dp->aux_power_domain;
> + intel_display_power_get(dev_priv, aux_domain);
> +}
> +
> +static void psr_power_put(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> + enum intel_display_power_domain aux_domain;
> +
> + if (INTEL_GEN(dev_priv) < 10)
> + return;
> +
> + aux_domain = intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> + intel_dp->aux_power_domain;
> + intel_display_power_put(dev_priv, aux_domain);
> +}
> +
> static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -459,6 +492,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> u32 chicken;
>
> + psr_power_get(intel_dp);
> +
> if (dev_priv->psr.psr2_support) {
> chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> if (dev_priv->psr.y_cord_support)
> @@ -617,6 +652,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> else
> WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> }
> +
> + psr_power_put(intel_dp);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index b7924feb9f27..53ea564f971e 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> return "AUX_D";
> case POWER_DOMAIN_AUX_F:
> return "AUX_F";
> + case POWER_DOMAIN_AUX_IO_A:
> + return "AUX_IO_A";
> case POWER_DOMAIN_GMBUS:
> return "GMBUS";
> case POWER_DOMAIN_INIT:
> @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> BIT_ULL(POWER_DOMAIN_INIT))
> #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> BIT_ULL(POWER_DOMAIN_AUX_A) | \
> + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> BIT_ULL(POWER_DOMAIN_INIT))
> #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> BIT_ULL(POWER_DOMAIN_AUX_B) | \
> --
> 2.14.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] drm/i915/cnl: New power domain for AUX IO.
2018-02-22 10:28 ` Imre Deak
@ 2018-02-22 18:32 ` Imre Deak
0 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2018-02-22 18:32 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx, Rodrigo Vivi
On Thu, Feb 22, 2018 at 12:28:11PM +0200, Imre Deak wrote:
> On Wed, Feb 21, 2018 at 11:28:56PM -0800, Dhinakaran Pandiyan wrote:
> > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > channels re-use the existing AUX domains as they do need power well 2.
> >
> > v2: Add AUX IO domain only for AUX-A
> > Rebased on top of Ville's AUX series.
> >
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Suggested-by: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.h | 1 +
> > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_psr.c | 37 +++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > 5 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > index f5733a2576e7..4e7418b345bc 100644
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > POWER_DOMAIN_AUX_C,
> > POWER_DOMAIN_AUX_D,
> > POWER_DOMAIN_AUX_F,
> > + POWER_DOMAIN_AUX_IO_A,
> > POWER_DOMAIN_GMBUS,
> > POWER_DOMAIN_MODESET,
> > POWER_DOMAIN_GT_IRQ,
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index eeb8a026fd08..777682a925c9 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > return ret;
> > }
> >
> > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > {
> > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index b70ed154c4ce..725a5b8ab611 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1684,6 +1684,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > int intel_dp_link_required(int pixel_clock, int bpp);
> > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> >
> > /* intel_dp_aux_backlight.c */
> > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 2ef374f936b9..ff77b505534c 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -56,6 +56,39 @@
> > #include "intel_drv.h"
> > #include "i915_drv.h"
> >
> > +static void psr_power_get(struct intel_dp *intel_dp)
> > +{
> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > + enum intel_display_power_domain aux_domain;
> > +
> > + if (INTEL_GEN(dev_priv) < 10)
> > + return;
> > +
> > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > + * does not require the driver to disable DC states, but the rest do.
> > + * Although PSR is enabled only on Port A currently, let's do this
> > + * correctly for other ports too.
> > + */
> > + aux_domain = intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
>
> The AUX power well is port specific, while there could be an alternative
> port->aux-channel mapping. So we should check the port here.
At least that's how the driver worked so far .. but that was incorrect
as confirmed now by Art, and Ville's patch fixed it. So you can ignore
the above:/
It's still unclear how this works on ICL onwards at least in TBT and
USBC modes, the selection there could be port specific. Will try to
clarify that with Art.
--Imre
>
> > + intel_dp->aux_power_domain;
> > + intel_display_power_get(dev_priv, aux_domain);
> > +}
> > +
> > +static void psr_power_put(struct intel_dp *intel_dp)
> > +{
> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > + enum intel_display_power_domain aux_domain;
> > +
> > + if (INTEL_GEN(dev_priv) < 10)
> > + return;
> > +
> > + aux_domain = intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > + intel_dp->aux_power_domain;
> > + intel_display_power_put(dev_priv, aux_domain);
> > +}
> > +
> > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > {
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > @@ -459,6 +492,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > u32 chicken;
> >
> > + psr_power_get(intel_dp);
> > +
> > if (dev_priv->psr.psr2_support) {
> > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > if (dev_priv->psr.y_cord_support)
> > @@ -617,6 +652,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > else
> > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > }
> > +
> > + psr_power_put(intel_dp);
> > }
> >
> > /**
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index b7924feb9f27..53ea564f971e 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > return "AUX_D";
> > case POWER_DOMAIN_AUX_F:
> > return "AUX_F";
> > + case POWER_DOMAIN_AUX_IO_A:
> > + return "AUX_IO_A";
> > case POWER_DOMAIN_GMBUS:
> > return "GMBUS";
> > case POWER_DOMAIN_INIT:
> > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > BIT_ULL(POWER_DOMAIN_INIT))
> > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > BIT_ULL(POWER_DOMAIN_INIT))
> > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > --
> > 2.14.1
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] drm/i915/cnl: New power domain for AUX IO.
2018-02-22 7:28 ` [PATCH 2/2] drm/i915/cnl: New power domain for AUX IO Dhinakaran Pandiyan
2018-02-22 10:28 ` Imre Deak
@ 2018-02-22 18:54 ` Ville Syrjälä
2018-02-22 20:28 ` [PATCH v3] drm/i915/psr: " Dhinakaran Pandiyan
1 sibling, 1 reply; 26+ messages in thread
From: Ville Syrjälä @ 2018-02-22 18:54 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx, Rodrigo Vivi
On Wed, Feb 21, 2018 at 11:28:56PM -0800, Dhinakaran Pandiyan wrote:
> PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> for AUX-A enables DC_OFF well too. This is not required, so add a new
> AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> channels re-use the existing AUX domains as they do need power well 2.
>
> v2: Add AUX IO domain only for AUX-A
> Rebased on top of Ville's AUX series.
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Suggested-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.h | 1 +
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_psr.c | 37 +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> 5 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index f5733a2576e7..4e7418b345bc 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> POWER_DOMAIN_AUX_C,
> POWER_DOMAIN_AUX_D,
> POWER_DOMAIN_AUX_F,
> + POWER_DOMAIN_AUX_IO_A,
> POWER_DOMAIN_GMBUS,
> POWER_DOMAIN_MODESET,
> POWER_DOMAIN_GT_IRQ,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index eeb8a026fd08..777682a925c9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> return ret;
> }
>
> -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> {
> struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b70ed154c4ce..725a5b8ab611 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1684,6 +1684,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> int intel_dp_link_required(int pixel_clock, int bpp);
> int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> bool intel_digital_port_connected(struct intel_encoder *encoder);
> +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
>
> /* intel_dp_aux_backlight.c */
> int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 2ef374f936b9..ff77b505534c 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -56,6 +56,39 @@
> #include "intel_drv.h"
> #include "i915_drv.h"
>
> +static void psr_power_get(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> + enum intel_display_power_domain aux_domain;
> +
> + if (INTEL_GEN(dev_priv) < 10)
> + return;
> +
> + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> + * does not require the driver to disable DC states, but the rest do.
> + * Although PSR is enabled only on Port A currently, let's do this
> + * correctly for other ports too.
> + */
> + aux_domain = intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> + intel_dp->aux_power_domain;
Could put that logic into a small helper to avoid having to duplicate it
in both get and put.
> + intel_display_power_get(dev_priv, aux_domain);
> +}
> +
> +static void psr_power_put(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> + enum intel_display_power_domain aux_domain;
> +
> + if (INTEL_GEN(dev_priv) < 10)
> + return;
> +
> + aux_domain = intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> + intel_dp->aux_power_domain;
> + intel_display_power_put(dev_priv, aux_domain);
> +}
> +
> static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -459,6 +492,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> u32 chicken;
>
> + psr_power_get(intel_dp);
> +
> if (dev_priv->psr.psr2_support) {
> chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> if (dev_priv->psr.y_cord_support)
> @@ -617,6 +652,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> else
> WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> }
> +
> + psr_power_put(intel_dp);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index b7924feb9f27..53ea564f971e 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> return "AUX_D";
> case POWER_DOMAIN_AUX_F:
> return "AUX_F";
> + case POWER_DOMAIN_AUX_IO_A:
> + return "AUX_IO_A";
> case POWER_DOMAIN_GMBUS:
> return "GMBUS";
> case POWER_DOMAIN_INIT:
> @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> BIT_ULL(POWER_DOMAIN_INIT))
> #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> BIT_ULL(POWER_DOMAIN_AUX_A) | \
> + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> BIT_ULL(POWER_DOMAIN_INIT))
> #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> BIT_ULL(POWER_DOMAIN_AUX_B) | \
> --
> 2.14.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 18:54 ` Ville Syrjälä
@ 2018-02-22 20:28 ` Dhinakaran Pandiyan
2018-02-22 20:44 ` Rodrigo Vivi
0 siblings, 1 reply; 26+ messages in thread
From: Dhinakaran Pandiyan @ 2018-02-22 20:28 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
for AUX-A enables DC_OFF well too. This is not required, so add a new
AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
channels re-use the existing AUX domains as they do need power well 2.
v3: Extract aux domain selection into a function (Ville)
v2: Add AUX IO domain only for AUX-A
Rebased on top of Ville's AUX series.
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Suggested-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/intel_display.h | 1 +
drivers/gpu/drm/i915/intel_dp.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
5 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index f5733a2576e7..4e7418b345bc 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -186,6 +186,7 @@ enum intel_display_power_domain {
POWER_DOMAIN_AUX_C,
POWER_DOMAIN_AUX_D,
POWER_DOMAIN_AUX_F,
+ POWER_DOMAIN_AUX_IO_A,
POWER_DOMAIN_GMBUS,
POWER_DOMAIN_MODESET,
POWER_DOMAIN_GT_IRQ,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 005735a7fc29..b78b9972ebae 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
return ret;
}
-static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
+enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
{
struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5132f6a58c6d..bcd6dc9fb13d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
int intel_dp_link_required(int pixel_clock, int bpp);
int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
bool intel_digital_port_connected(struct intel_encoder *encoder);
+enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
/* intel_dp_aux_backlight.c */
int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2ef374f936b9..03814f7bc2d3 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,6 +56,40 @@
#include "intel_drv.h"
#include "i915_drv.h"
+static inline enum intel_display_power_domain
+psr_aux_domain(struct intel_dp *intel_dp)
+{
+ /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
+ * does not require the driver to disable DC states, but the rest do.
+ * Although PSR is enabled only on Port A currently, let's do this
+ * correctly for other ports too.
+ */
+ return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
+ intel_dp->aux_power_domain;
+}
+
+static void psr_power_get(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
+
+ if (INTEL_GEN(dev_priv) < 10)
+ return;
+
+ intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
+}
+
+static void psr_power_put(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
+
+ if (INTEL_GEN(dev_priv) < 10)
+ return;
+
+ intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
+}
+
static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
{
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
u32 chicken;
+ psr_power_get(intel_dp);
+
if (dev_priv->psr.psr2_support) {
chicken = PSR2_VSC_ENABLE_PROG_HEADER;
if (dev_priv->psr.y_cord_support)
@@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
else
WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
}
+
+ psr_power_put(intel_dp);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index b7924feb9f27..53ea564f971e 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
return "AUX_D";
case POWER_DOMAIN_AUX_F:
return "AUX_F";
+ case POWER_DOMAIN_AUX_IO_A:
+ return "AUX_IO_A";
case POWER_DOMAIN_GMBUS:
return "GMBUS";
case POWER_DOMAIN_INIT:
@@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
BIT_ULL(POWER_DOMAIN_INIT))
#define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
BIT_ULL(POWER_DOMAIN_AUX_A) | \
+ BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
BIT_ULL(POWER_DOMAIN_INIT))
#define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
BIT_ULL(POWER_DOMAIN_AUX_B) | \
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 20:28 ` [PATCH v3] drm/i915/psr: " Dhinakaran Pandiyan
@ 2018-02-22 20:44 ` Rodrigo Vivi
2018-02-22 21:33 ` Pandiyan, Dhinakaran
0 siblings, 1 reply; 26+ messages in thread
From: Rodrigo Vivi @ 2018-02-22 20:44 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> for AUX-A enables DC_OFF well too. This is not required, so add a new
> AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> channels re-use the existing AUX domains as they do need power well 2.
>
> v3: Extract aux domain selection into a function (Ville)
> v2: Add AUX IO domain only for AUX-A
> Rebased on top of Ville's AUX series.
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Suggested-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.h | 1 +
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> 5 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index f5733a2576e7..4e7418b345bc 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> POWER_DOMAIN_AUX_C,
> POWER_DOMAIN_AUX_D,
> POWER_DOMAIN_AUX_F,
> + POWER_DOMAIN_AUX_IO_A,
> POWER_DOMAIN_GMBUS,
> POWER_DOMAIN_MODESET,
> POWER_DOMAIN_GT_IRQ,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 005735a7fc29..b78b9972ebae 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> return ret;
> }
>
> -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> {
> struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5132f6a58c6d..bcd6dc9fb13d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> int intel_dp_link_required(int pixel_clock, int bpp);
> int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> bool intel_digital_port_connected(struct intel_encoder *encoder);
> +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
>
> /* intel_dp_aux_backlight.c */
> int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 2ef374f936b9..03814f7bc2d3 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -56,6 +56,40 @@
> #include "intel_drv.h"
> #include "i915_drv.h"
>
> +static inline enum intel_display_power_domain
> +psr_aux_domain(struct intel_dp *intel_dp)
> +{
> + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> + * does not require the driver to disable DC states, but the rest do.
This phrase is strange. Specially "the rest do" part.
It seems that we need to disable DC states on other ports than A,
what it is not true.
> + * Although PSR is enabled only on Port A currently, let's do this
> + * correctly for other ports too.
> + */
> + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> + intel_dp->aux_power_domain;
> +}
> +
> +static void psr_power_get(struct intel_dp *intel_dp)
nip: psr_aux_io_power_get ?!
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> +
> + if (INTEL_GEN(dev_priv) < 10)
> + return;
> +
> + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> +}
> +
> +static void psr_power_put(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> +
> + if (INTEL_GEN(dev_priv) < 10)
> + return;
> +
> + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> +}
> +
> static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> u32 chicken;
>
> + psr_power_get(intel_dp);
> +
> if (dev_priv->psr.psr2_support) {
> chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> if (dev_priv->psr.y_cord_support)
> @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> else
> WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> }
> +
> + psr_power_put(intel_dp);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index b7924feb9f27..53ea564f971e 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> return "AUX_D";
> case POWER_DOMAIN_AUX_F:
> return "AUX_F";
> + case POWER_DOMAIN_AUX_IO_A:
> + return "AUX_IO_A";
> case POWER_DOMAIN_GMBUS:
> return "GMBUS";
> case POWER_DOMAIN_INIT:
> @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> BIT_ULL(POWER_DOMAIN_INIT))
> #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> BIT_ULL(POWER_DOMAIN_AUX_A) | \
> + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> BIT_ULL(POWER_DOMAIN_INIT))
> #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> BIT_ULL(POWER_DOMAIN_AUX_B) | \
> --
> 2.14.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev2)
2018-02-22 7:28 [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Dhinakaran Pandiyan
` (4 preceding siblings ...)
2018-02-22 10:12 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2018-02-22 21:22 ` Patchwork
2018-02-23 0:25 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev3) Patchwork
` (2 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2018-02-22 21:22 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev2)
URL : https://patchwork.freedesktop.org/series/38744/
State : success
== Summary ==
Series 38744v2 series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
https://patchwork.freedesktop.org/api/1.0/series/38744/revisions/2/mbox/
Test debugfs_test:
Subgroup read_all_entries:
pass -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test gem_mmap_gtt:
Subgroup basic-small-bo-tiledx:
fail -> PASS (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a:
fail -> PASS (fi-skl-6700k2)
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> FAIL (fi-ivb-3770) fdo#104008
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:415s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:422s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:373s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:480s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:284s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:476s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:479s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:467s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:451s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:560s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:414s
fi-gdg-551 total:288 pass:180 dwarn:0 dfail:0 fail:0 skip:108 time:283s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:508s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:386s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:404s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:452s
fi-ivb-3770 total:288 pass:254 dwarn:0 dfail:0 fail:1 skip:33 time:409s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:453s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:488s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:450s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:491s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:587s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:426s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:500s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:518s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:485s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:472s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:404s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:428s
fi-snb-2520m total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:396s
Blacklisted hosts:
fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:389s
b9f1df86d4379ec76dddc07cf6e1cc85ffb26ffd drm-tip: 2018y-02m-22d-18h-25m-51s UTC integration manifest
37c6bf3d8eab drm/i915/psr: New power domain for AUX IO.
ef0d7df2f508 drm/i915: Add enum aux_ch and clean up the aux init to use it
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8137/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 20:44 ` Rodrigo Vivi
@ 2018-02-22 21:33 ` Pandiyan, Dhinakaran
2018-02-22 21:47 ` Ville Syrjälä
2018-02-22 21:53 ` [PATCH v3] " Imre Deak
0 siblings, 2 replies; 26+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-02-22 21:33 UTC (permalink / raw)
To: ville.syrjala, Vivi, Rodrigo, Deak, Imre; +Cc: intel-gfx
On Thu, 2018-02-22 at 12:44 -0800, Rodrigo Vivi wrote:
> On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > channels re-use the existing AUX domains as they do need power well 2.
> >
> > v3: Extract aux domain selection into a function (Ville)
> > v2: Add AUX IO domain only for AUX-A
> > Rebased on top of Ville's AUX series.
> >
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Suggested-by: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.h | 1 +
> > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > 5 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > index f5733a2576e7..4e7418b345bc 100644
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > POWER_DOMAIN_AUX_C,
> > POWER_DOMAIN_AUX_D,
> > POWER_DOMAIN_AUX_F,
> > + POWER_DOMAIN_AUX_IO_A,
> > POWER_DOMAIN_GMBUS,
> > POWER_DOMAIN_MODESET,
> > POWER_DOMAIN_GT_IRQ,
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 005735a7fc29..b78b9972ebae 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > return ret;
> > }
> >
> > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > {
> > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 5132f6a58c6d..bcd6dc9fb13d 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > int intel_dp_link_required(int pixel_clock, int bpp);
> > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> >
> > /* intel_dp_aux_backlight.c */
> > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 2ef374f936b9..03814f7bc2d3 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -56,6 +56,40 @@
> > #include "intel_drv.h"
> > #include "i915_drv.h"
> >
> > +static inline enum intel_display_power_domain
> > +psr_aux_domain(struct intel_dp *intel_dp)
> > +{
> > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > + * does not require the driver to disable DC states, but the rest do.
>
> This phrase is strange. Specially "the rest do" part.
> It seems that we need to disable DC states on other ports than A,
> what it is not true.
Okay, let's back up a little bit.
AUX domain for port A enables DC_OFF and AUX-IO. AUX domains for non-A
ports enable DC_OFF, power well 2 and AUX-IO, in fact all domains that
require power well 2 enable DC_OFF too. I can't see this is in bspec but
that's what the code does currently. So, this is for AUX transfers.
which means, this comment for the previous iteration of the patch
"I mean, on CNL
POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
POWER_DOMAIN_AUX__IO_{B,C,D,F}"
was not accurate. _AUX_IO_{B,C,D,F} domains enable only AUX_IO well and
_AUX_{B,C,D,F} domains enable power well 2, DC_OFF and AUX_IO.
For PSR, we want only the AUX_IO to be enabled for any port because the
spec says -
"PSR spontaneously sends Aux transactions. If PSR is enabled on a port,
then the associated Aux IO must be kept powered up."
which means, this comment
"Hm, so in general (for AUX transfers) to enable AUX-A we first need to
disable DC states _except_ if we enable AUX-A for PSR where we want
DC5/6 to stay enabled." applies to all ports.
To just enable the AUX-IO well, the correct version is ->
https://patchwork.freedesktop.org/patch/205328/
-DK
>
> > + * Although PSR is enabled only on Port A currently, let's do this
> > + * correctly for other ports too.
> > + */
> > + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > + intel_dp->aux_power_domain;
> > +}
> > +
> > +static void psr_power_get(struct intel_dp *intel_dp)
>
> nip: psr_aux_io_power_get ?!
>
> > +{
> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > +
> > + if (INTEL_GEN(dev_priv) < 10)
> > + return;
> > +
> > + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> > +}
> > +
> > +static void psr_power_put(struct intel_dp *intel_dp)
> > +{
> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > +
> > + if (INTEL_GEN(dev_priv) < 10)
> > + return;
> > +
> > + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > +}
> > +
> > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > {
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > u32 chicken;
> >
> > + psr_power_get(intel_dp);
> > +
> > if (dev_priv->psr.psr2_support) {
> > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > if (dev_priv->psr.y_cord_support)
> > @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > else
> > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > }
> > +
> > + psr_power_put(intel_dp);
> > }
> >
> > /**
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index b7924feb9f27..53ea564f971e 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > return "AUX_D";
> > case POWER_DOMAIN_AUX_F:
> > return "AUX_F";
> > + case POWER_DOMAIN_AUX_IO_A:
> > + return "AUX_IO_A";
> > case POWER_DOMAIN_GMBUS:
> > return "GMBUS";
> > case POWER_DOMAIN_INIT:
> > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > BIT_ULL(POWER_DOMAIN_INIT))
> > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > BIT_ULL(POWER_DOMAIN_INIT))
> > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > --
> > 2.14.1
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 21:33 ` Pandiyan, Dhinakaran
@ 2018-02-22 21:47 ` Ville Syrjälä
2018-02-22 22:10 ` Pandiyan, Dhinakaran
2018-02-22 21:53 ` [PATCH v3] " Imre Deak
1 sibling, 1 reply; 26+ messages in thread
From: Ville Syrjälä @ 2018-02-22 21:47 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx, Vivi, Rodrigo
On Thu, Feb 22, 2018 at 09:33:10PM +0000, Pandiyan, Dhinakaran wrote:
>
>
>
> On Thu, 2018-02-22 at 12:44 -0800, Rodrigo Vivi wrote:
> > On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> > > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > > channels re-use the existing AUX domains as they do need power well 2.
> > >
> > > v3: Extract aux domain selection into a function (Ville)
> > > v2: Add AUX IO domain only for AUX-A
> > > Rebased on top of Ville's AUX series.
> > >
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Suggested-by: Imre Deak <imre.deak@intel.com>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.h | 1 +
> > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > 5 files changed, 44 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > index f5733a2576e7..4e7418b345bc 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > > POWER_DOMAIN_AUX_C,
> > > POWER_DOMAIN_AUX_D,
> > > POWER_DOMAIN_AUX_F,
> > > + POWER_DOMAIN_AUX_IO_A,
> > > POWER_DOMAIN_GMBUS,
> > > POWER_DOMAIN_MODESET,
> > > POWER_DOMAIN_GT_IRQ,
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 005735a7fc29..b78b9972ebae 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > > return ret;
> > > }
> > >
> > > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > {
> > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 5132f6a58c6d..bcd6dc9fb13d 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > > int intel_dp_link_required(int pixel_clock, int bpp);
> > > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> > >
> > > /* intel_dp_aux_backlight.c */
> > > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > index 2ef374f936b9..03814f7bc2d3 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -56,6 +56,40 @@
> > > #include "intel_drv.h"
> > > #include "i915_drv.h"
> > >
> > > +static inline enum intel_display_power_domain
> > > +psr_aux_domain(struct intel_dp *intel_dp)
> > > +{
> > > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > > + * does not require the driver to disable DC states, but the rest do.
> >
> > This phrase is strange. Specially "the rest do" part.
> > It seems that we need to disable DC states on other ports than A,
> > what it is not true.
>
> Okay, let's back up a little bit.
>
> AUX domain for port A enables DC_OFF and AUX-IO. AUX domains for non-A
> ports enable DC_OFF, power well 2 and AUX-IO, in fact all domains that
> require power well 2 enable DC_OFF too. I can't see this is in bspec but
> that's what the code does currently. So, this is for AUX transfers.
>
> which means, this comment for the previous iteration of the patch
> "I mean, on CNL
>
> POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
> POWER_DOMAIN_AUX__IO_{B,C,D,F}"
>
> was not accurate. _AUX_IO_{B,C,D,F} domains enable only AUX_IO well and
> _AUX_{B,C,D,F} domains enable power well 2, DC_OFF and AUX_IO.
>
>
> For PSR, we want only the AUX_IO to be enabled for any port because the
> spec says -
> "PSR spontaneously sends Aux transactions. If PSR is enabled on a port,
> then the associated Aux IO must be kept powered up."
>
> which means, this comment
> "Hm, so in general (for AUX transfers) to enable AUX-A we first need to
> disable DC states _except_ if we enable AUX-A for PSR where we want
> DC5/6 to stay enabled." applies to all ports.
>
> To just enable the AUX-IO well, the correct version is ->
> https://patchwork.freedesktop.org/patch/205328/
While that is true I don't think it actually matters in the end. As
long as transcoder A-C is enabled DC will be off and PG2 will be
enabled. So I suppose we can (ab)use that fact to save a few power
domain bits and only define AUX_IO_A. The comment should probably
be reworded to say as much.
>
>
>
> -DK
> >
> > > + * Although PSR is enabled only on Port A currently, let's do this
> > > + * correctly for other ports too.
> > > + */
> > > + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > + intel_dp->aux_power_domain;
> > > +}
> > > +
> > > +static void psr_power_get(struct intel_dp *intel_dp)
> >
> > nip: psr_aux_io_power_get ?!
> >
> > > +{
> > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > +
> > > + if (INTEL_GEN(dev_priv) < 10)
> > > + return;
> > > +
> > > + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> > > +}
> > > +
> > > +static void psr_power_put(struct intel_dp *intel_dp)
> > > +{
> > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > +
> > > + if (INTEL_GEN(dev_priv) < 10)
> > > + return;
> > > +
> > > + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > > +}
> > > +
> > > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > u32 chicken;
> > >
> > > + psr_power_get(intel_dp);
> > > +
> > > if (dev_priv->psr.psr2_support) {
> > > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > > if (dev_priv->psr.y_cord_support)
> > > @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > > else
> > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > > }
> > > +
> > > + psr_power_put(intel_dp);
> > > }
> > >
> > > /**
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index b7924feb9f27..53ea564f971e 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > > return "AUX_D";
> > > case POWER_DOMAIN_AUX_F:
> > > return "AUX_F";
> > > + case POWER_DOMAIN_AUX_IO_A:
> > > + return "AUX_IO_A";
> > > case POWER_DOMAIN_GMBUS:
> > > return "GMBUS";
> > > case POWER_DOMAIN_INIT:
> > > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > BIT_ULL(POWER_DOMAIN_INIT))
> > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > > BIT_ULL(POWER_DOMAIN_INIT))
> > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > > --
> > > 2.14.1
> > >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 21:33 ` Pandiyan, Dhinakaran
2018-02-22 21:47 ` Ville Syrjälä
@ 2018-02-22 21:53 ` Imre Deak
2018-02-22 22:05 ` Imre Deak
1 sibling, 1 reply; 26+ messages in thread
From: Imre Deak @ 2018-02-22 21:53 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx, Vivi, Rodrigo
On Thu, Feb 22, 2018 at 11:33:10PM +0200, Pandiyan, Dhinakaran wrote:
>
>
>
> On Thu, 2018-02-22 at 12:44 -0800, Rodrigo Vivi wrote:
> > On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> > > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > > channels re-use the existing AUX domains as they do need power well 2.
> > >
> > > v3: Extract aux domain selection into a function (Ville)
> > > v2: Add AUX IO domain only for AUX-A
> > > Rebased on top of Ville's AUX series.
> > >
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Suggested-by: Imre Deak <imre.deak@intel.com>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.h | 1 +
> > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > 5 files changed, 44 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > index f5733a2576e7..4e7418b345bc 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > > POWER_DOMAIN_AUX_C,
> > > POWER_DOMAIN_AUX_D,
> > > POWER_DOMAIN_AUX_F,
> > > + POWER_DOMAIN_AUX_IO_A,
> > > POWER_DOMAIN_GMBUS,
> > > POWER_DOMAIN_MODESET,
> > > POWER_DOMAIN_GT_IRQ,
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 005735a7fc29..b78b9972ebae 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > > return ret;
> > > }
> > >
> > > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > {
> > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 5132f6a58c6d..bcd6dc9fb13d 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > > int intel_dp_link_required(int pixel_clock, int bpp);
> > > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> > >
> > > /* intel_dp_aux_backlight.c */
> > > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > index 2ef374f936b9..03814f7bc2d3 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -56,6 +56,40 @@
> > > #include "intel_drv.h"
> > > #include "i915_drv.h"
> > >
> > > +static inline enum intel_display_power_domain
> > > +psr_aux_domain(struct intel_dp *intel_dp)
> > > +{
> > > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > > + * does not require the driver to disable DC states, but the rest do.
> >
> > This phrase is strange. Specially "the rest do" part.
> > It seems that we need to disable DC states on other ports than A,
> > what it is not true.
>
> Okay, let's back up a little bit.
>
> AUX domain for port A enables DC_OFF and AUX-IO. AUX domains for non-A
> ports enable DC_OFF, power well 2 and AUX-IO, in fact all domains that
> require power well 2 enable DC_OFF too. I can't see this is in bspec but
> that's what the code does currently. So, this is for AUX transfers.
To enable DC5/6 power well 2 needs to be disabled; I see this is still
missing from the BSpec DC5/6 enable sequence, though I asked already for
an update there before. Will ask again.
Non-A AUX channels are contained in power well 2, so we have to disable
DC states and enable power well 2 for non-A AUX channels.
>
> which means, this comment for the previous iteration of the patch
> "I mean, on CNL
>
> POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
> POWER_DOMAIN_AUX__IO_{B,C,D,F}"
>
> was not accurate. _AUX_IO_{B,C,D,F} domains enable only AUX_IO well and
> _AUX_{B,C,D,F} domains enable power well 2, DC_OFF and AUX_IO.
>
>
> For PSR, we want only the AUX_IO to be enabled for any port because the
> spec says -
> "PSR spontaneously sends Aux transactions. If PSR is enabled on a port,
> then the associated Aux IO must be kept powered up."
>
> which means, this comment
> "Hm, so in general (for AUX transfers) to enable AUX-A we first need to
> disable DC states _except_ if we enable AUX-A for PSR where we want
> DC5/6 to stay enabled." applies to all ports.
It doesn't make sense and we can't enable DC5/6 for non-A AUX channels,
since those are contained in power well 2, which won't be powered on/off
dynamically by DMC.
--Imre
>
> To just enable the AUX-IO well, the correct version is ->
> https://patchwork.freedesktop.org/patch/205328/
>
>
>
> -DK
> >
> > > + * Although PSR is enabled only on Port A currently, let's do this
> > > + * correctly for other ports too.
> > > + */
> > > + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > + intel_dp->aux_power_domain;
> > > +}
> > > +
> > > +static void psr_power_get(struct intel_dp *intel_dp)
> >
> > nip: psr_aux_io_power_get ?!
> >
> > > +{
> > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > +
> > > + if (INTEL_GEN(dev_priv) < 10)
> > > + return;
> > > +
> > > + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> > > +}
> > > +
> > > +static void psr_power_put(struct intel_dp *intel_dp)
> > > +{
> > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > +
> > > + if (INTEL_GEN(dev_priv) < 10)
> > > + return;
> > > +
> > > + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > > +}
> > > +
> > > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > u32 chicken;
> > >
> > > + psr_power_get(intel_dp);
> > > +
> > > if (dev_priv->psr.psr2_support) {
> > > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > > if (dev_priv->psr.y_cord_support)
> > > @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > > else
> > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > > }
> > > +
> > > + psr_power_put(intel_dp);
> > > }
> > >
> > > /**
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index b7924feb9f27..53ea564f971e 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > > return "AUX_D";
> > > case POWER_DOMAIN_AUX_F:
> > > return "AUX_F";
> > > + case POWER_DOMAIN_AUX_IO_A:
> > > + return "AUX_IO_A";
> > > case POWER_DOMAIN_GMBUS:
> > > return "GMBUS";
> > > case POWER_DOMAIN_INIT:
> > > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > BIT_ULL(POWER_DOMAIN_INIT))
> > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > > BIT_ULL(POWER_DOMAIN_INIT))
> > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > > --
> > > 2.14.1
> > >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 21:53 ` [PATCH v3] " Imre Deak
@ 2018-02-22 22:05 ` Imre Deak
2018-02-22 22:30 ` Pandiyan, Dhinakaran
0 siblings, 1 reply; 26+ messages in thread
From: Imre Deak @ 2018-02-22 22:05 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx, Vivi, Rodrigo
On Thu, Feb 22, 2018 at 11:53:30PM +0200, Imre Deak wrote:
> On Thu, Feb 22, 2018 at 11:33:10PM +0200, Pandiyan, Dhinakaran wrote:
> >
> >
> >
> > On Thu, 2018-02-22 at 12:44 -0800, Rodrigo Vivi wrote:
> > > On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> > > > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > > > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > > > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > > > channels re-use the existing AUX domains as they do need power well 2.
> > > >
> > > > v3: Extract aux domain selection into a function (Ville)
> > > > v2: Add AUX IO domain only for AUX-A
> > > > Rebased on top of Ville's AUX series.
> > > >
> > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Suggested-by: Imre Deak <imre.deak@intel.com>
> > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.h | 1 +
> > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > > 5 files changed, 44 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > > index f5733a2576e7..4e7418b345bc 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > > > POWER_DOMAIN_AUX_C,
> > > > POWER_DOMAIN_AUX_D,
> > > > POWER_DOMAIN_AUX_F,
> > > > + POWER_DOMAIN_AUX_IO_A,
> > > > POWER_DOMAIN_GMBUS,
> > > > POWER_DOMAIN_MODESET,
> > > > POWER_DOMAIN_GT_IRQ,
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 005735a7fc29..b78b9972ebae 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > > > return ret;
> > > > }
> > > >
> > > > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > {
> > > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > index 5132f6a58c6d..bcd6dc9fb13d 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > > > int intel_dp_link_required(int pixel_clock, int bpp);
> > > > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > > > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> > > >
> > > > /* intel_dp_aux_backlight.c */
> > > > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > > index 2ef374f936b9..03814f7bc2d3 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -56,6 +56,40 @@
> > > > #include "intel_drv.h"
> > > > #include "i915_drv.h"
> > > >
> > > > +static inline enum intel_display_power_domain
> > > > +psr_aux_domain(struct intel_dp *intel_dp)
> > > > +{
> > > > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > > > + * does not require the driver to disable DC states, but the rest do.
> > >
> > > This phrase is strange. Specially "the rest do" part.
> > > It seems that we need to disable DC states on other ports than A,
> > > what it is not true.
> >
> > Okay, let's back up a little bit.
> >
> > AUX domain for port A enables DC_OFF and AUX-IO. AUX domains for non-A
> > ports enable DC_OFF, power well 2 and AUX-IO, in fact all domains that
> > require power well 2 enable DC_OFF too. I can't see this is in bspec but
> > that's what the code does currently. So, this is for AUX transfers.
>
> To enable DC5/6 power well 2 needs to be disabled; I see this is still
> missing from the BSpec DC5/6 enable sequence, though I asked already for
> an update there before. Will ask again.
Actually it's at "Mode Set[SKL+]/Sequences for Display C5 and C6":
"""
...
2. Configure display engine to have power wells above power well 1
disabled, following the appropriate mode set disable sequences for any
ports using power wells above power well 1. This can be done earlier if
desired.
...
4. Set DC_STATE_EN Dynamic DC State Enable = "Enable up to DC5" for DC5
or "Enable up to DC6" for DC6.
"""
>
> Non-A AUX channels are contained in power well 2, so we have to disable
> DC states and enable power well 2 for non-A AUX channels.
>
> >
> > which means, this comment for the previous iteration of the patch
> > "I mean, on CNL
> >
> > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
> > POWER_DOMAIN_AUX__IO_{B,C,D,F}"
> >
> > was not accurate. _AUX_IO_{B,C,D,F} domains enable only AUX_IO well and
> > _AUX_{B,C,D,F} domains enable power well 2, DC_OFF and AUX_IO.
> >
> >
> > For PSR, we want only the AUX_IO to be enabled for any port because the
> > spec says -
> > "PSR spontaneously sends Aux transactions. If PSR is enabled on a port,
> > then the associated Aux IO must be kept powered up."
> >
> > which means, this comment
> > "Hm, so in general (for AUX transfers) to enable AUX-A we first need to
> > disable DC states _except_ if we enable AUX-A for PSR where we want
> > DC5/6 to stay enabled." applies to all ports.
>
> It doesn't make sense and we can't enable DC5/6 for non-A AUX channels,
> since those are contained in power well 2, which won't be powered on/off
> dynamically by DMC.
>
> --Imre
>
> >
> > To just enable the AUX-IO well, the correct version is ->
> > https://patchwork.freedesktop.org/patch/205328/
> >
> >
> >
> > -DK
> > >
> > > > + * Although PSR is enabled only on Port A currently, let's do this
> > > > + * correctly for other ports too.
> > > > + */
> > > > + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > > + intel_dp->aux_power_domain;
> > > > +}
> > > > +
> > > > +static void psr_power_get(struct intel_dp *intel_dp)
> > >
> > > nip: psr_aux_io_power_get ?!
> > >
> > > > +{
> > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > +
> > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > + return;
> > > > +
> > > > + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> > > > +}
> > > > +
> > > > +static void psr_power_put(struct intel_dp *intel_dp)
> > > > +{
> > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > +
> > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > + return;
> > > > +
> > > > + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > > > +}
> > > > +
> > > > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > > > {
> > > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > > @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > > u32 chicken;
> > > >
> > > > + psr_power_get(intel_dp);
> > > > +
> > > > if (dev_priv->psr.psr2_support) {
> > > > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > > > if (dev_priv->psr.y_cord_support)
> > > > @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > > > else
> > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > > > }
> > > > +
> > > > + psr_power_put(intel_dp);
> > > > }
> > > >
> > > > /**
> > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > index b7924feb9f27..53ea564f971e 100644
> > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > > > return "AUX_D";
> > > > case POWER_DOMAIN_AUX_F:
> > > > return "AUX_F";
> > > > + case POWER_DOMAIN_AUX_IO_A:
> > > > + return "AUX_IO_A";
> > > > case POWER_DOMAIN_GMBUS:
> > > > return "GMBUS";
> > > > case POWER_DOMAIN_INIT:
> > > > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > > > --
> > > > 2.14.1
> > > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 21:47 ` Ville Syrjälä
@ 2018-02-22 22:10 ` Pandiyan, Dhinakaran
2018-02-22 22:55 ` Rodrigo Vivi
0 siblings, 1 reply; 26+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-02-22 22:10 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Vivi, Rodrigo
On Thu, 2018-02-22 at 23:47 +0200, Ville Syrjälä wrote:
> On Thu, Feb 22, 2018 at 09:33:10PM +0000, Pandiyan, Dhinakaran wrote:
> >
> >
> >
> > On Thu, 2018-02-22 at 12:44 -0800, Rodrigo Vivi wrote:
> > > On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> > > > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > > > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > > > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > > > channels re-use the existing AUX domains as they do need power well 2.
> > > >
> > > > v3: Extract aux domain selection into a function (Ville)
> > > > v2: Add AUX IO domain only for AUX-A
> > > > Rebased on top of Ville's AUX series.
> > > >
> > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Suggested-by: Imre Deak <imre.deak@intel.com>
> > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.h | 1 +
> > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > > 5 files changed, 44 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > > index f5733a2576e7..4e7418b345bc 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > > > POWER_DOMAIN_AUX_C,
> > > > POWER_DOMAIN_AUX_D,
> > > > POWER_DOMAIN_AUX_F,
> > > > + POWER_DOMAIN_AUX_IO_A,
> > > > POWER_DOMAIN_GMBUS,
> > > > POWER_DOMAIN_MODESET,
> > > > POWER_DOMAIN_GT_IRQ,
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 005735a7fc29..b78b9972ebae 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > > > return ret;
> > > > }
> > > >
> > > > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > {
> > > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > index 5132f6a58c6d..bcd6dc9fb13d 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > > > int intel_dp_link_required(int pixel_clock, int bpp);
> > > > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > > > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> > > >
> > > > /* intel_dp_aux_backlight.c */
> > > > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > > index 2ef374f936b9..03814f7bc2d3 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -56,6 +56,40 @@
> > > > #include "intel_drv.h"
> > > > #include "i915_drv.h"
> > > >
> > > > +static inline enum intel_display_power_domain
> > > > +psr_aux_domain(struct intel_dp *intel_dp)
> > > > +{
> > > > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > > > + * does not require the driver to disable DC states, but the rest do.
> > >
> > > This phrase is strange. Specially "the rest do" part.
> > > It seems that we need to disable DC states on other ports than A,
> > > what it is not true.
> >
> > Okay, let's back up a little bit.
> >
> > AUX domain for port A enables DC_OFF and AUX-IO. AUX domains for non-A
> > ports enable DC_OFF, power well 2 and AUX-IO, in fact all domains that
> > require power well 2 enable DC_OFF too. I can't see this is in bspec but
> > that's what the code does currently. So, this is for AUX transfers.
> >
> > which means, this comment for the previous iteration of the patch
> > "I mean, on CNL
> >
> > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
> > POWER_DOMAIN_AUX__IO_{B,C,D,F}"
> >
> > was not accurate. _AUX_IO_{B,C,D,F} domains enable only AUX_IO well and
> > _AUX_{B,C,D,F} domains enable power well 2, DC_OFF and AUX_IO.
> >
> >
> > For PSR, we want only the AUX_IO to be enabled for any port because the
> > spec says -
> > "PSR spontaneously sends Aux transactions. If PSR is enabled on a port,
> > then the associated Aux IO must be kept powered up."
> >
> > which means, this comment
> > "Hm, so in general (for AUX transfers) to enable AUX-A we first need to
> > disable DC states _except_ if we enable AUX-A for PSR where we want
> > DC5/6 to stay enabled." applies to all ports.
> >
> > To just enable the AUX-IO well, the correct version is ->
> > https://patchwork.freedesktop.org/patch/205328/
>
> While that is true I don't think it actually matters in the end. As
> long as transcoder A-C is enabled DC will be off and PG2 will be
> enabled. So I suppose we can (ab)use that fact to save a few power
> domain bits and only define AUX_IO_A. The comment should probably
> be reworded to say as much.
>
"CNL HW requires corresponding AUX IOs to be powered up for PSR.
However, for non-A AUX ports the corresponding non-EDP transcoders
would have already enabled power well 2 and DC_OFF. This means we can
acquire a wider POWER_DOMAIN_AUX_{B,C,D,F} reference instead of a
specific AUX_IO reference without powering up any extra wells."
Sound good?
> >
> >
> >
> > -DK
> > >
> > > > + * Although PSR is enabled only on Port A currently, let's do this
> > > > + * correctly for other ports too.
> > > > + */
> > > > + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > > + intel_dp->aux_power_domain;
> > > > +}
> > > > +
> > > > +static void psr_power_get(struct intel_dp *intel_dp)
> > >
> > > nip: psr_aux_io_power_get ?!
> > >
> > > > +{
> > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > +
> > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > + return;
> > > > +
> > > > + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> > > > +}
> > > > +
> > > > +static void psr_power_put(struct intel_dp *intel_dp)
> > > > +{
> > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > +
> > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > + return;
> > > > +
> > > > + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > > > +}
> > > > +
> > > > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > > > {
> > > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > > @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > > u32 chicken;
> > > >
> > > > + psr_power_get(intel_dp);
> > > > +
> > > > if (dev_priv->psr.psr2_support) {
> > > > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > > > if (dev_priv->psr.y_cord_support)
> > > > @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > > > else
> > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > > > }
> > > > +
> > > > + psr_power_put(intel_dp);
> > > > }
> > > >
> > > > /**
> > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > index b7924feb9f27..53ea564f971e 100644
> > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > > > return "AUX_D";
> > > > case POWER_DOMAIN_AUX_F:
> > > > return "AUX_F";
> > > > + case POWER_DOMAIN_AUX_IO_A:
> > > > + return "AUX_IO_A";
> > > > case POWER_DOMAIN_GMBUS:
> > > > return "GMBUS";
> > > > case POWER_DOMAIN_INIT:
> > > > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > > > --
> > > > 2.14.1
> > > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 22:05 ` Imre Deak
@ 2018-02-22 22:30 ` Pandiyan, Dhinakaran
2018-02-22 22:42 ` Imre Deak
0 siblings, 1 reply; 26+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-02-22 22:30 UTC (permalink / raw)
To: Deak, Imre; +Cc: intel-gfx, Vivi, Rodrigo
On Fri, 2018-02-23 at 00:05 +0200, Imre Deak wrote:
> On Thu, Feb 22, 2018 at 11:53:30PM +0200, Imre Deak wrote:
> > On Thu, Feb 22, 2018 at 11:33:10PM +0200, Pandiyan, Dhinakaran wrote:
> > >
> > >
> > >
> > > On Thu, 2018-02-22 at 12:44 -0800, Rodrigo Vivi wrote:
> > > > On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> > > > > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > > > > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > > > > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > > > > channels re-use the existing AUX domains as they do need power well 2.
> > > > >
> > > > > v3: Extract aux domain selection into a function (Ville)
> > > > > v2: Add AUX IO domain only for AUX-A
> > > > > Rebased on top of Ville's AUX series.
> > > > >
> > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Suggested-by: Imre Deak <imre.deak@intel.com>
> > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_display.h | 1 +
> > > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > > drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > > > 5 files changed, 44 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > > > index f5733a2576e7..4e7418b345bc 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > > > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > > > > POWER_DOMAIN_AUX_C,
> > > > > POWER_DOMAIN_AUX_D,
> > > > > POWER_DOMAIN_AUX_F,
> > > > > + POWER_DOMAIN_AUX_IO_A,
> > > > > POWER_DOMAIN_GMBUS,
> > > > > POWER_DOMAIN_MODESET,
> > > > > POWER_DOMAIN_GT_IRQ,
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index 005735a7fc29..b78b9972ebae 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > > > > return ret;
> > > > > }
> > > > >
> > > > > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > > {
> > > > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > > index 5132f6a58c6d..bcd6dc9fb13d 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > > > > int intel_dp_link_required(int pixel_clock, int bpp);
> > > > > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > > > > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> > > > >
> > > > > /* intel_dp_aux_backlight.c */
> > > > > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > > > index 2ef374f936b9..03814f7bc2d3 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > @@ -56,6 +56,40 @@
> > > > > #include "intel_drv.h"
> > > > > #include "i915_drv.h"
> > > > >
> > > > > +static inline enum intel_display_power_domain
> > > > > +psr_aux_domain(struct intel_dp *intel_dp)
> > > > > +{
> > > > > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > > > > + * does not require the driver to disable DC states, but the rest do.
> > > >
> > > > This phrase is strange. Specially "the rest do" part.
> > > > It seems that we need to disable DC states on other ports than A,
> > > > what it is not true.
> > >
> > > Okay, let's back up a little bit.
> > >
> > > AUX domain for port A enables DC_OFF and AUX-IO. AUX domains for non-A
> > > ports enable DC_OFF, power well 2 and AUX-IO, in fact all domains that
> > > require power well 2 enable DC_OFF too. I can't see this is in bspec but
> > > that's what the code does currently. So, this is for AUX transfers.
> >
> > To enable DC5/6 power well 2 needs to be disabled; I see this is still
> > missing from the BSpec DC5/6 enable sequence, though I asked already for
> > an update there before. Will ask again.
>
> Actually it's at "Mode Set[SKL+]/Sequences for Display C5 and C6":
>
> """
> ...
> 2. Configure display engine to have power wells above power well 1
> disabled, following the appropriate mode set disable sequences for any
> ports using power wells above power well 1. This can be done earlier if
> desired.
> ...
> 4. Set DC_STATE_EN Dynamic DC State Enable = "Enable up to DC5" for DC5
> or "Enable up to DC6" for DC6.
>
> """
>
Thanks, finding this information would have been much easier if it was
all in one place. We still have to assume the converse from the enable
step. I wonder if enabling power well 2 is sufficient to bring out the
hardware from DC6 without having to explicitly disable it.
> >
> > Non-A AUX channels are contained in power well 2, so we have to disable
> > DC states and enable power well 2 for non-A AUX channels.
> >
> > >
> > > which means, this comment for the previous iteration of the patch
> > > "I mean, on CNL
> > >
> > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
> > > POWER_DOMAIN_AUX__IO_{B,C,D,F}"
> > >
> > > was not accurate. _AUX_IO_{B,C,D,F} domains enable only AUX_IO well and
> > > _AUX_{B,C,D,F} domains enable power well 2, DC_OFF and AUX_IO.
> > >
> > >
> > > For PSR, we want only the AUX_IO to be enabled for any port because the
> > > spec says -
> > > "PSR spontaneously sends Aux transactions. If PSR is enabled on a port,
> > > then the associated Aux IO must be kept powered up."
> > >
> > > which means, this comment
> > > "Hm, so in general (for AUX transfers) to enable AUX-A we first need to
> > > disable DC states _except_ if we enable AUX-A for PSR where we want
> > > DC5/6 to stay enabled." applies to all ports.
> >
> > It doesn't make sense and we can't enable DC5/6 for non-A AUX channels,
> > since those are contained in power well 2, which won't be powered on/off
> > dynamically by DMC.
Is the AUX-IO contained in power well 2? The AUX_CTL registers are.
If the hardware is sending AUX transactions on AUX_A when PSR is
enabled, does it mean it is also powering up power well 1 or just the
AUX-IO?
> >
> > --Imre
> >
> > >
> > > To just enable the AUX-IO well, the correct version is ->
> > > https://patchwork.freedesktop.org/patch/205328/
> > >
> > >
> > >
> > > -DK
> > > >
> > > > > + * Although PSR is enabled only on Port A currently, let's do this
> > > > > + * correctly for other ports too.
> > > > > + */
> > > > > + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > > > + intel_dp->aux_power_domain;
> > > > > +}
> > > > > +
> > > > > +static void psr_power_get(struct intel_dp *intel_dp)
> > > >
> > > > nip: psr_aux_io_power_get ?!
> > > >
> > > > > +{
> > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > > +
> > > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > > + return;
> > > > > +
> > > > > + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> > > > > +}
> > > > > +
> > > > > +static void psr_power_put(struct intel_dp *intel_dp)
> > > > > +{
> > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > > +
> > > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > > + return;
> > > > > +
> > > > > + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > > > > +}
> > > > > +
> > > > > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > > > > {
> > > > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > > > @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > > > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > > > u32 chicken;
> > > > >
> > > > > + psr_power_get(intel_dp);
> > > > > +
> > > > > if (dev_priv->psr.psr2_support) {
> > > > > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > > > > if (dev_priv->psr.y_cord_support)
> > > > > @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > > > > else
> > > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > > > > }
> > > > > +
> > > > > + psr_power_put(intel_dp);
> > > > > }
> > > > >
> > > > > /**
> > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > index b7924feb9f27..53ea564f971e 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > > > > return "AUX_D";
> > > > > case POWER_DOMAIN_AUX_F:
> > > > > return "AUX_F";
> > > > > + case POWER_DOMAIN_AUX_IO_A:
> > > > > + return "AUX_IO_A";
> > > > > case POWER_DOMAIN_GMBUS:
> > > > > return "GMBUS";
> > > > > case POWER_DOMAIN_INIT:
> > > > > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > > > > --
> > > > > 2.14.1
> > > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 22:30 ` Pandiyan, Dhinakaran
@ 2018-02-22 22:42 ` Imre Deak
2018-02-22 22:58 ` Pandiyan, Dhinakaran
0 siblings, 1 reply; 26+ messages in thread
From: Imre Deak @ 2018-02-22 22:42 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx, Vivi, Rodrigo
On Fri, Feb 23, 2018 at 12:30:14AM +0200, Pandiyan, Dhinakaran wrote:
> On Fri, 2018-02-23 at 00:05 +0200, Imre Deak wrote:
> > On Thu, Feb 22, 2018 at 11:53:30PM +0200, Imre Deak wrote:
> > > On Thu, Feb 22, 2018 at 11:33:10PM +0200, Pandiyan, Dhinakaran wrote:
> > > >
> > > >
> > > >
> > > > On Thu, 2018-02-22 at 12:44 -0800, Rodrigo Vivi wrote:
> > > > > On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> > > > > > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > > > > > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > > > > > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > > > > > channels re-use the existing AUX domains as they do need power well 2.
> > > > > >
> > > > > > v3: Extract aux domain selection into a function (Ville)
> > > > > > v2: Add AUX IO domain only for AUX-A
> > > > > > Rebased on top of Ville's AUX series.
> > > > > >
> > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > Suggested-by: Imre Deak <imre.deak@intel.com>
> > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/intel_display.h | 1 +
> > > > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > > > drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> > > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > > > > 5 files changed, 44 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > > > > index f5733a2576e7..4e7418b345bc 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > > > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > > > > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > > > > > POWER_DOMAIN_AUX_C,
> > > > > > POWER_DOMAIN_AUX_D,
> > > > > > POWER_DOMAIN_AUX_F,
> > > > > > + POWER_DOMAIN_AUX_IO_A,
> > > > > > POWER_DOMAIN_GMBUS,
> > > > > > POWER_DOMAIN_MODESET,
> > > > > > POWER_DOMAIN_GT_IRQ,
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > index 005735a7fc29..b78b9972ebae 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > > > > > return ret;
> > > > > > }
> > > > > >
> > > > > > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > > > {
> > > > > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > > > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > > > index 5132f6a58c6d..bcd6dc9fb13d 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > > @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > > > > > int intel_dp_link_required(int pixel_clock, int bpp);
> > > > > > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > > > > > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > > > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> > > > > >
> > > > > > /* intel_dp_aux_backlight.c */
> > > > > > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > index 2ef374f936b9..03814f7bc2d3 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > @@ -56,6 +56,40 @@
> > > > > > #include "intel_drv.h"
> > > > > > #include "i915_drv.h"
> > > > > >
> > > > > > +static inline enum intel_display_power_domain
> > > > > > +psr_aux_domain(struct intel_dp *intel_dp)
> > > > > > +{
> > > > > > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > > > > > + * does not require the driver to disable DC states, but the rest do.
> > > > >
> > > > > This phrase is strange. Specially "the rest do" part.
> > > > > It seems that we need to disable DC states on other ports than A,
> > > > > what it is not true.
> > > >
> > > > Okay, let's back up a little bit.
> > > >
> > > > AUX domain for port A enables DC_OFF and AUX-IO. AUX domains for non-A
> > > > ports enable DC_OFF, power well 2 and AUX-IO, in fact all domains that
> > > > require power well 2 enable DC_OFF too. I can't see this is in bspec but
> > > > that's what the code does currently. So, this is for AUX transfers.
> > >
> > > To enable DC5/6 power well 2 needs to be disabled; I see this is still
> > > missing from the BSpec DC5/6 enable sequence, though I asked already for
> > > an update there before. Will ask again.
> >
> > Actually it's at "Mode Set[SKL+]/Sequences for Display C5 and C6":
> >
> > """
> > ...
> > 2. Configure display engine to have power wells above power well 1
> > disabled, following the appropriate mode set disable sequences for any
> > ports using power wells above power well 1. This can be done earlier if
> > desired.
> > ...
> > 4. Set DC_STATE_EN Dynamic DC State Enable = "Enable up to DC5" for DC5
> > or "Enable up to DC6" for DC6.
> >
> > """
> >
> Thanks, finding this information would have been much easier if it was
> all in one place. We still have to assume the converse from the enable
> step. I wonder if enabling power well 2 is sufficient to bring out the
> hardware from DC6 without having to explicitly disable it.
Yep, that's not that clear, but I guess in any caes we can do that always
manually. Should file an update request for this.
> > > Non-A AUX channels are contained in power well 2, so we have to disable
> > > DC states and enable power well 2 for non-A AUX channels.
> > >
> > > >
> > > > which means, this comment for the previous iteration of the patch
> > > > "I mean, on CNL
> > > >
> > > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
> > > > POWER_DOMAIN_AUX__IO_{B,C,D,F}"
> > > >
> > > > was not accurate. _AUX_IO_{B,C,D,F} domains enable only AUX_IO well and
> > > > _AUX_{B,C,D,F} domains enable power well 2, DC_OFF and AUX_IO.
> > > >
> > > >
> > > > For PSR, we want only the AUX_IO to be enabled for any port because the
> > > > spec says -
> > > > "PSR spontaneously sends Aux transactions. If PSR is enabled on a port,
> > > > then the associated Aux IO must be kept powered up."
> > > >
> > > > which means, this comment
> > > > "Hm, so in general (for AUX transfers) to enable AUX-A we first need to
> > > > disable DC states _except_ if we enable AUX-A for PSR where we want
> > > > DC5/6 to stay enabled." applies to all ports.
> > >
> > > It doesn't make sense and we can't enable DC5/6 for non-A AUX channels,
> > > since those are contained in power well 2, which won't be powered on/off
> > > dynamically by DMC.
>
> Is the AUX-IO contained in power well 2? The AUX_CTL registers are.
Not sure if the two things are divided but I would assume the HW/FW
needs the same registers to perform an AUX transfer as what the driver
programs. (Hence the need for the HW mutex.)
> If the hardware is sending AUX transactions on AUX_A when PSR is
> enabled, does it mean it is also powering up power well 1 or just the
> AUX-IO?
Again, I'd say it needs the same power as what's needed in case the
driver does AUX transfers. So yes, power well 1 would be powered on.
--Imre
>
>
> > >
> > > --Imre
> > >
> > > >
> > > > To just enable the AUX-IO well, the correct version is ->
> > > > https://patchwork.freedesktop.org/patch/205328/
> > > >
> > > >
> > > >
> > > > -DK
> > > > >
> > > > > > + * Although PSR is enabled only on Port A currently, let's do this
> > > > > > + * correctly for other ports too.
> > > > > > + */
> > > > > > + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > > > > + intel_dp->aux_power_domain;
> > > > > > +}
> > > > > > +
> > > > > > +static void psr_power_get(struct intel_dp *intel_dp)
> > > > >
> > > > > nip: psr_aux_io_power_get ?!
> > > > >
> > > > > > +{
> > > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > > > +
> > > > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > > > + return;
> > > > > > +
> > > > > > + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> > > > > > +}
> > > > > > +
> > > > > > +static void psr_power_put(struct intel_dp *intel_dp)
> > > > > > +{
> > > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > > > +
> > > > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > > > + return;
> > > > > > +
> > > > > > + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > > > > > +}
> > > > > > +
> > > > > > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > > > > > {
> > > > > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > > > > @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > > > > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > > > > u32 chicken;
> > > > > >
> > > > > > + psr_power_get(intel_dp);
> > > > > > +
> > > > > > if (dev_priv->psr.psr2_support) {
> > > > > > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > > > > > if (dev_priv->psr.y_cord_support)
> > > > > > @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > > > > > else
> > > > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > > > > > }
> > > > > > +
> > > > > > + psr_power_put(intel_dp);
> > > > > > }
> > > > > >
> > > > > > /**
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > > index b7924feb9f27..53ea564f971e 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > > > > > return "AUX_D";
> > > > > > case POWER_DOMAIN_AUX_F:
> > > > > > return "AUX_F";
> > > > > > + case POWER_DOMAIN_AUX_IO_A:
> > > > > > + return "AUX_IO_A";
> > > > > > case POWER_DOMAIN_GMBUS:
> > > > > > return "GMBUS";
> > > > > > case POWER_DOMAIN_INIT:
> > > > > > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > > > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > > > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > > > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > > > > > --
> > > > > > 2.14.1
> > > > > >
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 22:10 ` Pandiyan, Dhinakaran
@ 2018-02-22 22:55 ` Rodrigo Vivi
2018-02-22 23:54 ` [PATCH v4] " Dhinakaran Pandiyan
0 siblings, 1 reply; 26+ messages in thread
From: Rodrigo Vivi @ 2018-02-22 22:55 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
On Thu, Feb 22, 2018 at 10:10:24PM +0000, Pandiyan, Dhinakaran wrote:
>
>
> On Thu, 2018-02-22 at 23:47 +0200, Ville Syrjälä wrote:
> > On Thu, Feb 22, 2018 at 09:33:10PM +0000, Pandiyan, Dhinakaran wrote:
> > >
> > >
> > >
> > > On Thu, 2018-02-22 at 12:44 -0800, Rodrigo Vivi wrote:
> > > > On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> > > > > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > > > > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > > > > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > > > > channels re-use the existing AUX domains as they do need power well 2.
> > > > >
> > > > > v3: Extract aux domain selection into a function (Ville)
> > > > > v2: Add AUX IO domain only for AUX-A
> > > > > Rebased on top of Ville's AUX series.
> > > > >
> > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Suggested-by: Imre Deak <imre.deak@intel.com>
> > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_display.h | 1 +
> > > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > > drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > > > 5 files changed, 44 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > > > index f5733a2576e7..4e7418b345bc 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > > > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > > > > POWER_DOMAIN_AUX_C,
> > > > > POWER_DOMAIN_AUX_D,
> > > > > POWER_DOMAIN_AUX_F,
> > > > > + POWER_DOMAIN_AUX_IO_A,
> > > > > POWER_DOMAIN_GMBUS,
> > > > > POWER_DOMAIN_MODESET,
> > > > > POWER_DOMAIN_GT_IRQ,
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index 005735a7fc29..b78b9972ebae 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > > > > return ret;
> > > > > }
> > > > >
> > > > > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > > {
> > > > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > > index 5132f6a58c6d..bcd6dc9fb13d 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > > > > int intel_dp_link_required(int pixel_clock, int bpp);
> > > > > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > > > > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> > > > >
> > > > > /* intel_dp_aux_backlight.c */
> > > > > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > > > index 2ef374f936b9..03814f7bc2d3 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > @@ -56,6 +56,40 @@
> > > > > #include "intel_drv.h"
> > > > > #include "i915_drv.h"
> > > > >
> > > > > +static inline enum intel_display_power_domain
> > > > > +psr_aux_domain(struct intel_dp *intel_dp)
> > > > > +{
> > > > > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > > > > + * does not require the driver to disable DC states, but the rest do.
> > > >
> > > > This phrase is strange. Specially "the rest do" part.
> > > > It seems that we need to disable DC states on other ports than A,
> > > > what it is not true.
> > >
> > > Okay, let's back up a little bit.
> > >
> > > AUX domain for port A enables DC_OFF and AUX-IO. AUX domains for non-A
> > > ports enable DC_OFF, power well 2 and AUX-IO, in fact all domains that
> > > require power well 2 enable DC_OFF too. I can't see this is in bspec but
> > > that's what the code does currently. So, this is for AUX transfers.
> > >
> > > which means, this comment for the previous iteration of the patch
> > > "I mean, on CNL
> > >
> > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
> > > POWER_DOMAIN_AUX__IO_{B,C,D,F}"
> > >
> > > was not accurate. _AUX_IO_{B,C,D,F} domains enable only AUX_IO well and
> > > _AUX_{B,C,D,F} domains enable power well 2, DC_OFF and AUX_IO.
> > >
> > >
> > > For PSR, we want only the AUX_IO to be enabled for any port because the
> > > spec says -
> > > "PSR spontaneously sends Aux transactions. If PSR is enabled on a port,
> > > then the associated Aux IO must be kept powered up."
> > >
> > > which means, this comment
> > > "Hm, so in general (for AUX transfers) to enable AUX-A we first need to
> > > disable DC states _except_ if we enable AUX-A for PSR where we want
> > > DC5/6 to stay enabled." applies to all ports.
> > >
> > > To just enable the AUX-IO well, the correct version is ->
> > > https://patchwork.freedesktop.org/patch/205328/
> >
> > While that is true I don't think it actually matters in the end. As
> > long as transcoder A-C is enabled DC will be off and PG2 will be
> > enabled. So I suppose we can (ab)use that fact to save a few power
> > domain bits and only define AUX_IO_A. The comment should probably
> > be reworded to say as much.
> >
>
> "CNL HW requires corresponding AUX IOs to be powered up for PSR.
> However, for non-A AUX ports the corresponding non-EDP transcoders
> would have already enabled power well 2 and DC_OFF. This means we can
> acquire a wider POWER_DOMAIN_AUX_{B,C,D,F} reference instead of a
> specific AUX_IO reference without powering up any extra wells."
>
> Sound good?
Oh ok... thanks for explaining it to me in person as well...
I was really missing that AUX B to F were already part for PG2 hence
part of DC_OFF...
Maybe the first version was less confusing actually... but we can
go with this optmization.
If you want to go with this version:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
and ville can merge when merging the aux_ch one.
>
> > >
> > >
> > >
> > > -DK
> > > >
> > > > > + * Although PSR is enabled only on Port A currently, let's do this
> > > > > + * correctly for other ports too.
> > > > > + */
> > > > > + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > > > + intel_dp->aux_power_domain;
> > > > > +}
> > > > > +
> > > > > +static void psr_power_get(struct intel_dp *intel_dp)
> > > >
> > > > nip: psr_aux_io_power_get ?!
> > > >
> > > > > +{
> > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > > +
> > > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > > + return;
> > > > > +
> > > > > + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> > > > > +}
> > > > > +
> > > > > +static void psr_power_put(struct intel_dp *intel_dp)
> > > > > +{
> > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > > +
> > > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > > + return;
> > > > > +
> > > > > + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > > > > +}
> > > > > +
> > > > > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > > > > {
> > > > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > > > @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > > > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > > > u32 chicken;
> > > > >
> > > > > + psr_power_get(intel_dp);
> > > > > +
> > > > > if (dev_priv->psr.psr2_support) {
> > > > > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > > > > if (dev_priv->psr.y_cord_support)
> > > > > @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > > > > else
> > > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > > > > }
> > > > > +
> > > > > + psr_power_put(intel_dp);
> > > > > }
> > > > >
> > > > > /**
> > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > index b7924feb9f27..53ea564f971e 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > > > > return "AUX_D";
> > > > > case POWER_DOMAIN_AUX_F:
> > > > > return "AUX_F";
> > > > > + case POWER_DOMAIN_AUX_IO_A:
> > > > > + return "AUX_IO_A";
> > > > > case POWER_DOMAIN_GMBUS:
> > > > > return "GMBUS";
> > > > > case POWER_DOMAIN_INIT:
> > > > > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > > > > --
> > > > > 2.14.1
> > > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 22:42 ` Imre Deak
@ 2018-02-22 22:58 ` Pandiyan, Dhinakaran
2018-02-22 23:06 ` Imre Deak
0 siblings, 1 reply; 26+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-02-22 22:58 UTC (permalink / raw)
To: Deak, Imre; +Cc: intel-gfx, Vivi, Rodrigo
On Fri, 2018-02-23 at 00:42 +0200, Imre Deak wrote:
> On Fri, Feb 23, 2018 at 12:30:14AM +0200, Pandiyan, Dhinakaran wrote:
> > On Fri, 2018-02-23 at 00:05 +0200, Imre Deak wrote:
> > > On Thu, Feb 22, 2018 at 11:53:30PM +0200, Imre Deak wrote:
> > > > On Thu, Feb 22, 2018 at 11:33:10PM +0200, Pandiyan, Dhinakaran wrote:
> > > > >
> > > > >
> > > > >
> > > > > On Thu, 2018-02-22 at 12:44 -0800, Rodrigo Vivi wrote:
> > > > > > On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> > > > > > > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > > > > > > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > > > > > > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > > > > > > channels re-use the existing AUX domains as they do need power well 2.
> > > > > > >
> > > > > > > v3: Extract aux domain selection into a function (Ville)
> > > > > > > v2: Add AUX IO domain only for AUX-A
> > > > > > > Rebased on top of Ville's AUX series.
> > > > > > >
> > > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > Suggested-by: Imre Deak <imre.deak@intel.com>
> > > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > > ---
> > > > > > > drivers/gpu/drm/i915/intel_display.h | 1 +
> > > > > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > > > > drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> > > > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > > > > > 5 files changed, 44 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > > > > > index f5733a2576e7..4e7418b345bc 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > > > > > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > > > > > > POWER_DOMAIN_AUX_C,
> > > > > > > POWER_DOMAIN_AUX_D,
> > > > > > > POWER_DOMAIN_AUX_F,
> > > > > > > + POWER_DOMAIN_AUX_IO_A,
> > > > > > > POWER_DOMAIN_GMBUS,
> > > > > > > POWER_DOMAIN_MODESET,
> > > > > > > POWER_DOMAIN_GT_IRQ,
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > index 005735a7fc29..b78b9972ebae 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > > > > > > return ret;
> > > > > > > }
> > > > > > >
> > > > > > > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > > > > {
> > > > > > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > > > > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > > > > index 5132f6a58c6d..bcd6dc9fb13d 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > > > @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > > > > > > int intel_dp_link_required(int pixel_clock, int bpp);
> > > > > > > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > > > > > > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > > > > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> > > > > > >
> > > > > > > /* intel_dp_aux_backlight.c */
> > > > > > > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > > index 2ef374f936b9..03814f7bc2d3 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > > @@ -56,6 +56,40 @@
> > > > > > > #include "intel_drv.h"
> > > > > > > #include "i915_drv.h"
> > > > > > >
> > > > > > > +static inline enum intel_display_power_domain
> > > > > > > +psr_aux_domain(struct intel_dp *intel_dp)
> > > > > > > +{
> > > > > > > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > > > > > > + * does not require the driver to disable DC states, but the rest do.
> > > > > >
> > > > > > This phrase is strange. Specially "the rest do" part.
> > > > > > It seems that we need to disable DC states on other ports than A,
> > > > > > what it is not true.
> > > > >
> > > > > Okay, let's back up a little bit.
> > > > >
> > > > > AUX domain for port A enables DC_OFF and AUX-IO. AUX domains for non-A
> > > > > ports enable DC_OFF, power well 2 and AUX-IO, in fact all domains that
> > > > > require power well 2 enable DC_OFF too. I can't see this is in bspec but
> > > > > that's what the code does currently. So, this is for AUX transfers.
> > > >
> > > > To enable DC5/6 power well 2 needs to be disabled; I see this is still
> > > > missing from the BSpec DC5/6 enable sequence, though I asked already for
> > > > an update there before. Will ask again.
> > >
> > > Actually it's at "Mode Set[SKL+]/Sequences for Display C5 and C6":
> > >
> > > """
> > > ...
> > > 2. Configure display engine to have power wells above power well 1
> > > disabled, following the appropriate mode set disable sequences for any
> > > ports using power wells above power well 1. This can be done earlier if
> > > desired.
> > > ...
> > > 4. Set DC_STATE_EN Dynamic DC State Enable = "Enable up to DC5" for DC5
> > > or "Enable up to DC6" for DC6.
> > >
> > > """
> > >
> > Thanks, finding this information would have been much easier if it was
> > all in one place. We still have to assume the converse from the enable
> > step. I wonder if enabling power well 2 is sufficient to bring out the
> > hardware from DC6 without having to explicitly disable it.
>
> Yep, that's not that clear, but I guess in any caes we can do that always
> manually. Should file an update request for this.
>
> > > > Non-A AUX channels are contained in power well 2, so we have to disable
> > > > DC states and enable power well 2 for non-A AUX channels.
> > > >
> > > > >
> > > > > which means, this comment for the previous iteration of the patch
> > > > > "I mean, on CNL
> > > > >
> > > > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
> > > > > POWER_DOMAIN_AUX__IO_{B,C,D,F}"
> > > > >
> > > > > was not accurate. _AUX_IO_{B,C,D,F} domains enable only AUX_IO well and
> > > > > _AUX_{B,C,D,F} domains enable power well 2, DC_OFF and AUX_IO.
> > > > >
> > > > >
> > > > > For PSR, we want only the AUX_IO to be enabled for any port because the
> > > > > spec says -
> > > > > "PSR spontaneously sends Aux transactions. If PSR is enabled on a port,
> > > > > then the associated Aux IO must be kept powered up."
> > > > >
> > > > > which means, this comment
> > > > > "Hm, so in general (for AUX transfers) to enable AUX-A we first need to
> > > > > disable DC states _except_ if we enable AUX-A for PSR where we want
> > > > > DC5/6 to stay enabled." applies to all ports.
> > > >
> > > > It doesn't make sense and we can't enable DC5/6 for non-A AUX channels,
> > > > since those are contained in power well 2, which won't be powered on/off
> > > > dynamically by DMC.
> >
> > Is the AUX-IO contained in power well 2? The AUX_CTL registers are.
>
> Not sure if the two things are divided but I would assume the HW/FW
> needs the same registers to perform an AUX transfer as what the driver
> programs. (Hence the need for the HW mutex.)
>
That's a fair guess but kinda defeats the point of having a separate
power well for AUX IO, doesn't it? We should perhaps check this with
Art.
> > If the hardware is sending AUX transactions on AUX_A when PSR is
> > enabled, does it mean it is also powering up power well 1 or just the
> > AUX-IO?
>
> Again, I'd say it needs the same power as what's needed in case the
> driver does AUX transfers. So yes, power well 1 would be powered on.
>
> --Imre
>
> >
> >
> > > >
> > > > --Imre
> > > >
> > > > >
> > > > > To just enable the AUX-IO well, the correct version is ->
> > > > > https://patchwork.freedesktop.org/patch/205328/
> > > > >
> > > > >
> > > > >
> > > > > -DK
> > > > > >
> > > > > > > + * Although PSR is enabled only on Port A currently, let's do this
> > > > > > > + * correctly for other ports too.
> > > > > > > + */
> > > > > > > + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > > > > > + intel_dp->aux_power_domain;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void psr_power_get(struct intel_dp *intel_dp)
> > > > > >
> > > > > > nip: psr_aux_io_power_get ?!
> > > > > >
> > > > > > > +{
> > > > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > > > > +
> > > > > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > > > > + return;
> > > > > > > +
> > > > > > > + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void psr_power_put(struct intel_dp *intel_dp)
> > > > > > > +{
> > > > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > > > > +
> > > > > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > > > > + return;
> > > > > > > +
> > > > > > > + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > > > > > > +}
> > > > > > > +
> > > > > > > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > > > > > > {
> > > > > > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > > > > > @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > > > > > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > > > > > u32 chicken;
> > > > > > >
> > > > > > > + psr_power_get(intel_dp);
> > > > > > > +
> > > > > > > if (dev_priv->psr.psr2_support) {
> > > > > > > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > > > > > > if (dev_priv->psr.y_cord_support)
> > > > > > > @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > > > > > > else
> > > > > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > > > > > > }
> > > > > > > +
> > > > > > > + psr_power_put(intel_dp);
> > > > > > > }
> > > > > > >
> > > > > > > /**
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > > > index b7924feb9f27..53ea564f971e 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > > > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > > > > > > return "AUX_D";
> > > > > > > case POWER_DOMAIN_AUX_F:
> > > > > > > return "AUX_F";
> > > > > > > + case POWER_DOMAIN_AUX_IO_A:
> > > > > > > + return "AUX_IO_A";
> > > > > > > case POWER_DOMAIN_GMBUS:
> > > > > > > return "GMBUS";
> > > > > > > case POWER_DOMAIN_INIT:
> > > > > > > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > > > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > > > > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > > > > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > > > > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > > > > > > --
> > > > > > > 2.14.1
> > > > > > >
> > > > > > _______________________________________________
> > > > > > Intel-gfx mailing list
> > > > > > Intel-gfx@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] drm/i915/psr: New power domain for AUX IO.
2018-02-22 22:58 ` Pandiyan, Dhinakaran
@ 2018-02-22 23:06 ` Imre Deak
0 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2018-02-22 23:06 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx, Vivi, Rodrigo
On Fri, Feb 23, 2018 at 12:58:44AM +0200, Pandiyan, Dhinakaran wrote:
> On Fri, 2018-02-23 at 00:42 +0200, Imre Deak wrote:
> > On Fri, Feb 23, 2018 at 12:30:14AM +0200, Pandiyan, Dhinakaran wrote:
> > > On Fri, 2018-02-23 at 00:05 +0200, Imre Deak wrote:
> > > > On Thu, Feb 22, 2018 at 11:53:30PM +0200, Imre Deak wrote:
> > > > > On Thu, Feb 22, 2018 at 11:33:10PM +0200, Pandiyan, Dhinakaran wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Thu, 2018-02-22 at 12:44 -0800, Rodrigo Vivi wrote:
> > > > > > > On Thu, Feb 22, 2018 at 12:28:50PM -0800, Dhinakaran Pandiyan wrote:
> > > > > > > > PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
> > > > > > > > for AUX-A enables DC_OFF well too. This is not required, so add a new
> > > > > > > > AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
> > > > > > > > channels re-use the existing AUX domains as they do need power well 2.
> > > > > > > >
> > > > > > > > v3: Extract aux domain selection into a function (Ville)
> > > > > > > > v2: Add AUX IO domain only for AUX-A
> > > > > > > > Rebased on top of Ville's AUX series.
> > > > > > > >
> > > > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > Suggested-by: Imre Deak <imre.deak@intel.com>
> > > > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > > > ---
> > > > > > > > drivers/gpu/drm/i915/intel_display.h | 1 +
> > > > > > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > > > > > drivers/gpu/drm/i915/intel_psr.c | 38 +++++++++++++++++++++++++++++++++
> > > > > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > > > > > > 5 files changed, 44 insertions(+), 1 deletion(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > > > > > > index f5733a2576e7..4e7418b345bc 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > > > > > > @@ -186,6 +186,7 @@ enum intel_display_power_domain {
> > > > > > > > POWER_DOMAIN_AUX_C,
> > > > > > > > POWER_DOMAIN_AUX_D,
> > > > > > > > POWER_DOMAIN_AUX_F,
> > > > > > > > + POWER_DOMAIN_AUX_IO_A,
> > > > > > > > POWER_DOMAIN_GMBUS,
> > > > > > > > POWER_DOMAIN_MODESET,
> > > > > > > > POWER_DOMAIN_GT_IRQ,
> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > > index 005735a7fc29..b78b9972ebae 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > > @@ -1331,7 +1331,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > > > > > > > return ret;
> > > > > > > > }
> > > > > > > >
> > > > > > > > -static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > > > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
> > > > > > > > {
> > > > > > > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > > > > > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > > > > > index 5132f6a58c6d..bcd6dc9fb13d 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > > > > @@ -1685,6 +1685,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > > > > > > > int intel_dp_link_required(int pixel_clock, int bpp);
> > > > > > > > int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > > > > > > > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > > > > > > > +enum aux_ch intel_aux_ch(struct intel_dp *intel_dp);
> > > > > > > >
> > > > > > > > /* intel_dp_aux_backlight.c */
> > > > > > > > int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > > > index 2ef374f936b9..03814f7bc2d3 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > > > @@ -56,6 +56,40 @@
> > > > > > > > #include "intel_drv.h"
> > > > > > > > #include "i915_drv.h"
> > > > > > > >
> > > > > > > > +static inline enum intel_display_power_domain
> > > > > > > > +psr_aux_domain(struct intel_dp *intel_dp)
> > > > > > > > +{
> > > > > > > > + /* CNL HW requires corresponding AUX IOs to be powered up for PSR, AUX-A
> > > > > > > > + * does not require the driver to disable DC states, but the rest do.
> > > > > > >
> > > > > > > This phrase is strange. Specially "the rest do" part.
> > > > > > > It seems that we need to disable DC states on other ports than A,
> > > > > > > what it is not true.
> > > > > >
> > > > > > Okay, let's back up a little bit.
> > > > > >
> > > > > > AUX domain for port A enables DC_OFF and AUX-IO. AUX domains for non-A
> > > > > > ports enable DC_OFF, power well 2 and AUX-IO, in fact all domains that
> > > > > > require power well 2 enable DC_OFF too. I can't see this is in bspec but
> > > > > > that's what the code does currently. So, this is for AUX transfers.
> > > > >
> > > > > To enable DC5/6 power well 2 needs to be disabled; I see this is still
> > > > > missing from the BSpec DC5/6 enable sequence, though I asked already for
> > > > > an update there before. Will ask again.
> > > >
> > > > Actually it's at "Mode Set[SKL+]/Sequences for Display C5 and C6":
> > > >
> > > > """
> > > > ...
> > > > 2. Configure display engine to have power wells above power well 1
> > > > disabled, following the appropriate mode set disable sequences for any
> > > > ports using power wells above power well 1. This can be done earlier if
> > > > desired.
> > > > ...
> > > > 4. Set DC_STATE_EN Dynamic DC State Enable = "Enable up to DC5" for DC5
> > > > or "Enable up to DC6" for DC6.
> > > >
> > > > """
> > > >
> > > Thanks, finding this information would have been much easier if it was
> > > all in one place. We still have to assume the converse from the enable
> > > step. I wonder if enabling power well 2 is sufficient to bring out the
> > > hardware from DC6 without having to explicitly disable it.
> >
> > Yep, that's not that clear, but I guess in any caes we can do that always
> > manually. Should file an update request for this.
> >
> > > > > Non-A AUX channels are contained in power well 2, so we have to disable
> > > > > DC states and enable power well 2 for non-A AUX channels.
> > > > >
> > > > > >
> > > > > > which means, this comment for the previous iteration of the patch
> > > > > > "I mean, on CNL
> > > > > >
> > > > > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
> > > > > > POWER_DOMAIN_AUX__IO_{B,C,D,F}"
> > > > > >
> > > > > > was not accurate. _AUX_IO_{B,C,D,F} domains enable only AUX_IO well and
> > > > > > _AUX_{B,C,D,F} domains enable power well 2, DC_OFF and AUX_IO.
> > > > > >
> > > > > >
> > > > > > For PSR, we want only the AUX_IO to be enabled for any port because the
> > > > > > spec says -
> > > > > > "PSR spontaneously sends Aux transactions. If PSR is enabled on a port,
> > > > > > then the associated Aux IO must be kept powered up."
> > > > > >
> > > > > > which means, this comment
> > > > > > "Hm, so in general (for AUX transfers) to enable AUX-A we first need to
> > > > > > disable DC states _except_ if we enable AUX-A for PSR where we want
> > > > > > DC5/6 to stay enabled." applies to all ports.
> > > > >
> > > > > It doesn't make sense and we can't enable DC5/6 for non-A AUX channels,
> > > > > since those are contained in power well 2, which won't be powered on/off
> > > > > dynamically by DMC.
> > >
> > > Is the AUX-IO contained in power well 2? The AUX_CTL registers are.
> >
> > Not sure if the two things are divided but I would assume the HW/FW
> > needs the same registers to perform an AUX transfer as what the driver
> > programs. (Hence the need for the HW mutex.)
> >
> That's a fair guess but kinda defeats the point of having a separate
> power well for AUX IO, doesn't it?
No, if you have also other functionality backed by PW#1/2. Then by
powering off the AUX-IO power well when AUX transfers are not needed
(like when you have only the main link on) saves power even if PW#1/2 is
enabled.
> We should perhaps check this with Art.
Sure, I agree it's not completely clear.
> > > If the hardware is sending AUX transactions on AUX_A when PSR is
> > > enabled, does it mean it is also powering up power well 1 or just the
> > > AUX-IO?
> >
> > Again, I'd say it needs the same power as what's needed in case the
> > driver does AUX transfers. So yes, power well 1 would be powered on.
> >
> > --Imre
> >
> > >
> > >
> > > > >
> > > > > --Imre
> > > > >
> > > > > >
> > > > > > To just enable the AUX-IO well, the correct version is ->
> > > > > > https://patchwork.freedesktop.org/patch/205328/
> > > > > >
> > > > > >
> > > > > >
> > > > > > -DK
> > > > > > >
> > > > > > > > + * Although PSR is enabled only on Port A currently, let's do this
> > > > > > > > + * correctly for other ports too.
> > > > > > > > + */
> > > > > > > > + return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > > > > > > + intel_dp->aux_power_domain;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void psr_power_get(struct intel_dp *intel_dp)
> > > > > > >
> > > > > > > nip: psr_aux_io_power_get ?!
> > > > > > >
> > > > > > > > +{
> > > > > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > > > > > +
> > > > > > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > > > > > + return;
> > > > > > > > +
> > > > > > > > + intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void psr_power_put(struct intel_dp *intel_dp)
> > > > > > > > +{
> > > > > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > > > > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > > > > > > > +
> > > > > > > > + if (INTEL_GEN(dev_priv) < 10)
> > > > > > > > + return;
> > > > > > > > +
> > > > > > > > + intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> > > > > > > > {
> > > > > > > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > > > > > > @@ -459,6 +493,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > > > > > > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > > > > > > u32 chicken;
> > > > > > > >
> > > > > > > > + psr_power_get(intel_dp);
> > > > > > > > +
> > > > > > > > if (dev_priv->psr.psr2_support) {
> > > > > > > > chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > > > > > > > if (dev_priv->psr.y_cord_support)
> > > > > > > > @@ -617,6 +653,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > > > > > > > else
> > > > > > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > > > > > > > }
> > > > > > > > +
> > > > > > > > + psr_power_put(intel_dp);
> > > > > > > > }
> > > > > > > >
> > > > > > > > /**
> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > > > > index b7924feb9f27..53ea564f971e 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > > > > > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > > > > > > > return "AUX_D";
> > > > > > > > case POWER_DOMAIN_AUX_F:
> > > > > > > > return "AUX_F";
> > > > > > > > + case POWER_DOMAIN_AUX_IO_A:
> > > > > > > > + return "AUX_IO_A";
> > > > > > > > case POWER_DOMAIN_GMBUS:
> > > > > > > > return "GMBUS";
> > > > > > > > case POWER_DOMAIN_INIT:
> > > > > > > > @@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > > > > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > > > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
> > > > > > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \
> > > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
> > > > > > > > BIT_ULL(POWER_DOMAIN_INIT))
> > > > > > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
> > > > > > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \
> > > > > > > > --
> > > > > > > > 2.14.1
> > > > > > > >
> > > > > > > _______________________________________________
> > > > > > > Intel-gfx mailing list
> > > > > > > Intel-gfx@lists.freedesktop.org
> > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4] drm/i915/psr: New power domain for AUX IO.
2018-02-22 22:55 ` Rodrigo Vivi
@ 2018-02-22 23:54 ` Dhinakaran Pandiyan
0 siblings, 0 replies; 26+ messages in thread
From: Dhinakaran Pandiyan @ 2018-02-22 23:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
PSR on CNL requires AUX IO wells to be kept on and the existing AUX domain
for AUX-A enables DC_OFF well too. This is not required, so add a new
AUX_IO_A domain for AUX-A to allow DC states to remain enabled. Other AUX
channels re-use the existing AUX domains.
v4: Reword comment (Rodrigo and Ville)
Rename _get and _put functions to include aux_io substring(Rodrigo)
Remove unnecessary diff that got included.
v3: Extract aux domain selection into a function (Ville)
v2: Add AUX IO domain only for AUX-A
Rebased on top of Ville's AUX series.
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Suggested-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_display.h | 1 +
drivers/gpu/drm/i915/intel_psr.c | 41 +++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
3 files changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index f5733a2576e7..4e7418b345bc 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -186,6 +186,7 @@ enum intel_display_power_domain {
POWER_DOMAIN_AUX_C,
POWER_DOMAIN_AUX_D,
POWER_DOMAIN_AUX_F,
+ POWER_DOMAIN_AUX_IO_A,
POWER_DOMAIN_GMBUS,
POWER_DOMAIN_MODESET,
POWER_DOMAIN_GT_IRQ,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2ef374f936b9..04430d4c99c9 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,6 +56,43 @@
#include "intel_drv.h"
#include "i915_drv.h"
+static inline enum intel_display_power_domain
+psr_aux_domain(struct intel_dp *intel_dp)
+{
+ /* CNL HW requires corresponding AUX IOs to be powered up for PSR.
+ * However, for non-A AUX ports the corresponding non-EDP transcoders
+ * would have already enabled power well 2 and DC_OFF. This means we can
+ * acquire a wider POWER_DOMAIN_AUX_{B,C,D,F} reference instead of a
+ * specific AUX_IO reference without powering up any extra wells.
+ * Note that PSR is enabled only on Port A even though this function
+ * returns the correct domain for other ports too.
+ */
+ return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
+ intel_dp->aux_power_domain;
+}
+
+static void psr_aux_io_power_get(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
+
+ if (INTEL_GEN(dev_priv) < 10)
+ return;
+
+ intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
+}
+
+static void psr_aux_io_power_put(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
+
+ if (INTEL_GEN(dev_priv) < 10)
+ return;
+
+ intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
+}
+
static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
{
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -459,6 +496,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
u32 chicken;
+ psr_aux_io_power_get(intel_dp);
+
if (dev_priv->psr.psr2_support) {
chicken = PSR2_VSC_ENABLE_PROG_HEADER;
if (dev_priv->psr.y_cord_support)
@@ -617,6 +656,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
else
WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
}
+
+ psr_aux_io_power_put(intel_dp);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index b7924feb9f27..53ea564f971e 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
return "AUX_D";
case POWER_DOMAIN_AUX_F:
return "AUX_F";
+ case POWER_DOMAIN_AUX_IO_A:
+ return "AUX_IO_A";
case POWER_DOMAIN_GMBUS:
return "GMBUS";
case POWER_DOMAIN_INIT:
@@ -1853,6 +1855,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
BIT_ULL(POWER_DOMAIN_INIT))
#define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \
BIT_ULL(POWER_DOMAIN_AUX_A) | \
+ BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
BIT_ULL(POWER_DOMAIN_INIT))
#define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \
BIT_ULL(POWER_DOMAIN_AUX_B) | \
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev3)
2018-02-22 7:28 [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Dhinakaran Pandiyan
` (5 preceding siblings ...)
2018-02-22 21:22 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev2) Patchwork
@ 2018-02-23 0:25 ` Patchwork
2018-02-23 4:07 ` ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev2) Patchwork
2018-02-23 7:09 ` ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev3) Patchwork
8 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2018-02-23 0:25 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev3)
URL : https://patchwork.freedesktop.org/series/38744/
State : success
== Summary ==
Series 38744v3 series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it
https://patchwork.freedesktop.org/api/1.0/series/38744/revisions/3/mbox/
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:421s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:425s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:373s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:479s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:283s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:478s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:479s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:463s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:455s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:565s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:408s
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:290s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:503s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:386s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:406s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:446s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:414s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:452s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:495s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:448s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:493s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:588s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:426s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:495s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:520s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:480s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:467s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:407s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:425s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:518s
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:394s
Blacklisted hosts:
fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:392s
f57b51c21b8307ba66b5212ceb886e52f506fa5e drm-tip: 2018y-02m-22d-23h-14m-05s UTC integration manifest
8d2603ac2831 drm/i915/psr: New power domain for AUX IO.
72f2fe3467be drm/i915: Add enum aux_ch and clean up the aux init to use it
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8140/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev2)
2018-02-22 7:28 [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Dhinakaran Pandiyan
` (6 preceding siblings ...)
2018-02-23 0:25 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev3) Patchwork
@ 2018-02-23 4:07 ` Patchwork
2018-02-23 7:09 ` ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev3) Patchwork
8 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2018-02-23 4:07 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev2)
URL : https://patchwork.freedesktop.org/series/38744/
State : failure
== Summary ==
Test gem_eio:
Subgroup in-flight-contexts:
pass -> INCOMPLETE (shard-apl) fdo#104945
Test kms_frontbuffer_tracking:
Subgroup fbc-1p-primscrn-indfb-msflip-blt:
fail -> PASS (shard-apl) fdo#103167
Test kms_flip_tiling:
Subgroup flip-to-yf-tiled:
fail -> PASS (shard-apl) fdo#103822
Test perf:
Subgroup buffer-fill:
pass -> FAIL (shard-apl) fdo#103755
Test kms_plane:
Subgroup plane-panning-bottom-right-suspend-pipe-b-planes:
pass -> INCOMPLETE (shard-hsw) fdo#103540
Test kms_vblank:
Subgroup pipe-b-ts-continuation-suspend:
pass -> FAIL (shard-hsw)
Test kms_flip:
Subgroup 2x-wf_vblank-ts-check:
fail -> PASS (shard-hsw) fdo#100368 +1
fdo#104945 https://bugs.freedesktop.org/show_bug.cgi?id=104945
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
fdo#103755 https://bugs.freedesktop.org/show_bug.cgi?id=103755
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
shard-apl total:3428 pass:1798 dwarn:1 dfail:0 fail:13 skip:1615 time:12113s
shard-hsw total:3448 pass:1759 dwarn:1 dfail:0 fail:3 skip:1683 time:11171s
shard-snb total:3465 pass:1358 dwarn:1 dfail:0 fail:2 skip:2104 time:6585s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8137/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev3)
2018-02-22 7:28 [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Dhinakaran Pandiyan
` (7 preceding siblings ...)
2018-02-23 4:07 ` ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev2) Patchwork
@ 2018-02-23 7:09 ` Patchwork
8 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2018-02-23 7:09 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev3)
URL : https://patchwork.freedesktop.org/series/38744/
State : success
== Summary ==
Test kms_flip:
Subgroup 2x-plain-flip-fb-recreate-interruptible:
fail -> PASS (shard-hsw) fdo#100368 +1
Subgroup modeset-vs-vblank-race:
pass -> FAIL (shard-hsw) fdo#103060 +1
Subgroup 2x-flip-vs-expired-vblank-interruptible:
pass -> FAIL (shard-hsw) fdo#102887
Test gem_eio:
Subgroup in-flight:
pass -> INCOMPLETE (shard-apl) fdo#104945 +1
Test kms_frontbuffer_tracking:
Subgroup fbc-1p-primscrn-spr-indfb-fullscreen:
pass -> FAIL (shard-snb) fdo#101623
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#104945 https://bugs.freedesktop.org/show_bug.cgi?id=104945
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
shard-apl total:3236 pass:1699 dwarn:1 dfail:0 fail:12 skip:1521 time:11339s
shard-hsw total:3465 pass:1766 dwarn:1 dfail:0 fail:4 skip:1693 time:11719s
shard-snb total:3465 pass:1357 dwarn:1 dfail:0 fail:3 skip:2104 time:6600s
Blacklisted hosts:
shard-kbl total:3422 pass:1944 dwarn:1 dfail:0 fail:14 skip:1462 time:9352s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8140/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2018-02-23 7:09 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-22 7:28 [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Dhinakaran Pandiyan
2018-02-22 7:28 ` [PATCH 2/2] drm/i915/cnl: New power domain for AUX IO Dhinakaran Pandiyan
2018-02-22 10:28 ` Imre Deak
2018-02-22 18:32 ` Imre Deak
2018-02-22 18:54 ` Ville Syrjälä
2018-02-22 20:28 ` [PATCH v3] drm/i915/psr: " Dhinakaran Pandiyan
2018-02-22 20:44 ` Rodrigo Vivi
2018-02-22 21:33 ` Pandiyan, Dhinakaran
2018-02-22 21:47 ` Ville Syrjälä
2018-02-22 22:10 ` Pandiyan, Dhinakaran
2018-02-22 22:55 ` Rodrigo Vivi
2018-02-22 23:54 ` [PATCH v4] " Dhinakaran Pandiyan
2018-02-22 21:53 ` [PATCH v3] " Imre Deak
2018-02-22 22:05 ` Imre Deak
2018-02-22 22:30 ` Pandiyan, Dhinakaran
2018-02-22 22:42 ` Imre Deak
2018-02-22 22:58 ` Pandiyan, Dhinakaran
2018-02-22 23:06 ` Imre Deak
2018-02-22 7:35 ` [PATCH 1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it Pandiyan, Dhinakaran
2018-02-22 7:54 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] " Patchwork
2018-02-22 8:12 ` ✓ Fi.CI.BAT: success " Patchwork
2018-02-22 10:12 ` ✗ Fi.CI.IGT: warning " Patchwork
2018-02-22 21:22 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev2) Patchwork
2018-02-23 0:25 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev3) Patchwork
2018-02-23 4:07 ` ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev2) Patchwork
2018-02-23 7:09 ` ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Add enum aux_ch and clean up the aux init to use it (rev3) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.