[PATCH v2 1/2] drm/i915/debugfs: Print sink PSR status

[PATCH v2 1/2] drm/i915/debugfs: Print sink PSR status

[José Roberto de Souza]

IGT tests could be improved with sink status, knowing for sure that
hardware have activate or exit PSR.

Reviewed-by: 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/i915_debugfs.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1dba2c451255..c9ac946b62c9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2603,6 +2603,26 @@ static const char *psr2_live_status(u32 val)
 	return "unknown";
 }
 
+static const char *psr_sink_status(u8 val)
+{
+	static const char * const sink_status[] = {
+		"inactive",
+		"transition to active, capture and display",
+		"active, display from RFB",
+		"active, capture and display on sink device timings",
+		"transition to inactive, capture and display, timing re-sync",
+		"reserved",
+		"reserved",
+		"sink internal error"
+	};
+
+	val &= DP_PSR_SINK_STATE_MASK;
+	if (val < ARRAY_SIZE(sink_status))
+		return sink_status[val];
+
+	return "unknown";
+}
+
 static int i915_edp_psr_status(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
 		seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
 			   psr2, psr2_live_status(psr2));
 	}
+
+	if (dev_priv->psr.enabled) {
+		struct drm_dp_aux *aux = &dev_priv->psr.enabled->aux;
+		u8 val;
+
+		if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, &val) == 1)
+			seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
+				   psr_sink_status(val));
+	}
 	mutex_unlock(&dev_priv->psr.lock);
 
 	intel_runtime_pm_put(dev_priv);
-- 
2.16.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/2] drm/i915/psr/cnl: Set y-coordinate as valid in SDP

[José Roberto de Souza]

This was my bad, spec says that the name of this bit is
'Y-coordinate valid' but the values for it is:
0: Include Y-coordinate valid eDP1.4a
1: Do not include Y-coordinate valid eDP 1.4
So not setting it.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---

Changelog:
v2: Not renaming EDP_Y_COORDINATE_VALID

 drivers/gpu/drm/i915/intel_psr.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2d53f7398a6d..f12111438bcf 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -400,9 +400,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 	 * good enough. */
 	val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
-	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
-		val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE;
-	}
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+		val |= EDP_Y_COORDINATE_ENABLE;
 
 	val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency + 1);
 
-- 
2.16.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915/debugfs: Print sink PSR status

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/debugfs: Print sink PSR status
URL   : https://patchwork.freedesktop.org/series/41022/
State : success

== Summary ==

Series 41022v1 series starting with [v2,1/2] drm/i915/debugfs: Print sink PSR status
https://patchwork.freedesktop.org/api/1.0/series/41022/revisions/1/mbox/

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:432s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:441s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:378s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:541s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:296s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:512s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:518s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:521s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:509s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:409s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:561s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:510s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:577s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:422s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:320s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:535s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:402s
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:432s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:470s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:462s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:506s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:670s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:443s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:534s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:502s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:513s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:430s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:443s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:597s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:397s
Blacklisted hosts:
fi-cnl-psr       total:285  pass:255  dwarn:3   dfail:0   fail:1   skip:26  time:527s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:485s

c46052cde6a50c5459e00791ffc4d5aa1ec58a9e drm-tip: 2018y-03m-30d-18h-56m-26s UTC integration manifest
4c3d5eaa3722 drm/i915/psr/cnl: Set y-coordinate as valid in SDP
b029e056dae9 drm/i915/debugfs: Print sink PSR status

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8563/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for series starting with [v2,1/2] drm/i915/debugfs: Print sink PSR status

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/debugfs: Print sink PSR status
URL   : https://patchwork.freedesktop.org/series/41022/
State : warning

== Summary ==

---- Possible new issues:

Test kms_chv_cursor_fail:
        Subgroup pipe-b-64x64-top-edge:
                pass       -> SKIP       (shard-snb)
Test kms_cursor_crc:
        Subgroup cursor-64x64-suspend:
                dmesg-warn -> PASS       (shard-snb)

---- Known issues:

Test gem_softpin:
        Subgroup noreloc-s3:
                incomplete -> PASS       (shard-hsw) fdo#103540
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-shrfb-msflip-blt:
                dmesg-fail -> PASS       (shard-apl) fdo#104727
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-c-planes:
                pass       -> INCOMPLETE (shard-hsw) fdo#103375

fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#104727 https://bugs.freedesktop.org/show_bug.cgi?id=104727
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

shard-apl        total:3495 pass:1831 dwarn:1   dfail:0   fail:7   skip:1655 time:12912s
shard-hsw        total:3493 pass:1780 dwarn:1   dfail:0   fail:1   skip:1709 time:10871s
shard-snb        total:3495 pass:1373 dwarn:1   dfail:0   fail:3   skip:2118 time:6990s
Blacklisted hosts:
shard-kbl        total:3495 pass:1958 dwarn:1   dfail:1   fail:7   skip:1528 time:9294s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8563/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/debugfs: Print sink PSR status

[Pandiyan, Dhinakaran]

On Mon, 2018-04-02 at 13:51 -0700, José Roberto de Souza wrote:
> IGT tests could be improved with sink status, knowing for sure that
> hardware have activate or exit PSR.
> 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>


Please don't merge this patch yet. While the patch itself is correct,
testing it brings up an interesting problem.

Printing the sink_status() leads to power_get(aux_domain) which wakes up
the display engine from DC5/6. This results in a HW triggered PSR exit,
effectively altering the state that we are trying to read. I would like
to understand the problem fully before merging the patch.


> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1dba2c451255..c9ac946b62c9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2603,6 +2603,26 @@ static const char *psr2_live_status(u32 val)
>  	return "unknown";
>  }
>  
> +static const char *psr_sink_status(u8 val)
> +{
> +	static const char * const sink_status[] = {
> +		"inactive",
> +		"transition to active, capture and display",
> +		"active, display from RFB",
> +		"active, capture and display on sink device timings",
> +		"transition to inactive, capture and display, timing re-sync",
> +		"reserved",
> +		"reserved",
> +		"sink internal error"
> +	};
> +
> +	val &= DP_PSR_SINK_STATE_MASK;
> +	if (val < ARRAY_SIZE(sink_status))
> +		return sink_status[val];
> +
> +	return "unknown";
> +}
> +
>  static int i915_edp_psr_status(struct seq_file *m, void *data)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> @@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>  		seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
>  			   psr2, psr2_live_status(psr2));
>  	}
> +
> +	if (dev_priv->psr.enabled) {
> +		struct drm_dp_aux *aux = &dev_priv->psr.enabled->aux;
> +		u8 val;
> +
> +		if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, &val) == 1)
> +			seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
> +				   psr_sink_status(val));
> +	}
>  	mutex_unlock(&dev_priv->psr.lock);
>  
>  	intel_runtime_pm_put(dev_priv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/debugfs: Print sink PSR status

[Rodrigo Vivi]

On Mon, Apr 02, 2018 at 03:38:26PM -0700, Pandiyan, Dhinakaran wrote:
> On Mon, 2018-04-02 at 13:51 -0700, José Roberto de Souza wrote:
> > IGT tests could be improved with sink status, knowing for sure that
> > hardware have activate or exit PSR.
> > 
> > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> 
> Please don't merge this patch yet. While the patch itself is correct,
> testing it brings up an interesting problem.
> 
> Printing the sink_status() leads to power_get(aux_domain) which wakes up
> the display engine from DC5/6. This results in a HW triggered PSR exit,
> effectively altering the state that we are trying to read. I would like
> to understand the problem fully before merging the patch.

ack

> 
> 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 1dba2c451255..c9ac946b62c9 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2603,6 +2603,26 @@ static const char *psr2_live_status(u32 val)
> >  	return "unknown";
> >  }
> >  
> > +static const char *psr_sink_status(u8 val)
> > +{
> > +	static const char * const sink_status[] = {
> > +		"inactive",
> > +		"transition to active, capture and display",
> > +		"active, display from RFB",
> > +		"active, capture and display on sink device timings",
> > +		"transition to inactive, capture and display, timing re-sync",
> > +		"reserved",
> > +		"reserved",
> > +		"sink internal error"
> > +	};
> > +
> > +	val &= DP_PSR_SINK_STATE_MASK;
> > +	if (val < ARRAY_SIZE(sink_status))
> > +		return sink_status[val];
> > +
> > +	return "unknown";
> > +}
> > +
> >  static int i915_edp_psr_status(struct seq_file *m, void *data)
> >  {
> >  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> > @@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> >  		seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> >  			   psr2, psr2_live_status(psr2));
> >  	}
> > +
> > +	if (dev_priv->psr.enabled) {
> > +		struct drm_dp_aux *aux = &dev_priv->psr.enabled->aux;
> > +		u8 val;
> > +
> > +		if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, &val) == 1)
> > +			seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
> > +				   psr_sink_status(val));
> > +	}
> >  	mutex_unlock(&dev_priv->psr.lock);
> >  
> >  	intel_runtime_pm_put(dev_priv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/debugfs: Print sink PSR status

[Souza, Jose]

On Mon, 2018-04-02 at 15:38 -0700, Pandiyan, Dhinakaran wrote:
> On Mon, 2018-04-02 at 13:51 -0700, José Roberto de Souza wrote:
> > IGT tests could be improved with sink status, knowing for sure that
> > hardware have activate or exit PSR.
> > 
> > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> 
> Please don't merge this patch yet. While the patch itself is correct,
> testing it brings up an interesting problem.
> 
> Printing the sink_status() leads to power_get(aux_domain) which wakes
> up
> the display engine from DC5/6. This results in a HW triggered PSR
> exit,
> effectively altering the state that we are trying to read. I would
> like
> to understand the problem fully before merging the patch.

In my tests(PSR1 only) with DMC loaded, I can see HW trigerring PSR
exit when reading i915_edp_psr_status with or without this patch, the
event that caused is 'SRD Mask Reg Write'.
After masking EDP_PSR_DEBUG_MASK_DISP_REG_WRITE in EDP_PSR_DEBUG it do
not happens with or without this patch.

Masking this event did not bring more IGT errors than we have now, what
do you think Dhinakaran? Should we mask it?

> 
> 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 29
> > +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 1dba2c451255..c9ac946b62c9 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2603,6 +2603,26 @@ static const char *psr2_live_status(u32 val)
> >  	return "unknown";
> >  }
> >  
> > +static const char *psr_sink_status(u8 val)
> > +{
> > +	static const char * const sink_status[] = {
> > +		"inactive",
> > +		"transition to active, capture and display",
> > +		"active, display from RFB",
> > +		"active, capture and display on sink device
> > timings",
> > +		"transition to inactive, capture and display,
> > timing re-sync",
> > +		"reserved",
> > +		"reserved",
> > +		"sink internal error"
> > +	};
> > +
> > +	val &= DP_PSR_SINK_STATE_MASK;
> > +	if (val < ARRAY_SIZE(sink_status))
> > +		return sink_status[val];
> > +
> > +	return "unknown";
> > +}
> > +
> >  static int i915_edp_psr_status(struct seq_file *m, void *data)
> >  {
> >  	struct drm_i915_private *dev_priv = node_to_i915(m-
> > >private);
> > @@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct
> > seq_file *m, void *data)
> >  		seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> >  			   psr2, psr2_live_status(psr2));
> >  	}
> > +
> > +	if (dev_priv->psr.enabled) {
> > +		struct drm_dp_aux *aux = &dev_priv->psr.enabled-
> > >aux;
> > +		u8 val;
> > +
> > +		if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, &val) ==
> > 1)
> > +			seq_printf(m, "Sink PSR status: 0x%x
> > [%s]\n", val,
> > +				   psr_sink_status(val));
> > +	}
> >  	mutex_unlock(&dev_priv->psr.lock);
> >  
> >  	intel_runtime_pm_put(dev_priv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/debugfs: Print sink PSR status

[Souza, Jose]

On Fri, 2018-04-06 at 16:36 -0700, José Roberto de Souza wrote:
> On Mon, 2018-04-02 at 15:38 -0700, Pandiyan, Dhinakaran wrote:
> > On Mon, 2018-04-02 at 13:51 -0700, José Roberto de Souza wrote:
> > > IGT tests could be improved with sink status, knowing for sure
> > > that
> > > hardware have activate or exit PSR.
> > > 
> > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > 
> > 
> > Please don't merge this patch yet. While the patch itself is
> > correct,
> > testing it brings up an interesting problem.
> > 
> > Printing the sink_status() leads to power_get(aux_domain) which
> > wakes
> > up
> > the display engine from DC5/6. This results in a HW triggered PSR
> > exit,
> > effectively altering the state that we are trying to read. I would
> > like
> > to understand the problem fully before merging the patch.
> 
> In my tests(PSR1 only) with DMC loaded, I can see HW trigerring PSR
> exit when reading i915_edp_psr_status with or without this patch, the
> event that caused is 'SRD Mask Reg Write'.
> After masking EDP_PSR_DEBUG_MASK_DISP_REG_WRITE in EDP_PSR_DEBUG it
> do
> not happens with or without this patch.
> 
> Masking this event did not bring more IGT errors than we have now,
> what
> do you think Dhinakaran? Should we mask it?

Another information missed:

When doing a dpcd(I did not look at what exacly is causing it)
transaction it is causing the HW to be awaken from DC6 with or without
masking EDP_PSR_DEBUG_MASK_DISP_REG_WRITE but when masking it do not
cause a PSR exit.
Reading i915_sr_status also cause HW to be awaken from DC6.

> 
> > 
> > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c | 29
> > > +++++++++++++++++++++++++++++
> > >  1 file changed, 29 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 1dba2c451255..c9ac946b62c9 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2603,6 +2603,26 @@ static const char *psr2_live_status(u32
> > > val)
> > >  	return "unknown";
> > >  }
> > >  
> > > +static const char *psr_sink_status(u8 val)
> > > +{
> > > +	static const char * const sink_status[] = {
> > > +		"inactive",
> > > +		"transition to active, capture and display",
> > > +		"active, display from RFB",
> > > +		"active, capture and display on sink device
> > > timings",
> > > +		"transition to inactive, capture and display,
> > > timing re-sync",
> > > +		"reserved",
> > > +		"reserved",
> > > +		"sink internal error"
> > > +	};
> > > +
> > > +	val &= DP_PSR_SINK_STATE_MASK;
> > > +	if (val < ARRAY_SIZE(sink_status))
> > > +		return sink_status[val];
> > > +
> > > +	return "unknown";
> > > +}
> > > +
> > >  static int i915_edp_psr_status(struct seq_file *m, void *data)
> > >  {
> > >  	struct drm_i915_private *dev_priv = node_to_i915(m-
> > > > private);
> > > 
> > > @@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct
> > > seq_file *m, void *data)
> > >  		seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> > >  			   psr2, psr2_live_status(psr2));
> > >  	}
> > > +
> > > +	if (dev_priv->psr.enabled) {
> > > +		struct drm_dp_aux *aux = &dev_priv->psr.enabled-
> > > > aux;
> > > 
> > > +		u8 val;
> > > +
> > > +		if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, &val)
> > > ==
> > > 1)
> > > +			seq_printf(m, "Sink PSR status: 0x%x
> > > [%s]\n", val,
> > > +				   psr_sink_status(val));
> > > +	}
> > >  	mutex_unlock(&dev_priv->psr.lock);
> > >  
> > >  	intel_runtime_pm_put(dev_priv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/debugfs: Print sink PSR status

[Dhinakaran Pandiyan]

On Sat, 2018-04-07 at 00:49 +0000, Souza, Jose wrote:
> On Fri, 2018-04-06 at 16:36 -0700, José Roberto de Souza wrote:
> > On Mon, 2018-04-02 at 15:38 -0700, Pandiyan, Dhinakaran wrote:
> > > On Mon, 2018-04-02 at 13:51 -0700, José Roberto de Souza wrote:
> > > > IGT tests could be improved with sink status, knowing for sure
> > > > that
> > > > hardware have activate or exit PSR.
> > > > 
> > > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > 
> > > 
> > > Please don't merge this patch yet. While the patch itself is
> > > correct,
> > > testing it brings up an interesting problem.
> > > 
> > > Printing the sink_status() leads to power_get(aux_domain) which
> > > wakes
> > > up
> > > the display engine from DC5/6. This results in a HW triggered PSR
> > > exit,
> > > effectively altering the state that we are trying to read. I would
> > > like
> > > to understand the problem fully before merging the patch.
> > 
> > In my tests(PSR1 only) with DMC loaded, I can see HW trigerring PSR
> > exit when reading i915_edp_psr_status with or without this patch, the
> > event that caused is 'SRD Mask Reg Write'.
> > After masking EDP_PSR_DEBUG_MASK_DISP_REG_WRITE in EDP_PSR_DEBUG it
> > do
> > not happens with or without this patch.
> > 

Reads causing PSR exit is a concern, but I have seen edp_psr_status
showing PSR live status as active. If reads triggered exit, then it
should always be inactive. Unless the exit happens after the status is
read.

> > Masking this event did not bring more IGT errors than we have now,
> > what
> > do you think Dhinakaran? Should we mask it?
> 

We'll need PSR to exit on some register writes, example flips or cursor
moves. 

> Another information missed:
> 
> When doing a dpcd(I did not look at what exacly is causing it)
> transaction it is causing the HW to be awaken from DC6 with or without
> masking EDP_PSR_DEBUG_MASK_DISP_REG_WRITE 

This is expected, AUX_A is in the DC_OFF power domain, so the driver
forces a DC6 exit.

> but when masking it do not
> cause a PSR exit.
> Reading i915_sr_status also cause HW to be awaken from DC6.
> 

What hardware are you testing on? And is this a PSR2 panel?



> > 
> > > 
> > > 
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_debugfs.c | 29
> > > > +++++++++++++++++++++++++++++
> > > >  1 file changed, 29 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > index 1dba2c451255..c9ac946b62c9 100644
> > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > @@ -2603,6 +2603,26 @@ static const char *psr2_live_status(u32
> > > > val)
> > > >  	return "unknown";
> > > >  }
> > > >  
> > > > +static const char *psr_sink_status(u8 val)
> > > > +{
> > > > +	static const char * const sink_status[] = {
> > > > +		"inactive",
> > > > +		"transition to active, capture and display",
> > > > +		"active, display from RFB",
> > > > +		"active, capture and display on sink device
> > > > timings",
> > > > +		"transition to inactive, capture and display,
> > > > timing re-sync",
> > > > +		"reserved",
> > > > +		"reserved",
> > > > +		"sink internal error"
> > > > +	};
> > > > +
> > > > +	val &= DP_PSR_SINK_STATE_MASK;
> > > > +	if (val < ARRAY_SIZE(sink_status))
> > > > +		return sink_status[val];
> > > > +
> > > > +	return "unknown";
> > > > +}
> > > > +
> > > >  static int i915_edp_psr_status(struct seq_file *m, void *data)
> > > >  {
> > > >  	struct drm_i915_private *dev_priv = node_to_i915(m-
> > > > > private);
> > > > 
> > > > @@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct
> > > > seq_file *m, void *data)
> > > >  		seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> > > >  			   psr2, psr2_live_status(psr2));
> > > >  	}
> > > > +
> > > > +	if (dev_priv->psr.enabled) {
> > > > +		struct drm_dp_aux *aux = &dev_priv->psr.enabled-
> > > > > aux;
> > > > 
> > > > +		u8 val;
> > > > +
> > > > +		if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, &val)
> > > > ==
> > > > 1)
> > > > +			seq_printf(m, "Sink PSR status: 0x%x
> > > > [%s]\n", val,
> > > > +				   psr_sink_status(val));
> > > > +	}
> > > >  	mutex_unlock(&dev_priv->psr.lock);
> > > >  
> > > >  	intel_runtime_pm_put(dev_priv);
> _______________________________________________
> 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

Re: [PATCH v2 1/2] drm/i915/debugfs: Print sink PSR status

[Souza, Jose]

On Fri, 2018-04-06 at 18:40 -0700, Dhinakaran Pandiyan wrote:
> 
> 
> On Sat, 2018-04-07 at 00:49 +0000, Souza, Jose wrote:
> > On Fri, 2018-04-06 at 16:36 -0700, José Roberto de Souza wrote:
> > > On Mon, 2018-04-02 at 15:38 -0700, Pandiyan, Dhinakaran wrote:
> > > > On Mon, 2018-04-02 at 13:51 -0700, José Roberto de Souza wrote:
> > > > > IGT tests could be improved with sink status, knowing for
> > > > > sure
> > > > > that
> > > > > hardware have activate or exit PSR.
> > > > > 
> > > > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.c
> > > > > om>
> > > > 
> > > > 
> > > > Please don't merge this patch yet. While the patch itself is
> > > > correct,
> > > > testing it brings up an interesting problem.
> > > > 
> > > > Printing the sink_status() leads to power_get(aux_domain) which
> > > > wakes
> > > > up
> > > > the display engine from DC5/6. This results in a HW triggered
> > > > PSR
> > > > exit,
> > > > effectively altering the state that we are trying to read. I
> > > > would
> > > > like
> > > > to understand the problem fully before merging the patch.
> > > 
> > > In my tests(PSR1 only) with DMC loaded, I can see HW trigerring
> > > PSR
> > > exit when reading i915_edp_psr_status with or without this patch,
> > > the
> > > event that caused is 'SRD Mask Reg Write'.
> > > After masking EDP_PSR_DEBUG_MASK_DISP_REG_WRITE in EDP_PSR_DEBUG
> > > it
> > > do
> > > not happens with or without this patch.
> > > 
> 
> Reads causing PSR exit is a concern, but I have seen edp_psr_status
> showing PSR live status as active. If reads triggered exit, then it
> should always be inactive. Unless the exit happens after the status
> is
> read.

Same thing for sink status, I can see that PSR is exiting but the
status is still active on sink.

> 
> > > Masking this event did not bring more IGT errors than we have
> > > now,
> > > what
> > > do you think Dhinakaran? Should we mask it?
> 
> We'll need PSR to exit on some register writes, example flips or
> cursor
> moves. 

PIPE related writes are still causing PSR to exit, PSR_EVENT have a bit
for Register Update and other for Pipe Registers Update but it don't
have a counter part in PSR_MASK/SRD_DEBUG to make the pipe register
updated not trigger a PSR exit.

> 
> > Another information missed:
> > 
> > When doing a dpcd(I did not look at what exacly is causing it)
> > transaction it is causing the HW to be awaken from DC6 with or
> > without
> > masking EDP_PSR_DEBUG_MASK_DISP_REG_WRITE 
> 
> This is expected, AUX_A is in the DC_OFF power domain, so the driver
> forces a DC6 exit.
> 
> > but when masking it do not
> > cause a PSR exit.
> > Reading i915_sr_status also cause HW to be awaken from DC6.
> > 
> 
> What hardware are you testing on? And is this a PSR2 panel?

KBL it is a PSR2 pannels but I did some changes to only enable PSR1.

> 
> 
> 
> > > 
> > > > 
> > > > 
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_debugfs.c | 29
> > > > > +++++++++++++++++++++++++++++
> > > > >  1 file changed, 29 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > index 1dba2c451255..c9ac946b62c9 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > @@ -2603,6 +2603,26 @@ static const char
> > > > > *psr2_live_status(u32
> > > > > val)
> > > > >  	return "unknown";
> > > > >  }
> > > > >  
> > > > > +static const char *psr_sink_status(u8 val)
> > > > > +{
> > > > > +	static const char * const sink_status[] = {
> > > > > +		"inactive",
> > > > > +		"transition to active, capture and display",
> > > > > +		"active, display from RFB",
> > > > > +		"active, capture and display on sink device
> > > > > timings",
> > > > > +		"transition to inactive, capture and
> > > > > display,
> > > > > timing re-sync",
> > > > > +		"reserved",
> > > > > +		"reserved",
> > > > > +		"sink internal error"
> > > > > +	};
> > > > > +
> > > > > +	val &= DP_PSR_SINK_STATE_MASK;
> > > > > +	if (val < ARRAY_SIZE(sink_status))
> > > > > +		return sink_status[val];
> > > > > +
> > > > > +	return "unknown";
> > > > > +}
> > > > > +
> > > > >  static int i915_edp_psr_status(struct seq_file *m, void
> > > > > *data)
> > > > >  {
> > > > >  	struct drm_i915_private *dev_priv = node_to_i915(m-
> > > > > > private);
> > > > > 
> > > > > @@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct
> > > > > seq_file *m, void *data)
> > > > >  		seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> > > > >  			   psr2, psr2_live_status(psr2));
> > > > >  	}
> > > > > +
> > > > > +	if (dev_priv->psr.enabled) {
> > > > > +		struct drm_dp_aux *aux = &dev_priv-
> > > > > >psr.enabled-
> > > > > > aux;
> > > > > 
> > > > > +		u8 val;
> > > > > +
> > > > > +		if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS,
> > > > > &val)
> > > > > ==
> > > > > 1)
> > > > > +			seq_printf(m, "Sink PSR status: 0x%x
> > > > > [%s]\n", val,
> > > > > +				   psr_sink_status(val));
> > > > > +	}
> > > > >  	mutex_unlock(&dev_priv->psr.lock);
> > > > >  
> > > > >  	intel_runtime_pm_put(dev_priv);
> > 
> > _______________________________________________
> > 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