* [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
@ 2018-07-05 0:31 Dhinakaran Pandiyan
2018-07-05 0:37 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (7 more replies)
0 siblings, 8 replies; 14+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-05 0:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
This allows to read i915_edp_psr_status from tests without triggering
any AUX communication. Take this opportunity to move this under the
eDP-1 connector directory as the status we print is of the sink.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 69 +++++++++++++++++++++----------------
1 file changed, 39 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f6142d78ede4..5069d5dedafe 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2592,6 +2592,41 @@ static const struct file_operations i915_guc_log_relay_fops = {
.release = i915_guc_log_relay_release,
};
+static int i915_psr_sink_status_show(struct seq_file *m, void *data)
+{
+ 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",
+ };
+ struct drm_connector *connector = m->private;
+ struct intel_dp *intel_dp =
+ enc_to_intel_dp(&intel_attached_encoder(connector)->base);
+
+ if (connector->status != connector_status_connected)
+ return -ENODEV;
+
+ if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) == 1) {
+ const char *str = "unknown";
+
+ val &= DP_PSR_SINK_STATE_MASK;
+ if (val < ARRAY_SIZE(sink_status))
+ str = sink_status[val];
+ seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val, str);
+ } else {
+ DRM_ERROR("dpcd read (at %u) failed\n", DP_PSR_STATUS);
+ }
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
+
static void
psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
{
@@ -2643,26 +2678,6 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status, "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);
@@ -2706,15 +2721,6 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
}
psr_source_status(dev_priv, m);
-
- 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);
if (READ_ONCE(dev_priv->psr.debug)) {
@@ -4971,9 +4977,12 @@ int i915_debugfs_connector_add(struct drm_connector *connector)
debugfs_create_file("i915_dpcd", S_IRUGO, root,
connector, &i915_dpcd_fops);
- if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
+ if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
debugfs_create_file("i915_panel_timings", S_IRUGO, root,
connector, &i915_panel_fops);
+ debugfs_create_file("i915_psr_sink_status", S_IRUGO, root,
+ connector, &i915_psr_sink_status_fops);
+ }
return 0;
}
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/psr: Split sink status into a separate debugfs node
2018-07-05 0:31 [PATCH] drm/i915/psr: Split sink status into a separate debugfs node Dhinakaran Pandiyan
@ 2018-07-05 0:37 ` Patchwork
2018-07-05 0:55 ` ✓ Fi.CI.BAT: success " Patchwork
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-07-05 0:37 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/psr: Split sink status into a separate debugfs node
URL : https://patchwork.freedesktop.org/series/45952/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
687e1ec30b27 drm/i915/psr: Split sink status into a separate debugfs node
-:115: WARNING:SYMBOLIC_PERMS: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
#115: FILE: drivers/gpu/drm/i915/i915_debugfs.c:4983:
+ debugfs_create_file("i915_psr_sink_status", S_IRUGO, root,
-:116: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#116: FILE: drivers/gpu/drm/i915/i915_debugfs.c:4984:
+ debugfs_create_file("i915_psr_sink_status", S_IRUGO, root,
+ connector, &i915_psr_sink_status_fops);
total: 0 errors, 1 warnings, 1 checks, 95 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/psr: Split sink status into a separate debugfs node
2018-07-05 0:31 [PATCH] drm/i915/psr: Split sink status into a separate debugfs node Dhinakaran Pandiyan
2018-07-05 0:37 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-07-05 0:55 ` Patchwork
2018-07-05 4:54 ` ✗ Fi.CI.IGT: failure " Patchwork
` (5 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-07-05 0:55 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/psr: Split sink status into a separate debugfs node
URL : https://patchwork.freedesktop.org/series/45952/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4428 -> Patchwork_9529 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/45952/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9529 that come from known issues:
=== IGT changes ===
==== Possible fixes ====
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
== Participating hosts (46 -> 41) ==
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4428 -> Patchwork_9529
CI_DRM_4428: 7bc1be8128e30b4d581b913feff5c78909c00945 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4536: aaa23eff21a148809beb22e928f3cd72530ea3de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9529: 687e1ec30b274d3e69509de0b4b873a2db0333cc @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
687e1ec30b27 drm/i915/psr: Split sink status into a separate debugfs node
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9529/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✗ Fi.CI.IGT: failure for drm/i915/psr: Split sink status into a separate debugfs node
2018-07-05 0:31 [PATCH] drm/i915/psr: Split sink status into a separate debugfs node Dhinakaran Pandiyan
2018-07-05 0:37 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-07-05 0:55 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-07-05 4:54 ` Patchwork
2018-07-05 21:04 ` [PATCH] " Rodrigo Vivi
` (4 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-07-05 4:54 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/psr: Split sink status into a separate debugfs node
URL : https://patchwork.freedesktop.org/series/45952/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_4428_full -> Patchwork_9529_full =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_9529_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9529_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9529_full:
=== IGT changes ===
==== Possible regressions ====
igt@drv_selftest@live_gtt:
shard-glk: NOTRUN -> DMESG-FAIL
igt@drv_selftest@mock_requests:
shard-glk: NOTRUN -> DMESG-WARN
==== Warnings ====
igt@drv_selftest@live_gtt:
shard-apl: INCOMPLETE (fdo#103927) -> DMESG-FAIL
igt@gem_exec_schedule@deep-bsd1:
shard-kbl: SKIP -> PASS +2
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
shard-snb: SKIP -> PASS +1
== Known issues ==
Here are the changes found in Patchwork_9529_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@mock_scatterlist:
shard-glk: NOTRUN -> DMESG-WARN (fdo#103667)
igt@drv_suspend@shrink:
shard-snb: PASS -> FAIL (fdo#106886)
igt@gem_exec_schedule@pi-ringfull-render:
shard-glk: NOTRUN -> FAIL (fdo#103158)
igt@kms_flip_tiling@flip-to-x-tiled:
shard-glk: NOTRUN -> FAIL (fdo#104724)
igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-cpu:
shard-glk: NOTRUN -> FAIL (fdo#104724, fdo#103167)
igt@kms_frontbuffer_tracking@fbc-suspend:
shard-kbl: PASS -> INCOMPLETE (fdo#105959, fdo#103665)
igt@prime_vgem@coherency-gtt:
shard-glk: NOTRUN -> FAIL (fdo#100587)
igt@testdisplay:
shard-glk: NOTRUN -> INCOMPLETE (k.org#198133, fdo#103359)
==== Possible fixes ====
igt@drv_suspend@shrink:
shard-kbl: INCOMPLETE (fdo#106886, fdo#103665) -> PASS
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-hsw: FAIL (fdo#100368) -> PASS
igt@kms_flip_tiling@flip-y-tiled:
shard-glk: FAIL (fdo#103822, fdo#104724) -> PASS
==== Warnings ====
igt@drv_selftest@live_gtt:
shard-kbl: DMESG-FAIL -> INCOMPLETE (fdo#103665)
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#100587 https://bugs.freedesktop.org/show_bug.cgi?id=100587
fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103667 https://bugs.freedesktop.org/show_bug.cgi?id=103667
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
fdo#105959 https://bugs.freedesktop.org/show_bug.cgi?id=105959
fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4428 -> Patchwork_9529
CI_DRM_4428: 7bc1be8128e30b4d581b913feff5c78909c00945 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4536: aaa23eff21a148809beb22e928f3cd72530ea3de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9529: 687e1ec30b274d3e69509de0b4b873a2db0333cc @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9529/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
2018-07-05 0:31 [PATCH] drm/i915/psr: Split sink status into a separate debugfs node Dhinakaran Pandiyan
` (2 preceding siblings ...)
2018-07-05 4:54 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-07-05 21:04 ` Rodrigo Vivi
2018-07-05 21:27 ` Dhinakaran Pandiyan
2018-07-05 22:42 ` ✓ Fi.CI.BAT: success for " Patchwork
` (3 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Rodrigo Vivi @ 2018-07-05 21:04 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx, Dhinakaran Pandiyan
On Wed, Jul 04, 2018 at 05:31:21PM -0700, Dhinakaran Pandiyan wrote:
> This allows to read i915_edp_psr_status from tests without triggering
> any AUX communication. Take this opportunity to move this under the
> eDP-1 connector directory as the status we print is of the sink.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 69 +++++++++++++++++++++----------------
> 1 file changed, 39 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index f6142d78ede4..5069d5dedafe 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2592,6 +2592,41 @@ static const struct file_operations i915_guc_log_relay_fops = {
> .release = i915_guc_log_relay_release,
> };
>
> +static int i915_psr_sink_status_show(struct seq_file *m, void *data)
> +{
> + 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",
> + };
> + struct drm_connector *connector = m->private;
> + struct intel_dp *intel_dp =
> + enc_to_intel_dp(&intel_attached_encoder(connector)->base);
> +
> + if (connector->status != connector_status_connected)
> + return -ENODEV;
> +
> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) == 1) {
> + const char *str = "unknown";
> +
> + val &= DP_PSR_SINK_STATE_MASK;
> + if (val < ARRAY_SIZE(sink_status))
> + str = sink_status[val];
> + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val, str);
> + } else {
> + DRM_ERROR("dpcd read (at %u) failed\n", DP_PSR_STATUS);
> + }
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> +
> static void
> psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
> {
> @@ -2643,26 +2678,6 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
> seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status, "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);
> @@ -2706,15 +2721,6 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> }
>
> psr_source_status(dev_priv, m);
> -
> - 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);
I wonder if we shouldn't get this lock there to make sure that no
PSR transition from our driver is getting called.
>
> if (READ_ONCE(dev_priv->psr.debug)) {
> @@ -4971,9 +4977,12 @@ int i915_debugfs_connector_add(struct drm_connector *connector)
> debugfs_create_file("i915_dpcd", S_IRUGO, root,
> connector, &i915_dpcd_fops);
>
> - if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
it seems that we should also move the other status file here to this block...
> debugfs_create_file("i915_panel_timings", S_IRUGO, root,
> connector, &i915_panel_fops);
> + debugfs_create_file("i915_psr_sink_status", S_IRUGO, root,
> + connector, &i915_psr_sink_status_fops);
... and also rename the other 2 entries to be in pair with this new one:
or all i915_edp_psr or all i915_psr...
> + }
>
> return 0;
> }
> --
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
2018-07-05 21:04 ` [PATCH] " Rodrigo Vivi
@ 2018-07-05 21:27 ` Dhinakaran Pandiyan
2018-07-05 21:37 ` Rodrigo Vivi
0 siblings, 1 reply; 14+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-05 21:27 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Dhinakaran Pandiyan
On Thursday, July 5, 2018 2:04:18 PM PDT Rodrigo Vivi wrote:
> On Wed, Jul 04, 2018 at 05:31:21PM -0700, Dhinakaran Pandiyan wrote:
> > This allows to read i915_edp_psr_status from tests without triggering
> > any AUX communication. Take this opportunity to move this under the
> > eDP-1 connector directory as the status we print is of the sink.
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >
> > drivers/gpu/drm/i915/i915_debugfs.c | 69
> > +++++++++++++++++++++---------------- 1 file changed, 39 insertions(+),
> > 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c index f6142d78ede4..5069d5dedafe
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2592,6 +2592,41 @@ static const struct file_operations
> > i915_guc_log_relay_fops = {>
> > .release = i915_guc_log_relay_release,
> >
> > };
> >
> > +static int i915_psr_sink_status_show(struct seq_file *m, void *data)
> > +{
> > + 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",
> > + };
> > + struct drm_connector *connector = m->private;
> > + struct intel_dp *intel_dp =
> > + enc_to_intel_dp(&intel_attached_encoder(connector)->base);
> > +
> > + if (connector->status != connector_status_connected)
> > + return -ENODEV;
> > +
> > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) == 1) {
> > + const char *str = "unknown";
> > +
> > + val &= DP_PSR_SINK_STATE_MASK;
> > + if (val < ARRAY_SIZE(sink_status))
> > + str = sink_status[val];
> > + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val, str);
> > + } else {
> > + DRM_ERROR("dpcd read (at %u) failed\n", DP_PSR_STATUS);
> > + }
> > +
> > + return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > +
> >
> > static void
> > psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
> > {
> >
> > @@ -2643,26 +2678,6 @@ psr_source_status(struct drm_i915_private
> > *dev_priv, struct seq_file *m)>
> > seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status,
"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);
> >
> > @@ -2706,15 +2721,6 @@ static int i915_edp_psr_status(struct seq_file *m,
> > void *data)>
> > }
> >
> > psr_source_status(dev_priv, m);
> >
> > -
> > - 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);
>
> I wonder if we shouldn't get this lock there to make sure taat no
> PSR transition from our driver is getting called.
>
This lock was useful as we wanted dev_priv->psr.enabled == intel_dp to be
valid. I don't see why we have to serialize sink DPCD reads w.r.t the driver's
PSR state transitions.
> > if (READ_ONCE(dev_priv->psr.debug)) {
> >
> > @@ -4971,9 +4977,12 @@ int i915_debugfs_connector_add(struct drm_connector
> > *connector)>
> > debugfs_create_file("i915_dpcd", S_IRUGO, root,
> >
> > connector, &i915_dpcd_fops);
> >
> > - if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> > + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
>
> it seems that we should also move the other status file here to this
> block...
Do you mean i915_edp_psr_status? As I see it, that is a feature specific
debugfs node that includes source information as well. The ones under
$debugfs/eDP-1/ are all panel related.
> > debugfs_create_file("i915_panel_timings", S_IRUGO, root,
> >
> > connector, &i915_panel_fops);
> >
> > + debugfs_create_file("i915_psr_sink_status", S_IRUGO, root,
> > + connector, &i915_psr_sink_status_fops);
>
> ... and also rename the other 2 entries to be in pair with this new one:
>
> or all i915_edp_psr or all i915_psr...
>
I wrote this as i915_psr and realized that it might get confusing since we
also have i915_edp_psr_status for feature status. And "_edp_" is redundant
because all the nodes are under the eDP-1 directory. But, yeah I can change
it to "i915_psr" if you think that is better.
> > + }
> >
> > return 0;
> >
> > }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
2018-07-05 21:27 ` Dhinakaran Pandiyan
@ 2018-07-05 21:37 ` Rodrigo Vivi
0 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2018-07-05 21:37 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx, Dhinakaran Pandiyan
On Thu, Jul 05, 2018 at 02:27:46PM -0700, Dhinakaran Pandiyan wrote:
> On Thursday, July 5, 2018 2:04:18 PM PDT Rodrigo Vivi wrote:
> > On Wed, Jul 04, 2018 at 05:31:21PM -0700, Dhinakaran Pandiyan wrote:
> > > This allows to read i915_edp_psr_status from tests without triggering
> > > any AUX communication. Take this opportunity to move this under the
> > > eDP-1 connector directory as the status we print is of the sink.
> > >
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >
> > > drivers/gpu/drm/i915/i915_debugfs.c | 69
> > > +++++++++++++++++++++---------------- 1 file changed, 39 insertions(+),
> > > 30 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c index f6142d78ede4..5069d5dedafe
> > > 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2592,6 +2592,41 @@ static const struct file_operations
> > > i915_guc_log_relay_fops = {>
> > > .release = i915_guc_log_relay_release,
> > >
> > > };
> > >
> > > +static int i915_psr_sink_status_show(struct seq_file *m, void *data)
> > > +{
> > > + 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",
> > > + };
> > > + struct drm_connector *connector = m->private;
> > > + struct intel_dp *intel_dp =
> > > + enc_to_intel_dp(&intel_attached_encoder(connector)->base);
> > > +
> > > + if (connector->status != connector_status_connected)
> > > + return -ENODEV;
> > > +
> > > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) == 1) {
> > > + const char *str = "unknown";
> > > +
> > > + val &= DP_PSR_SINK_STATE_MASK;
> > > + if (val < ARRAY_SIZE(sink_status))
> > > + str = sink_status[val];
> > > + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val, str);
> > > + } else {
> > > + DRM_ERROR("dpcd read (at %u) failed\n", DP_PSR_STATUS);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > > +
> > >
> > > static void
> > > psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
> > > {
> > >
> > > @@ -2643,26 +2678,6 @@ psr_source_status(struct drm_i915_private
> > > *dev_priv, struct seq_file *m)>
> > > seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status,
> "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);
> > >
> > > @@ -2706,15 +2721,6 @@ static int i915_edp_psr_status(struct seq_file *m,
> > > void *data)>
> > > }
> > >
> > > psr_source_status(dev_priv, m);
> > >
> > > -
> > > - 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);
> >
> > I wonder if we shouldn't get this lock there to make sure taat no
> > PSR transition from our driver is getting called.
> >
> This lock was useful as we wanted dev_priv->psr.enabled == intel_dp to be
> valid. I don't see why we have to serialize sink DPCD reads w.r.t the driver's
> PSR state transitions.
hm... makes sense...
>
> > > if (READ_ONCE(dev_priv->psr.debug)) {
> > >
> > > @@ -4971,9 +4977,12 @@ int i915_debugfs_connector_add(struct drm_connector
> > > *connector)>
> > > debugfs_create_file("i915_dpcd", S_IRUGO, root,
> > >
> > > connector, &i915_dpcd_fops);
> > >
> > > - if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> > > + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> >
> > it seems that we should also move the other status file here to this
> > block...
>
> Do you mean i915_edp_psr_status? As I see it, that is a feature specific
> debugfs node that includes source information as well. The ones under
> $debugfs/eDP-1/ are all panel related.
oh good point...
>
> > > debugfs_create_file("i915_panel_timings", S_IRUGO, root,
> > >
> > > connector, &i915_panel_fops);
> > >
> > > + debugfs_create_file("i915_psr_sink_status", S_IRUGO, root,
> > > + connector, &i915_psr_sink_status_fops);
> >
> > ... and also rename the other 2 entries to be in pair with this new one:
> >
> > or all i915_edp_psr or all i915_psr...
> >
> I wrote this as i915_psr and realized that it might get confusing since we
> also have i915_edp_psr_status for feature status. And "_edp_" is redundant
> because all the nodes are under the eDP-1 directory. But, yeah I can change
> it to "i915_psr" if you think that is better.
well, the i915_edp_psr_status is before we had this directory I think
and the idea of having the "_edp_" was to name it as eDP-1 when we had multiple cases...
So, whatever we decide for that... move or not to eDP-1, remove or not "_edp_"
I think we should do in a separated patch...
So, for this one here:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> > > + }
> > >
> > > return 0;
> > >
> > > }
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/psr: Split sink status into a separate debugfs node
2018-07-05 0:31 [PATCH] drm/i915/psr: Split sink status into a separate debugfs node Dhinakaran Pandiyan
` (3 preceding siblings ...)
2018-07-05 21:04 ` [PATCH] " Rodrigo Vivi
@ 2018-07-05 22:42 ` Patchwork
2018-07-06 13:43 ` ✓ Fi.CI.IGT: " Patchwork
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-07-05 22:42 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/psr: Split sink status into a separate debugfs node
URL : https://patchwork.freedesktop.org/series/45952/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4438 -> Patchwork_9555 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/45952/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9555 that come from known issues:
=== IGT changes ===
==== Possible fixes ====
igt@kms_frontbuffer_tracking@basic:
fi-hsw-peppy: DMESG-FAIL (fdo#106103, fdo#102614) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
== Participating hosts (47 -> 42) ==
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4438 -> Patchwork_9555
CI_DRM_4438: b689733af687b4b8072fb62a6bfe267c4e888f5f @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4539: 8b3cc74c6911e9b2835fe6e160f84bae463a70ef @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9555: 1fa6ece46478fac57189bac261cece9348ba807e @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
1fa6ece46478 drm/i915/psr: Split sink status into a separate debugfs node
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9555/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915/psr: Split sink status into a separate debugfs node
2018-07-05 0:31 [PATCH] drm/i915/psr: Split sink status into a separate debugfs node Dhinakaran Pandiyan
` (4 preceding siblings ...)
2018-07-05 22:42 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-07-06 13:43 ` Patchwork
2018-07-12 23:26 ` [PATCH] " Rodrigo Vivi
2018-07-19 6:18 ` Chris Wilson
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-07-06 13:43 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/psr: Split sink status into a separate debugfs node
URL : https://patchwork.freedesktop.org/series/45952/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4438_full -> Patchwork_9555_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9555_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9555_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9555_full:
=== IGT changes ===
==== Warnings ====
igt@gem_mocs_settings@mocs-rc6-blt:
shard-kbl: PASS -> SKIP +2
igt@gem_mocs_settings@mocs-rc6-bsd1:
shard-kbl: SKIP -> PASS +1
== Known issues ==
Here are the changes found in Patchwork_9555_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_flip@wf_vblank-ts-check:
shard-glk: PASS -> FAIL (fdo#100368)
igt@kms_flip_tiling@flip-y-tiled:
shard-glk: PASS -> FAIL (fdo#103822)
igt@kms_rotation_crc@sprite-rotation-180:
shard-hsw: PASS -> FAIL (fdo#103925)
igt@kms_setmode@basic:
shard-apl: PASS -> FAIL (fdo#99912)
==== Possible fixes ====
igt@kms_flip@2x-plain-flip-ts-check:
shard-glk: FAIL (fdo#100368) -> PASS
igt@kms_flip_tiling@flip-to-y-tiled:
shard-glk: FAIL -> PASS
==== Warnings ====
igt@drv_selftest@live_gtt:
shard-kbl: INCOMPLETE (fdo#107127, fdo#103665) -> FAIL (fdo#107127, fdo#105347)
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
fdo#107127 https://bugs.freedesktop.org/show_bug.cgi?id=107127
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4438 -> Patchwork_9555
CI_DRM_4438: b689733af687b4b8072fb62a6bfe267c4e888f5f @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4539: 8b3cc74c6911e9b2835fe6e160f84bae463a70ef @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9555: 1fa6ece46478fac57189bac261cece9348ba807e @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9555/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
2018-07-05 0:31 [PATCH] drm/i915/psr: Split sink status into a separate debugfs node Dhinakaran Pandiyan
` (5 preceding siblings ...)
2018-07-06 13:43 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-07-12 23:26 ` Rodrigo Vivi
2018-07-13 3:10 ` Dhinakaran Pandiyan
2018-07-19 6:18 ` Chris Wilson
7 siblings, 1 reply; 14+ messages in thread
From: Rodrigo Vivi @ 2018-07-12 23:26 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx, Dhinakaran Pandiyan
On Wed, Jul 04, 2018 at 05:31:21PM -0700, Dhinakaran Pandiyan wrote:
> This allows to read i915_edp_psr_status from tests without triggering
> any AUX communication. Take this opportunity to move this under the
> eDP-1 connector directory as the status we print is of the sink.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 69 +++++++++++++++++++++----------------
> 1 file changed, 39 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index f6142d78ede4..5069d5dedafe 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2592,6 +2592,41 @@ static const struct file_operations i915_guc_log_relay_fops = {
> .release = i915_guc_log_relay_release,
> };
>
> +static int i915_psr_sink_status_show(struct seq_file *m, void *data)
> +{
> + 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",
> + };
> + struct drm_connector *connector = m->private;
> + struct intel_dp *intel_dp =
> + enc_to_intel_dp(&intel_attached_encoder(connector)->base);
> +
> + if (connector->status != connector_status_connected)
> + return -ENODEV;
> +
> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) == 1) {
> + const char *str = "unknown";
> +
> + val &= DP_PSR_SINK_STATE_MASK;
> + if (val < ARRAY_SIZE(sink_status))
> + str = sink_status[val];
> + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val, str);
> + } else {
> + DRM_ERROR("dpcd read (at %u) failed\n", DP_PSR_STATUS);
> + }
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> +
> static void
> psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
> {
> @@ -2643,26 +2678,6 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
> seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status, "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);
> @@ -2706,15 +2721,6 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> }
>
> psr_source_status(dev_priv, m);
> -
> - 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);
>
> if (READ_ONCE(dev_priv->psr.debug)) {
> @@ -4971,9 +4977,12 @@ int i915_debugfs_connector_add(struct drm_connector *connector)
> debugfs_create_file("i915_dpcd", S_IRUGO, root,
> connector, &i915_dpcd_fops);
>
> - if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> debugfs_create_file("i915_panel_timings", S_IRUGO, root,
> connector, &i915_panel_fops);
> + debugfs_create_file("i915_psr_sink_status", S_IRUGO, root,
> + connector, &i915_psr_sink_status_fops);
my OCD on names here about i915_psr vs i915_edp_psr is still a real thing although
they are on different places ;)
We could probably remove "_edp" from the others, because psr is an edp only after all....
Anyways let's not block the progress here ;)
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> + }
>
> return 0;
> }
> --
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
2018-07-12 23:26 ` [PATCH] " Rodrigo Vivi
@ 2018-07-13 3:10 ` Dhinakaran Pandiyan
0 siblings, 0 replies; 14+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-13 3:10 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Thu, 2018-07-12 at 16:26 -0700, Rodrigo Vivi wrote:
> On Wed, Jul 04, 2018 at 05:31:21PM -0700, Dhinakaran Pandiyan wrote:
> >
> > This allows to read i915_edp_psr_status from tests without
> > triggering
> > any AUX communication. Take this opportunity to move this under the
> > eDP-1 connector directory as the status we print is of the sink.
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 69 +++++++++++++++++++++--
> > --------------
> > 1 file changed, 39 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index f6142d78ede4..5069d5dedafe 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2592,6 +2592,41 @@ static const struct file_operations
> > i915_guc_log_relay_fops = {
> > .release = i915_guc_log_relay_release,
> > };
> >
> > +static int i915_psr_sink_status_show(struct seq_file *m, void
> > *data)
> > +{
> > + 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",
> > + };
> > + struct drm_connector *connector = m->private;
> > + struct intel_dp *intel_dp =
> > + enc_to_intel_dp(&intel_attached_encoder(connector)
> > ->base);
> > +
> > + if (connector->status != connector_status_connected)
> > + return -ENODEV;
> > +
> > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val)
> > == 1) {
> > + const char *str = "unknown";
> > +
> > + val &= DP_PSR_SINK_STATE_MASK;
> > + if (val < ARRAY_SIZE(sink_status))
> > + str = sink_status[val];
> > + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
> > str);
> > + } else {
> > + DRM_ERROR("dpcd read (at %u) failed\n",
> > DP_PSR_STATUS);
> > + }
> > +
> > + return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > +
> > static void
> > psr_source_status(struct drm_i915_private *dev_priv, struct
> > seq_file *m)
> > {
> > @@ -2643,26 +2678,6 @@ psr_source_status(struct drm_i915_private
> > *dev_priv, struct seq_file *m)
> > seq_printf(m, "Source PSR status: 0x%x [%s]\n",
> > psr_status, "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);
> > @@ -2706,15 +2721,6 @@ static int i915_edp_psr_status(struct
> > seq_file *m, void *data)
> > }
> >
> > psr_source_status(dev_priv, m);
> > -
> > - 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);
> >
> > if (READ_ONCE(dev_priv->psr.debug)) {
> > @@ -4971,9 +4977,12 @@ int i915_debugfs_connector_add(struct
> > drm_connector *connector)
> > debugfs_create_file("i915_dpcd", S_IRUGO, root,
> > connector, &i915_dpcd_fops);
> >
> > - if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> > + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> > debugfs_create_file("i915_panel_timings", S_IRUGO,
> > root,
> > connector, &i915_panel_fops);
> > + debugfs_create_file("i915_psr_sink_status",
> > S_IRUGO, root,
> > + connector,
> > &i915_psr_sink_status_fops);
> my OCD on names here about i915_psr vs i915_edp_psr is still a real
> thing although
> they are on different places ;)
>
> We could probably remove "_edp" from the others, because psr is an
> edp only after all....
>
> Anyways let's not block the progress here ;)
>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Thank you, I pushed this to -queued.
>
>
>
> >
> > + }
> >
> > return 0;
> > }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
2018-07-05 0:31 [PATCH] drm/i915/psr: Split sink status into a separate debugfs node Dhinakaran Pandiyan
` (6 preceding siblings ...)
2018-07-12 23:26 ` [PATCH] " Rodrigo Vivi
@ 2018-07-19 6:18 ` Chris Wilson
2018-07-19 7:18 ` Dhinakaran Pandiyan
7 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2018-07-19 6:18 UTC (permalink / raw)
To: Dhinakaran Pandiyan, intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Quoting Dhinakaran Pandiyan (2018-07-05 01:31:21)
> This allows to read i915_edp_psr_status from tests without triggering
> any AUX communication. Take this opportunity to move this under the
> eDP-1 connector directory as the status we print is of the sink.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 69 +++++++++++++++++++++----------------
> 1 file changed, 39 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index f6142d78ede4..5069d5dedafe 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2592,6 +2592,41 @@ static const struct file_operations i915_guc_log_relay_fops = {
> .release = i915_guc_log_relay_release,
> };
>
> +static int i915_psr_sink_status_show(struct seq_file *m, void *data)
> +{
> + 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",
> + };
> + struct drm_connector *connector = m->private;
> + struct intel_dp *intel_dp =
> + enc_to_intel_dp(&intel_attached_encoder(connector)->base);
> +
> + if (connector->status != connector_status_connected)
> + return -ENODEV;
> +
> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) == 1) {
> + const char *str = "unknown";
> +
> + val &= DP_PSR_SINK_STATE_MASK;
> + if (val < ARRAY_SIZE(sink_status))
> + str = sink_status[val];
> + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val, str);
> + } else {
> + DRM_ERROR("dpcd read (at %u) failed\n", DP_PSR_STATUS);
Why is this common occurrence (any DP that doesn't support PSR) an error?
Why report it via a backchannel when you are already directly
communicating with the user?!!!
Please fix this mess.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
2018-07-19 7:18 ` Dhinakaran Pandiyan
@ 2018-07-19 7:00 ` Rodrigo Vivi
0 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2018-07-19 7:00 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
On Thu, Jul 19, 2018 at 12:18:39AM -0700, Dhinakaran Pandiyan wrote:
> On Thu, 2018-07-19 at 07:18 +0100, Chris Wilson wrote:
> > Quoting Dhinakaran Pandiyan (2018-07-05 01:31:21)
> > >
> > > This allows to read i915_edp_psr_status from tests without
> > > triggering
> > > any AUX communication. Take this opportunity to move this under the
> > > eDP-1 connector directory as the status we print is of the sink.
> > >
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_debugfs.c | 69 +++++++++++++++++++++--
> > > --------------
> > > 1 file changed, 39 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index f6142d78ede4..5069d5dedafe 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2592,6 +2592,41 @@ static const struct file_operations
> > > i915_guc_log_relay_fops = {
> > > .release = i915_guc_log_relay_release,
> > > };
> > >
> > > +static int i915_psr_sink_status_show(struct seq_file *m, void
> > > *data)
> > > +{
> > > + 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",
> > > + };
> > > + struct drm_connector *connector = m->private;
> > > + struct intel_dp *intel_dp =
> > > + enc_to_intel_dp(&intel_attached_encoder(connector)-
> > > >base);
> > > +
> > > + if (connector->status != connector_status_connected)
> > > + return -ENODEV;
> > > +
> > > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val)
> > > == 1) {
> > > + const char *str = "unknown";
> > > +
> > > + val &= DP_PSR_SINK_STATE_MASK;
> > > + if (val < ARRAY_SIZE(sink_status))
> > > + str = sink_status[val];
> > > + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
> > > str);
> > > + } else {
> > > + DRM_ERROR("dpcd read (at %u) failed\n",
> > > DP_PSR_STATUS);
> > Why is this common occurrence (any DP that doesn't support PSR) an
> > error?
> We create this file only for eDPs and I did not think dpcd_read would
> return an error if the panel did not support PSR. Do you happen to have
> any logs?
>
> > Why report it via a backchannel when you are already directly
> > communicating with the user?!!!
> >
> You are right, i915_dpcd_show() also has the same issue. How about
> returning the error to the user?
>
> @@ -2595,6 +2595,7 @@ static const struct file_operations
> i915_guc_log_relay_fops = {
> static int i915_psr_sink_status_show(struct seq_file *m, void *data)
> {
> u8 val;
> + int ret;
> static const char * const sink_status[] = {
> "inactive",
> "transition to active, capture and display",
> @@ -2605,6 +2606,7 @@ static int i915_psr_sink_status_show(struct
> seq_file *m, void *data)
> "reserved",
> "sink internal error",
> };
> + const char *str = "unknown";
> struct drm_connector *connector = m->private;
> struct intel_dp *intel_dp =
> enc_to_intel_dp(&intel_attached_encoder(connector)-
> >base);
> @@ -2612,17 +2614,16 @@ static int i915_psr_sink_status_show(struct
> seq_file *m, void *data)
> if (connector->status != connector_status_connected)
> return -ENODEV;
>
> - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) ==
> 1) {
> - const char *str = "unknown";
> + ret = drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val);
>
> + if (ret < 0) {
> + return ret;
> + } else if (ret == 1) {
> val &= DP_PSR_SINK_STATE_MASK;
> if (val < ARRAY_SIZE(sink_status))
> str = sink_status[val];
> - seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
> str);
> - } else {
> - DRM_ERROR("dpcd read (at %u) failed\n", DP_PSR_STATUS);
> }
> -
> + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val, str);
> return 0;
> }
ops, just saw this after sending the patch...
I think this should work as well.
However I believe it is better not even read the dpcd if psr is not supported.
>
> > Please fix this mess.
> > -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
2018-07-19 6:18 ` Chris Wilson
@ 2018-07-19 7:18 ` Dhinakaran Pandiyan
2018-07-19 7:00 ` Rodrigo Vivi
0 siblings, 1 reply; 14+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-19 7:18 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Rodrigo Vivi
On Thu, 2018-07-19 at 07:18 +0100, Chris Wilson wrote:
> Quoting Dhinakaran Pandiyan (2018-07-05 01:31:21)
> >
> > This allows to read i915_edp_psr_status from tests without
> > triggering
> > any AUX communication. Take this opportunity to move this under the
> > eDP-1 connector directory as the status we print is of the sink.
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 69 +++++++++++++++++++++--
> > --------------
> > 1 file changed, 39 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index f6142d78ede4..5069d5dedafe 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2592,6 +2592,41 @@ static const struct file_operations
> > i915_guc_log_relay_fops = {
> > .release = i915_guc_log_relay_release,
> > };
> >
> > +static int i915_psr_sink_status_show(struct seq_file *m, void
> > *data)
> > +{
> > + 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",
> > + };
> > + struct drm_connector *connector = m->private;
> > + struct intel_dp *intel_dp =
> > + enc_to_intel_dp(&intel_attached_encoder(connector)-
> > >base);
> > +
> > + if (connector->status != connector_status_connected)
> > + return -ENODEV;
> > +
> > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val)
> > == 1) {
> > + const char *str = "unknown";
> > +
> > + val &= DP_PSR_SINK_STATE_MASK;
> > + if (val < ARRAY_SIZE(sink_status))
> > + str = sink_status[val];
> > + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
> > str);
> > + } else {
> > + DRM_ERROR("dpcd read (at %u) failed\n",
> > DP_PSR_STATUS);
> Why is this common occurrence (any DP that doesn't support PSR) an
> error?
We create this file only for eDPs and I did not think dpcd_read would
return an error if the panel did not support PSR. Do you happen to have
any logs?
> Why report it via a backchannel when you are already directly
> communicating with the user?!!!
>
You are right, i915_dpcd_show() also has the same issue. How about
returning the error to the user?
@@ -2595,6 +2595,7 @@ static const struct file_operations
i915_guc_log_relay_fops = {
static int i915_psr_sink_status_show(struct seq_file *m, void *data)
{
u8 val;
+ int ret;
static const char * const sink_status[] = {
"inactive",
"transition to active, capture and display",
@@ -2605,6 +2606,7 @@ static int i915_psr_sink_status_show(struct
seq_file *m, void *data)
"reserved",
"sink internal error",
};
+ const char *str = "unknown";
struct drm_connector *connector = m->private;
struct intel_dp *intel_dp =
enc_to_intel_dp(&intel_attached_encoder(connector)-
>base);
@@ -2612,17 +2614,16 @@ static int i915_psr_sink_status_show(struct
seq_file *m, void *data)
if (connector->status != connector_status_connected)
return -ENODEV;
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) ==
1) {
- const char *str = "unknown";
+ ret = drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val);
+ if (ret < 0) {
+ return ret;
+ } else if (ret == 1) {
val &= DP_PSR_SINK_STATE_MASK;
if (val < ARRAY_SIZE(sink_status))
str = sink_status[val];
- seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
str);
- } else {
- DRM_ERROR("dpcd read (at %u) failed\n", DP_PSR_STATUS);
}
-
+ seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val, str);
return 0;
}
> Please fix this mess.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-07-19 7:00 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-05 0:31 [PATCH] drm/i915/psr: Split sink status into a separate debugfs node Dhinakaran Pandiyan
2018-07-05 0:37 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-07-05 0:55 ` ✓ Fi.CI.BAT: success " Patchwork
2018-07-05 4:54 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-07-05 21:04 ` [PATCH] " Rodrigo Vivi
2018-07-05 21:27 ` Dhinakaran Pandiyan
2018-07-05 21:37 ` Rodrigo Vivi
2018-07-05 22:42 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-07-06 13:43 ` ✓ Fi.CI.IGT: " Patchwork
2018-07-12 23:26 ` [PATCH] " Rodrigo Vivi
2018-07-13 3:10 ` Dhinakaran Pandiyan
2018-07-19 6:18 ` Chris Wilson
2018-07-19 7:18 ` Dhinakaran Pandiyan
2018-07-19 7:00 ` Rodrigo Vivi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.