* [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function
@ 2018-03-30 22:23 José Roberto de Souza
2018-03-30 22:23 ` [PATCH 02/11] drm/i915/psr: Move PSR exit specific code to hardware specific function José Roberto de Souza
` (12 more replies)
0 siblings, 13 replies; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
It was reading some random register in VLV and CHV.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2d53f7398a6d..fc7c36efd401 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -424,6 +424,11 @@ static void hsw_psr_activate(struct intel_dp *intel_dp)
struct drm_device *dev = dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
+ if (dev_priv->psr.psr2_enabled)
+ WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
+ else
+ WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
+
/* On HSW+ after we enable PSR on source it will activate it
* as soon as it match configure idle_frame count. So
* we just actually enable it here on activation time.
@@ -549,10 +554,6 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- if (dev_priv->psr.psr2_enabled)
- WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
- else
- WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
WARN_ON(dev_priv->psr.active);
lockdep_assert_held(&dev_priv->psr.lock);
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 02/11] drm/i915/psr: Move PSR exit specific code to hardware specific function
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
@ 2018-03-30 22:23 ` José Roberto de Souza
2018-04-02 18:09 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 03/11] drm/i915/psr: Share code between disable and exit José Roberto de Souza
` (11 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
To proper execute PSR exit it was using 'if (HAS_DDI(dev_priv))' to
differentiate between VLV/CHV and HSW+ hardware, so here moving each
hardware handling to his own function.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_psr.c | 94 +++++++++++++++++++++++-----------------
2 files changed, 56 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5373b171bb96..a8d300280a2c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -617,6 +617,7 @@ struct i915_psr {
void (*enable_sink)(struct intel_dp *);
void (*activate)(struct intel_dp *);
void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
+ void (*exit)(struct intel_dp *intel_dp);
};
enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index fc7c36efd401..bcaac9e69f8c 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -838,53 +838,67 @@ static void intel_psr_work(struct work_struct *work)
mutex_unlock(&dev_priv->psr.lock);
}
-static void intel_psr_exit(struct drm_i915_private *dev_priv)
+static void hsw_psr_exit(struct intel_dp *intel_dp)
{
- struct intel_dp *intel_dp = dev_priv->psr.enabled;
- struct drm_crtc *crtc = dp_to_dig_port(intel_dp)->base.base.crtc;
+ struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ u32 val;
+
+ if (dev_priv->psr.psr2_enabled) {
+ val = I915_READ(EDP_PSR2_CTL);
+ WARN_ON(!(val & EDP_PSR2_ENABLE));
+ I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
+ } else {
+ val = I915_READ(EDP_PSR_CTL);
+ WARN_ON(!(val & EDP_PSR_ENABLE));
+ I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
+ }
+}
+
+static void vlv_psr_exit(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_crtc *crtc = dig_port->base.base.crtc;
enum pipe pipe = to_intel_crtc(crtc)->pipe;
u32 val;
- if (!dev_priv->psr.active)
- return;
+ val = I915_READ(VLV_PSRCTL(pipe));
- if (HAS_DDI(dev_priv)) {
- if (dev_priv->psr.psr2_enabled) {
- val = I915_READ(EDP_PSR2_CTL);
- WARN_ON(!(val & EDP_PSR2_ENABLE));
- I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
- } else {
- val = I915_READ(EDP_PSR_CTL);
- WARN_ON(!(val & EDP_PSR_ENABLE));
- I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
- }
- } else {
- val = I915_READ(VLV_PSRCTL(pipe));
+ /*
+ * Here we do the transition drirectly from
+ * PSR_state 3 (active - no Remote Frame Buffer (RFB) update) to
+ * PSR_state 5 (exit).
+ * PSR State 4 (active with single frame update) can be skipped.
+ * On PSR_state 5 (exit) Hardware is responsible to transition
+ * back to PSR_state 1 (inactive).
+ * Now we are at Same state after vlv_psr_enable_source.
+ */
+ val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
+ I915_WRITE(VLV_PSRCTL(pipe), val);
- /*
- * Here we do the transition drirectly from
- * PSR_state 3 (active - no Remote Frame Buffer (RFB) update) to
- * PSR_state 5 (exit).
- * PSR State 4 (active with single frame update) can be skipped.
- * On PSR_state 5 (exit) Hardware is responsible to transition
- * back to PSR_state 1 (inactive).
- * Now we are at Same state after vlv_psr_enable_source.
- */
- val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
- I915_WRITE(VLV_PSRCTL(pipe), val);
+ /*
+ * Send AUX wake up - Spec says after transitioning to PSR
+ * active we have to send AUX wake up by writing 01h in DPCD
+ * 600h of sink device.
+ * XXX: This might slow down the transition, but without this
+ * HW doesn't complete the transition to PSR_state 1 and we
+ * never get the screen updated.
+ */
+ drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+ DP_SET_POWER_D0);
+}
- /*
- * Send AUX wake up - Spec says after transitioning to PSR
- * active we have to send AUX wake up by writing 01h in DPCD
- * 600h of sink device.
- * XXX: This might slow down the transition, but without this
- * HW doesn't complete the transition to PSR_state 1 and we
- * never get the screen updated.
- */
- drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
- DP_SET_POWER_D0);
- }
+static void intel_psr_exit(struct drm_i915_private *dev_priv)
+{
+ struct intel_dp *intel_dp = dev_priv->psr.enabled;
+
+ if (!dev_priv->psr.active)
+ return;
+ dev_priv->psr.exit(intel_dp);
dev_priv->psr.active = false;
}
@@ -1094,6 +1108,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
dev_priv->psr.enable_sink = vlv_psr_enable_sink;
dev_priv->psr.activate = vlv_psr_activate;
dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
+ dev_priv->psr.exit = vlv_psr_exit;
} else {
dev_priv->psr.has_hw_tracking = true;
dev_priv->psr.enable_source = hsw_psr_enable_source;
@@ -1101,5 +1116,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
dev_priv->psr.enable_sink = hsw_psr_enable_sink;
dev_priv->psr.activate = hsw_psr_activate;
dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
+ dev_priv->psr.exit = hsw_psr_exit;
}
}
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 03/11] drm/i915/psr: Share code between disable and exit
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
2018-03-30 22:23 ` [PATCH 02/11] drm/i915/psr: Move PSR exit specific code to hardware specific function José Roberto de Souza
@ 2018-03-30 22:23 ` José Roberto de Souza
2018-04-02 18:11 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 04/11] drm/i915/psr: Remove intel_crtc_state parameter from disable() José Roberto de Souza
` (10 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
The disable and exit sequence are very similar with a lot common
code between both, so here sharing the code.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_psr.c | 84 ++++++++++++++++++----------------------
2 files changed, 38 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a8d300280a2c..cb72ee27422f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -617,7 +617,7 @@ struct i915_psr {
void (*enable_sink)(struct intel_dp *);
void (*activate)(struct intel_dp *);
void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
- void (*exit)(struct intel_dp *intel_dp);
+ void (*exit)(struct intel_dp *intel_dp, bool disabling);
};
enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index bcaac9e69f8c..d3451afeb8bb 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -672,23 +672,9 @@ static void vlv_psr_disable(struct intel_dp *intel_dp,
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
- uint32_t val;
if (dev_priv->psr.active) {
- /* Put VLV PSR back to PSR_state 0 (disabled). */
- if (intel_wait_for_register(dev_priv,
- VLV_PSRSTAT(crtc->pipe),
- VLV_EDP_PSR_IN_TRANS,
- 0,
- 1))
- WARN(1, "PSR transition took longer than expected\n");
-
- val = I915_READ(VLV_PSRCTL(crtc->pipe));
- val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
- val &= ~VLV_EDP_PSR_ENABLE;
- val &= ~VLV_EDP_PSR_MODE_MASK;
- I915_WRITE(VLV_PSRCTL(crtc->pipe), val);
-
+ dev_priv->psr.exit(intel_dp, true);
dev_priv->psr.active = false;
} else {
WARN_ON(vlv_is_psr_active_on_pipe(dev, crtc->pipe));
@@ -703,31 +689,7 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
struct drm_i915_private *dev_priv = to_i915(dev);
if (dev_priv->psr.active) {
- i915_reg_t psr_status;
- u32 psr_status_mask;
-
- if (dev_priv->psr.psr2_enabled) {
- psr_status = EDP_PSR2_STATUS;
- psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
-
- I915_WRITE(EDP_PSR2_CTL,
- I915_READ(EDP_PSR2_CTL) &
- ~(EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE));
-
- } else {
- psr_status = EDP_PSR_STATUS;
- psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
-
- I915_WRITE(EDP_PSR_CTL,
- I915_READ(EDP_PSR_CTL) & ~EDP_PSR_ENABLE);
- }
-
- /* Wait till PSR is idle */
- if (intel_wait_for_register(dev_priv,
- psr_status, psr_status_mask, 0,
- 2000))
- DRM_ERROR("Timed out waiting for PSR Idle State\n");
-
+ dev_priv->psr.exit(intel_dp, true);
dev_priv->psr.active = false;
} else {
if (dev_priv->psr.psr2_enabled)
@@ -838,25 +800,41 @@ static void intel_psr_work(struct work_struct *work)
mutex_unlock(&dev_priv->psr.lock);
}
-static void hsw_psr_exit(struct intel_dp *intel_dp)
+static void hsw_psr_exit(struct intel_dp *intel_dp, bool disabling)
{
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
+ i915_reg_t psr_status;
+ u32 psr_status_mask;
u32 val;
if (dev_priv->psr.psr2_enabled) {
+ psr_status = EDP_PSR2_STATUS;
+ psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
+
val = I915_READ(EDP_PSR2_CTL);
- WARN_ON(!(val & EDP_PSR2_ENABLE));
- I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
+ WARN_ON(!disabling && !(val & EDP_PSR2_ENABLE));
+ val &= ~EDP_PSR2_ENABLE;
+ if (disabling)
+ val &= ~EDP_SU_TRACK_ENABLE;
+ I915_WRITE(EDP_PSR2_CTL, val);
} else {
+ psr_status = EDP_PSR_STATUS;
+ psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
+
val = I915_READ(EDP_PSR_CTL);
- WARN_ON(!(val & EDP_PSR_ENABLE));
+ WARN_ON(!disabling && !(val & EDP_PSR_ENABLE));
I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
}
+
+ /* When disabling wait till PSR is idle */
+ if (disabling && intel_wait_for_register(dev_priv, psr_status,
+ psr_status_mask, 0, 2000))
+ DRM_ERROR("Timed out waiting for PSR Idle State\n");
}
-static void vlv_psr_exit(struct intel_dp *intel_dp)
+static void vlv_psr_exit(struct intel_dp *intel_dp, bool disabling)
{
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = dig_port->base.base.dev;
@@ -865,10 +843,14 @@ static void vlv_psr_exit(struct intel_dp *intel_dp)
enum pipe pipe = to_intel_crtc(crtc)->pipe;
u32 val;
+ if (disabling && intel_wait_for_register(dev_priv, VLV_PSRSTAT(pipe),
+ VLV_EDP_PSR_IN_TRANS, 0, 1))
+ DRM_WARN("PSR transition took longer than expected\n");
+
val = I915_READ(VLV_PSRCTL(pipe));
/*
- * Here we do the transition drirectly from
+ * Here we do the transition directly from
* PSR_state 3 (active - no Remote Frame Buffer (RFB) update) to
* PSR_state 5 (exit).
* PSR State 4 (active with single frame update) can be skipped.
@@ -877,8 +859,16 @@ static void vlv_psr_exit(struct intel_dp *intel_dp)
* Now we are at Same state after vlv_psr_enable_source.
*/
val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
+ if (disabling) {
+ /* Put VLV PSR back to PSR_state 0 (disabled). */
+ val &= ~VLV_EDP_PSR_ENABLE;
+ val &= ~VLV_EDP_PSR_MODE_MASK;
+ }
I915_WRITE(VLV_PSRCTL(pipe), val);
+ if (disabling)
+ return;
+
/*
* Send AUX wake up - Spec says after transitioning to PSR
* active we have to send AUX wake up by writing 01h in DPCD
@@ -898,7 +888,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
if (!dev_priv->psr.active)
return;
- dev_priv->psr.exit(intel_dp);
+ dev_priv->psr.exit(intel_dp, false);
dev_priv->psr.active = false;
}
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 04/11] drm/i915/psr: Remove intel_crtc_state parameter from disable()
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
2018-03-30 22:23 ` [PATCH 02/11] drm/i915/psr: Move PSR exit specific code to hardware specific function José Roberto de Souza
2018-03-30 22:23 ` [PATCH 03/11] drm/i915/psr: Share code between disable and exit José Roberto de Souza
@ 2018-03-30 22:23 ` José Roberto de Souza
2018-04-02 18:13 ` Rodrigo Vivi
2018-04-03 17:52 ` Ville Syrjälä
2018-03-30 22:23 ` [PATCH 05/11] drm/i915/psr: Export intel_psr_activate/exit() José Roberto de Souza
` (9 subsequent siblings)
12 siblings, 2 replies; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
It is not necessary as is possible to get the pipe information
from intel_dp.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +--
drivers/gpu/drm/i915/intel_psr.c | 13 ++++++-------
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cb72ee27422f..99af9169d792 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -612,8 +612,7 @@ struct i915_psr {
void (*enable_source)(struct intel_dp *,
const struct intel_crtc_state *);
- void (*disable_source)(struct intel_dp *,
- const struct intel_crtc_state *);
+ void (*disable_source)(struct intel_dp *intel_dp);
void (*enable_sink)(struct intel_dp *);
void (*activate)(struct intel_dp *);
void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index d3451afeb8bb..c4720b0152c3 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -665,24 +665,23 @@ void intel_psr_enable(struct intel_dp *intel_dp,
mutex_unlock(&dev_priv->psr.lock);
}
-static void vlv_psr_disable(struct intel_dp *intel_dp,
- const struct intel_crtc_state *old_crtc_state)
+static void vlv_psr_disable(struct intel_dp *intel_dp)
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
+ struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
+ enum pipe pipe = to_intel_crtc(crtc)->pipe;
if (dev_priv->psr.active) {
dev_priv->psr.exit(intel_dp, true);
dev_priv->psr.active = false;
} else {
- WARN_ON(vlv_is_psr_active_on_pipe(dev, crtc->pipe));
+ WARN_ON(vlv_is_psr_active_on_pipe(dev, pipe));
}
}
-static void hsw_psr_disable(struct intel_dp *intel_dp,
- const struct intel_crtc_state *old_crtc_state)
+static void hsw_psr_disable(struct intel_dp *intel_dp)
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -727,7 +726,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
return;
}
- dev_priv->psr.disable_source(intel_dp, old_crtc_state);
+ dev_priv->psr.disable_source(intel_dp);
/* Disable PSR on Sink */
drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 05/11] drm/i915/psr: Export intel_psr_activate/exit()
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
` (2 preceding siblings ...)
2018-03-30 22:23 ` [PATCH 04/11] drm/i915/psr: Remove intel_crtc_state parameter from disable() José Roberto de Souza
@ 2018-03-30 22:23 ` José Roberto de Souza
2018-04-02 18:18 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 06/11] drm/i915/psr: Add intel_psr_activate_block_get()/put() José Roberto de Souza
` (8 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Export this functions so other modules can activate and exit PSR.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_psr.c | 94 +++++++++++++++++++++++++++++++++++++---
2 files changed, 89 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d1452fd2a58d..70026b772721 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1891,6 +1891,8 @@ void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
unsigned frontbuffer_bits);
void intel_psr_compute_config(struct intel_dp *intel_dp,
struct intel_crtc_state *crtc_state);
+void intel_psr_exit(struct intel_dp *intel_dp, bool wait_idle);
+void intel_psr_activate(struct intel_dp *intel_dp, bool schedule);
/* intel_runtime_pm.c */
int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c4720b0152c3..906a12ea934d 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,6 +56,8 @@
#include "intel_drv.h"
#include "i915_drv.h"
+#define ACTIVATE_WORK_MSEC_DELAY 100
+
static inline enum intel_display_power_domain
psr_aux_domain(struct intel_dp *intel_dp)
{
@@ -548,7 +550,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
}
-static void intel_psr_activate(struct intel_dp *intel_dp)
+static void __intel_psr_activate(struct intel_dp *intel_dp)
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -645,7 +647,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
dev_priv->psr.enabled = intel_dp;
if (INTEL_GEN(dev_priv) >= 9) {
- intel_psr_activate(intel_dp);
+ __intel_psr_activate(intel_dp);
} else {
/*
* FIXME: Activation should happen immediately since this
@@ -794,7 +796,7 @@ static void intel_psr_work(struct work_struct *work)
if (dev_priv->psr.busy_frontbuffer_bits)
goto unlock;
- intel_psr_activate(intel_dp);
+ __intel_psr_activate(intel_dp);
unlock:
mutex_unlock(&dev_priv->psr.lock);
}
@@ -880,7 +882,7 @@ static void vlv_psr_exit(struct intel_dp *intel_dp, bool disabling)
DP_SET_POWER_D0);
}
-static void intel_psr_exit(struct drm_i915_private *dev_priv)
+static void __intel_psr_exit(struct drm_i915_private *dev_priv)
{
struct intel_dp *intel_dp = dev_priv->psr.enabled;
@@ -977,7 +979,7 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
dev_priv->psr.busy_frontbuffer_bits |= frontbuffer_bits;
if (frontbuffer_bits)
- intel_psr_exit(dev_priv);
+ __intel_psr_exit(dev_priv);
mutex_unlock(&dev_priv->psr.lock);
}
@@ -1023,7 +1025,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
if (frontbuffer_bits) {
if (dev_priv->psr.psr2_enabled ||
IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
- intel_psr_exit(dev_priv);
+ __intel_psr_exit(dev_priv);
} else {
/*
* Display WA #0884: all
@@ -1041,7 +1043,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
if (!work_busy(&dev_priv->psr.work.work))
schedule_delayed_work(&dev_priv->psr.work,
- msecs_to_jiffies(100));
+ msecs_to_jiffies(ACTIVATE_WORK_MSEC_DELAY));
mutex_unlock(&dev_priv->psr.lock);
}
@@ -1108,3 +1110,81 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
dev_priv->psr.exit = hsw_psr_exit;
}
}
+
+/**
+ * intel_psr_exit - Exit PSR
+ * @intel_dp: DisplayPort that should have PSR exited if active
+ * @wait_idle: if true wait until PSR is inactive otherwise just request it
+ *
+ * This function exit PSR if enabled and active in this DisplayPort.
+ */
+void intel_psr_exit(struct intel_dp *intel_dp, bool wait_idle)
+{
+ struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+
+ if (!CAN_PSR(dev_priv))
+ return;
+
+ mutex_lock(&dev_priv->psr.lock);
+ if (dev_priv->psr.enabled != intel_dp)
+ goto out;
+ if (!dev_priv->psr.active && !wait_idle)
+ goto out;
+
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+ struct drm_crtc *crtc = dig_port->base.base.crtc;
+ enum pipe pipe = to_intel_crtc(crtc)->pipe;
+
+ __intel_psr_exit(dev_priv);
+
+ if (!wait_idle)
+ goto out;
+
+ if (intel_wait_for_register(dev_priv, VLV_PSRSTAT(pipe),
+ VLV_EDP_PSR_IN_TRANS, 0, 2000))
+ DRM_ERROR("Timed out waiting for PSR Idle State\n");
+ } else {
+ /* HSW+ PSR disable is actually, exit + wait transition */
+ dev_priv->psr.exit(intel_dp, wait_idle);
+ dev_priv->psr.active = false;
+ }
+
+out:
+ mutex_unlock(&dev_priv->psr.lock);
+}
+
+/**
+ * intel_psr_activate - Activate PSR
+ * @intel_dp: DisplayPort that should have PSR activated if enabled
+ * @schedule: if true schedule the active to happens in a few milliseconds
+ * otherwise activate right now.
+ *
+ * This function activate PSR if enabled in this DisplayPort.
+ */
+void intel_psr_activate(struct intel_dp *intel_dp, bool schedule)
+{
+ struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+
+ if (!CAN_PSR(dev_priv))
+ return;
+
+ mutex_lock(&dev_priv->psr.lock);
+ if (dev_priv->psr.enabled != intel_dp || dev_priv->psr.active)
+ goto out;
+
+ if (dev_priv->psr.busy_frontbuffer_bits)
+ goto out;
+
+ if (schedule) {
+ if (!work_busy(&dev_priv->psr.work.work))
+ schedule_delayed_work(&dev_priv->psr.work,
+ msecs_to_jiffies(ACTIVATE_WORK_MSEC_DELAY));
+ } else
+ __intel_psr_activate(intel_dp);
+out:
+ mutex_unlock(&dev_priv->psr.lock);
+}
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 06/11] drm/i915/psr: Add intel_psr_activate_block_get()/put()
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
` (3 preceding siblings ...)
2018-03-30 22:23 ` [PATCH 05/11] drm/i915/psr: Export intel_psr_activate/exit() José Roberto de Souza
@ 2018-03-30 22:23 ` José Roberto de Souza
2018-04-02 18:20 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction José Roberto de Souza
` (7 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
intel_psr_activate_block_get() should be called when by some reason
PSR should not be activated for some time, it will increment counter
and it should the decremented by intel_psr_activate_block_put()
when PSR can be activated again.
intel_psr_activate_block_put() will not actually activate PSR, users
of this function should also call intel_psr_activate().
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_psr.c | 54 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 99af9169d792..41ebb144594e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -609,6 +609,7 @@ struct i915_psr {
bool has_hw_tracking;
bool psr2_enabled;
u8 sink_sync_latency;
+ unsigned int activate_block_count;
void (*enable_source)(struct intel_dp *,
const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 70026b772721..020b96324135 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1893,6 +1893,8 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
struct intel_crtc_state *crtc_state);
void intel_psr_exit(struct intel_dp *intel_dp, bool wait_idle);
void intel_psr_activate(struct intel_dp *intel_dp, bool schedule);
+void intel_psr_activate_block_get(struct intel_dp *intel_dp);
+void intel_psr_activate_block_put(struct intel_dp *intel_dp);
/* intel_runtime_pm.c */
int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 906a12ea934d..8702dbafb42d 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -558,6 +558,8 @@ static void __intel_psr_activate(struct intel_dp *intel_dp)
WARN_ON(dev_priv->psr.active);
lockdep_assert_held(&dev_priv->psr.lock);
+ if (dev_priv->psr.activate_block_count)
+ return;
dev_priv->psr.activate(intel_dp);
dev_priv->psr.active = true;
@@ -1188,3 +1190,55 @@ void intel_psr_activate(struct intel_dp *intel_dp, bool schedule)
out:
mutex_unlock(&dev_priv->psr.lock);
}
+
+/**
+ * intel_psr_activate_block_get - Block further attempts to activate PSR
+ * @intel_dp: DisplayPort that have PSR enabled
+ *
+ * It have a internal reference count, so each intel_psr_activate_block_get()
+ * should have a intel_psr_activate_block_put() counterpart.
+ */
+void intel_psr_activate_block_get(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+
+ if (!CAN_PSR(dev_priv))
+ return;
+
+ mutex_lock(&dev_priv->psr.lock);
+ if (dev_priv->psr.enabled != intel_dp)
+ goto out;
+
+ dev_priv->psr.activate_block_count++;
+out:
+ mutex_unlock(&dev_priv->psr.lock);
+}
+
+
+/**
+ * intel_psr_activate_block_put - Unblock further attempts to activate PSR
+ * @intel_dp: DisplayPort that have PSR enabled
+ *
+ * Decrease the reference counter incremented by intel_psr_activate_block_get()
+ * when zero it allows PSR be activate. To actually activate PSR when reference
+ * counter is zero intel_psr_activate() should be called.
+ */
+void intel_psr_activate_block_put(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+
+ if (!CAN_PSR(dev_priv))
+ return;
+
+ mutex_lock(&dev_priv->psr.lock);
+ if (dev_priv->psr.enabled != intel_dp)
+ goto out;
+
+ dev_priv->psr.activate_block_count--;
+out:
+ mutex_unlock(&dev_priv->psr.lock);
+}
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
` (4 preceding siblings ...)
2018-03-30 22:23 ` [PATCH 06/11] drm/i915/psr: Add intel_psr_activate_block_get()/put() José Roberto de Souza
@ 2018-03-30 22:23 ` José Roberto de Souza
2018-04-02 18:23 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 08/11] drm/i915: Keep IGT PSR and frontbuffer tests functional José Roberto de Souza
` (6 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
When PSR/PSR2 is enabled hardware can do aux ch transactions by it
self.
Spec requires that PSR is inactive before do any aux ch transaction
in HSW and BDW, for skl+ there is a aux ch mutex but the use is not
recommended.
So exiting PSR/PSR2 and waiting the transition to inactive to prevent
any concurrent access between hardware and software in aux ch
registers.
VLV and CHV hardware don't do any aux as software is responsible to
do all the buffer tracking and it sends the wake up aux ch message
to sink.
BSpec: 7530, 799 and 7532
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 38 ++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_psr.c | 8 ++++++++
3 files changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 62f82c4298ac..fedee4e7ed24 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1062,6 +1062,41 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
}
+/**
+ * intel_dp_aux_ch_get - Get total control of aux ch registers
+ *
+ * By exiting or disabling any hardware feature that can also use the aux ch
+ * registers at the same time as driver, this function will give total control
+ * of aux ch registers to driver.
+ */
+static void intel_dp_aux_ch_get(struct intel_dp *intel_dp)
+{
+ if (!intel_dp->exit_psr_on_aux_ch_xfer)
+ return;
+
+ intel_psr_activate_block_get(intel_dp);
+ intel_psr_exit(intel_dp, true);
+}
+
+/**
+ * intel_dp_aux_ch_put - Release aux ch registers control
+ *
+ * It is the intel_dp_aux_ch_get() counterpart.
+ */
+static void intel_dp_aux_ch_put(struct intel_dp *intel_dp)
+{
+ if (!intel_dp->exit_psr_on_aux_ch_xfer)
+ return;
+
+ intel_psr_activate_block_put(intel_dp);
+ /* Usually more than just one aux ch transaction is executed when
+ * handling some event, activating PSR right way would cause several
+ * msecs of delay waiting PSR to exit for each aux ch transaction, so
+ * here asking it to be scheduled.
+ */
+ intel_psr_activate(intel_dp, true);
+}
+
static int
intel_dp_aux_xfer(struct intel_dp *intel_dp,
const uint8_t *send, int send_bytes,
@@ -1101,6 +1136,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
intel_dp_check_edp(intel_dp);
+ intel_dp_aux_ch_get(intel_dp);
+
/* Try to wait for any previous AUX channel activity */
for (try = 0; try < 3; try++) {
status = I915_READ_NOTRACE(ch_ctl);
@@ -1223,6 +1260,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
ret = recv_bytes;
out:
+ intel_dp_aux_ch_put(intel_dp);
pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
if (vdd)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 020b96324135..177478f0b032 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1139,6 +1139,8 @@ struct intel_dp {
/* Displayport compliance testing */
struct intel_dp_compliance compliance;
+
+ bool exit_psr_on_aux_ch_xfer;
};
struct intel_lspcon {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 8702dbafb42d..f88f12246a23 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -665,6 +665,13 @@ void intel_psr_enable(struct intel_dp *intel_dp,
msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5));
}
+ /* From all platforms that supports PSR/PSR2 this 2 is the ones that
+ * don't have restrictions about use of the aux ch while PSR/PSR2 is
+ * enabled.
+ */
+ if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
+ intel_dp->exit_psr_on_aux_ch_xfer = true;
+
unlock:
mutex_unlock(&dev_priv->psr.lock);
}
@@ -732,6 +739,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
dev_priv->psr.disable_source(intel_dp);
+ intel_dp->exit_psr_on_aux_ch_xfer = false;
/* Disable PSR on Sink */
drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 08/11] drm/i915: Keep IGT PSR and frontbuffer tests functional
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
` (5 preceding siblings ...)
2018-03-30 22:23 ` [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction José Roberto de Souza
@ 2018-03-30 22:23 ` José Roberto de Souza
2018-04-02 18:26 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 09/11] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink José Roberto de Souza
` (5 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
'drm/i915/dp: Exit PSR before do a aux channel transaction' cause all
IGT PSR and frontbuffer tracking tests to not be userful.
Those tests depend in reading the calculated CRC value of the
frontbuffer in sink and doing a aux transaction now causes the PSR
to exit.
So any bug in software and hardware buffer tracking will be masked by
this forced PSR exit.
This is a dirty workaround that makes those tests functional again
at the risk of causing concurrent access in the aux ch registers
while running_igt_psr_tests is set.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 24 +++++++++++++++++++++++-
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++++
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1dba2c451255..519f67598d44 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4732,6 +4732,27 @@ static int i915_drrs_ctl_set(void *data, u64 val)
DEFINE_SIMPLE_ATTRIBUTE(i915_drrs_ctl_fops, NULL, i915_drrs_ctl_set, "%llu\n");
+static int i915_running_igt_psr_tests_get(void *data, u64 *val)
+{
+ struct drm_i915_private *dev_priv = data;
+
+ *val = dev_priv->running_igt_psr_tests;
+ return 0;
+}
+
+static int i915_running_igt_psr_tests_set(void *data, u64 val)
+{
+ struct drm_i915_private *dev_priv = data;
+
+ dev_priv->running_igt_psr_tests = !!val;
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_running_igt_psr_tests_fops,
+ i915_running_igt_psr_tests_get,
+ i915_running_igt_psr_tests_set,
+ "%llu\n");
+
static const struct drm_info_list i915_debugfs_list[] = {
{"i915_capabilities", i915_capabilities, 0},
{"i915_gem_objects", i915_gem_object_info, 0},
@@ -4812,7 +4833,8 @@ static const struct i915_debugfs_files {
{"i915_guc_log_relay", &i915_guc_log_relay_fops},
{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
{"i915_ipc_status", &i915_ipc_status_fops},
- {"i915_drrs_ctl", &i915_drrs_ctl_fops}
+ {"i915_drrs_ctl", &i915_drrs_ctl_fops},
+ {"i915_running_igt_psr_tests", &i915_running_igt_psr_tests_fops}
};
int i915_debugfs_register(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 41ebb144594e..9e0a5e29f48e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2110,6 +2110,8 @@ struct drm_i915_private {
struct i915_pmu pmu;
+ bool running_igt_psr_tests;
+
/*
* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
* will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fedee4e7ed24..c655f6c08a02 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1071,8 +1071,20 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
*/
static void intel_dp_aux_ch_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_dp->exit_psr_on_aux_ch_xfer)
return;
+ /*
+ * HACK: This is a workaround to keep IGT PSR and frontbuffer tracking
+ * tests functional, otherwise when IGT request the CRC of the
+ * frontbuffer to sink it would cause this function to complete execute
+ * and exit PSR.
+ */
+ if (dev_priv->running_igt_psr_tests)
+ return;
intel_psr_activate_block_get(intel_dp);
intel_psr_exit(intel_dp, true);
@@ -1085,8 +1097,14 @@ static void intel_dp_aux_ch_get(struct intel_dp *intel_dp)
*/
static void intel_dp_aux_ch_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_dp->exit_psr_on_aux_ch_xfer)
return;
+ if (dev_priv->running_igt_psr_tests)
+ return;
intel_psr_activate_block_put(intel_dp);
/* Usually more than just one aux ch transaction is executed when
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 09/11] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
` (6 preceding siblings ...)
2018-03-30 22:23 ` [PATCH 08/11] drm/i915: Keep IGT PSR and frontbuffer tests functional José Roberto de Souza
@ 2018-03-30 22:23 ` José Roberto de Souza
2018-04-02 18:28 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 10/11] drm/i915/psr: Handle PSR RFB storage error José Roberto de Souza
` (4 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
eDP spec states that sink device will do a short pulse in HPD
line when there is a PSR/PSR2 error that needs to be handled by
source, this is handling the first and most simples error:
DP_PSR_SINK_INTERNAL_ERROR.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 2 ++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_psr.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c655f6c08a02..fc8b86bc0120 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4518,6 +4518,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
if (intel_dp_needs_link_retrain(intel_dp))
return false;
+ intel_psr_hpd_short_pulse_handle(intel_dp);
+
if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
/* Send a Hotplug Uevent to userspace to start modeset */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 177478f0b032..d4c7e1e0fb86 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1897,6 +1897,7 @@ void intel_psr_exit(struct intel_dp *intel_dp, bool wait_idle);
void intel_psr_activate(struct intel_dp *intel_dp, bool schedule);
void intel_psr_activate_block_get(struct intel_dp *intel_dp);
void intel_psr_activate_block_put(struct intel_dp *intel_dp);
+void intel_psr_hpd_short_pulse_handle(struct intel_dp *intel_dp);
/* intel_runtime_pm.c */
int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index f88f12246a23..85d201fce287 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -1250,3 +1250,34 @@ void intel_psr_activate_block_put(struct intel_dp *intel_dp)
out:
mutex_unlock(&dev_priv->psr.lock);
}
+
+void intel_psr_hpd_short_pulse_handle(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct i915_psr *psr = &dev_priv->psr;
+ uint8_t val;
+
+ if (!HAS_PSR(dev_priv))
+ return;
+
+ mutex_lock(&psr->lock);
+ if (psr->enabled != intel_dp)
+ goto out;
+
+ if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) != 1) {
+ DRM_DEBUG_KMS("PSR_STATUS read failed\n");
+ goto dpcd_read_error;
+ }
+
+ if ((val & DP_PSR_SINK_STATE_MASK) == DP_PSR_SINK_INTERNAL_ERROR)
+ __intel_psr_exit(dev_priv);
+
+ /* TODO: handle other PSR/PSR2 errors */
+dpcd_read_error:
+ if (!dev_priv->psr.busy_frontbuffer_bits)
+ __intel_psr_activate(intel_dp);
+out:
+ mutex_unlock(&psr->lock);
+}
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 10/11] drm/i915/psr: Handle PSR RFB storage error
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
` (7 preceding siblings ...)
2018-03-30 22:23 ` [PATCH 09/11] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink José Roberto de Souza
@ 2018-03-30 22:23 ` José Roberto de Souza
2018-03-30 22:23 ` [PATCH 11/11] drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side José Roberto de Souza
` (3 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Sink will interrupt source when it have any problem saving or reading
the remote frame buffer.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_psr.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 85d201fce287..d76eece63ebd 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -1274,6 +1274,16 @@ void intel_psr_hpd_short_pulse_handle(struct intel_dp *intel_dp)
if ((val & DP_PSR_SINK_STATE_MASK) == DP_PSR_SINK_INTERNAL_ERROR)
__intel_psr_exit(dev_priv);
+ if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_ERROR_STATUS, &val) != 1) {
+ DRM_DEBUG_KMS("PSR_ERROR_STATUS read failed\n");
+ goto dpcd_read_error;
+ }
+
+ if (val & DP_PSR_RFB_STORAGE_ERROR)
+ __intel_psr_exit(dev_priv);
+ /* clear status register */
+ drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
+
/* TODO: handle other PSR/PSR2 errors */
dpcd_read_error:
if (!dev_priv->psr.busy_frontbuffer_bits)
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 11/11] drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
` (8 preceding siblings ...)
2018-03-30 22:23 ` [PATCH 10/11] drm/i915/psr: Handle PSR RFB storage error José Roberto de Souza
@ 2018-03-30 22:23 ` José Roberto de Souza
2018-03-30 22:38 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function Patchwork
` (2 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: José Roberto de Souza @ 2018-03-30 22:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Sink can be configured to calculate the CRC over the static frame and
compare with the CRC calculated and transmited in the VSC SDP by
source, if there is a mismatch sink will do a short pulse in HPD
and set DP_PSR_LINK_CRC_ERROR on DP_PSR_ERROR_STATUS.
Spec: 7723
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_psr.c | 9 +++++++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 176dca6554f4..3bd8f5edee87 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4001,6 +4001,7 @@ enum {
#define EDP_PSR_SKIP_AUX_EXIT (1<<12)
#define EDP_PSR_TP1_TP2_SEL (0<<11)
#define EDP_PSR_TP1_TP3_SEL (1<<11)
+#define EDP_PSR_CRC_ENABLE (1<<10) /* BDW+ */
#define EDP_PSR_TP2_TP3_TIME_500us (0<<8)
#define EDP_PSR_TP2_TP3_TIME_100us (1<<8)
#define EDP_PSR_TP2_TP3_TIME_2500us (2<<8)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index d76eece63ebd..1f4bf079dd39 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -292,6 +292,8 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
dpcd_val |= DP_PSR_ENABLE_PSR2;
if (dev_priv->psr.link_standby)
dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
+ if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
+ dpcd_val |= DP_PSR_CRC_VERIFICATION;
drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
@@ -379,6 +381,9 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp)
else
val |= EDP_PSR_TP1_TP2_SEL;
+ if (INTEL_GEN(dev_priv) >= 8)
+ val |= EDP_PSR_CRC_ENABLE;
+
val |= I915_READ(EDP_PSR_CTL) & EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK;
I915_WRITE(EDP_PSR_CTL, val);
}
@@ -1279,12 +1284,12 @@ void intel_psr_hpd_short_pulse_handle(struct intel_dp *intel_dp)
goto dpcd_read_error;
}
- if (val & DP_PSR_RFB_STORAGE_ERROR)
+ if (val & (DP_PSR_RFB_STORAGE_ERROR | DP_PSR_LINK_CRC_ERROR))
__intel_psr_exit(dev_priv);
/* clear status register */
drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
- /* TODO: handle other PSR/PSR2 errors */
+ /* TODO: handle PSR2 errors */
dpcd_read_error:
if (!dev_priv->psr.busy_frontbuffer_bits)
__intel_psr_activate(intel_dp);
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
` (9 preceding siblings ...)
2018-03-30 22:23 ` [PATCH 11/11] drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side José Roberto de Souza
@ 2018-03-30 22:38 ` Patchwork
2018-03-30 22:53 ` ✗ Fi.CI.BAT: " Patchwork
2018-04-02 18:06 ` [PATCH 01/11] " Rodrigo Vivi
12 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2018-03-30 22:38 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function
URL : https://patchwork.freedesktop.org/series/40978/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
499cc3f929a4 drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function
5f0bc3f1c19d drm/i915/psr: Move PSR exit specific code to hardware specific function
00837d74661e drm/i915/psr: Share code between disable and exit
384712a1fb4a drm/i915/psr: Remove intel_crtc_state parameter from disable()
af1fc4a61d32 drm/i915/psr: Export intel_psr_activate/exit()
-:177: CHECK:BRACES: braces {} should be used on all arms of this statement
#177: FILE: drivers/gpu/drm/i915/intel_psr.c:1182:
+ if (schedule) {
[...]
+ } else
[...]
-:181: CHECK:BRACES: Unbalanced braces around else statement
#181: FILE: drivers/gpu/drm/i915/intel_psr.c:1186:
+ } else
total: 0 errors, 0 warnings, 2 checks, 153 lines checked
6d6bc90eccc3 drm/i915/psr: Add intel_psr_activate_block_get()/put()
-:88: CHECK:LINE_SPACING: Please don't use multiple blank lines
#88: FILE: drivers/gpu/drm/i915/intel_psr.c:1219:
+
+
total: 0 errors, 0 warnings, 1 checks, 78 lines checked
f42690dc389a drm/i915/dp: Exit PSR before do a aux channel transaction
ac18ae2ade2c drm/i915: Keep IGT PSR and frontbuffer tests functional
191d9682d215 drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink
164ed46e42e1 drm/i915/psr: Handle PSR RFB storage error
67745738f4d0 drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side
-:29: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#29: FILE: drivers/gpu/drm/i915/i915_reg.h:4004:
+#define EDP_PSR_CRC_ENABLE (1<<10) /* BDW+ */
^
total: 0 errors, 0 warnings, 1 checks, 38 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
` (10 preceding siblings ...)
2018-03-30 22:38 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function Patchwork
@ 2018-03-30 22:53 ` Patchwork
2018-04-02 18:06 ` [PATCH 01/11] " Rodrigo Vivi
12 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2018-03-30 22:53 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function
URL : https://patchwork.freedesktop.org/series/40978/
State : warning
== Summary ==
Series 40978v1 series starting with [01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function
https://patchwork.freedesktop.org/api/1.0/series/40978/revisions/1/mbox/
---- Possible new issues:
Test kms_busy:
Subgroup basic-flip-a:
pass -> DMESG-WARN (fi-skl-6700k2)
pass -> DMESG-WARN (fi-skl-guc)
Subgroup basic-flip-c:
pass -> DMESG-WARN (fi-kbl-7567u)
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:431s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:442s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:381s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:544s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:295s
fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:515s
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:506s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:517s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:508s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:410s
fi-cfl-s3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:564s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:513s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:588s
fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:418s
fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:319s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:539s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:403s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:419s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:467s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:428s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:464s
fi-kbl-7567u total:285 pass:264 dwarn:1 dfail:0 fail:0 skip:20 time:457s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:509s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:660s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:437s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:531s
fi-skl-6700k2 total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:508s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:477s
fi-skl-guc total:285 pass:256 dwarn:1 dfail:0 fail:0 skip:28 time:428s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:446s
fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:565s
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:407s
Blacklisted hosts:
fi-cnl-psr total:285 pass:256 dwarn:3 dfail:0 fail:0 skip:26 time:505s
fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:487s
c46052cde6a50c5459e00791ffc4d5aa1ec58a9e drm-tip: 2018y-03m-30d-18h-56m-26s UTC integration manifest
67745738f4d0 drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side
164ed46e42e1 drm/i915/psr: Handle PSR RFB storage error
191d9682d215 drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink
ac18ae2ade2c drm/i915: Keep IGT PSR and frontbuffer tests functional
f42690dc389a drm/i915/dp: Exit PSR before do a aux channel transaction
6d6bc90eccc3 drm/i915/psr: Add intel_psr_activate_block_get()/put()
af1fc4a61d32 drm/i915/psr: Export intel_psr_activate/exit()
384712a1fb4a drm/i915/psr: Remove intel_crtc_state parameter from disable()
00837d74661e drm/i915/psr: Share code between disable and exit
5f0bc3f1c19d drm/i915/psr: Move PSR exit specific code to hardware specific function
499cc3f929a4 drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8554/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
` (11 preceding siblings ...)
2018-03-30 22:53 ` ✗ Fi.CI.BAT: " Patchwork
@ 2018-04-02 18:06 ` Rodrigo Vivi
12 siblings, 0 replies; 31+ messages in thread
From: Rodrigo Vivi @ 2018-04-02 18:06 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Mar 30, 2018 at 03:23:26PM -0700, José Roberto de Souza wrote:
> It was reading some random register in VLV and CHV.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
I'm sure that my patch [1] will conflict with this series.
[1] https://patchwork.freedesktop.org/series/39650/
But the approach here is right and I couldn't get back there
yet and address latest DK's requests. So:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 2d53f7398a6d..fc7c36efd401 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -424,6 +424,11 @@ static void hsw_psr_activate(struct intel_dp *intel_dp)
> struct drm_device *dev = dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> + if (dev_priv->psr.psr2_enabled)
> + WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> + else
> + WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> +
> /* On HSW+ after we enable PSR on source it will activate it
> * as soon as it match configure idle_frame count. So
> * we just actually enable it here on activation time.
> @@ -549,10 +554,6 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> - if (dev_priv->psr.psr2_enabled)
> - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> - else
> - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> WARN_ON(dev_priv->psr.active);
> lockdep_assert_held(&dev_priv->psr.lock);
>
> --
> 2.16.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 02/11] drm/i915/psr: Move PSR exit specific code to hardware specific function
2018-03-30 22:23 ` [PATCH 02/11] drm/i915/psr: Move PSR exit specific code to hardware specific function José Roberto de Souza
@ 2018-04-02 18:09 ` Rodrigo Vivi
0 siblings, 0 replies; 31+ messages in thread
From: Rodrigo Vivi @ 2018-04-02 18:09 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Mar 30, 2018 at 03:23:27PM -0700, José Roberto de Souza wrote:
> To proper execute PSR exit it was using 'if (HAS_DDI(dev_priv))' to
> differentiate between VLV/CHV and HSW+ hardware, so here moving each
> hardware handling to his own function.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_psr.c | 94 +++++++++++++++++++++++-----------------
> 2 files changed, 56 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5373b171bb96..a8d300280a2c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -617,6 +617,7 @@ struct i915_psr {
> void (*enable_sink)(struct intel_dp *);
> void (*activate)(struct intel_dp *);
> void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
> + void (*exit)(struct intel_dp *intel_dp);
> };
>
> enum intel_pch {
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index fc7c36efd401..bcaac9e69f8c 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -838,53 +838,67 @@ static void intel_psr_work(struct work_struct *work)
> mutex_unlock(&dev_priv->psr.lock);
> }
>
> -static void intel_psr_exit(struct drm_i915_private *dev_priv)
> +static void hsw_psr_exit(struct intel_dp *intel_dp)
> {
> - struct intel_dp *intel_dp = dev_priv->psr.enabled;
> - struct drm_crtc *crtc = dp_to_dig_port(intel_dp)->base.base.crtc;
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + u32 val;
> +
> + if (dev_priv->psr.psr2_enabled) {
> + val = I915_READ(EDP_PSR2_CTL);
> + WARN_ON(!(val & EDP_PSR2_ENABLE));
> + I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
> + } else {
> + val = I915_READ(EDP_PSR_CTL);
> + WARN_ON(!(val & EDP_PSR_ENABLE));
> + I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
> + }
> +}
> +
> +static void vlv_psr_exit(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_crtc *crtc = dig_port->base.base.crtc;
> enum pipe pipe = to_intel_crtc(crtc)->pipe;
> u32 val;
>
> - if (!dev_priv->psr.active)
> - return;
> + val = I915_READ(VLV_PSRCTL(pipe));
>
> - if (HAS_DDI(dev_priv)) {
> - if (dev_priv->psr.psr2_enabled) {
> - val = I915_READ(EDP_PSR2_CTL);
> - WARN_ON(!(val & EDP_PSR2_ENABLE));
> - I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
> - } else {
> - val = I915_READ(EDP_PSR_CTL);
> - WARN_ON(!(val & EDP_PSR_ENABLE));
> - I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
> - }
> - } else {
> - val = I915_READ(VLV_PSRCTL(pipe));
> + /*
> + * Here we do the transition drirectly from
> + * PSR_state 3 (active - no Remote Frame Buffer (RFB) update) to
> + * PSR_state 5 (exit).
> + * PSR State 4 (active with single frame update) can be skipped.
> + * On PSR_state 5 (exit) Hardware is responsible to transition
> + * back to PSR_state 1 (inactive).
> + * Now we are at Same state after vlv_psr_enable_source.
> + */
> + val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> + I915_WRITE(VLV_PSRCTL(pipe), val);
>
> - /*
> - * Here we do the transition drirectly from
> - * PSR_state 3 (active - no Remote Frame Buffer (RFB) update) to
> - * PSR_state 5 (exit).
> - * PSR State 4 (active with single frame update) can be skipped.
> - * On PSR_state 5 (exit) Hardware is responsible to transition
> - * back to PSR_state 1 (inactive).
> - * Now we are at Same state after vlv_psr_enable_source.
> - */
> - val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> - I915_WRITE(VLV_PSRCTL(pipe), val);
> + /*
> + * Send AUX wake up - Spec says after transitioning to PSR
> + * active we have to send AUX wake up by writing 01h in DPCD
> + * 600h of sink device.
> + * XXX: This might slow down the transition, but without this
> + * HW doesn't complete the transition to PSR_state 1 and we
> + * never get the screen updated.
> + */
> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> + DP_SET_POWER_D0);
> +}
>
> - /*
> - * Send AUX wake up - Spec says after transitioning to PSR
> - * active we have to send AUX wake up by writing 01h in DPCD
> - * 600h of sink device.
> - * XXX: This might slow down the transition, but without this
> - * HW doesn't complete the transition to PSR_state 1 and we
> - * never get the screen updated.
> - */
> - drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> - DP_SET_POWER_D0);
> - }
> +static void intel_psr_exit(struct drm_i915_private *dev_priv)
> +{
> + struct intel_dp *intel_dp = dev_priv->psr.enabled;
> +
> + if (!dev_priv->psr.active)
> + return;
>
> + dev_priv->psr.exit(intel_dp);
> dev_priv->psr.active = false;
> }
>
> @@ -1094,6 +1108,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
> dev_priv->psr.enable_sink = vlv_psr_enable_sink;
> dev_priv->psr.activate = vlv_psr_activate;
> dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
> + dev_priv->psr.exit = vlv_psr_exit;
> } else {
> dev_priv->psr.has_hw_tracking = true;
> dev_priv->psr.enable_source = hsw_psr_enable_source;
> @@ -1101,5 +1116,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
> dev_priv->psr.enable_sink = hsw_psr_enable_sink;
> dev_priv->psr.activate = hsw_psr_activate;
> dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
> + dev_priv->psr.exit = hsw_psr_exit;
> }
> }
> --
> 2.16.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 03/11] drm/i915/psr: Share code between disable and exit
2018-03-30 22:23 ` [PATCH 03/11] drm/i915/psr: Share code between disable and exit José Roberto de Souza
@ 2018-04-02 18:11 ` Rodrigo Vivi
2018-04-02 21:15 ` Souza, Jose
0 siblings, 1 reply; 31+ messages in thread
From: Rodrigo Vivi @ 2018-04-02 18:11 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Mar 30, 2018 at 03:23:28PM -0700, José Roberto de Souza wrote:
> The disable and exit sequence are very similar with a lot common
> code between both, so here sharing the code.
I don't believe that we should do this.
Disable as is has some extra wait/timeouts that could slow things down.
Or at least not do this while we don't have a proper robuts test in place
and to test also performance impacts...
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_psr.c | 84 ++++++++++++++++++----------------------
> 2 files changed, 38 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a8d300280a2c..cb72ee27422f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -617,7 +617,7 @@ struct i915_psr {
> void (*enable_sink)(struct intel_dp *);
> void (*activate)(struct intel_dp *);
> void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
> - void (*exit)(struct intel_dp *intel_dp);
> + void (*exit)(struct intel_dp *intel_dp, bool disabling);
> };
>
> enum intel_pch {
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index bcaac9e69f8c..d3451afeb8bb 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -672,23 +672,9 @@ static void vlv_psr_disable(struct intel_dp *intel_dp,
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> - uint32_t val;
>
> if (dev_priv->psr.active) {
> - /* Put VLV PSR back to PSR_state 0 (disabled). */
> - if (intel_wait_for_register(dev_priv,
> - VLV_PSRSTAT(crtc->pipe),
> - VLV_EDP_PSR_IN_TRANS,
> - 0,
> - 1))
> - WARN(1, "PSR transition took longer than expected\n");
> -
> - val = I915_READ(VLV_PSRCTL(crtc->pipe));
> - val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> - val &= ~VLV_EDP_PSR_ENABLE;
> - val &= ~VLV_EDP_PSR_MODE_MASK;
> - I915_WRITE(VLV_PSRCTL(crtc->pipe), val);
> -
> + dev_priv->psr.exit(intel_dp, true);
> dev_priv->psr.active = false;
> } else {
> WARN_ON(vlv_is_psr_active_on_pipe(dev, crtc->pipe));
> @@ -703,31 +689,7 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> if (dev_priv->psr.active) {
> - i915_reg_t psr_status;
> - u32 psr_status_mask;
> -
> - if (dev_priv->psr.psr2_enabled) {
> - psr_status = EDP_PSR2_STATUS;
> - psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> -
> - I915_WRITE(EDP_PSR2_CTL,
> - I915_READ(EDP_PSR2_CTL) &
> - ~(EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE));
> -
> - } else {
> - psr_status = EDP_PSR_STATUS;
> - psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> -
> - I915_WRITE(EDP_PSR_CTL,
> - I915_READ(EDP_PSR_CTL) & ~EDP_PSR_ENABLE);
> - }
> -
> - /* Wait till PSR is idle */
> - if (intel_wait_for_register(dev_priv,
> - psr_status, psr_status_mask, 0,
> - 2000))
> - DRM_ERROR("Timed out waiting for PSR Idle State\n");
> -
> + dev_priv->psr.exit(intel_dp, true);
> dev_priv->psr.active = false;
> } else {
> if (dev_priv->psr.psr2_enabled)
> @@ -838,25 +800,41 @@ static void intel_psr_work(struct work_struct *work)
> mutex_unlock(&dev_priv->psr.lock);
> }
>
> -static void hsw_psr_exit(struct intel_dp *intel_dp)
> +static void hsw_psr_exit(struct intel_dp *intel_dp, bool disabling)
> {
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> + i915_reg_t psr_status;
> + u32 psr_status_mask;
> u32 val;
>
> if (dev_priv->psr.psr2_enabled) {
> + psr_status = EDP_PSR2_STATUS;
> + psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> +
> val = I915_READ(EDP_PSR2_CTL);
> - WARN_ON(!(val & EDP_PSR2_ENABLE));
> - I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
> + WARN_ON(!disabling && !(val & EDP_PSR2_ENABLE));
> + val &= ~EDP_PSR2_ENABLE;
> + if (disabling)
> + val &= ~EDP_SU_TRACK_ENABLE;
> + I915_WRITE(EDP_PSR2_CTL, val);
> } else {
> + psr_status = EDP_PSR_STATUS;
> + psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> +
> val = I915_READ(EDP_PSR_CTL);
> - WARN_ON(!(val & EDP_PSR_ENABLE));
> + WARN_ON(!disabling && !(val & EDP_PSR_ENABLE));
> I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
> }
> +
> + /* When disabling wait till PSR is idle */
> + if (disabling && intel_wait_for_register(dev_priv, psr_status,
> + psr_status_mask, 0, 2000))
> + DRM_ERROR("Timed out waiting for PSR Idle State\n");
> }
>
> -static void vlv_psr_exit(struct intel_dp *intel_dp)
> +static void vlv_psr_exit(struct intel_dp *intel_dp, bool disabling)
> {
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = dig_port->base.base.dev;
> @@ -865,10 +843,14 @@ static void vlv_psr_exit(struct intel_dp *intel_dp)
> enum pipe pipe = to_intel_crtc(crtc)->pipe;
> u32 val;
>
> + if (disabling && intel_wait_for_register(dev_priv, VLV_PSRSTAT(pipe),
> + VLV_EDP_PSR_IN_TRANS, 0, 1))
> + DRM_WARN("PSR transition took longer than expected\n");
> +
> val = I915_READ(VLV_PSRCTL(pipe));
>
> /*
> - * Here we do the transition drirectly from
> + * Here we do the transition directly from
> * PSR_state 3 (active - no Remote Frame Buffer (RFB) update) to
> * PSR_state 5 (exit).
> * PSR State 4 (active with single frame update) can be skipped.
> @@ -877,8 +859,16 @@ static void vlv_psr_exit(struct intel_dp *intel_dp)
> * Now we are at Same state after vlv_psr_enable_source.
> */
> val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> + if (disabling) {
> + /* Put VLV PSR back to PSR_state 0 (disabled). */
> + val &= ~VLV_EDP_PSR_ENABLE;
> + val &= ~VLV_EDP_PSR_MODE_MASK;
> + }
> I915_WRITE(VLV_PSRCTL(pipe), val);
>
> + if (disabling)
> + return;
> +
> /*
> * Send AUX wake up - Spec says after transitioning to PSR
> * active we have to send AUX wake up by writing 01h in DPCD
> @@ -898,7 +888,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
> if (!dev_priv->psr.active)
> return;
>
> - dev_priv->psr.exit(intel_dp);
> + dev_priv->psr.exit(intel_dp, false);
> dev_priv->psr.active = false;
> }
>
> --
> 2.16.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 04/11] drm/i915/psr: Remove intel_crtc_state parameter from disable()
2018-03-30 22:23 ` [PATCH 04/11] drm/i915/psr: Remove intel_crtc_state parameter from disable() José Roberto de Souza
@ 2018-04-02 18:13 ` Rodrigo Vivi
2018-04-03 17:52 ` Ville Syrjälä
1 sibling, 0 replies; 31+ messages in thread
From: Rodrigo Vivi @ 2018-04-02 18:13 UTC (permalink / raw)
To: José Roberto de Souza
Cc: intel-gfx, Lucas De Marchi, Dhinakaran Pandiyan
On Fri, Mar 30, 2018 at 03:23:29PM -0700, José Roberto de Souza wrote:
> It is not necessary as is possible to get the pipe information
> from intel_dp.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Lucas
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +--
> drivers/gpu/drm/i915/intel_psr.c | 13 ++++++-------
> 2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cb72ee27422f..99af9169d792 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -612,8 +612,7 @@ struct i915_psr {
>
> void (*enable_source)(struct intel_dp *,
> const struct intel_crtc_state *);
> - void (*disable_source)(struct intel_dp *,
> - const struct intel_crtc_state *);
> + void (*disable_source)(struct intel_dp *intel_dp);
> void (*enable_sink)(struct intel_dp *);
> void (*activate)(struct intel_dp *);
> void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index d3451afeb8bb..c4720b0152c3 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -665,24 +665,23 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> mutex_unlock(&dev_priv->psr.lock);
> }
>
> -static void vlv_psr_disable(struct intel_dp *intel_dp,
> - const struct intel_crtc_state *old_crtc_state)
> +static void vlv_psr_disable(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> + struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
Lucas, is this the one you noticed we are using wrongly?
> + enum pipe pipe = to_intel_crtc(crtc)->pipe;
>
> if (dev_priv->psr.active) {
> dev_priv->psr.exit(intel_dp, true);
> dev_priv->psr.active = false;
> } else {
> - WARN_ON(vlv_is_psr_active_on_pipe(dev, crtc->pipe));
> + WARN_ON(vlv_is_psr_active_on_pipe(dev, pipe));
> }
> }
>
> -static void hsw_psr_disable(struct intel_dp *intel_dp,
> - const struct intel_crtc_state *old_crtc_state)
> +static void hsw_psr_disable(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = intel_dig_port->base.base.dev;
> @@ -727,7 +726,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> return;
> }
>
> - dev_priv->psr.disable_source(intel_dp, old_crtc_state);
> + dev_priv->psr.disable_source(intel_dp);
>
> /* Disable PSR on Sink */
> drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> --
> 2.16.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 05/11] drm/i915/psr: Export intel_psr_activate/exit()
2018-03-30 22:23 ` [PATCH 05/11] drm/i915/psr: Export intel_psr_activate/exit() José Roberto de Souza
@ 2018-04-02 18:18 ` Rodrigo Vivi
0 siblings, 0 replies; 31+ messages in thread
From: Rodrigo Vivi @ 2018-04-02 18:18 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Mar 30, 2018 at 03:23:30PM -0700, José Roberto de Souza wrote:
> Export this functions so other modules can activate and exit PSR.
"module" is not a right word here.
But I also don't like the approach. In the past we had the use spread
on our driver and it was confusing and hard to deal with more corner
cases. frontbuffer tracking was created to fix that.
I understand where you are going because we need to disable it on aux
transactions, etc. But I don't believe the right way is to exporting
active/exit.
But to create one unified infrastructure to block and release psr
with atomic counters.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> drivers/gpu/drm/i915/intel_psr.c | 94 +++++++++++++++++++++++++++++++++++++---
> 2 files changed, 89 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d1452fd2a58d..70026b772721 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1891,6 +1891,8 @@ void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
> unsigned frontbuffer_bits);
> void intel_psr_compute_config(struct intel_dp *intel_dp,
> struct intel_crtc_state *crtc_state);
> +void intel_psr_exit(struct intel_dp *intel_dp, bool wait_idle);
> +void intel_psr_activate(struct intel_dp *intel_dp, bool schedule);
>
> /* intel_runtime_pm.c */
> int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index c4720b0152c3..906a12ea934d 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -56,6 +56,8 @@
> #include "intel_drv.h"
> #include "i915_drv.h"
>
> +#define ACTIVATE_WORK_MSEC_DELAY 100
> +
> static inline enum intel_display_power_domain
> psr_aux_domain(struct intel_dp *intel_dp)
> {
> @@ -548,7 +550,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
> }
>
> -static void intel_psr_activate(struct intel_dp *intel_dp)
> +static void __intel_psr_activate(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = intel_dig_port->base.base.dev;
> @@ -645,7 +647,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> dev_priv->psr.enabled = intel_dp;
>
> if (INTEL_GEN(dev_priv) >= 9) {
> - intel_psr_activate(intel_dp);
> + __intel_psr_activate(intel_dp);
> } else {
> /*
> * FIXME: Activation should happen immediately since this
> @@ -794,7 +796,7 @@ static void intel_psr_work(struct work_struct *work)
> if (dev_priv->psr.busy_frontbuffer_bits)
> goto unlock;
>
> - intel_psr_activate(intel_dp);
> + __intel_psr_activate(intel_dp);
> unlock:
> mutex_unlock(&dev_priv->psr.lock);
> }
> @@ -880,7 +882,7 @@ static void vlv_psr_exit(struct intel_dp *intel_dp, bool disabling)
> DP_SET_POWER_D0);
> }
>
> -static void intel_psr_exit(struct drm_i915_private *dev_priv)
> +static void __intel_psr_exit(struct drm_i915_private *dev_priv)
> {
> struct intel_dp *intel_dp = dev_priv->psr.enabled;
>
> @@ -977,7 +979,7 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
> dev_priv->psr.busy_frontbuffer_bits |= frontbuffer_bits;
>
> if (frontbuffer_bits)
> - intel_psr_exit(dev_priv);
> + __intel_psr_exit(dev_priv);
>
> mutex_unlock(&dev_priv->psr.lock);
> }
> @@ -1023,7 +1025,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> if (frontbuffer_bits) {
> if (dev_priv->psr.psr2_enabled ||
> IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> - intel_psr_exit(dev_priv);
> + __intel_psr_exit(dev_priv);
> } else {
> /*
> * Display WA #0884: all
> @@ -1041,7 +1043,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
> if (!work_busy(&dev_priv->psr.work.work))
> schedule_delayed_work(&dev_priv->psr.work,
> - msecs_to_jiffies(100));
> + msecs_to_jiffies(ACTIVATE_WORK_MSEC_DELAY));
> mutex_unlock(&dev_priv->psr.lock);
> }
>
> @@ -1108,3 +1110,81 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
> dev_priv->psr.exit = hsw_psr_exit;
> }
> }
> +
> +/**
> + * intel_psr_exit - Exit PSR
> + * @intel_dp: DisplayPort that should have PSR exited if active
> + * @wait_idle: if true wait until PSR is inactive otherwise just request it
> + *
> + * This function exit PSR if enabled and active in this DisplayPort.
> + */
> +void intel_psr_exit(struct intel_dp *intel_dp, bool wait_idle)
> +{
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (!CAN_PSR(dev_priv))
> + return;
> +
> + mutex_lock(&dev_priv->psr.lock);
> + if (dev_priv->psr.enabled != intel_dp)
> + goto out;
> + if (!dev_priv->psr.active && !wait_idle)
> + goto out;
> +
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> + struct drm_crtc *crtc = dig_port->base.base.crtc;
> + enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +
> + __intel_psr_exit(dev_priv);
> +
> + if (!wait_idle)
> + goto out;
> +
> + if (intel_wait_for_register(dev_priv, VLV_PSRSTAT(pipe),
> + VLV_EDP_PSR_IN_TRANS, 0, 2000))
> + DRM_ERROR("Timed out waiting for PSR Idle State\n");
> + } else {
> + /* HSW+ PSR disable is actually, exit + wait transition */
> + dev_priv->psr.exit(intel_dp, wait_idle);
> + dev_priv->psr.active = false;
> + }
> +
> +out:
> + mutex_unlock(&dev_priv->psr.lock);
> +}
> +
> +/**
> + * intel_psr_activate - Activate PSR
> + * @intel_dp: DisplayPort that should have PSR activated if enabled
> + * @schedule: if true schedule the active to happens in a few milliseconds
> + * otherwise activate right now.
> + *
> + * This function activate PSR if enabled in this DisplayPort.
> + */
> +void intel_psr_activate(struct intel_dp *intel_dp, bool schedule)
> +{
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (!CAN_PSR(dev_priv))
> + return;
> +
> + mutex_lock(&dev_priv->psr.lock);
> + if (dev_priv->psr.enabled != intel_dp || dev_priv->psr.active)
> + goto out;
> +
> + if (dev_priv->psr.busy_frontbuffer_bits)
> + goto out;
> +
> + if (schedule) {
> + if (!work_busy(&dev_priv->psr.work.work))
> + schedule_delayed_work(&dev_priv->psr.work,
> + msecs_to_jiffies(ACTIVATE_WORK_MSEC_DELAY));
> + } else
> + __intel_psr_activate(intel_dp);
> +out:
> + mutex_unlock(&dev_priv->psr.lock);
> +}
> --
> 2.16.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 06/11] drm/i915/psr: Add intel_psr_activate_block_get()/put()
2018-03-30 22:23 ` [PATCH 06/11] drm/i915/psr: Add intel_psr_activate_block_get()/put() José Roberto de Souza
@ 2018-04-02 18:20 ` Rodrigo Vivi
2018-04-02 22:11 ` Souza, Jose
0 siblings, 1 reply; 31+ messages in thread
From: Rodrigo Vivi @ 2018-04-02 18:20 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Mar 30, 2018 at 03:23:31PM -0700, José Roberto de Souza wrote:
> intel_psr_activate_block_get() should be called when by some reason
> PSR should not be activated for some time, it will increment counter
> and it should the decremented by intel_psr_activate_block_put()
> when PSR can be activated again.
> intel_psr_activate_block_put() will not actually activate PSR, users
> of this function should also call intel_psr_activate().
Ohh cool! you made the counter.
probably we will need to change things from mutex to spin locker.
But also the blocker functions here could already introduce the function
calls to really block and release psr.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_psr.c | 54 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 57 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 99af9169d792..41ebb144594e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -609,6 +609,7 @@ struct i915_psr {
> bool has_hw_tracking;
> bool psr2_enabled;
> u8 sink_sync_latency;
> + unsigned int activate_block_count;
>
> void (*enable_source)(struct intel_dp *,
> const struct intel_crtc_state *);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 70026b772721..020b96324135 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1893,6 +1893,8 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> struct intel_crtc_state *crtc_state);
> void intel_psr_exit(struct intel_dp *intel_dp, bool wait_idle);
> void intel_psr_activate(struct intel_dp *intel_dp, bool schedule);
> +void intel_psr_activate_block_get(struct intel_dp *intel_dp);
> +void intel_psr_activate_block_put(struct intel_dp *intel_dp);
>
> /* intel_runtime_pm.c */
> int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 906a12ea934d..8702dbafb42d 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -558,6 +558,8 @@ static void __intel_psr_activate(struct intel_dp *intel_dp)
>
> WARN_ON(dev_priv->psr.active);
> lockdep_assert_held(&dev_priv->psr.lock);
> + if (dev_priv->psr.activate_block_count)
> + return;
>
> dev_priv->psr.activate(intel_dp);
> dev_priv->psr.active = true;
> @@ -1188,3 +1190,55 @@ void intel_psr_activate(struct intel_dp *intel_dp, bool schedule)
> out:
> mutex_unlock(&dev_priv->psr.lock);
> }
> +
> +/**
> + * intel_psr_activate_block_get - Block further attempts to activate PSR
> + * @intel_dp: DisplayPort that have PSR enabled
> + *
> + * It have a internal reference count, so each intel_psr_activate_block_get()
> + * should have a intel_psr_activate_block_put() counterpart.
> + */
> +void intel_psr_activate_block_get(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (!CAN_PSR(dev_priv))
> + return;
> +
> + mutex_lock(&dev_priv->psr.lock);
> + if (dev_priv->psr.enabled != intel_dp)
> + goto out;
> +
> + dev_priv->psr.activate_block_count++;
> +out:
> + mutex_unlock(&dev_priv->psr.lock);
> +}
> +
> +
> +/**
> + * intel_psr_activate_block_put - Unblock further attempts to activate PSR
> + * @intel_dp: DisplayPort that have PSR enabled
> + *
> + * Decrease the reference counter incremented by intel_psr_activate_block_get()
> + * when zero it allows PSR be activate. To actually activate PSR when reference
> + * counter is zero intel_psr_activate() should be called.
> + */
> +void intel_psr_activate_block_put(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (!CAN_PSR(dev_priv))
> + return;
> +
> + mutex_lock(&dev_priv->psr.lock);
> + if (dev_priv->psr.enabled != intel_dp)
> + goto out;
> +
> + dev_priv->psr.activate_block_count--;
> +out:
> + mutex_unlock(&dev_priv->psr.lock);
> +}
> --
> 2.16.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction
2018-03-30 22:23 ` [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction José Roberto de Souza
@ 2018-04-02 18:23 ` Rodrigo Vivi
2018-04-02 18:42 ` Pandiyan, Dhinakaran
2018-04-02 19:00 ` Pandiyan, Dhinakaran
0 siblings, 2 replies; 31+ messages in thread
From: Rodrigo Vivi @ 2018-04-02 18:23 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Mar 30, 2018 at 03:23:32PM -0700, José Roberto de Souza wrote:
> When PSR/PSR2 is enabled hardware can do aux ch transactions by it
> self.
> Spec requires that PSR is inactive before do any aux ch transaction
> in HSW and BDW, for skl+ there is a aux ch mutex but the use is not
> recommended.
> So exiting PSR/PSR2 and waiting the transition to inactive to prevent
> any concurrent access between hardware and software in aux ch
> registers.
>
> VLV and CHV hardware don't do any aux as software is responsible to
> do all the buffer tracking and it sends the wake up aux ch message
> to sink.
>
ahh cool... I get back what I said on some previous patch.
I like where it is going, but...
> BSpec: 7530, 799 and 7532
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 38 ++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_psr.c | 8 ++++++++
> 3 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 62f82c4298ac..fedee4e7ed24 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1062,6 +1062,41 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> }
>
> +/**
> + * intel_dp_aux_ch_get - Get total control of aux ch registers
> + *
> + * By exiting or disabling any hardware feature that can also use the aux ch
> + * registers at the same time as driver, this function will give total control
> + * of aux ch registers to driver.
> + */
> +static void intel_dp_aux_ch_get(struct intel_dp *intel_dp)
> +{
> + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> + return;
> +
> + intel_psr_activate_block_get(intel_dp);
> + intel_psr_exit(intel_dp, true);
decision on exit and activate should be inside the block_get and block_put,
based on current state of the counters.
> +}
> +
> +/**
> + * intel_dp_aux_ch_put - Release aux ch registers control
> + *
> + * It is the intel_dp_aux_ch_get() counterpart.
> + */
> +static void intel_dp_aux_ch_put(struct intel_dp *intel_dp)
> +{
> + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> + return;
> +
> + intel_psr_activate_block_put(intel_dp);
> + /* Usually more than just one aux ch transaction is executed when
> + * handling some event, activating PSR right way would cause several
> + * msecs of delay waiting PSR to exit for each aux ch transaction, so
> + * here asking it to be scheduled.
> + */
> + intel_psr_activate(intel_dp, true);
> +}
> +
> static int
> intel_dp_aux_xfer(struct intel_dp *intel_dp,
> const uint8_t *send, int send_bytes,
> @@ -1101,6 +1136,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>
> intel_dp_check_edp(intel_dp);
>
> + intel_dp_aux_ch_get(intel_dp);
> +
> /* Try to wait for any previous AUX channel activity */
> for (try = 0; try < 3; try++) {
> status = I915_READ_NOTRACE(ch_ctl);
> @@ -1223,6 +1260,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>
> ret = recv_bytes;
> out:
> + intel_dp_aux_ch_put(intel_dp);
> pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
>
> if (vdd)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 020b96324135..177478f0b032 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1139,6 +1139,8 @@ struct intel_dp {
>
> /* Displayport compliance testing */
> struct intel_dp_compliance compliance;
> +
> + bool exit_psr_on_aux_ch_xfer;
> };
>
> struct intel_lspcon {
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 8702dbafb42d..f88f12246a23 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -665,6 +665,13 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5));
> }
>
> + /* From all platforms that supports PSR/PSR2 this 2 is the ones that
> + * don't have restrictions about use of the aux ch while PSR/PSR2 is
> + * enabled.
> + */
> + if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
> + intel_dp->exit_psr_on_aux_ch_xfer = true;
> +
> unlock:
> mutex_unlock(&dev_priv->psr.lock);
> }
> @@ -732,6 +739,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
>
> dev_priv->psr.disable_source(intel_dp);
>
> + intel_dp->exit_psr_on_aux_ch_xfer = false;
> /* Disable PSR on Sink */
> drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
>
> --
> 2.16.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 08/11] drm/i915: Keep IGT PSR and frontbuffer tests functional
2018-03-30 22:23 ` [PATCH 08/11] drm/i915: Keep IGT PSR and frontbuffer tests functional José Roberto de Souza
@ 2018-04-02 18:26 ` Rodrigo Vivi
0 siblings, 0 replies; 31+ messages in thread
From: Rodrigo Vivi @ 2018-04-02 18:26 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Mar 30, 2018 at 03:23:33PM -0700, José Roberto de Souza wrote:
> 'drm/i915/dp: Exit PSR before do a aux channel transaction' cause all
> IGT PSR and frontbuffer tracking tests to not be userful.
> Those tests depend in reading the calculated CRC value of the
> frontbuffer in sink and doing a aux transaction now causes the PSR
> to exit.
> So any bug in software and hardware buffer tracking will be masked by
> this forced PSR exit.
>
> This is a dirty workaround that makes those tests functional again
> at the risk of causing concurrent access in the aux ch registers
> while running_igt_psr_tests is set.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 24 +++++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++++
> 3 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1dba2c451255..519f67598d44 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4732,6 +4732,27 @@ static int i915_drrs_ctl_set(void *data, u64 val)
>
> DEFINE_SIMPLE_ATTRIBUTE(i915_drrs_ctl_fops, NULL, i915_drrs_ctl_set, "%llu\n");
>
> +static int i915_running_igt_psr_tests_get(void *data, u64 *val)
> +{
> + struct drm_i915_private *dev_priv = data;
> +
> + *val = dev_priv->running_igt_psr_tests;
> + return 0;
> +}
> +
> +static int i915_running_igt_psr_tests_set(void *data, u64 val)
> +{
> + struct drm_i915_private *dev_priv = data;
> +
> + dev_priv->running_igt_psr_tests = !!val;
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(i915_running_igt_psr_tests_fops,
> + i915_running_igt_psr_tests_get,
> + i915_running_igt_psr_tests_set,
> + "%llu\n");
> +
> static const struct drm_info_list i915_debugfs_list[] = {
> {"i915_capabilities", i915_capabilities, 0},
> {"i915_gem_objects", i915_gem_object_info, 0},
> @@ -4812,7 +4833,8 @@ static const struct i915_debugfs_files {
> {"i915_guc_log_relay", &i915_guc_log_relay_fops},
> {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
> {"i915_ipc_status", &i915_ipc_status_fops},
> - {"i915_drrs_ctl", &i915_drrs_ctl_fops}
> + {"i915_drrs_ctl", &i915_drrs_ctl_fops},
> + {"i915_running_igt_psr_tests", &i915_running_igt_psr_tests_fops}
we don't need new ops. All we want is to block the exit when running
sink_crc tests, so add the blocker wa there...
> };
>
> int i915_debugfs_register(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 41ebb144594e..9e0a5e29f48e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2110,6 +2110,8 @@ struct drm_i915_private {
>
> struct i915_pmu pmu;
>
> + bool running_igt_psr_tests;
> +
> /*
> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> * will be rejected. Instead look for a better place.
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fedee4e7ed24..c655f6c08a02 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1071,8 +1071,20 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> */
> static void intel_dp_aux_ch_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_dp->exit_psr_on_aux_ch_xfer)
> return;
> + /*
> + * HACK: This is a workaround to keep IGT PSR and frontbuffer tracking
> + * tests functional, otherwise when IGT request the CRC of the
> + * frontbuffer to sink it would cause this function to complete execute
> + * and exit PSR.
> + */
> + if (dev_priv->running_igt_psr_tests)
> + return;
>
> intel_psr_activate_block_get(intel_dp);
> intel_psr_exit(intel_dp, true);
> @@ -1085,8 +1097,14 @@ static void intel_dp_aux_ch_get(struct intel_dp *intel_dp)
> */
> static void intel_dp_aux_ch_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_dp->exit_psr_on_aux_ch_xfer)
> return;
> + if (dev_priv->running_igt_psr_tests)
> + return;
>
> intel_psr_activate_block_put(intel_dp);
> /* Usually more than just one aux ch transaction is executed when
> --
> 2.16.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 09/11] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink
2018-03-30 22:23 ` [PATCH 09/11] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink José Roberto de Souza
@ 2018-04-02 18:28 ` Rodrigo Vivi
0 siblings, 0 replies; 31+ messages in thread
From: Rodrigo Vivi @ 2018-04-02 18:28 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Mar 30, 2018 at 03:23:34PM -0700, José Roberto de Souza wrote:
> eDP spec states that sink device will do a short pulse in HPD
> line when there is a PSR/PSR2 error that needs to be handled by
> source, this is handling the first and most simples error:
> DP_PSR_SINK_INTERNAL_ERROR.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 2 ++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_psr.c | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c655f6c08a02..fc8b86bc0120 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4518,6 +4518,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
> if (intel_dp_needs_link_retrain(intel_dp))
> return false;
>
> + intel_psr_hpd_short_pulse_handle(intel_dp);
> +
> if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
> /* Send a Hotplug Uevent to userspace to start modeset */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 177478f0b032..d4c7e1e0fb86 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1897,6 +1897,7 @@ void intel_psr_exit(struct intel_dp *intel_dp, bool wait_idle);
> void intel_psr_activate(struct intel_dp *intel_dp, bool schedule);
> void intel_psr_activate_block_get(struct intel_dp *intel_dp);
> void intel_psr_activate_block_put(struct intel_dp *intel_dp);
> +void intel_psr_hpd_short_pulse_handle(struct intel_dp *intel_dp);
>
> /* intel_runtime_pm.c */
> int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index f88f12246a23..85d201fce287 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1250,3 +1250,34 @@ void intel_psr_activate_block_put(struct intel_dp *intel_dp)
> out:
> mutex_unlock(&dev_priv->psr.lock);
> }
> +
> +void intel_psr_hpd_short_pulse_handle(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct i915_psr *psr = &dev_priv->psr;
> + uint8_t val;
> +
> + if (!HAS_PSR(dev_priv))
> + return;
> +
> + mutex_lock(&psr->lock);
> + if (psr->enabled != intel_dp)
> + goto out;
> +
> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) != 1) {
> + DRM_DEBUG_KMS("PSR_STATUS read failed\n");
> + goto dpcd_read_error;
> + }
> +
> + if ((val & DP_PSR_SINK_STATE_MASK) == DP_PSR_SINK_INTERNAL_ERROR)
> + __intel_psr_exit(dev_priv);
shouldn't we add a debug msg here?
> +
> + /* TODO: handle other PSR/PSR2 errors */
> +dpcd_read_error:
> + if (!dev_priv->psr.busy_frontbuffer_bits)
> + __intel_psr_activate(intel_dp);
> +out:
> + mutex_unlock(&psr->lock);
> +}
> --
> 2.16.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction
2018-04-02 18:23 ` Rodrigo Vivi
@ 2018-04-02 18:42 ` Pandiyan, Dhinakaran
2018-04-02 22:16 ` Souza, Jose
2018-04-02 19:00 ` Pandiyan, Dhinakaran
1 sibling, 1 reply; 31+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-04-02 18:42 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx
On Mon, 2018-04-02 at 11:23 -0700, Rodrigo Vivi wrote:
> On Fri, Mar 30, 2018 at 03:23:32PM -0700, José Roberto de Souza wrote:
> > When PSR/PSR2 is enabled hardware can do aux ch transactions by it
> > self.
> > Spec requires that PSR is inactive before do any aux ch transaction
> > in HSW and BDW, for skl+ there is a aux ch mutex but the use is not
> > recommended.
> > So exiting PSR/PSR2 and waiting the transition to inactive to prevent
> > any concurrent access between hardware and software in aux ch
> > registers.
> >
> > VLV and CHV hardware don't do any aux as software is responsible to
> > do all the buffer tracking and it sends the wake up aux ch message
> > to sink.
> >
>
> ahh cool... I get back what I said on some previous patch.
> I like where it is going, but...
>
> > BSpec: 7530, 799 and 7532
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 38 ++++++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_psr.c | 8 ++++++++
> > 3 files changed, 48 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 62f82c4298ac..fedee4e7ed24 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1062,6 +1062,41 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> > DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> > }
> >
> > +/**
> > + * intel_dp_aux_ch_get - Get total control of aux ch registers
> > + *
> > + * By exiting or disabling any hardware feature that can also use the aux ch
> > + * registers at the same time as driver, this function will give total control
> > + * of aux ch registers to driver.
> > + */
> > +static void intel_dp_aux_ch_get(struct intel_dp *intel_dp)
> > +{
> > + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> > + return;
> > +
> > + intel_psr_activate_block_get(intel_dp);
> > + intel_psr_exit(intel_dp, true);
>
> decision on exit and activate should be inside the block_get and block_put,
> based on current state of the counters.
>
I haven't read the patches yet, but you could use the kref infrastructure for
reference counting.
> > +}
> > +
> > +/**
> > + * intel_dp_aux_ch_put - Release aux ch registers control
> > + *
> > + * It is the intel_dp_aux_ch_get() counterpart.
> > + */
> > +static void intel_dp_aux_ch_put(struct intel_dp *intel_dp)
> > +{
> > + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> > + return;
> > +
> > + intel_psr_activate_block_put(intel_dp);
> > + /* Usually more than just one aux ch transaction is executed when
> > + * handling some event, activating PSR right way would cause several
> > + * msecs of delay waiting PSR to exit for each aux ch transaction, so
> > + * here asking it to be scheduled.
> > + */
> > + intel_psr_activate(intel_dp, true);
> > +}
> > +
> > static int
> > intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > const uint8_t *send, int send_bytes,
> > @@ -1101,6 +1136,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >
> > intel_dp_check_edp(intel_dp);
> >
> > + intel_dp_aux_ch_get(intel_dp);
> > +
> > /* Try to wait for any previous AUX channel activity */
> > for (try = 0; try < 3; try++) {
> > status = I915_READ_NOTRACE(ch_ctl);
> > @@ -1223,6 +1260,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >
> > ret = recv_bytes;
> > out:
> > + intel_dp_aux_ch_put(intel_dp);
> > pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
> >
> > if (vdd)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 020b96324135..177478f0b032 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1139,6 +1139,8 @@ struct intel_dp {
> >
> > /* Displayport compliance testing */
> > struct intel_dp_compliance compliance;
> > +
> > + bool exit_psr_on_aux_ch_xfer;
> > };
> >
> > struct intel_lspcon {
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 8702dbafb42d..f88f12246a23 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -665,6 +665,13 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> > msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5));
> > }
> >
> > + /* From all platforms that supports PSR/PSR2 this 2 is the ones that
> > + * don't have restrictions about use of the aux ch while PSR/PSR2 is
> > + * enabled.
> > + */
> > + if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
> > + intel_dp->exit_psr_on_aux_ch_xfer = true;
> > +
> > unlock:
> > mutex_unlock(&dev_priv->psr.lock);
> > }
> > @@ -732,6 +739,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> >
> > dev_priv->psr.disable_source(intel_dp);
> >
> > + intel_dp->exit_psr_on_aux_ch_xfer = false;
> > /* Disable PSR on Sink */
> > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> >
> > --
> > 2.16.3
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction
2018-04-02 18:23 ` Rodrigo Vivi
2018-04-02 18:42 ` Pandiyan, Dhinakaran
@ 2018-04-02 19:00 ` Pandiyan, Dhinakaran
2018-04-02 22:15 ` Souza, Jose
1 sibling, 1 reply; 31+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-04-02 19:00 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx
On Mon, 2018-04-02 at 11:23 -0700, Rodrigo Vivi wrote:
> On Fri, Mar 30, 2018 at 03:23:32PM -0700, José Roberto de Souza wrote:
> > When PSR/PSR2 is enabled hardware can do aux ch transactions by it
> > self.
> > Spec requires that PSR is inactive before do any aux ch transaction
> > in HSW and BDW, for skl+ there is a aux ch mutex but the use is not
> > recommended.
> > So exiting PSR/PSR2 and waiting the transition to inactive to prevent
> > any concurrent access between hardware and software in aux ch
> > registers.
Wouldn't the debugfs interface that you added to read sink PSR status
always show PSR as disabled? Or do you intend to set
intel_dp->exit_psr_on_aux_ch_xfer = true?
> >
> > VLV and CHV hardware don't do any aux as software is responsible to
> > do all the buffer tracking and it sends the wake up aux ch message
> > to sink.
> >
>
> ahh cool... I get back what I said on some previous patch.
> I like where it is going, but...
>
> > BSpec: 7530, 799 and 7532
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 38 ++++++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_psr.c | 8 ++++++++
> > 3 files changed, 48 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 62f82c4298ac..fedee4e7ed24 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1062,6 +1062,41 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> > DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> > }
> >
> > +/**
> > + * intel_dp_aux_ch_get - Get total control of aux ch registers
> > + *
> > + * By exiting or disabling any hardware feature that can also use the aux ch
> > + * registers at the same time as driver, this function will give total control
> > + * of aux ch registers to driver.
> > + */
> > +static void intel_dp_aux_ch_get(struct intel_dp *intel_dp)
> > +{
> > + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> > + return;
> > +
> > + intel_psr_activate_block_get(intel_dp);
> > + intel_psr_exit(intel_dp, true);
>
> decision on exit and activate should be inside the block_get and block_put,
> based on current state of the counters.
>
> > +}
> > +
> > +/**
> > + * intel_dp_aux_ch_put - Release aux ch registers control
> > + *
> > + * It is the intel_dp_aux_ch_get() counterpart.
> > + */
> > +static void intel_dp_aux_ch_put(struct intel_dp *intel_dp)
> > +{
> > + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> > + return;
> > +
> > + intel_psr_activate_block_put(intel_dp);
> > + /* Usually more than just one aux ch transaction is executed when
> > + * handling some event, activating PSR right way would cause several
> > + * msecs of delay waiting PSR to exit for each aux ch transaction, so
> > + * here asking it to be scheduled.
> > + */
> > + intel_psr_activate(intel_dp, true);
> > +}
> > +
> > static int
> > intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > const uint8_t *send, int send_bytes,
> > @@ -1101,6 +1136,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >
> > intel_dp_check_edp(intel_dp);
> >
> > + intel_dp_aux_ch_get(intel_dp);
> > +
> > /* Try to wait for any previous AUX channel activity */
> > for (try = 0; try < 3; try++) {
> > status = I915_READ_NOTRACE(ch_ctl);
> > @@ -1223,6 +1260,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >
> > ret = recv_bytes;
> > out:
> > + intel_dp_aux_ch_put(intel_dp);
> > pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
> >
> > if (vdd)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 020b96324135..177478f0b032 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1139,6 +1139,8 @@ struct intel_dp {
> >
> > /* Displayport compliance testing */
> > struct intel_dp_compliance compliance;
> > +
> > + bool exit_psr_on_aux_ch_xfer;
> > };
> >
> > struct intel_lspcon {
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 8702dbafb42d..f88f12246a23 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -665,6 +665,13 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> > msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5));
> > }
> >
> > + /* From all platforms that supports PSR/PSR2 this 2 is the ones that
> > + * don't have restrictions about use of the aux ch while PSR/PSR2 is
> > + * enabled.
> > + */
> > + if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
> > + intel_dp->exit_psr_on_aux_ch_xfer = true;
> > +
> > unlock:
> > mutex_unlock(&dev_priv->psr.lock);
> > }
> > @@ -732,6 +739,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> >
> > dev_priv->psr.disable_source(intel_dp);
> >
> > + intel_dp->exit_psr_on_aux_ch_xfer = false;
> > /* Disable PSR on Sink */
> > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> >
> > --
> > 2.16.3
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 03/11] drm/i915/psr: Share code between disable and exit
2018-04-02 18:11 ` Rodrigo Vivi
@ 2018-04-02 21:15 ` Souza, Jose
0 siblings, 0 replies; 31+ messages in thread
From: Souza, Jose @ 2018-04-02 21:15 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx, Pandiyan, Dhinakaran
On Mon, 2018-04-02 at 11:11 -0700, Rodrigo Vivi wrote:
> On Fri, Mar 30, 2018 at 03:23:28PM -0700, José Roberto de Souza
> wrote:
> > The disable and exit sequence are very similar with a lot common
> > code between both, so here sharing the code.
>
> I don't believe that we should do this.
> Disable as is has some extra wait/timeouts that could slow things
> down.
>
> Or at least not do this while we don't have a proper robuts test in
> place
> and to test also performance impacts...
When the disabling argument is false it don't have the disable
wait/timeouts, I kept the same behavior to both just sharing the common
part.
>
> >
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > drivers/gpu/drm/i915/intel_psr.c | 84 ++++++++++++++++++--------
> > --------------
> > 2 files changed, 38 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index a8d300280a2c..cb72ee27422f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -617,7 +617,7 @@ struct i915_psr {
> > void (*enable_sink)(struct intel_dp *);
> > void (*activate)(struct intel_dp *);
> > void (*setup_vsc)(struct intel_dp *, const struct
> > intel_crtc_state *);
> > - void (*exit)(struct intel_dp *intel_dp);
> > + void (*exit)(struct intel_dp *intel_dp, bool disabling);
> > };
> >
> > enum intel_pch {
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index bcaac9e69f8c..d3451afeb8bb 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -672,23 +672,9 @@ static void vlv_psr_disable(struct intel_dp
> > *intel_dp,
> > struct drm_device *dev = intel_dig_port->base.base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_crtc *crtc = to_intel_crtc(old_crtc_state-
> > >base.crtc);
> > - uint32_t val;
> >
> > if (dev_priv->psr.active) {
> > - /* Put VLV PSR back to PSR_state 0 (disabled). */
> > - if (intel_wait_for_register(dev_priv,
> > - VLV_PSRSTAT(crtc-
> > >pipe),
> > - VLV_EDP_PSR_IN_TRANS,
> > - 0,
> > - 1))
> > - WARN(1, "PSR transition took longer than
> > expected\n");
> > -
> > - val = I915_READ(VLV_PSRCTL(crtc->pipe));
> > - val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> > - val &= ~VLV_EDP_PSR_ENABLE;
> > - val &= ~VLV_EDP_PSR_MODE_MASK;
> > - I915_WRITE(VLV_PSRCTL(crtc->pipe), val);
> > -
> > + dev_priv->psr.exit(intel_dp, true);
> > dev_priv->psr.active = false;
> > } else {
> > WARN_ON(vlv_is_psr_active_on_pipe(dev, crtc-
> > >pipe));
> > @@ -703,31 +689,7 @@ static void hsw_psr_disable(struct intel_dp
> > *intel_dp,
> > struct drm_i915_private *dev_priv = to_i915(dev);
> >
> > if (dev_priv->psr.active) {
> > - i915_reg_t psr_status;
> > - u32 psr_status_mask;
> > -
> > - if (dev_priv->psr.psr2_enabled) {
> > - psr_status = EDP_PSR2_STATUS;
> > - psr_status_mask =
> > EDP_PSR2_STATUS_STATE_MASK;
> > -
> > - I915_WRITE(EDP_PSR2_CTL,
> > - I915_READ(EDP_PSR2_CTL) &
> > - ~(EDP_PSR2_ENABLE |
> > EDP_SU_TRACK_ENABLE));
> > -
> > - } else {
> > - psr_status = EDP_PSR_STATUS;
> > - psr_status_mask =
> > EDP_PSR_STATUS_STATE_MASK;
> > -
> > - I915_WRITE(EDP_PSR_CTL,
> > - I915_READ(EDP_PSR_CTL) &
> > ~EDP_PSR_ENABLE);
> > - }
> > -
> > - /* Wait till PSR is idle */
> > - if (intel_wait_for_register(dev_priv,
> > - psr_status,
> > psr_status_mask, 0,
> > - 2000))
> > - DRM_ERROR("Timed out waiting for PSR Idle
> > State\n");
> > -
> > + dev_priv->psr.exit(intel_dp, true);
> > dev_priv->psr.active = false;
> > } else {
> > if (dev_priv->psr.psr2_enabled)
> > @@ -838,25 +800,41 @@ static void intel_psr_work(struct work_struct
> > *work)
> > mutex_unlock(&dev_priv->psr.lock);
> > }
> >
> > -static void hsw_psr_exit(struct intel_dp *intel_dp)
> > +static void hsw_psr_exit(struct intel_dp *intel_dp, bool
> > disabling)
> > {
> > struct intel_digital_port *dig_port =
> > dp_to_dig_port(intel_dp);
> > struct drm_device *dev = dig_port->base.base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > + i915_reg_t psr_status;
> > + u32 psr_status_mask;
> > u32 val;
> >
> > if (dev_priv->psr.psr2_enabled) {
> > + psr_status = EDP_PSR2_STATUS;
> > + psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> > +
> > val = I915_READ(EDP_PSR2_CTL);
> > - WARN_ON(!(val & EDP_PSR2_ENABLE));
> > - I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
> > + WARN_ON(!disabling && !(val & EDP_PSR2_ENABLE));
> > + val &= ~EDP_PSR2_ENABLE;
> > + if (disabling)
> > + val &= ~EDP_SU_TRACK_ENABLE;
> > + I915_WRITE(EDP_PSR2_CTL, val);
> > } else {
> > + psr_status = EDP_PSR_STATUS;
> > + psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> > +
> > val = I915_READ(EDP_PSR_CTL);
> > - WARN_ON(!(val & EDP_PSR_ENABLE));
> > + WARN_ON(!disabling && !(val & EDP_PSR_ENABLE));
> > I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
> > }
> > +
> > + /* When disabling wait till PSR is idle */
> > + if (disabling && intel_wait_for_register(dev_priv,
> > psr_status,
> > + psr_status_mask,
> > 0, 2000))
> > + DRM_ERROR("Timed out waiting for PSR Idle
> > State\n");
> > }
> >
> > -static void vlv_psr_exit(struct intel_dp *intel_dp)
> > +static void vlv_psr_exit(struct intel_dp *intel_dp, bool
> > disabling)
> > {
> > struct intel_digital_port *dig_port =
> > dp_to_dig_port(intel_dp);
> > struct drm_device *dev = dig_port->base.base.dev;
> > @@ -865,10 +843,14 @@ static void vlv_psr_exit(struct intel_dp
> > *intel_dp)
> > enum pipe pipe = to_intel_crtc(crtc)->pipe;
> > u32 val;
> >
> > + if (disabling && intel_wait_for_register(dev_priv,
> > VLV_PSRSTAT(pipe),
> > + VLV_EDP_PSR_IN_TR
> > ANS, 0, 1))
> > + DRM_WARN("PSR transition took longer than
> > expected\n");
> > +
> > val = I915_READ(VLV_PSRCTL(pipe));
> >
> > /*
> > - * Here we do the transition drirectly from
> > + * Here we do the transition directly from
> > * PSR_state 3 (active - no Remote Frame Buffer (RFB)
> > update) to
> > * PSR_state 5 (exit).
> > * PSR State 4 (active with single frame update) can be
> > skipped.
> > @@ -877,8 +859,16 @@ static void vlv_psr_exit(struct intel_dp
> > *intel_dp)
> > * Now we are at Same state after vlv_psr_enable_source.
> > */
> > val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> > + if (disabling) {
> > + /* Put VLV PSR back to PSR_state 0 (disabled). */
> > + val &= ~VLV_EDP_PSR_ENABLE;
> > + val &= ~VLV_EDP_PSR_MODE_MASK;
> > + }
> > I915_WRITE(VLV_PSRCTL(pipe), val);
> >
> > + if (disabling)
> > + return;
> > +
> > /*
> > * Send AUX wake up - Spec says after transitioning to PSR
> > * active we have to send AUX wake up by writing 01h in
> > DPCD
> > @@ -898,7 +888,7 @@ static void intel_psr_exit(struct
> > drm_i915_private *dev_priv)
> > if (!dev_priv->psr.active)
> > return;
> >
> > - dev_priv->psr.exit(intel_dp);
> > + dev_priv->psr.exit(intel_dp, false);
> > dev_priv->psr.active = false;
> > }
> >
> > --
> > 2.16.3
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 06/11] drm/i915/psr: Add intel_psr_activate_block_get()/put()
2018-04-02 18:20 ` Rodrigo Vivi
@ 2018-04-02 22:11 ` Souza, Jose
2018-04-03 17:32 ` Rodrigo Vivi
0 siblings, 1 reply; 31+ messages in thread
From: Souza, Jose @ 2018-04-02 22:11 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx, Pandiyan, Dhinakaran
On Mon, 2018-04-02 at 11:20 -0700, Rodrigo Vivi wrote:
> On Fri, Mar 30, 2018 at 03:23:31PM -0700, José Roberto de Souza
> wrote:
> > intel_psr_activate_block_get() should be called when by some reason
> > PSR should not be activated for some time, it will increment
> > counter
> > and it should the decremented by intel_psr_activate_block_put()
> > when PSR can be activated again.
> > intel_psr_activate_block_put() will not actually activate PSR,
> > users
> > of this function should also call intel_psr_activate().
>
> Ohh cool! you made the counter.
> probably we will need to change things from mutex to spin locker.
> But also the blocker functions here could already introduce the
> function
> calls to really block and release psr.
Oh so drop the 'drm/i915/psr: Export intel_psr_activate/exit()' and
call PSR exit and activate from intel_psr_activate_block_get()/put()?
I dind't understand why you want to change struct mutex lock; to
spinlock_t?
>
> >
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_psr.c | 54
> > ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 57 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 99af9169d792..41ebb144594e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -609,6 +609,7 @@ struct i915_psr {
> > bool has_hw_tracking;
> > bool psr2_enabled;
> > u8 sink_sync_latency;
> > + unsigned int activate_block_count;
> >
> > void (*enable_source)(struct intel_dp *,
> > const struct intel_crtc_state *);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 70026b772721..020b96324135 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1893,6 +1893,8 @@ void intel_psr_compute_config(struct intel_dp
> > *intel_dp,
> > struct intel_crtc_state
> > *crtc_state);
> > void intel_psr_exit(struct intel_dp *intel_dp, bool wait_idle);
> > void intel_psr_activate(struct intel_dp *intel_dp, bool schedule);
> > +void intel_psr_activate_block_get(struct intel_dp *intel_dp);
> > +void intel_psr_activate_block_put(struct intel_dp *intel_dp);
> >
> > /* intel_runtime_pm.c */
> > int intel_power_domains_init(struct drm_i915_private *);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 906a12ea934d..8702dbafb42d 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -558,6 +558,8 @@ static void __intel_psr_activate(struct
> > intel_dp *intel_dp)
> >
> > WARN_ON(dev_priv->psr.active);
> > lockdep_assert_held(&dev_priv->psr.lock);
> > + if (dev_priv->psr.activate_block_count)
> > + return;
> >
> > dev_priv->psr.activate(intel_dp);
> > dev_priv->psr.active = true;
> > @@ -1188,3 +1190,55 @@ void intel_psr_activate(struct intel_dp
> > *intel_dp, bool schedule)
> > out:
> > mutex_unlock(&dev_priv->psr.lock);
> > }
> > +
> > +/**
> > + * intel_psr_activate_block_get - Block further attempts to
> > activate PSR
> > + * @intel_dp: DisplayPort that have PSR enabled
> > + *
> > + * It have a internal reference count, so each
> > intel_psr_activate_block_get()
> > + * should have a intel_psr_activate_block_put() counterpart.
> > + */
> > +void intel_psr_activate_block_get(struct intel_dp *intel_dp)
> > +{
> > + struct intel_digital_port *dig_port =
> > dp_to_dig_port(intel_dp);
> > + struct drm_device *dev = dig_port->base.base.dev;
> > + struct drm_i915_private *dev_priv = to_i915(dev);
> > +
> > + if (!CAN_PSR(dev_priv))
> > + return;
> > +
> > + mutex_lock(&dev_priv->psr.lock);
> > + if (dev_priv->psr.enabled != intel_dp)
> > + goto out;
> > +
> > + dev_priv->psr.activate_block_count++;
> > +out:
> > + mutex_unlock(&dev_priv->psr.lock);
> > +}
> > +
> > +
> > +/**
> > + * intel_psr_activate_block_put - Unblock further attempts to
> > activate PSR
> > + * @intel_dp: DisplayPort that have PSR enabled
> > + *
> > + * Decrease the reference counter incremented by
> > intel_psr_activate_block_get()
> > + * when zero it allows PSR be activate. To actually activate PSR
> > when reference
> > + * counter is zero intel_psr_activate() should be called.
> > + */
> > +void intel_psr_activate_block_put(struct intel_dp *intel_dp)
> > +{
> > + struct intel_digital_port *dig_port =
> > dp_to_dig_port(intel_dp);
> > + struct drm_device *dev = dig_port->base.base.dev;
> > + struct drm_i915_private *dev_priv = to_i915(dev);
> > +
> > + if (!CAN_PSR(dev_priv))
> > + return;
> > +
> > + mutex_lock(&dev_priv->psr.lock);
> > + if (dev_priv->psr.enabled != intel_dp)
> > + goto out;
> > +
> > + dev_priv->psr.activate_block_count--;
> > +out:
> > + mutex_unlock(&dev_priv->psr.lock);
> > +}
> > --
> > 2.16.3
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction
2018-04-02 19:00 ` Pandiyan, Dhinakaran
@ 2018-04-02 22:15 ` Souza, Jose
0 siblings, 0 replies; 31+ messages in thread
From: Souza, Jose @ 2018-04-02 22:15 UTC (permalink / raw)
To: Vivi, Rodrigo, Pandiyan, Dhinakaran; +Cc: intel-gfx
On Mon, 2018-04-02 at 12:00 -0700, Pandiyan, Dhinakaran wrote:
>
>
> On Mon, 2018-04-02 at 11:23 -0700, Rodrigo Vivi wrote:
> > On Fri, Mar 30, 2018 at 03:23:32PM -0700, José Roberto de Souza
> > wrote:
> > > When PSR/PSR2 is enabled hardware can do aux ch transactions by
> > > it
> > > self.
> > > Spec requires that PSR is inactive before do any aux ch
> > > transaction
> > > in HSW and BDW, for skl+ there is a aux ch mutex but the use is
> > > not
> > > recommended.
> > > So exiting PSR/PSR2 and waiting the transition to inactive to
> > > prevent
> > > any concurrent access between hardware and software in aux ch
> > > registers.
>
> Wouldn't the debugfs interface that you added to read sink PSR status
> always show PSR as disabled? Or do you intend to set
> intel_dp->exit_psr_on_aux_ch_xfer = true?
Yeah it will always be disabled, unless when the hack in 'drm/i915:
Keep IGT PSR and frontbuffer tests functional' is set, Rodrigo asked to
remove the debugfs file that was controlling the state of the hack and
enable/disable it around CRC read so I could do that for the whole
i915_edp_psr_status too.
>
>
> > >
> > > VLV and CHV hardware don't do any aux as software is responsible
> > > to
> > > do all the buffer tracking and it sends the wake up aux ch
> > > message
> > > to sink.
> > >
> >
> > ahh cool... I get back what I said on some previous patch.
> > I like where it is going, but...
> >
> > > BSpec: 7530, 799 and 7532
> > >
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 38
> > > ++++++++++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > drivers/gpu/drm/i915/intel_psr.c | 8 ++++++++
> > > 3 files changed, 48 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 62f82c4298ac..fedee4e7ed24 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -1062,6 +1062,41 @@ static uint32_t
> > > skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> > > }
> > >
> > > +/**
> > > + * intel_dp_aux_ch_get - Get total control of aux ch registers
> > > + *
> > > + * By exiting or disabling any hardware feature that can also
> > > use the aux ch
> > > + * registers at the same time as driver, this function will give
> > > total control
> > > + * of aux ch registers to driver.
> > > + */
> > > +static void intel_dp_aux_ch_get(struct intel_dp *intel_dp)
> > > +{
> > > + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> > > + return;
> > > +
> > > + intel_psr_activate_block_get(intel_dp);
> > > + intel_psr_exit(intel_dp, true);
> >
> > decision on exit and activate should be inside the block_get and
> > block_put,
> > based on current state of the counters.
> >
> > > +}
> > > +
> > > +/**
> > > + * intel_dp_aux_ch_put - Release aux ch registers control
> > > + *
> > > + * It is the intel_dp_aux_ch_get() counterpart.
> > > + */
> > > +static void intel_dp_aux_ch_put(struct intel_dp *intel_dp)
> > > +{
> > > + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> > > + return;
> > > +
> > > + intel_psr_activate_block_put(intel_dp);
> > > + /* Usually more than just one aux ch transaction is
> > > executed when
> > > + * handling some event, activating PSR right way would
> > > cause several
> > > + * msecs of delay waiting PSR to exit for each aux ch
> > > transaction, so
> > > + * here asking it to be scheduled.
> > > + */
> > > + intel_psr_activate(intel_dp, true);
> > > +}
> > > +
> > > static int
> > > intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > const uint8_t *send, int send_bytes,
> > > @@ -1101,6 +1136,8 @@ intel_dp_aux_xfer(struct intel_dp
> > > *intel_dp,
> > >
> > > intel_dp_check_edp(intel_dp);
> > >
> > > + intel_dp_aux_ch_get(intel_dp);
> > > +
> > > /* Try to wait for any previous AUX channel activity */
> > > for (try = 0; try < 3; try++) {
> > > status = I915_READ_NOTRACE(ch_ctl);
> > > @@ -1223,6 +1260,7 @@ intel_dp_aux_xfer(struct intel_dp
> > > *intel_dp,
> > >
> > > ret = recv_bytes;
> > > out:
> > > + intel_dp_aux_ch_put(intel_dp);
> > > pm_qos_update_request(&dev_priv->pm_qos,
> > > PM_QOS_DEFAULT_VALUE);
> > >
> > > if (vdd)
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 020b96324135..177478f0b032 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1139,6 +1139,8 @@ struct intel_dp {
> > >
> > > /* Displayport compliance testing */
> > > struct intel_dp_compliance compliance;
> > > +
> > > + bool exit_psr_on_aux_ch_xfer;
> > > };
> > >
> > > struct intel_lspcon {
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 8702dbafb42d..f88f12246a23 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -665,6 +665,13 @@ void intel_psr_enable(struct intel_dp
> > > *intel_dp,
> > > msecs_to_jiffies(intel_dp-
> > > >panel_power_cycle_delay * 5));
> > > }
> > >
> > > + /* From all platforms that supports PSR/PSR2 this 2 is
> > > the ones that
> > > + * don't have restrictions about use of the aux ch while
> > > PSR/PSR2 is
> > > + * enabled.
> > > + */
> > > + if (!(IS_VALLEYVIEW(dev_priv) ||
> > > IS_CHERRYVIEW(dev_priv)))
> > > + intel_dp->exit_psr_on_aux_ch_xfer = true;
> > > +
> > > unlock:
> > > mutex_unlock(&dev_priv->psr.lock);
> > > }
> > > @@ -732,6 +739,7 @@ void intel_psr_disable(struct intel_dp
> > > *intel_dp,
> > >
> > > dev_priv->psr.disable_source(intel_dp);
> > >
> > > + intel_dp->exit_psr_on_aux_ch_xfer = false;
> > > /* Disable PSR on Sink */
> > > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> > >
> > > --
> > > 2.16.3
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction
2018-04-02 18:42 ` Pandiyan, Dhinakaran
@ 2018-04-02 22:16 ` Souza, Jose
2018-04-02 23:21 ` Souza, Jose
0 siblings, 1 reply; 31+ messages in thread
From: Souza, Jose @ 2018-04-02 22:16 UTC (permalink / raw)
To: Vivi, Rodrigo, Pandiyan, Dhinakaran; +Cc: intel-gfx
On Mon, 2018-04-02 at 11:42 -0700, Pandiyan, Dhinakaran wrote:
>
>
> On Mon, 2018-04-02 at 11:23 -0700, Rodrigo Vivi wrote:
> > On Fri, Mar 30, 2018 at 03:23:32PM -0700, José Roberto de Souza
> > wrote:
> > > When PSR/PSR2 is enabled hardware can do aux ch transactions by
> > > it
> > > self.
> > > Spec requires that PSR is inactive before do any aux ch
> > > transaction
> > > in HSW and BDW, for skl+ there is a aux ch mutex but the use is
> > > not
> > > recommended.
> > > So exiting PSR/PSR2 and waiting the transition to inactive to
> > > prevent
> > > any concurrent access between hardware and software in aux ch
> > > registers.
> > >
> > > VLV and CHV hardware don't do any aux as software is responsible
> > > to
> > > do all the buffer tracking and it sends the wake up aux ch
> > > message
> > > to sink.
> > >
> >
> > ahh cool... I get back what I said on some previous patch.
> > I like where it is going, but...
> >
> > > BSpec: 7530, 799 and 7532
> > >
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 38
> > > ++++++++++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > drivers/gpu/drm/i915/intel_psr.c | 8 ++++++++
> > > 3 files changed, 48 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 62f82c4298ac..fedee4e7ed24 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -1062,6 +1062,41 @@ static uint32_t
> > > skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> > > }
> > >
> > > +/**
> > > + * intel_dp_aux_ch_get - Get total control of aux ch registers
> > > + *
> > > + * By exiting or disabling any hardware feature that can also
> > > use the aux ch
> > > + * registers at the same time as driver, this function will give
> > > total control
> > > + * of aux ch registers to driver.
> > > + */
> > > +static void intel_dp_aux_ch_get(struct intel_dp *intel_dp)
> > > +{
> > > + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> > > + return;
> > > +
> > > + intel_psr_activate_block_get(intel_dp);
> > > + intel_psr_exit(intel_dp, true);
> >
> > decision on exit and activate should be inside the block_get and
> > block_put,
> > based on current state of the counters.
> >
>
> I haven't read the patches yet, but you could use the kref
> infrastructure for
> reference counting.
Cool I will use it, thanks.
>
> > > +}
> > > +
> > > +/**
> > > + * intel_dp_aux_ch_put - Release aux ch registers control
> > > + *
> > > + * It is the intel_dp_aux_ch_get() counterpart.
> > > + */
> > > +static void intel_dp_aux_ch_put(struct intel_dp *intel_dp)
> > > +{
> > > + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> > > + return;
> > > +
> > > + intel_psr_activate_block_put(intel_dp);
> > > + /* Usually more than just one aux ch transaction is
> > > executed when
> > > + * handling some event, activating PSR right way would
> > > cause several
> > > + * msecs of delay waiting PSR to exit for each aux ch
> > > transaction, so
> > > + * here asking it to be scheduled.
> > > + */
> > > + intel_psr_activate(intel_dp, true);
> > > +}
> > > +
> > > static int
> > > intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > const uint8_t *send, int send_bytes,
> > > @@ -1101,6 +1136,8 @@ intel_dp_aux_xfer(struct intel_dp
> > > *intel_dp,
> > >
> > > intel_dp_check_edp(intel_dp);
> > >
> > > + intel_dp_aux_ch_get(intel_dp);
> > > +
> > > /* Try to wait for any previous AUX channel activity */
> > > for (try = 0; try < 3; try++) {
> > > status = I915_READ_NOTRACE(ch_ctl);
> > > @@ -1223,6 +1260,7 @@ intel_dp_aux_xfer(struct intel_dp
> > > *intel_dp,
> > >
> > > ret = recv_bytes;
> > > out:
> > > + intel_dp_aux_ch_put(intel_dp);
> > > pm_qos_update_request(&dev_priv->pm_qos,
> > > PM_QOS_DEFAULT_VALUE);
> > >
> > > if (vdd)
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 020b96324135..177478f0b032 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1139,6 +1139,8 @@ struct intel_dp {
> > >
> > > /* Displayport compliance testing */
> > > struct intel_dp_compliance compliance;
> > > +
> > > + bool exit_psr_on_aux_ch_xfer;
> > > };
> > >
> > > struct intel_lspcon {
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 8702dbafb42d..f88f12246a23 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -665,6 +665,13 @@ void intel_psr_enable(struct intel_dp
> > > *intel_dp,
> > > msecs_to_jiffies(intel_dp-
> > > >panel_power_cycle_delay * 5));
> > > }
> > >
> > > + /* From all platforms that supports PSR/PSR2 this 2 is
> > > the ones that
> > > + * don't have restrictions about use of the aux ch while
> > > PSR/PSR2 is
> > > + * enabled.
> > > + */
> > > + if (!(IS_VALLEYVIEW(dev_priv) ||
> > > IS_CHERRYVIEW(dev_priv)))
> > > + intel_dp->exit_psr_on_aux_ch_xfer = true;
> > > +
> > > unlock:
> > > mutex_unlock(&dev_priv->psr.lock);
> > > }
> > > @@ -732,6 +739,7 @@ void intel_psr_disable(struct intel_dp
> > > *intel_dp,
> > >
> > > dev_priv->psr.disable_source(intel_dp);
> > >
> > > + intel_dp->exit_psr_on_aux_ch_xfer = false;
> > > /* Disable PSR on Sink */
> > > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> > >
> > > --
> > > 2.16.3
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction
2018-04-02 22:16 ` Souza, Jose
@ 2018-04-02 23:21 ` Souza, Jose
0 siblings, 0 replies; 31+ messages in thread
From: Souza, Jose @ 2018-04-02 23:21 UTC (permalink / raw)
To: Vivi, Rodrigo, Pandiyan, Dhinakaran; +Cc: intel-gfx
On Mon, 2018-04-02 at 22:16 +0000, Souza, Jose wrote:
> On Mon, 2018-04-02 at 11:42 -0700, Pandiyan, Dhinakaran wrote:
> >
> >
> > On Mon, 2018-04-02 at 11:23 -0700, Rodrigo Vivi wrote:
> > > On Fri, Mar 30, 2018 at 03:23:32PM -0700, José Roberto de Souza
> > > wrote:
> > > > When PSR/PSR2 is enabled hardware can do aux ch transactions by
> > > > it
> > > > self.
> > > > Spec requires that PSR is inactive before do any aux ch
> > > > transaction
> > > > in HSW and BDW, for skl+ there is a aux ch mutex but the use is
> > > > not
> > > > recommended.
> > > > So exiting PSR/PSR2 and waiting the transition to inactive to
> > > > prevent
> > > > any concurrent access between hardware and software in aux ch
> > > > registers.
> > > >
> > > > VLV and CHV hardware don't do any aux as software is
> > > > responsible
> > > > to
> > > > do all the buffer tracking and it sends the wake up aux ch
> > > > message
> > > > to sink.
> > > >
> > >
> > > ahh cool... I get back what I said on some previous patch.
> > > I like where it is going, but...
> > >
> > > > BSpec: 7530, 799 and 7532
> > > >
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_dp.c | 38
> > > > ++++++++++++++++++++++++++++++++++++++
> > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > > drivers/gpu/drm/i915/intel_psr.c | 8 ++++++++
> > > > 3 files changed, 48 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 62f82c4298ac..fedee4e7ed24 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -1062,6 +1062,41 @@ static uint32_t
> > > > skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> > > > }
> > > >
> > > > +/**
> > > > + * intel_dp_aux_ch_get - Get total control of aux ch registers
> > > > + *
> > > > + * By exiting or disabling any hardware feature that can also
> > > > use the aux ch
> > > > + * registers at the same time as driver, this function will
> > > > give
> > > > total control
> > > > + * of aux ch registers to driver.
> > > > + */
> > > > +static void intel_dp_aux_ch_get(struct intel_dp *intel_dp)
> > > > +{
> > > > + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> > > > + return;
> > > > +
> > > > + intel_psr_activate_block_get(intel_dp);
> > > > + intel_psr_exit(intel_dp, true);
> > >
> > > decision on exit and activate should be inside the block_get and
> > > block_put,
> > > based on current state of the counters.
> > >
> >
> > I haven't read the patches yet, but you could use the kref
> > infrastructure for
> > reference counting.
>
> Cool I will use it, thanks.
To use kref I would need to do something like this in intel_psr_init():
kref_init(&dev_priv->psr.activate_block_kref);
/* kref_init() initialize the reference as 1, so calling kref_put() to
* keep it as 0 but this will cause the release callback to be executed
* but it will not be fully executed as PSR is not enabled yet.
*/
kref_put(&dev_priv->psr.activate_block_kref,
intel_psr_activate_block_release);
That is okay? keep it?
>
> >
> > > > +}
> > > > +
> > > > +/**
> > > > + * intel_dp_aux_ch_put - Release aux ch registers control
> > > > + *
> > > > + * It is the intel_dp_aux_ch_get() counterpart.
> > > > + */
> > > > +static void intel_dp_aux_ch_put(struct intel_dp *intel_dp)
> > > > +{
> > > > + if (!intel_dp->exit_psr_on_aux_ch_xfer)
> > > > + return;
> > > > +
> > > > + intel_psr_activate_block_put(intel_dp);
> > > > + /* Usually more than just one aux ch transaction is
> > > > executed when
> > > > + * handling some event, activating PSR right way would
> > > > cause several
> > > > + * msecs of delay waiting PSR to exit for each aux ch
> > > > transaction, so
> > > > + * here asking it to be scheduled.
> > > > + */
> > > > + intel_psr_activate(intel_dp, true);
> > > > +}
> > > > +
> > > > static int
> > > > intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > > const uint8_t *send, int send_bytes,
> > > > @@ -1101,6 +1136,8 @@ intel_dp_aux_xfer(struct intel_dp
> > > > *intel_dp,
> > > >
> > > > intel_dp_check_edp(intel_dp);
> > > >
> > > > + intel_dp_aux_ch_get(intel_dp);
> > > > +
> > > > /* Try to wait for any previous AUX channel activity
> > > > */
> > > > for (try = 0; try < 3; try++) {
> > > > status = I915_READ_NOTRACE(ch_ctl);
> > > > @@ -1223,6 +1260,7 @@ intel_dp_aux_xfer(struct intel_dp
> > > > *intel_dp,
> > > >
> > > > ret = recv_bytes;
> > > > out:
> > > > + intel_dp_aux_ch_put(intel_dp);
> > > > pm_qos_update_request(&dev_priv->pm_qos,
> > > > PM_QOS_DEFAULT_VALUE);
> > > >
> > > > if (vdd)
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > > b/drivers/gpu/drm/i915/intel_drv.h
> > > > index 020b96324135..177478f0b032 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -1139,6 +1139,8 @@ struct intel_dp {
> > > >
> > > > /* Displayport compliance testing */
> > > > struct intel_dp_compliance compliance;
> > > > +
> > > > + bool exit_psr_on_aux_ch_xfer;
> > > > };
> > > >
> > > > struct intel_lspcon {
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index 8702dbafb42d..f88f12246a23 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -665,6 +665,13 @@ void intel_psr_enable(struct intel_dp
> > > > *intel_dp,
> > > > msecs_to_jiffies(intel_d
> > > > p-
> > > > > panel_power_cycle_delay * 5));
> > > >
> > > > }
> > > >
> > > > + /* From all platforms that supports PSR/PSR2 this 2 is
> > > > the ones that
> > > > + * don't have restrictions about use of the aux ch
> > > > while
> > > > PSR/PSR2 is
> > > > + * enabled.
> > > > + */
> > > > + if (!(IS_VALLEYVIEW(dev_priv) ||
> > > > IS_CHERRYVIEW(dev_priv)))
> > > > + intel_dp->exit_psr_on_aux_ch_xfer = true;
> > > > +
> > > > unlock:
> > > > mutex_unlock(&dev_priv->psr.lock);
> > > > }
> > > > @@ -732,6 +739,7 @@ void intel_psr_disable(struct intel_dp
> > > > *intel_dp,
> > > >
> > > > dev_priv->psr.disable_source(intel_dp);
> > > >
> > > > + intel_dp->exit_psr_on_aux_ch_xfer = false;
> > > > /* Disable PSR on Sink */
> > > > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> > > >
> > > > --
> > > > 2.16.3
> > > >
>
> _______________________________________________
> 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] 31+ messages in thread
* Re: [PATCH 06/11] drm/i915/psr: Add intel_psr_activate_block_get()/put()
2018-04-02 22:11 ` Souza, Jose
@ 2018-04-03 17:32 ` Rodrigo Vivi
0 siblings, 0 replies; 31+ messages in thread
From: Rodrigo Vivi @ 2018-04-03 17:32 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx, Pandiyan, Dhinakaran
On Mon, Apr 02, 2018 at 03:11:54PM -0700, Souza, Jose wrote:
> On Mon, 2018-04-02 at 11:20 -0700, Rodrigo Vivi wrote:
> > On Fri, Mar 30, 2018 at 03:23:31PM -0700, José Roberto de Souza
> > wrote:
> > > intel_psr_activate_block_get() should be called when by some reason
> > > PSR should not be activated for some time, it will increment
> > > counter
> > > and it should the decremented by intel_psr_activate_block_put()
> > > when PSR can be activated again.
> > > intel_psr_activate_block_put() will not actually activate PSR,
> > > users
> > > of this function should also call intel_psr_activate().
> >
> > Ohh cool! you made the counter.
> > probably we will need to change things from mutex to spin locker.
> > But also the blocker functions here could already introduce the
> > function
> > calls to really block and release psr.
>
> Oh so drop the 'drm/i915/psr: Export intel_psr_activate/exit()' and
> call PSR exit and activate from intel_psr_activate_block_get()/put()?
yeap.
>
> I dind't understand why you want to change struct mutex lock; to
> spinlock_t?
I'm not sure, but I believe that aux transactions can be called from
atomic areas protected with spin locks... if this assumption is true
than you cannot use any code that can sleep inside these areas, and
mutex can sleep. But in case no aux transaction is getting called
from atomic areas feel free to just ignore me ;)
>
> >
> > >
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > drivers/gpu/drm/i915/intel_psr.c | 54
> > > ++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 57 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 99af9169d792..41ebb144594e 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -609,6 +609,7 @@ struct i915_psr {
> > > bool has_hw_tracking;
> > > bool psr2_enabled;
> > > u8 sink_sync_latency;
> > > + unsigned int activate_block_count;
> > >
> > > void (*enable_source)(struct intel_dp *,
> > > const struct intel_crtc_state *);
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 70026b772721..020b96324135 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1893,6 +1893,8 @@ void intel_psr_compute_config(struct intel_dp
> > > *intel_dp,
> > > struct intel_crtc_state
> > > *crtc_state);
> > > void intel_psr_exit(struct intel_dp *intel_dp, bool wait_idle);
> > > void intel_psr_activate(struct intel_dp *intel_dp, bool schedule);
> > > +void intel_psr_activate_block_get(struct intel_dp *intel_dp);
> > > +void intel_psr_activate_block_put(struct intel_dp *intel_dp);
> > >
> > > /* intel_runtime_pm.c */
> > > int intel_power_domains_init(struct drm_i915_private *);
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 906a12ea934d..8702dbafb42d 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -558,6 +558,8 @@ static void __intel_psr_activate(struct
> > > intel_dp *intel_dp)
> > >
> > > WARN_ON(dev_priv->psr.active);
> > > lockdep_assert_held(&dev_priv->psr.lock);
> > > + if (dev_priv->psr.activate_block_count)
> > > + return;
> > >
> > > dev_priv->psr.activate(intel_dp);
> > > dev_priv->psr.active = true;
> > > @@ -1188,3 +1190,55 @@ void intel_psr_activate(struct intel_dp
> > > *intel_dp, bool schedule)
> > > out:
> > > mutex_unlock(&dev_priv->psr.lock);
> > > }
> > > +
> > > +/**
> > > + * intel_psr_activate_block_get - Block further attempts to
> > > activate PSR
> > > + * @intel_dp: DisplayPort that have PSR enabled
> > > + *
> > > + * It have a internal reference count, so each
> > > intel_psr_activate_block_get()
> > > + * should have a intel_psr_activate_block_put() counterpart.
> > > + */
> > > +void intel_psr_activate_block_get(struct intel_dp *intel_dp)
> > > +{
> > > + struct intel_digital_port *dig_port =
> > > dp_to_dig_port(intel_dp);
> > > + struct drm_device *dev = dig_port->base.base.dev;
> > > + struct drm_i915_private *dev_priv = to_i915(dev);
> > > +
> > > + if (!CAN_PSR(dev_priv))
> > > + return;
> > > +
> > > + mutex_lock(&dev_priv->psr.lock);
> > > + if (dev_priv->psr.enabled != intel_dp)
> > > + goto out;
> > > +
> > > + dev_priv->psr.activate_block_count++;
> > > +out:
> > > + mutex_unlock(&dev_priv->psr.lock);
> > > +}
> > > +
> > > +
> > > +/**
> > > + * intel_psr_activate_block_put - Unblock further attempts to
> > > activate PSR
> > > + * @intel_dp: DisplayPort that have PSR enabled
> > > + *
> > > + * Decrease the reference counter incremented by
> > > intel_psr_activate_block_get()
> > > + * when zero it allows PSR be activate. To actually activate PSR
> > > when reference
> > > + * counter is zero intel_psr_activate() should be called.
> > > + */
> > > +void intel_psr_activate_block_put(struct intel_dp *intel_dp)
> > > +{
> > > + struct intel_digital_port *dig_port =
> > > dp_to_dig_port(intel_dp);
> > > + struct drm_device *dev = dig_port->base.base.dev;
> > > + struct drm_i915_private *dev_priv = to_i915(dev);
> > > +
> > > + if (!CAN_PSR(dev_priv))
> > > + return;
> > > +
> > > + mutex_lock(&dev_priv->psr.lock);
> > > + if (dev_priv->psr.enabled != intel_dp)
> > > + goto out;
> > > +
> > > + dev_priv->psr.activate_block_count--;
> > > +out:
> > > + mutex_unlock(&dev_priv->psr.lock);
> > > +}
> > > --
> > > 2.16.3
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 04/11] drm/i915/psr: Remove intel_crtc_state parameter from disable()
2018-03-30 22:23 ` [PATCH 04/11] drm/i915/psr: Remove intel_crtc_state parameter from disable() José Roberto de Souza
2018-04-02 18:13 ` Rodrigo Vivi
@ 2018-04-03 17:52 ` Ville Syrjälä
1 sibling, 0 replies; 31+ messages in thread
From: Ville Syrjälä @ 2018-04-03 17:52 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan, Rodrigo Vivi
On Fri, Mar 30, 2018 at 03:23:29PM -0700, José Roberto de Souza wrote:
> It is not necessary as is possible to get the pipe information
> from intel_dp.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +--
> drivers/gpu/drm/i915/intel_psr.c | 13 ++++++-------
> 2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cb72ee27422f..99af9169d792 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -612,8 +612,7 @@ struct i915_psr {
>
> void (*enable_source)(struct intel_dp *,
> const struct intel_crtc_state *);
> - void (*disable_source)(struct intel_dp *,
> - const struct intel_crtc_state *);
> + void (*disable_source)(struct intel_dp *intel_dp);
> void (*enable_sink)(struct intel_dp *);
> void (*activate)(struct intel_dp *);
> void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index d3451afeb8bb..c4720b0152c3 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -665,24 +665,23 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> mutex_unlock(&dev_priv->psr.lock);
> }
>
> -static void vlv_psr_disable(struct intel_dp *intel_dp,
> - const struct intel_crtc_state *old_crtc_state)
> +static void vlv_psr_disable(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> + struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
That is a legacy pointer that should not be used anymore.
> + enum pipe pipe = to_intel_crtc(crtc)->pipe;
>
> if (dev_priv->psr.active) {
> dev_priv->psr.exit(intel_dp, true);
> dev_priv->psr.active = false;
> } else {
> - WARN_ON(vlv_is_psr_active_on_pipe(dev, crtc->pipe));
> + WARN_ON(vlv_is_psr_active_on_pipe(dev, pipe));
> }
> }
>
> -static void hsw_psr_disable(struct intel_dp *intel_dp,
> - const struct intel_crtc_state *old_crtc_state)
> +static void hsw_psr_disable(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = intel_dig_port->base.base.dev;
> @@ -727,7 +726,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> return;
> }
>
> - dev_priv->psr.disable_source(intel_dp, old_crtc_state);
> + dev_priv->psr.disable_source(intel_dp);
>
> /* Disable PSR on Sink */
> drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> --
> 2.16.3
>
> _______________________________________________
> 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] 31+ messages in thread
end of thread, other threads:[~2018-04-03 17:52 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-30 22:23 [PATCH 01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function José Roberto de Souza
2018-03-30 22:23 ` [PATCH 02/11] drm/i915/psr: Move PSR exit specific code to hardware specific function José Roberto de Souza
2018-04-02 18:09 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 03/11] drm/i915/psr: Share code between disable and exit José Roberto de Souza
2018-04-02 18:11 ` Rodrigo Vivi
2018-04-02 21:15 ` Souza, Jose
2018-03-30 22:23 ` [PATCH 04/11] drm/i915/psr: Remove intel_crtc_state parameter from disable() José Roberto de Souza
2018-04-02 18:13 ` Rodrigo Vivi
2018-04-03 17:52 ` Ville Syrjälä
2018-03-30 22:23 ` [PATCH 05/11] drm/i915/psr: Export intel_psr_activate/exit() José Roberto de Souza
2018-04-02 18:18 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 06/11] drm/i915/psr: Add intel_psr_activate_block_get()/put() José Roberto de Souza
2018-04-02 18:20 ` Rodrigo Vivi
2018-04-02 22:11 ` Souza, Jose
2018-04-03 17:32 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 07/11] drm/i915/dp: Exit PSR before do a aux channel transaction José Roberto de Souza
2018-04-02 18:23 ` Rodrigo Vivi
2018-04-02 18:42 ` Pandiyan, Dhinakaran
2018-04-02 22:16 ` Souza, Jose
2018-04-02 23:21 ` Souza, Jose
2018-04-02 19:00 ` Pandiyan, Dhinakaran
2018-04-02 22:15 ` Souza, Jose
2018-03-30 22:23 ` [PATCH 08/11] drm/i915: Keep IGT PSR and frontbuffer tests functional José Roberto de Souza
2018-04-02 18:26 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 09/11] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink José Roberto de Souza
2018-04-02 18:28 ` Rodrigo Vivi
2018-03-30 22:23 ` [PATCH 10/11] drm/i915/psr: Handle PSR RFB storage error José Roberto de Souza
2018-03-30 22:23 ` [PATCH 11/11] drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side José Roberto de Souza
2018-03-30 22:38 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/11] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function Patchwork
2018-03-30 22:53 ` ✗ Fi.CI.BAT: " Patchwork
2018-04-02 18:06 ` [PATCH 01/11] " Rodrigo Vivi
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.