| Message ID | 20180328223046.16125-10-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2018-03-28 at 15:30 -0700, José Roberto de Souza wrote: > IGT tests could be improved with sink status, knowing for sure that > hardware have activate or exit PSR. > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > > v3: rebased > > 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..89dc5b05ec24 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_self_refresh_status(u8 val) nit: psr_sink_status() is concise, "psr_self_refresh" sounds redundant. > +{ > + static const char * const sink_status[] = { > + "inactive", > + "transitioning to active", We muddle the meaning of these statuses by paraphrasing, it is better write these strings exactly as the states are defined in the spec. > + "active", > + "active receiving selective update", For example, this state corresponds to the capture of full static frame too. Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> with the nits addressed, thanks for the patch. > + "transitioning to inactive", > + "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 self refresh status: 0x%x [%s]\n", > + val, psr_sink_self_refresh_status(val)); > + } > mutex_unlock(&dev_priv->psr.lock); > > intel_runtime_pm_put(dev_priv);
On Fri, 2018-03-30 at 11:28 -0700, Pandiyan, Dhinakaran wrote: > On Wed, 2018-03-28 at 15:30 -0700, José Roberto de Souza wrote: > > IGT tests could be improved with sink status, knowing for sure that > > hardware have activate or exit PSR. > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > > > v3: rebased > > > > 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..89dc5b05ec24 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_self_refresh_status(u8 val) > > nit: psr_sink_status() is concise, > "psr_self_refresh" sounds > redundant. > > > +{ > > + static const char * const sink_status[] = { > > + "inactive", > > + "transitioning to active", > > We muddle the meaning of these statuses by paraphrasing, it is better > write these strings exactly as the states are defined in the spec. > > > + "active", > > + "active receiving selective update", > > For example, this state corresponds to the capture of full static > frame > too. It is fine this way? 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" }; > > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> with > the nits addressed, thanks for the patch. > > > + "transitioning to inactive", > > + "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 self refresh status: > > 0x%x [%s]\n", > > + val, > > psr_sink_self_refresh_status(val)); > > + } > > mutex_unlock(&dev_priv->psr.lock); > > > > intel_runtime_pm_put(dev_priv);
On Fri, 2018-03-30 at 19:19 +0000, Souza, Jose wrote: > On Fri, 2018-03-30 at 11:28 -0700, Pandiyan, Dhinakaran wrote: > > On Wed, 2018-03-28 at 15:30 -0700, José Roberto de Souza wrote: > > > IGT tests could be improved with sink status, knowing for sure that > > > hardware have activate or exit PSR. > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > --- > > > > > > v3: rebased > > > > > > 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..89dc5b05ec24 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_self_refresh_status(u8 val) > > > > nit: psr_sink_status() is concise, > > "psr_self_refresh" sounds > > redundant. > > > > > +{ > > > + static const char * const sink_status[] = { > > > + "inactive", > > > + "transitioning to active", > > > > We muddle the meaning of these statuses by paraphrasing, it is better > > write these strings exactly as the states are defined in the spec. > > > > > + "active", > > > + "active receiving selective update", > > > > For example, this state corresponds to the capture of full static > > frame > > too. > > > It is fine this way? > > 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" > }; > > Looks good. > > > > > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> with > > the nits addressed, thanks for the patch. > > > > > + "transitioning to inactive", > > > + "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 self refresh status: > > > 0x%x [%s]\n", > > > + val, > > > psr_sink_self_refresh_status(val)); > > > + } > > > mutex_unlock(&dev_priv->psr.lock); > > > > > > intel_runtime_pm_put(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 1dba2c451255..89dc5b05ec24 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_self_refresh_status(u8 val) +{ + static const char * const sink_status[] = { + "inactive", + "transitioning to active", + "active", + "active receiving selective update", + "transitioning to inactive", + "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 self refresh status: 0x%x [%s]\n", + val, psr_sink_self_refresh_status(val)); + } mutex_unlock(&dev_priv->psr.lock); intel_runtime_pm_put(dev_priv);
IGT tests could be improved with sink status, knowing for sure that hardware have activate or exit PSR. Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- v3: rebased drivers/gpu/drm/i915/i915_debugfs.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)