* [PATCH v3 02/10] drm/i915/psr: Always wait for idle state when disabling PSR
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
@ 2018-10-26 1:17 ` José Roberto de Souza
2018-10-26 16:45 ` Rodrigo Vivi
2018-10-26 1:17 ` [PATCH v3 03/10] drm/i915/psr: Move intel_psr_disable_source() code to intel_psr_disable_locked() José Roberto de Souza
` (10 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: José Roberto de Souza @ 2018-10-26 1:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
It should always wait for idle state when disabling PSR because PSR
could be inactive due a call to intel_psr_exit() and while PSR is
still being disabled asynchronously userspace could change the
modeset causing a call to psr_disable() that will not wait for PSR
idle and then PSR will be enabled again while PSR is still not idle.
v2: rebased on top of the patch reusing psr_exit()
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/intel_psr.c | 41 ++++++++++++++------------------
1 file changed, 18 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index f698b3f45c6d..4c226b78c3fc 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -661,8 +661,12 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
{
u32 val;
- if (!dev_priv->psr.active)
+ if (!dev_priv->psr.active) {
+ if (INTEL_GEN(dev_priv) >= 9)
+ WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
+ WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
return;
+ }
if (dev_priv->psr.psr2_enabled) {
val = I915_READ(EDP_PSR2_CTL);
@@ -680,32 +684,23 @@ static void
intel_psr_disable_source(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+ i915_reg_t psr_status;
+ u32 psr_status_mask;
- if (dev_priv->psr.active) {
- i915_reg_t psr_status;
- u32 psr_status_mask;
-
- intel_psr_exit(dev_priv);
+ intel_psr_exit(dev_priv);
- if (dev_priv->psr.psr2_enabled) {
- psr_status = EDP_PSR2_STATUS;
- psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
- } else {
- psr_status = EDP_PSR_STATUS;
- psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
- }
-
- /* 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");
+ if (dev_priv->psr.psr2_enabled) {
+ psr_status = EDP_PSR2_STATUS;
+ psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
} else {
- 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);
+ psr_status = EDP_PSR_STATUS;
+ psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
}
+
+ /* Wait till PSR is idle */
+ if (intel_wait_for_register(dev_priv, psr_status, psr_status_mask, 0,
+ 2000))
+ DRM_ERROR("Timed out waiting PSR idle state\n");
}
static void intel_psr_disable_locked(struct intel_dp *intel_dp)
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 02/10] drm/i915/psr: Always wait for idle state when disabling PSR
2018-10-26 1:17 ` [PATCH v3 02/10] drm/i915/psr: Always wait for idle state when disabling PSR José Roberto de Souza
@ 2018-10-26 16:45 ` Rodrigo Vivi
2018-10-26 17:10 ` Souza, Jose
0 siblings, 1 reply; 23+ messages in thread
From: Rodrigo Vivi @ 2018-10-26 16:45 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Thu, Oct 25, 2018 at 06:17:29PM -0700, José Roberto de Souza wrote:
> It should always wait for idle state when disabling PSR because PSR
> could be inactive due a call to intel_psr_exit() and while PSR is
> still being disabled asynchronously userspace could change the
> modeset causing a call to psr_disable() that will not wait for PSR
> idle and then PSR will be enabled again while PSR is still not idle.
>
> v2: rebased on top of the patch reusing psr_exit()
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/intel_psr.c | 41 ++++++++++++++------------------
> 1 file changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index f698b3f45c6d..4c226b78c3fc 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -661,8 +661,12 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
> {
> u32 val;
>
> - if (!dev_priv->psr.active)
> + if (!dev_priv->psr.active) {
> + if (INTEL_GEN(dev_priv) >= 9)
This should be PSR2 check instead of platform no?!
> + WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> + WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> return;
> + }
>
> if (dev_priv->psr.psr2_enabled) {
> val = I915_READ(EDP_PSR2_CTL);
> @@ -680,32 +684,23 @@ static void
> intel_psr_disable_source(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> + i915_reg_t psr_status;
> + u32 psr_status_mask;
>
> - if (dev_priv->psr.active) {
> - i915_reg_t psr_status;
> - u32 psr_status_mask;
> -
> - intel_psr_exit(dev_priv);
> + intel_psr_exit(dev_priv);
>
> - if (dev_priv->psr.psr2_enabled) {
> - psr_status = EDP_PSR2_STATUS;
> - psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> - } else {
> - psr_status = EDP_PSR_STATUS;
> - psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> - }
> -
> - /* 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");
> + if (dev_priv->psr.psr2_enabled) {
> + psr_status = EDP_PSR2_STATUS;
> + psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> } else {
> - 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);
> + psr_status = EDP_PSR_STATUS;
> + psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> }
> +
> + /* Wait till PSR is idle */
> + if (intel_wait_for_register(dev_priv, psr_status, psr_status_mask, 0,
> + 2000))
> + DRM_ERROR("Timed out waiting PSR idle state\n");
> }
>
> static void intel_psr_disable_locked(struct intel_dp *intel_dp)
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 02/10] drm/i915/psr: Always wait for idle state when disabling PSR
2018-10-26 16:45 ` Rodrigo Vivi
@ 2018-10-26 17:10 ` Souza, Jose
0 siblings, 0 replies; 23+ messages in thread
From: Souza, Jose @ 2018-10-26 17:10 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx@lists.freedesktop.org, Pandiyan, Dhinakaran
On Fri, 2018-10-26 at 09:45 -0700, Rodrigo Vivi wrote:
> On Thu, Oct 25, 2018 at 06:17:29PM -0700, José Roberto de Souza
> wrote:
> > It should always wait for idle state when disabling PSR because PSR
> > could be inactive due a call to intel_psr_exit() and while PSR is
> > still being disabled asynchronously userspace could change the
> > modeset causing a call to psr_disable() that will not wait for PSR
> > idle and then PSR will be enabled again while PSR is still not
> > idle.
> >
> > v2: rebased on top of the patch reusing psr_exit()
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_psr.c | 41 ++++++++++++++------------
> > ------
> > 1 file changed, 18 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index f698b3f45c6d..4c226b78c3fc 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -661,8 +661,12 @@ static void intel_psr_exit(struct
> > drm_i915_private *dev_priv)
> > {
> > u32 val;
> >
> > - if (!dev_priv->psr.active)
> > + if (!dev_priv->psr.active) {
> > + if (INTEL_GEN(dev_priv) >= 9)
>
> This should be PSR2 check instead of platform no?!
I did not changed this because this code was just moved but yes we
should check if psr2 is enabled.
>
> > + WARN_ON(I915_READ(EDP_PSR2_CTL) &
> > EDP_PSR2_ENABLE);
> > + WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > return;
> > + }
> >
> > if (dev_priv->psr.psr2_enabled) {
> > val = I915_READ(EDP_PSR2_CTL);
> > @@ -680,32 +684,23 @@ static void
> > intel_psr_disable_source(struct intel_dp *intel_dp)
> > {
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > + i915_reg_t psr_status;
> > + u32 psr_status_mask;
> >
> > - if (dev_priv->psr.active) {
> > - i915_reg_t psr_status;
> > - u32 psr_status_mask;
> > -
> > - intel_psr_exit(dev_priv);
> > + intel_psr_exit(dev_priv);
> >
> > - if (dev_priv->psr.psr2_enabled) {
> > - psr_status = EDP_PSR2_STATUS;
> > - psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> > - } else {
> > - psr_status = EDP_PSR_STATUS;
> > - psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> > - }
> > -
> > - /* 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");
> > + if (dev_priv->psr.psr2_enabled) {
> > + psr_status = EDP_PSR2_STATUS;
> > + psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> > } else {
> > - 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);
> > + psr_status = EDP_PSR_STATUS;
> > + psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> > }
> > +
> > + /* Wait till PSR is idle */
> > + if (intel_wait_for_register(dev_priv, psr_status,
> > psr_status_mask, 0,
> > + 2000))
> > + DRM_ERROR("Timed out waiting PSR idle state\n");
> > }
> >
> > static void intel_psr_disable_locked(struct intel_dp *intel_dp)
> > --
> > 2.19.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 03/10] drm/i915/psr: Move intel_psr_disable_source() code to intel_psr_disable_locked()
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
2018-10-26 1:17 ` [PATCH v3 02/10] drm/i915/psr: Always wait for idle state when disabling PSR José Roberto de Souza
@ 2018-10-26 1:17 ` José Roberto de Souza
2018-10-26 1:17 ` [PATCH v3 04/10] drm/i915: Avoid a full port detection in the first eDP short pulse José Roberto de Souza
` (9 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: José Roberto de Souza @ 2018-10-26 1:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
In the past we had hooks to configure HW for VLV/CHV too, in the drop
of VLV/CHV support the intel_psr_disable_source() code was not moved
to the caller, so doing it here.
Suggested-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/intel_psr.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 4c226b78c3fc..2d692ad41bf9 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -680,13 +680,20 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
dev_priv->psr.active = false;
}
-static void
-intel_psr_disable_source(struct intel_dp *intel_dp)
+static void intel_psr_disable_locked(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
i915_reg_t psr_status;
u32 psr_status_mask;
+ lockdep_assert_held(&dev_priv->psr.lock);
+
+ if (!dev_priv->psr.enabled)
+ return;
+
+ DRM_DEBUG_KMS("Disabling PSR%s\n",
+ dev_priv->psr.psr2_enabled ? "2" : "1");
+
intel_psr_exit(dev_priv);
if (dev_priv->psr.psr2_enabled) {
@@ -701,20 +708,6 @@ intel_psr_disable_source(struct intel_dp *intel_dp)
if (intel_wait_for_register(dev_priv, psr_status, psr_status_mask, 0,
2000))
DRM_ERROR("Timed out waiting PSR idle state\n");
-}
-
-static void intel_psr_disable_locked(struct intel_dp *intel_dp)
-{
- struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-
- lockdep_assert_held(&dev_priv->psr.lock);
-
- if (!dev_priv->psr.enabled)
- return;
-
- DRM_DEBUG_KMS("Disabling PSR%s\n",
- dev_priv->psr.psr2_enabled ? "2" : "1");
- intel_psr_disable_source(intel_dp);
/* Disable PSR on Sink */
drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 04/10] drm/i915: Avoid a full port detection in the first eDP short pulse
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
2018-10-26 1:17 ` [PATCH v3 02/10] drm/i915/psr: Always wait for idle state when disabling PSR José Roberto de Souza
2018-10-26 1:17 ` [PATCH v3 03/10] drm/i915/psr: Move intel_psr_disable_source() code to intel_psr_disable_locked() José Roberto de Souza
@ 2018-10-26 1:17 ` José Roberto de Souza
2018-10-26 1:17 ` [PATCH v3 05/10] drm/i915: Check PSR errors instead of retrain while PSR is enabled José Roberto de Souza
` (8 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: José Roberto de Souza @ 2018-10-26 1:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
Some eDP panels do not set a valid sink count value and even for the
ones that sets is should always be one for eDP, that is why it is not
cached in intel_edp_init_dpcd().
But intel_dp_short_pulse() compares the old count with the read one
if there is a mistmatch a full port detection will be executed, what
was happening in the first short pulse interruption of eDP panels
that sets sink count.
Instead of just skip the compasison for eDP panels, lets not read
the sink count at all for eDP.
v2: the previous version of this patch was caching the sink count
in intel_edp_init_dpcd() but I was pointed out by Ville a patch that
handled a case of a eDP panel that do not set sink count
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 44 +++++++++++++++++++--------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8e64f149ab09..fce67094c2a0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4039,8 +4039,6 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
static bool
intel_dp_get_dpcd(struct intel_dp *intel_dp)
{
- u8 sink_count;
-
if (!intel_dp_read_dpcd(intel_dp))
return false;
@@ -4050,25 +4048,35 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
intel_dp_set_common_rates(intel_dp);
}
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_COUNT, &sink_count) <= 0)
- return false;
-
/*
- * Sink count can change between short pulse hpd hence
- * a member variable in intel_dp will track any changes
- * between short pulse interrupts.
+ * Some eDP panels do not set a valid value for sink count, that is why
+ * it don't care about read it here and in intel_edp_init_dpcd().
*/
- intel_dp->sink_count = DP_GET_SINK_COUNT(sink_count);
+ if (!intel_dp_is_edp(intel_dp)) {
+ u8 count;
+ ssize_t r;
- /*
- * SINK_COUNT == 0 and DOWNSTREAM_PORT_PRESENT == 1 implies that
- * a dongle is present but no display. Unless we require to know
- * if a dongle is present or not, we don't need to update
- * downstream port information. So, an early return here saves
- * time from performing other operations which are not required.
- */
- if (!intel_dp_is_edp(intel_dp) && !intel_dp->sink_count)
- return false;
+ r = drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_COUNT, &count);
+ if (r < 1)
+ return false;
+
+ /*
+ * Sink count can change between short pulse hpd hence
+ * a member variable in intel_dp will track any changes
+ * between short pulse interrupts.
+ */
+ intel_dp->sink_count = DP_GET_SINK_COUNT(count);
+
+ /*
+ * SINK_COUNT == 0 and DOWNSTREAM_PORT_PRESENT == 1 implies that
+ * a dongle is present but no display. Unless we require to know
+ * if a dongle is present or not, we don't need to update
+ * downstream port information. So, an early return here saves
+ * time from performing other operations which are not required.
+ */
+ if (!intel_dp->sink_count)
+ return false;
+ }
if (!drm_dp_is_branch(intel_dp->dpcd))
return true; /* native DP sink */
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 05/10] drm/i915: Check PSR errors instead of retrain while PSR is enabled
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
` (2 preceding siblings ...)
2018-10-26 1:17 ` [PATCH v3 04/10] drm/i915: Avoid a full port detection in the first eDP short pulse José Roberto de Souza
@ 2018-10-26 1:17 ` José Roberto de Souza
2018-10-26 1:17 ` [PATCH v3 06/10] drm/i915: Unmask PSR interruptions before assert IIR José Roberto de Souza
` (7 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: José Roberto de Souza @ 2018-10-26 1:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
When a PSR error happens sink sets the PSR errors register and also
set the link to a error status.
So in the short pulse handling it was returning earlier and doing a
full detection and attempting to retrain but it fails as PSR HW is
in change of the main-link.
Just call intel_psr_short_pulse() before
intel_dp_needs_link_retrain() is not the right fix as
intel_dp_needs_link_retrain() would return true and trigger a full
detection while PSR HW is still in change of main-link.
Check for PSR active is also not safe as it could be inactive due a
frontbuffer invalidate and still doing the PSR exit sequence.
v3: added comment in intel_dp_needs_link_retrain()
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 11 +++++++++++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_psr.c | 15 +++++++++++++++
3 files changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fce67094c2a0..9bf3a128adb3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4401,6 +4401,17 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
if (!intel_dp->link_trained)
return false;
+ /*
+ * While PSR source HW is enabled, it will control main-link sending
+ * frames, enabling and disabling it so trying to do a retrain will fail
+ * as the link would or not be on or it could mix training patterns
+ * and frame data at the same time causing retrain to fail.
+ * Also when exiting PSR, HW will retrain the link anyways fixing
+ * any link status error.
+ */
+ if (intel_psr_enabled(intel_dp))
+ return false;
+
if (!intel_dp_get_link_status(intel_dp, link_status))
return false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index db24308729b4..ce89c92eaf0a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2031,6 +2031,7 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
void intel_psr_short_pulse(struct intel_dp *intel_dp);
int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
u32 *out_value);
+bool intel_psr_enabled(struct intel_dp *intel_dp);
/* intel_quirks.c */
void intel_init_quirks(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2d692ad41bf9..08b5351e4efb 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -1107,3 +1107,18 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
exit:
mutex_unlock(&psr->lock);
}
+
+bool intel_psr_enabled(struct intel_dp *intel_dp)
+{
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+ bool ret;
+
+ if (!CAN_PSR(dev_priv) || !intel_dp_is_edp(intel_dp))
+ return false;
+
+ mutex_lock(&dev_priv->psr.lock);
+ ret = (dev_priv->psr.dp == intel_dp && dev_priv->psr.enabled);
+ mutex_unlock(&dev_priv->psr.lock);
+
+ return ret;
+}
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 06/10] drm/i915: Unmask PSR interruptions before assert IIR
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
` (3 preceding siblings ...)
2018-10-26 1:17 ` [PATCH v3 05/10] drm/i915: Check PSR errors instead of retrain while PSR is enabled José Roberto de Souza
@ 2018-10-26 1:17 ` José Roberto de Souza
2018-10-26 17:06 ` Ville Syrjälä
2018-10-26 1:17 ` [PATCH v3 07/10] drm/i915/icl: Reset PSR interruptions José Roberto de Souza
` (6 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: José Roberto de Souza @ 2018-10-26 1:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
The IIR register is a result of a AND operation between the mask
register and the actual interruption state so checking IIR before
unmask interruptions will never get any errors even if they exits.
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5d1f53723388..21756e9a7523 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4086,8 +4086,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
}
if (IS_HASWELL(dev_priv)) {
- gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
+ gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
display_mask |= DE_EDP_PSR_INT_HSW;
}
@@ -4232,8 +4232,8 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
else if (IS_BROADWELL(dev_priv))
de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
- gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
+ gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
for_each_pipe(dev_priv, pipe) {
dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 06/10] drm/i915: Unmask PSR interruptions before assert IIR
2018-10-26 1:17 ` [PATCH v3 06/10] drm/i915: Unmask PSR interruptions before assert IIR José Roberto de Souza
@ 2018-10-26 17:06 ` Ville Syrjälä
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2018-10-26 17:06 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Thu, Oct 25, 2018 at 06:17:33PM -0700, José Roberto de Souza wrote:
> The IIR register is a result of a AND operation between the mask
> register and the actual interruption state so checking IIR before
> unmask interruptions will never get any errors even if they exits.
IIR is the latched state. The only thing we want to assert is that we
didn't leave anything in IIR after we masked via IMR. Just like
GEN3_IRQ_INIT() & co.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 5d1f53723388..21756e9a7523 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4086,8 +4086,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
> }
>
> if (IS_HASWELL(dev_priv)) {
> - gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
> + gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> display_mask |= DE_EDP_PSR_INT_HSW;
> }
>
> @@ -4232,8 +4232,8 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> else if (IS_BROADWELL(dev_priv))
> de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
>
> - gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
> + gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
>
> for_each_pipe(dev_priv, pipe) {
> dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 07/10] drm/i915/icl: Reset PSR interruptions
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
` (4 preceding siblings ...)
2018-10-26 1:17 ` [PATCH v3 06/10] drm/i915: Unmask PSR interruptions before assert IIR José Roberto de Souza
@ 2018-10-26 1:17 ` José Roberto de Souza
2018-10-26 17:13 ` Ville Syrjälä
2018-10-26 1:17 ` [PATCH v3 08/10] drm/i915: Disable PSR when a PSR aux error happen José Roberto de Souza
` (5 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: José Roberto de Souza @ 2018-10-26 1:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni
All other interruptions gen11 interruptions are reset in
gen11_irq_reset() also it is done for other gens that supports PSR.
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 21756e9a7523..09f7723dbd5c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3679,6 +3679,9 @@ static void gen11_irq_reset(struct drm_device *dev)
I915_WRITE(GEN11_DISPLAY_INT_CTL, 0);
+ I915_WRITE(EDP_PSR_IMR, 0xffffffff);
+ I915_WRITE(EDP_PSR_IIR, 0xffffffff);
+
for_each_pipe(dev_priv, pipe)
if (intel_display_power_is_enabled(dev_priv,
POWER_DOMAIN_PIPE(pipe)))
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 07/10] drm/i915/icl: Reset PSR interruptions
2018-10-26 1:17 ` [PATCH v3 07/10] drm/i915/icl: Reset PSR interruptions José Roberto de Souza
@ 2018-10-26 17:13 ` Ville Syrjälä
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2018-10-26 17:13 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan, Paulo Zanoni
On Thu, Oct 25, 2018 at 06:17:34PM -0700, José Roberto de Souza wrote:
> All other interruptions gen11 interruptions are reset in
> gen11_irq_reset() also it is done for other gens that supports PSR.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 21756e9a7523..09f7723dbd5c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3679,6 +3679,9 @@ static void gen11_irq_reset(struct drm_device *dev)
>
> I915_WRITE(GEN11_DISPLAY_INT_CTL, 0);
>
> + I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> + I915_WRITE(EDP_PSR_IIR, 0xffffffff);
> +
Might we want a gen8_de_irq_reset() ?
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> for_each_pipe(dev_priv, pipe)
> if (intel_display_power_is_enabled(dev_priv,
> POWER_DOMAIN_PIPE(pipe)))
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 08/10] drm/i915: Disable PSR when a PSR aux error happen
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
` (5 preceding siblings ...)
2018-10-26 1:17 ` [PATCH v3 07/10] drm/i915/icl: Reset PSR interruptions José Roberto de Souza
@ 2018-10-26 1:17 ` José Roberto de Souza
2018-10-26 1:17 ` [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error José Roberto de Souza
` (4 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: José Roberto de Souza @ 2018-10-26 1:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
While PSR is active hardware will do aux transactions by it self to
wakeup sink to receive a new frame when necessary. If that
transaction is not acked by sink, hardware will trigger this
interruption.
So let's disable PSR as it is a hint that there is problem with this
sink.
The removed FIXME was asking to manually train the link but we don't
need to do that as by spec sink should do a short pulse when it is
out of sync with source, we just need to make sure it is awaken and
the SDP header with PSR disable will trigger this condition.
v3: added workarround to fix scheduled work starvation cause by
to frequent PSR error interruption
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_psr.c | 53 +++++++++++++++++++++++++++++---
2 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2d7761b8ac07..2a2574f34b4a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -638,6 +638,7 @@ struct i915_psr {
u8 sink_sync_latency;
ktime_t last_entry_attempt;
ktime_t last_exit;
+ u32 irq_aux_error;
};
enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 08b5351e4efb..68201cc24d25 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -152,6 +152,7 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
u32 transcoders = BIT(TRANSCODER_EDP);
enum transcoder cpu_transcoder;
ktime_t time_ns = ktime_get();
+ u32 mask = 0;
if (INTEL_GEN(dev_priv) >= 8)
transcoders |= BIT(TRANSCODER_A) |
@@ -159,10 +160,23 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
BIT(TRANSCODER_C);
for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
- /* FIXME: Exit PSR and link train manually when this happens. */
- if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
- DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
- transcoder_name(cpu_transcoder));
+ if (psr_iir & EDP_PSR_ERROR(cpu_transcoder)) {
+ DRM_WARN("[transcoder %s] PSR aux error\n",
+ transcoder_name(cpu_transcoder));
+
+ spin_lock(&dev_priv->irq_lock);
+ dev_priv->psr.irq_aux_error |= BIT(cpu_transcoder);
+ spin_unlock(&dev_priv->irq_lock);
+
+ /*
+ * If this interruption is not masked it will keep
+ * interrupting so fast that it prevents the scheduled
+ * work to run.
+ * Also after a PSR error, we don't want to arm PSR
+ * again so we don't care about unmask the interruption.
+ */
+ mask |= EDP_PSR_ERROR(cpu_transcoder);
+ }
if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
dev_priv->psr.last_entry_attempt = time_ns;
@@ -184,6 +198,13 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
}
}
}
+
+ if (mask) {
+ mask |= I915_READ(EDP_PSR_IMR);
+ I915_WRITE(EDP_PSR_IMR, mask);
+
+ schedule_work(&dev_priv->psr.work);
+ }
}
static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
@@ -893,11 +914,35 @@ int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
return ret;
}
+static void intel_psr_handle_irq(struct drm_i915_private *dev_priv)
+{
+ struct i915_psr *psr = &dev_priv->psr;
+ u32 irq_aux_error;
+
+ spin_lock_irq(&dev_priv->irq_lock);
+ irq_aux_error = psr->irq_aux_error;
+ psr->irq_aux_error = 0;
+ spin_unlock_irq(&dev_priv->irq_lock);
+
+ WARN_ON(irq_aux_error & ~BIT(TRANSCODER_EDP));
+
+ mutex_lock(&psr->lock);
+
+ intel_psr_disable_locked(psr->dp);
+ /* let's make sure that sink is awaken */
+ drm_dp_dpcd_writeb(&psr->dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
+
+ mutex_unlock(&dev_priv->psr.lock);
+}
+
static void intel_psr_work(struct work_struct *work)
{
struct drm_i915_private *dev_priv =
container_of(work, typeof(*dev_priv), psr.work);
+ if (READ_ONCE(dev_priv->psr.irq_aux_error))
+ intel_psr_handle_irq(dev_priv);
+
mutex_lock(&dev_priv->psr.lock);
if (!dev_priv->psr.enabled)
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
` (6 preceding siblings ...)
2018-10-26 1:17 ` [PATCH v3 08/10] drm/i915: Disable PSR when a PSR aux error happen José Roberto de Souza
@ 2018-10-26 1:17 ` José Roberto de Souza
2018-10-26 17:53 ` Souza, Jose
2018-10-31 23:45 ` Dhinakaran Pandiyan
2018-10-26 1:17 ` [PATCH v3 10/10] drm/i915: Do not enable PSR in the next modeset after a error José Roberto de Souza
` (3 subsequent siblings)
11 siblings, 2 replies; 23+ messages in thread
From: José Roberto de Souza @ 2018-10-26 1:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
will still keep the error set even after the reset done in the
irq_preinstall and irq_uninstall hooks.
And enabling in this situation cause the screen to freeze in the
first time that PSR HW tries to activate so lets keep PSR disabled
to avoid any rendering problems.
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/intel_psr.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 68201cc24d25..718270da1061 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -529,6 +529,19 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
return;
}
+ /*
+ * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
+ * will still keep the error set even after the reset done in the
+ * irq_preinstall and irq_uninstall hooks.
+ * And enabling in this situation cause the screen to freeze in the
+ * first time that PSR HW tries to activate so lets keep PSR disabled
+ * to avoid any rendering problems.
+ */
+ if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP)) {
+ DRM_DEBUG_KMS("PSR interruption error set\n");
+ return;
+ }
+
if (IS_HASWELL(dev_priv) &&
I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
S3D_ENABLE) {
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error
2018-10-26 1:17 ` [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error José Roberto de Souza
@ 2018-10-26 17:53 ` Souza, Jose
2018-10-26 18:01 ` Ville Syrjälä
2018-10-31 23:45 ` Dhinakaran Pandiyan
1 sibling, 1 reply; 23+ messages in thread
From: Souza, Jose @ 2018-10-26 17:53 UTC (permalink / raw)
To: intel-gfx@lists.freedesktop.org
On Thu, 2018-10-25 at 18:17 -0700, José Roberto de Souza wrote:
> If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> will still keep the error set even after the reset done in the
> irq_preinstall and irq_uninstall hooks.
> And enabling in this situation cause the screen to freeze in the
> first time that PSR HW tries to activate so lets keep PSR disabled
> to avoid any rendering problems.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/intel_psr.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 68201cc24d25..718270da1061 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -529,6 +529,19 @@ void intel_psr_compute_config(struct intel_dp
> *intel_dp,
> return;
> }
>
> + /*
> + * If a PSR error happened and the driver is reloaded, the
> EDP_PSR_IIR
> + * will still keep the error set even after the reset done in
> the
> + * irq_preinstall and irq_uninstall hooks.
> + * And enabling in this situation cause the screen to freeze in
> the
> + * first time that PSR HW tries to activate so lets keep PSR
> disabled
> + * to avoid any rendering problems.
> + */
> + if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP)) {
> + DRM_DEBUG_KMS("PSR interruption error set\n");
> + return;
> + }
> +
> if (IS_HASWELL(dev_priv) &&
> I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
> S3D_ENABLE) {
diff --git a/drivers/gpu/drm/i915/intel_psr.c
b/drivers/gpu/drm/i915/intel_psr.c
index 68201cc24d25..47af87f45e03 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -529,6 +529,22 @@ void intel_psr_compute_config(struct intel_dp
*intel_dp,
return;
}
+ /*
+ * If a PSR error happened and the driver is reloaded, the
EDP_PSR_IIR
+ * will still keep the error set even after the reset done in
the
+ * irq_preinstall and irq_uninstall hooks.
+ * And enabling in this situation cause the screen to freeze in
the
+ * first time that PSR HW tries to activate so lets keep PSR
disabled
+ * to avoid any rendering problems.
+ */
+ intel_runtime_pm_get(dev_priv);
+ if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP)) {
+ intel_runtime_pm_put(dev_priv);
+ DRM_DEBUG_KMS("PSR interruption error set\n");
+ return;
+ }
+ intel_runtime_pm_put(dev_priv);
+
if (IS_HASWELL(dev_priv) &&
I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
S3D_ENABLE) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error
2018-10-26 17:53 ` Souza, Jose
@ 2018-10-26 18:01 ` Ville Syrjälä
2018-10-26 19:34 ` Souza, Jose
0 siblings, 1 reply; 23+ messages in thread
From: Ville Syrjälä @ 2018-10-26 18:01 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org
On Fri, Oct 26, 2018 at 05:53:47PM +0000, Souza, Jose wrote:
> On Thu, 2018-10-25 at 18:17 -0700, José Roberto de Souza wrote:
> > If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> > will still keep the error set even after the reset done in the
> > irq_preinstall and irq_uninstall hooks.
> > And enabling in this situation cause the screen to freeze in the
> > first time that PSR HW tries to activate so lets keep PSR disabled
> > to avoid any rendering problems.
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_psr.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 68201cc24d25..718270da1061 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -529,6 +529,19 @@ void intel_psr_compute_config(struct intel_dp
> > *intel_dp,
> > return;
> > }
> >
> > + /*
> > + * If a PSR error happened and the driver is reloaded, the
> > EDP_PSR_IIR
> > + * will still keep the error set even after the reset done in
> > the
> > + * irq_preinstall and irq_uninstall hooks.
> > + * And enabling in this situation cause the screen to freeze in
> > the
> > + * first time that PSR HW tries to activate so lets keep PSR
> > disabled
> > + * to avoid any rendering problems.
> > + */
> > + if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP)) {
> > + DRM_DEBUG_KMS("PSR interruption error set\n");
> > + return;
> > + }
> > +
> > if (IS_HASWELL(dev_priv) &&
> > I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
> > S3D_ENABLE) {
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 68201cc24d25..47af87f45e03 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -529,6 +529,22 @@ void intel_psr_compute_config(struct intel_dp
> *intel_dp,
> return;
> }
>
> + /*
> + * If a PSR error happened and the driver is reloaded, the
> EDP_PSR_IIR
> + * will still keep the error set even after the reset done in
> the
> + * irq_preinstall and irq_uninstall hooks.
> + * And enabling in this situation cause the screen to freeze in
> the
> + * first time that PSR HW tries to activate so lets keep PSR
> disabled
> + * to avoid any rendering problems.
> + */
> + intel_runtime_pm_get(dev_priv);
> + if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP)) {
> + intel_runtime_pm_put(dev_priv);
> + DRM_DEBUG_KMS("PSR interruption error set\n");
> + return;
> + }
> + intel_runtime_pm_put(dev_priv);
No hardware access in compute_config()
> +
> if (IS_HASWELL(dev_priv) &&
> I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
> S3D_ENABLE) {
^
This is nonsense. Don't follow the bad example. Also pls send a patch to
kill this thing ;)
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error
2018-10-26 18:01 ` Ville Syrjälä
@ 2018-10-26 19:34 ` Souza, Jose
2018-10-29 11:18 ` Ville Syrjälä
0 siblings, 1 reply; 23+ messages in thread
From: Souza, Jose @ 2018-10-26 19:34 UTC (permalink / raw)
To: ville.syrjala@linux.intel.com; +Cc: intel-gfx@lists.freedesktop.org
On Fri, 2018-10-26 at 21:01 +0300, Ville Syrjälä wrote:
> On Fri, Oct 26, 2018 at 05:53:47PM +0000, Souza, Jose wrote:
> > On Thu, 2018-10-25 at 18:17 -0700, José Roberto de Souza wrote:
> > > If a PSR error happened and the driver is reloaded, the
> > > EDP_PSR_IIR
> > > will still keep the error set even after the reset done in the
> > > irq_preinstall and irq_uninstall hooks.
> > > And enabling in this situation cause the screen to freeze in the
> > > first time that PSR HW tries to activate so lets keep PSR
> > > disabled
> > > to avoid any rendering problems.
> > >
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_psr.c | 13 +++++++++++++
> > > 1 file changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 68201cc24d25..718270da1061 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -529,6 +529,19 @@ void intel_psr_compute_config(struct
> > > intel_dp
> > > *intel_dp,
> > > return;
> > > }
> > >
> > > + /*
> > > + * If a PSR error happened and the driver is reloaded, the
> > > EDP_PSR_IIR
> > > + * will still keep the error set even after the reset done in
> > > the
> > > + * irq_preinstall and irq_uninstall hooks.
> > > + * And enabling in this situation cause the screen to freeze in
> > > the
> > > + * first time that PSR HW tries to activate so lets keep PSR
> > > disabled
> > > + * to avoid any rendering problems.
> > > + */
> > > + if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP)) {
> > > + DRM_DEBUG_KMS("PSR interruption error set\n");
> > > + return;
> > > + }
> > > +
> > > if (IS_HASWELL(dev_priv) &&
> > > I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
> > > S3D_ENABLE) {
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 68201cc24d25..47af87f45e03 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -529,6 +529,22 @@ void intel_psr_compute_config(struct intel_dp
> > *intel_dp,
> > return;
> > }
> >
> > + /*
> > + * If a PSR error happened and the driver is reloaded, the
> > EDP_PSR_IIR
> > + * will still keep the error set even after the reset done
> > in
> > the
> > + * irq_preinstall and irq_uninstall hooks.
> > + * And enabling in this situation cause the screen to
> > freeze in
> > the
> > + * first time that PSR HW tries to activate so lets keep
> > PSR
> > disabled
> > + * to avoid any rendering problems.
> > + */
> > + intel_runtime_pm_get(dev_priv);
> > + if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP))
> > {
> > + intel_runtime_pm_put(dev_priv);
> > + DRM_DEBUG_KMS("PSR interruption error set\n");
> > + return;
> > + }
> > + intel_runtime_pm_put(dev_priv);
>
> No hardware access in compute_config()
Oh did not know that, I will fix that.
Thanks
>
> > +
> > if (IS_HASWELL(dev_priv) &&
> > I915_READ(HSW_STEREO_3D_CTL(crtc_state-
> > >cpu_transcoder)) &
> > S3D_ENABLE) {
>
> ^
> This is nonsense. Don't follow the bad example. Also pls send a patch
> to
> kill this thing ;)
Sure, I can't find any place setting HSW_STEREO_3D_CTL so I will just
remove it.
I guess we don't support stereo 3D at all.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error
2018-10-26 19:34 ` Souza, Jose
@ 2018-10-29 11:18 ` Ville Syrjälä
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2018-10-29 11:18 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org
On Fri, Oct 26, 2018 at 07:34:42PM +0000, Souza, Jose wrote:
> On Fri, 2018-10-26 at 21:01 +0300, Ville Syrjälä wrote:
> > On Fri, Oct 26, 2018 at 05:53:47PM +0000, Souza, Jose wrote:
> > > On Thu, 2018-10-25 at 18:17 -0700, José Roberto de Souza wrote:
> > > > If a PSR error happened and the driver is reloaded, the
> > > > EDP_PSR_IIR
> > > > will still keep the error set even after the reset done in the
> > > > irq_preinstall and irq_uninstall hooks.
> > > > And enabling in this situation cause the screen to freeze in the
> > > > first time that PSR HW tries to activate so lets keep PSR
> > > > disabled
> > > > to avoid any rendering problems.
> > > >
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_psr.c | 13 +++++++++++++
> > > > 1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index 68201cc24d25..718270da1061 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -529,6 +529,19 @@ void intel_psr_compute_config(struct
> > > > intel_dp
> > > > *intel_dp,
> > > > return;
> > > > }
> > > >
> > > > + /*
> > > > + * If a PSR error happened and the driver is reloaded, the
> > > > EDP_PSR_IIR
> > > > + * will still keep the error set even after the reset done in
> > > > the
> > > > + * irq_preinstall and irq_uninstall hooks.
> > > > + * And enabling in this situation cause the screen to freeze in
> > > > the
> > > > + * first time that PSR HW tries to activate so lets keep PSR
> > > > disabled
> > > > + * to avoid any rendering problems.
> > > > + */
> > > > + if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP)) {
> > > > + DRM_DEBUG_KMS("PSR interruption error set\n");
> > > > + return;
> > > > + }
> > > > +
> > > > if (IS_HASWELL(dev_priv) &&
> > > > I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
> > > > S3D_ENABLE) {
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 68201cc24d25..47af87f45e03 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -529,6 +529,22 @@ void intel_psr_compute_config(struct intel_dp
> > > *intel_dp,
> > > return;
> > > }
> > >
> > > + /*
> > > + * If a PSR error happened and the driver is reloaded, the
> > > EDP_PSR_IIR
> > > + * will still keep the error set even after the reset done
> > > in
> > > the
> > > + * irq_preinstall and irq_uninstall hooks.
> > > + * And enabling in this situation cause the screen to
> > > freeze in
> > > the
> > > + * first time that PSR HW tries to activate so lets keep
> > > PSR
> > > disabled
> > > + * to avoid any rendering problems.
> > > + */
> > > + intel_runtime_pm_get(dev_priv);
> > > + if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP))
> > > {
> > > + intel_runtime_pm_put(dev_priv);
> > > + DRM_DEBUG_KMS("PSR interruption error set\n");
> > > + return;
> > > + }
> > > + intel_runtime_pm_put(dev_priv);
> >
> > No hardware access in compute_config()
>
> Oh did not know that, I will fix that.
> Thanks
>
> >
> > > +
> > > if (IS_HASWELL(dev_priv) &&
> > > I915_READ(HSW_STEREO_3D_CTL(crtc_state-
> > > >cpu_transcoder)) &
> > > S3D_ENABLE) {
> >
> > ^
> > This is nonsense. Don't follow the bad example. Also pls send a patch
> > to
> > kill this thing ;)
>
> Sure, I can't find any place setting HSW_STEREO_3D_CTL so I will just
> remove it.
> I guess we don't support stereo 3D at all.
Not this form of it.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error
2018-10-26 1:17 ` [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error José Roberto de Souza
2018-10-26 17:53 ` Souza, Jose
@ 2018-10-31 23:45 ` Dhinakaran Pandiyan
2018-11-06 0:00 ` Souza, Jose
1 sibling, 1 reply; 23+ messages in thread
From: Dhinakaran Pandiyan @ 2018-10-31 23:45 UTC (permalink / raw)
To: José Roberto de Souza, intel-gfx
On Thu, 2018-10-25 at 18:17 -0700, José Roberto de Souza wrote:
> If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> will still keep the error set even after the reset done in the
> irq_preinstall and irq_uninstall hooks.
Does this happen or are you suspecting it might? Is this because IIR
clearing did not work or is it because of a new unhandled interrupt?
-DK
> And enabling in this situation cause the screen to freeze in the
> first time that PSR HW tries to activate so lets keep PSR disabled
> to avoid any rendering problems.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/intel_psr.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 68201cc24d25..718270da1061 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -529,6 +529,19 @@ void intel_psr_compute_config(struct intel_dp
> *intel_dp,
> return;
> }
>
> + /*
> + * If a PSR error happened and the driver is reloaded, the
> EDP_PSR_IIR
> + * will still keep the error set even after the reset done in
> the
> + * irq_preinstall and irq_uninstall hooks.
> + * And enabling in this situation cause the screen to freeze in
> the
> + * first time that PSR HW tries to activate so lets keep PSR
> disabled
> + * to avoid any rendering problems.
> + */
> + if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP)) {
> + DRM_DEBUG_KMS("PSR interruption error set\n");
> + return;
> + }
> +
> if (IS_HASWELL(dev_priv) &&
> I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
> S3D_ENABLE) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error
2018-10-31 23:45 ` Dhinakaran Pandiyan
@ 2018-11-06 0:00 ` Souza, Jose
0 siblings, 0 replies; 23+ messages in thread
From: Souza, Jose @ 2018-11-06 0:00 UTC (permalink / raw)
To: intel-gfx@lists.freedesktop.org, Pandiyan, Dhinakaran
On Wed, 2018-10-31 at 16:45 -0700, Dhinakaran Pandiyan wrote:
> On Thu, 2018-10-25 at 18:17 -0700, José Roberto de Souza wrote:
> > If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> > will still keep the error set even after the reset done in the
> > irq_preinstall and irq_uninstall hooks.
>
> Does this happen or are you suspecting it might? Is this because IIR
> clearing did not work or is it because of a new unhandled interrupt?
It happens, it is cleared I can read it back right after clear it and
the value is correct but it is still set. I did some tests like clear
IIR with the interruption unmasked but it also don't work.
>
> -DK
>
>
>
> > And enabling in this situation cause the screen to freeze in the
> > first time that PSR HW tries to activate so lets keep PSR disabled
> > to avoid any rendering problems.
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_psr.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 68201cc24d25..718270da1061 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -529,6 +529,19 @@ void intel_psr_compute_config(struct intel_dp
> > *intel_dp,
> > return;
> > }
> >
> > + /*
> > + * If a PSR error happened and the driver is reloaded, the
> > EDP_PSR_IIR
> > + * will still keep the error set even after the reset done in
> > the
> > + * irq_preinstall and irq_uninstall hooks.
> > + * And enabling in this situation cause the screen to freeze in
> > the
> > + * first time that PSR HW tries to activate so lets keep PSR
> > disabled
> > + * to avoid any rendering problems.
> > + */
> > + if (I915_READ(EDP_PSR_IIR) & EDP_PSR_ERROR(TRANSCODER_EDP)) {
> > + DRM_DEBUG_KMS("PSR interruption error set\n");
> > + return;
> > + }
> > +
> > if (IS_HASWELL(dev_priv) &&
> > I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
> > S3D_ENABLE) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 10/10] drm/i915: Do not enable PSR in the next modeset after a error
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
` (7 preceding siblings ...)
2018-10-26 1:17 ` [PATCH v3 09/10] drm/i915: Keep PSR disabled after a driver reload after a PSR error José Roberto de Souza
@ 2018-10-26 1:17 ` José Roberto de Souza
2018-10-26 1:26 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() Patchwork
` (2 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: José Roberto de Souza @ 2018-10-26 1:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
When we detect a error and disable PSR, it is kept disable until the
next modeset but as the sink already show signs that it do not
properly work with PSR lets disabled it for good to avoid any
additional flickering.
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_psr.c | 11 ++++++++++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2a2574f34b4a..89479d0ee6b6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -639,6 +639,7 @@ struct i915_psr {
ktime_t last_entry_attempt;
ktime_t last_exit;
u32 irq_aux_error;
+ bool sink_not_reliable;
};
enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 718270da1061..47b334b6af16 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -542,6 +542,11 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
return;
}
+ if (dev_priv->psr.sink_not_reliable) {
+ DRM_DEBUG_KMS("Sink not reliable set\n");
+ return;
+ }
+
if (IS_HASWELL(dev_priv) &&
I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
S3D_ENABLE) {
@@ -942,6 +947,7 @@ static void intel_psr_handle_irq(struct drm_i915_private *dev_priv)
mutex_lock(&psr->lock);
intel_psr_disable_locked(psr->dp);
+ psr->sink_not_reliable = true;
/* let's make sure that sink is awaken */
drm_dp_dpcd_writeb(&psr->dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
@@ -1141,6 +1147,7 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
if ((val & DP_PSR_SINK_STATE_MASK) == DP_PSR_SINK_INTERNAL_ERROR) {
DRM_DEBUG_KMS("PSR sink internal error, disabling PSR\n");
intel_psr_disable_locked(intel_dp);
+ psr->sink_not_reliable = true;
}
if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_ERROR_STATUS, &val) != 1) {
@@ -1158,8 +1165,10 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
if (val & ~errors)
DRM_ERROR("PSR_ERROR_STATUS unhandled errors %x\n",
val & ~errors);
- if (val & errors)
+ if (val & errors) {
intel_psr_disable_locked(intel_dp);
+ psr->sink_not_reliable = true;
+ }
/* clear status register */
drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
exit:
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source()
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
` (8 preceding siblings ...)
2018-10-26 1:17 ` [PATCH v3 10/10] drm/i915: Do not enable PSR in the next modeset after a error José Roberto de Souza
@ 2018-10-26 1:26 ` Patchwork
2018-10-26 1:29 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-10-26 1:47 ` ✗ Fi.CI.BAT: failure " Patchwork
11 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-26 1:26 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source()
URL : https://patchwork.freedesktop.org/series/51562/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
3ed40d086a0b drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source()
2692c0e3b41e drm/i915/psr: Always wait for idle state when disabling PSR
811e7883ff20 drm/i915/psr: Move intel_psr_disable_source() code to intel_psr_disable_locked()
bcacc1b8ef43 drm/i915: Avoid a full port detection in the first eDP short pulse
804e6b686a8c drm/i915: Check PSR errors instead of retrain while PSR is enabled
57fe8ec72fc8 drm/i915: Unmask PSR interruptions before assert IIR
16514d3f1562 drm/i915/icl: Reset PSR interruptions
8722041afd17 drm/i915: Disable PSR when a PSR aux error happen
06f2c7c4845b drm/i915: Keep PSR disabled after a driver reload after a PSR error
4970b1f8ea58 drm/i915: Do not enable PSR in the next modeset after a error
-:25: CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384
#25: FILE: drivers/gpu/drm/i915/i915_drv.h:642:
+ bool sink_not_reliable;
total: 0 errors, 0 warnings, 1 checks, 43 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* ✗ Fi.CI.SPARSE: warning for series starting with [v3,01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source()
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
` (9 preceding siblings ...)
2018-10-26 1:26 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() Patchwork
@ 2018-10-26 1:29 ` Patchwork
2018-10-26 1:47 ` ✗ Fi.CI.BAT: failure " Patchwork
11 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-26 1:29 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source()
URL : https://patchwork.freedesktop.org/series/51562/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source()
Okay!
Commit: drm/i915/psr: Always wait for idle state when disabling PSR
Okay!
Commit: drm/i915/psr: Move intel_psr_disable_source() code to intel_psr_disable_locked()
Okay!
Commit: drm/i915: Avoid a full port detection in the first eDP short pulse
Okay!
Commit: drm/i915: Check PSR errors instead of retrain while PSR is enabled
Okay!
Commit: drm/i915: Unmask PSR interruptions before assert IIR
Okay!
Commit: drm/i915/icl: Reset PSR interruptions
Okay!
Commit: drm/i915: Disable PSR when a PSR aux error happen
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3707:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3708:16: warning: expression using sizeof(void)
Commit: drm/i915: Keep PSR disabled after a driver reload after a PSR error
Okay!
Commit: drm/i915: Do not enable PSR in the next modeset after a error
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3708:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3709:16: warning: expression using sizeof(void)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* ✗ Fi.CI.BAT: failure for series starting with [v3,01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source()
2018-10-26 1:17 [PATCH v3 01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source() José Roberto de Souza
` (10 preceding siblings ...)
2018-10-26 1:29 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-10-26 1:47 ` Patchwork
11 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-26 1:47 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,01/10] drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source()
URL : https://patchwork.freedesktop.org/series/51562/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_5038 -> Patchwork_10592 =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_10592 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10592, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/51562/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10592:
=== IGT changes ===
==== Possible regressions ====
igt@kms_busy@basic-flip-a:
fi-whl-u: PASS -> DMESG-WARN +2
fi-icl-u: NOTRUN -> DMESG-WARN
igt@pm_rpm@basic-pci-d3-state:
fi-skl-6600u: PASS -> DMESG-WARN +1
fi-kbl-7560u: PASS -> DMESG-WARN +1
fi-kbl-r: PASS -> DMESG-WARN +1
igt@pm_rpm@module-reload:
fi-cfl-s3: PASS -> DMESG-WARN +1
fi-skl-6700hq: PASS -> DMESG-WARN +2
fi-cnl-u: PASS -> DMESG-WARN +2
fi-icl-u2: PASS -> DMESG-WARN +2
== Known issues ==
Here are the changes found in Patchwork_10592 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_module_reload@basic-reload:
fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718)
igt@kms_flip@basic-flip-vs-dpms:
fi-glk-j4005: PASS -> DMESG-WARN (fdo#106000)
igt@kms_frontbuffer_tracking@basic:
fi-byt-clapper: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
fi-byt-clapper: PASS -> FAIL (fdo#103191, fdo#107362)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
fi-icl-u: NOTRUN -> INCOMPLETE (fdo#107713)
==== Possible fixes ====
igt@kms_frontbuffer_tracking@basic:
fi-hsw-peppy: DMESG-WARN (fdo#102614) -> PASS
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
fi-byt-clapper: FAIL (fdo#107362) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence:
fi-glk-j4005: DMESG-WARN (fdo#106000) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-byt-clapper: FAIL (fdo#103191, fdo#107362) -> PASS
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
== Participating hosts (46 -> 44) ==
Additional (1): fi-icl-u
Missing (3): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
== Build changes ==
* Linux: CI_DRM_5038 -> Patchwork_10592
CI_DRM_5038: 96ecfb04d5acfcc565068c09afd6d0d713b2ddef @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4695: 81b66cf2806d6a8e9516580fb31879677487d32b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10592: 4970b1f8ea5864ccf8177a81d69875d5cba0cfac @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
4970b1f8ea58 drm/i915: Do not enable PSR in the next modeset after a error
06f2c7c4845b drm/i915: Keep PSR disabled after a driver reload after a PSR error
8722041afd17 drm/i915: Disable PSR when a PSR aux error happen
16514d3f1562 drm/i915/icl: Reset PSR interruptions
57fe8ec72fc8 drm/i915: Unmask PSR interruptions before assert IIR
804e6b686a8c drm/i915: Check PSR errors instead of retrain while PSR is enabled
bcacc1b8ef43 drm/i915: Avoid a full port detection in the first eDP short pulse
811e7883ff20 drm/i915/psr: Move intel_psr_disable_source() code to intel_psr_disable_locked()
2692c0e3b41e drm/i915/psr: Always wait for idle state when disabling PSR
3ed40d086a0b drm/i915/psr: Use intel_psr_exit() in intel_psr_disable_source()
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10592/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread