Re: [Intel-gfx] [PATCH 2/5] drm/i915: Refactor PSR status debugfs

2019-01-02 Thread Dhinakaran Pandiyan
On Wed, 2019-01-02 at 09:09 -0800, Souza, Jose wrote:
> On Tue, 2018-12-11 at 18:54 +, Souza, Jose wrote:
> > On Tue, 2018-12-11 at 10:32 -0800, Dhinakaran Pandiyan wrote:
> > > On Tue, 2018-12-11 at 04:44 -0800, Souza, Jose wrote:
> > > > On Mon, 2018-12-10 at 22:51 -0800, Dhinakaran Pandiyan wrote:
> > > > > On Tue, 2018-12-04 at 15:00 -0800, José Roberto de Souza
> > > > > wrote:
> > > > > > The old debugfs fields was not following a naming partern
> > > > > > and
> > > > > > it
> > > > > > was
> > > > > > a bit confusing.
> > > > > > 
> > > > > > So it went from:
> > > > > > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> > > > > > Sink_Support: yes
> > > > > > PSR mode: PSR1
> > > > > > Enabled: yes
> > > > > > Busy frontbuffer bits: 0x000
> > > > > > Main link in standby mode: no
> > > > > > HW Enabled & Active bit: yes
> > > > > > Source PSR status: 0x24050006 [SRDONACK]
> > > > > > 
> > > > > > To:
> > > > > > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> > > > > > Sink support: yes [0x0003]
> > > > > > Status: PSR1 enabled
> > > > > > Source PSR ctl: enabled [0x81f00e26]
> > > > > > Source PSR status: SRDENT [0x40040006]
> > > > > > Busy frontbuffer bits: 0x
> > > > > > 
> > > > > > The 'Main link in standby mode' was removed as it is not
> > > > > > useful
> > > > > > but
> > > > > > if needed by someone the information is still in the
> > > > > > register
> > > > > > value
> > > > > > of 'Source PSR ctl' inside of the brackets, PSR mode and
> > > > > > Enabled
> > > > > > was
> > > > > > squashed into Status, some renames and reorders and we have
> > > > > > this
> > > > > > cleaner version. This will also make easy to parse debugfs
> > > > > > for
> > > > > > IGT
> > > > > > tests.
> > > > > > 
> > > > > > Cc: Rodrigo Vivi 
> > > > > > Cc: Dhinakaran Pandiyan 
> > > > > > Suggested-by: Dhinakaran Pandiyan <
> > > > > > dhinakaran.pandi...@intel.com>
> > > > > > Signed-off-by: José Roberto de Souza 
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/i915_debugfs.c | 96 +++--
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > >  1 file changed, 49 insertions(+), 47 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > > index 38dcee1ca062..86303ba02666 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > > @@ -2665,7 +2665,8 @@
> > > > > > DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > > > > >  static void
> > > > > >  psr_source_status(struct drm_i915_private *dev_priv,
> > > > > > struct
> > > > > > seq_file
> > > > > > *m)
> > > > > >  {
> > > > > > -   u32 val, psr_status;
> > > > > > +   u32 val, status_val;
> > > > > > +   const char *status = "unknown";
> > > > > >  
> > > > > > if (dev_priv->psr.psr2_enabled) {
> > > > > > static const char * const live_status[] = {
> > > > > > @@ -2681,14 +2682,11 @@ psr_source_status(struct
> > > > > > drm_i915_private
> > > > > > *dev_priv, struct seq_file *m)
> > > > > > "BUF_ON",
> > > > > > "TG_ON"
> > > > > > };
> > > > > > -   psr_status = I915_READ(EDP_PSR2_STATUS);
> > > > > > -   val = (psr_status & EDP_PSR2_STATUS_STATE_MASK)
> > > > > > -   EDP_PSR2_STATUS_STATE_SHIFT;
> > > > > > -   if (val < ARRAY_SIZE(live_status)) {
> > > > > > -   seq_printf(m, "Source PSR status: 0x%x
> > > > > > [%s]\n",
> > > > > > -  psr_status,
> > > > > > live_status[val]);
> > > > > > -   return;
> > > > > > -   }
> > > > > > +   val = I915_READ(EDP_PSR2_STATUS);
> > > > > > +   status_val = (val & EDP_PSR2_STATUS_STATE_MASK)
> > > > > > + EDP_PSR2_STATUS_STATE_SHIFT;
> > > > > > +   if (status_val < ARRAY_SIZE(live_status))
> > > > > > +   status = live_status[status_val];
> > > > > > } else {
> > > > > > static const char * const live_status[] = {
> > > > > > "IDLE",
> > > > > > @@ -2700,74 +2698,78 @@ psr_source_status(struct
> > > > > > drm_i915_private
> > > > > > *dev_priv, struct seq_file *m)
> > > > > > "SRDOFFACK",
> > > > > > "SRDENT_ON",
> > > > > > };
> > > > > > -   psr_status = I915_READ(EDP_PSR_STATUS);
> > > > > > -   val = (psr_status & EDP_PSR_STATUS_STATE_MASK)
> > > > > > -   EDP_PSR_STATUS_STATE_SHIFT;
> > > > > > -   if (val < ARRAY_SIZE(live_status)) {
> > > > > > -   seq_printf(m, "Source PSR status: 0x%x
> > > > > > [%s]\n",
> > > > > > -  psr_status,
> > > > > > live_status[val]);
> > > > > > -   return;
> > > > > > -   }
> > > > > > +   val = I915_READ(EDP_PSR_STATUS);
> > > > 

Re: [Intel-gfx] [PATCH 2/5] drm/i915: Refactor PSR status debugfs

2019-01-02 Thread Souza, Jose
On Tue, 2018-12-11 at 18:54 +, Souza, Jose wrote:
> On Tue, 2018-12-11 at 10:32 -0800, Dhinakaran Pandiyan wrote:
> > On Tue, 2018-12-11 at 04:44 -0800, Souza, Jose wrote:
> > > On Mon, 2018-12-10 at 22:51 -0800, Dhinakaran Pandiyan wrote:
> > > > On Tue, 2018-12-04 at 15:00 -0800, José Roberto de Souza wrote:
> > > > > The old debugfs fields was not following a naming partern and
> > > > > it
> > > > > was
> > > > > a bit confusing.
> > > > > 
> > > > > So it went from:
> > > > > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> > > > > Sink_Support: yes
> > > > > PSR mode: PSR1
> > > > > Enabled: yes
> > > > > Busy frontbuffer bits: 0x000
> > > > > Main link in standby mode: no
> > > > > HW Enabled & Active bit: yes
> > > > > Source PSR status: 0x24050006 [SRDONACK]
> > > > > 
> > > > > To:
> > > > > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> > > > > Sink support: yes [0x0003]
> > > > > Status: PSR1 enabled
> > > > > Source PSR ctl: enabled [0x81f00e26]
> > > > > Source PSR status: SRDENT [0x40040006]
> > > > > Busy frontbuffer bits: 0x
> > > > > 
> > > > > The 'Main link in standby mode' was removed as it is not
> > > > > useful
> > > > > but
> > > > > if needed by someone the information is still in the register
> > > > > value
> > > > > of 'Source PSR ctl' inside of the brackets, PSR mode and
> > > > > Enabled
> > > > > was
> > > > > squashed into Status, some renames and reorders and we have
> > > > > this
> > > > > cleaner version. This will also make easy to parse debugfs
> > > > > for
> > > > > IGT
> > > > > tests.
> > > > > 
> > > > > Cc: Rodrigo Vivi 
> > > > > Cc: Dhinakaran Pandiyan 
> > > > > Suggested-by: Dhinakaran Pandiyan <
> > > > > dhinakaran.pandi...@intel.com>
> > > > > Signed-off-by: José Roberto de Souza 
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_debugfs.c | 96 +++--
> > > > > 
> > > > > 
> > > > > 
> > > > >  1 file changed, 49 insertions(+), 47 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > index 38dcee1ca062..86303ba02666 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > > @@ -2665,7 +2665,8 @@
> > > > > DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > > > >  static void
> > > > >  psr_source_status(struct drm_i915_private *dev_priv, struct
> > > > > seq_file
> > > > > *m)
> > > > >  {
> > > > > - u32 val, psr_status;
> > > > > + u32 val, status_val;
> > > > > + const char *status = "unknown";
> > > > >  
> > > > >   if (dev_priv->psr.psr2_enabled) {
> > > > >   static const char * const live_status[] = {
> > > > > @@ -2681,14 +2682,11 @@ psr_source_status(struct
> > > > > drm_i915_private
> > > > > *dev_priv, struct seq_file *m)
> > > > >   "BUF_ON",
> > > > >   "TG_ON"
> > > > >   };
> > > > > - psr_status = I915_READ(EDP_PSR2_STATUS);
> > > > > - val = (psr_status & EDP_PSR2_STATUS_STATE_MASK)
> > > > > - EDP_PSR2_STATUS_STATE_SHIFT;
> > > > > - if (val < ARRAY_SIZE(live_status)) {
> > > > > - seq_printf(m, "Source PSR status: 0x%x
> > > > > [%s]\n",
> > > > > -psr_status,
> > > > > live_status[val]);
> > > > > - return;
> > > > > - }
> > > > > + val = I915_READ(EDP_PSR2_STATUS);
> > > > > + status_val = (val & EDP_PSR2_STATUS_STATE_MASK)
> > > > > +   EDP_PSR2_STATUS_STATE_SHIFT;
> > > > > + if (status_val < ARRAY_SIZE(live_status))
> > > > > + status = live_status[status_val];
> > > > >   } else {
> > > > >   static const char * const live_status[] = {
> > > > >   "IDLE",
> > > > > @@ -2700,74 +2698,78 @@ psr_source_status(struct
> > > > > drm_i915_private
> > > > > *dev_priv, struct seq_file *m)
> > > > >   "SRDOFFACK",
> > > > >   "SRDENT_ON",
> > > > >   };
> > > > > - psr_status = I915_READ(EDP_PSR_STATUS);
> > > > > - val = (psr_status & EDP_PSR_STATUS_STATE_MASK)
> > > > > - EDP_PSR_STATUS_STATE_SHIFT;
> > > > > - if (val < ARRAY_SIZE(live_status)) {
> > > > > - seq_printf(m, "Source PSR status: 0x%x
> > > > > [%s]\n",
> > > > > -psr_status,
> > > > > live_status[val]);
> > > > > - return;
> > > > > - }
> > > > > + val = I915_READ(EDP_PSR_STATUS);
> > > > > + status_val = (val & EDP_PSR_STATUS_STATE_MASK)
> > > > > +   EDP_PSR_STATUS_STATE_SHIFT;
> > > > > + if (status_val < ARRAY_SIZE(live_status))
> > > > > + status = 

Re: [Intel-gfx] [PATCH 2/5] drm/i915: Refactor PSR status debugfs

2018-12-11 Thread Souza, Jose
On Tue, 2018-12-11 at 10:32 -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2018-12-11 at 04:44 -0800, Souza, Jose wrote:
> > On Mon, 2018-12-10 at 22:51 -0800, Dhinakaran Pandiyan wrote:
> > > On Tue, 2018-12-04 at 15:00 -0800, José Roberto de Souza wrote:
> > > > The old debugfs fields was not following a naming partern and
> > > > it
> > > > was
> > > > a bit confusing.
> > > > 
> > > > So it went from:
> > > > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> > > > Sink_Support: yes
> > > > PSR mode: PSR1
> > > > Enabled: yes
> > > > Busy frontbuffer bits: 0x000
> > > > Main link in standby mode: no
> > > > HW Enabled & Active bit: yes
> > > > Source PSR status: 0x24050006 [SRDONACK]
> > > > 
> > > > To:
> > > > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> > > > Sink support: yes [0x0003]
> > > > Status: PSR1 enabled
> > > > Source PSR ctl: enabled [0x81f00e26]
> > > > Source PSR status: SRDENT [0x40040006]
> > > > Busy frontbuffer bits: 0x
> > > > 
> > > > The 'Main link in standby mode' was removed as it is not useful
> > > > but
> > > > if needed by someone the information is still in the register
> > > > value
> > > > of 'Source PSR ctl' inside of the brackets, PSR mode and
> > > > Enabled
> > > > was
> > > > squashed into Status, some renames and reorders and we have
> > > > this
> > > > cleaner version. This will also make easy to parse debugfs for
> > > > IGT
> > > > tests.
> > > > 
> > > > Cc: Rodrigo Vivi 
> > > > Cc: Dhinakaran Pandiyan 
> > > > Suggested-by: Dhinakaran Pandiyan <
> > > > dhinakaran.pandi...@intel.com>
> > > > Signed-off-by: José Roberto de Souza 
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_debugfs.c | 96 +++--
> > > > 
> > > > 
> > > >  1 file changed, 49 insertions(+), 47 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > index 38dcee1ca062..86303ba02666 100644
> > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > @@ -2665,7 +2665,8 @@
> > > > DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > > >  static void
> > > >  psr_source_status(struct drm_i915_private *dev_priv, struct
> > > > seq_file
> > > > *m)
> > > >  {
> > > > -   u32 val, psr_status;
> > > > +   u32 val, status_val;
> > > > +   const char *status = "unknown";
> > > >  
> > > > if (dev_priv->psr.psr2_enabled) {
> > > > static const char * const live_status[] = {
> > > > @@ -2681,14 +2682,11 @@ psr_source_status(struct
> > > > drm_i915_private
> > > > *dev_priv, struct seq_file *m)
> > > > "BUF_ON",
> > > > "TG_ON"
> > > > };
> > > > -   psr_status = I915_READ(EDP_PSR2_STATUS);
> > > > -   val = (psr_status & EDP_PSR2_STATUS_STATE_MASK)
> > > > >>
> > > > -   EDP_PSR2_STATUS_STATE_SHIFT;
> > > > -   if (val < ARRAY_SIZE(live_status)) {
> > > > -   seq_printf(m, "Source PSR status: 0x%x
> > > > [%s]\n",
> > > > -  psr_status,
> > > > live_status[val]);
> > > > -   return;
> > > > -   }
> > > > +   val = I915_READ(EDP_PSR2_STATUS);
> > > > +   status_val = (val & EDP_PSR2_STATUS_STATE_MASK)
> > > > >>
> > > > + EDP_PSR2_STATUS_STATE_SHIFT;
> > > > +   if (status_val < ARRAY_SIZE(live_status))
> > > > +   status = live_status[status_val];
> > > > } else {
> > > > static const char * const live_status[] = {
> > > > "IDLE",
> > > > @@ -2700,74 +2698,78 @@ psr_source_status(struct
> > > > drm_i915_private
> > > > *dev_priv, struct seq_file *m)
> > > > "SRDOFFACK",
> > > > "SRDENT_ON",
> > > > };
> > > > -   psr_status = I915_READ(EDP_PSR_STATUS);
> > > > -   val = (psr_status & EDP_PSR_STATUS_STATE_MASK)
> > > > >>
> > > > -   EDP_PSR_STATUS_STATE_SHIFT;
> > > > -   if (val < ARRAY_SIZE(live_status)) {
> > > > -   seq_printf(m, "Source PSR status: 0x%x
> > > > [%s]\n",
> > > > -  psr_status,
> > > > live_status[val]);
> > > > -   return;
> > > > -   }
> > > > +   val = I915_READ(EDP_PSR_STATUS);
> > > > +   status_val = (val & EDP_PSR_STATUS_STATE_MASK)
> > > > >>
> > > > + EDP_PSR_STATUS_STATE_SHIFT;
> > > > +   if (status_val < ARRAY_SIZE(live_status))
> > > > +   status = live_status[status_val];
> > > > }
> > > >  
> > > > -   seq_printf(m, "Source PSR status: 0x%x [%s]\n",
> > > > psr_status,
> > > > "unknown");
> > > > +   seq_printf(m, "Source PSR status: 

Re: [Intel-gfx] [PATCH 2/5] drm/i915: Refactor PSR status debugfs

2018-12-11 Thread Dhinakaran Pandiyan
On Tue, 2018-12-11 at 04:44 -0800, Souza, Jose wrote:
> On Mon, 2018-12-10 at 22:51 -0800, Dhinakaran Pandiyan wrote:
> > On Tue, 2018-12-04 at 15:00 -0800, José Roberto de Souza wrote:
> > > The old debugfs fields was not following a naming partern and it
> > > was
> > > a bit confusing.
> > > 
> > > So it went from:
> > > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> > > Sink_Support: yes
> > > PSR mode: PSR1
> > > Enabled: yes
> > > Busy frontbuffer bits: 0x000
> > > Main link in standby mode: no
> > > HW Enabled & Active bit: yes
> > > Source PSR status: 0x24050006 [SRDONACK]
> > > 
> > > To:
> > > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> > > Sink support: yes [0x0003]
> > > Status: PSR1 enabled
> > > Source PSR ctl: enabled [0x81f00e26]
> > > Source PSR status: SRDENT [0x40040006]
> > > Busy frontbuffer bits: 0x
> > > 
> > > The 'Main link in standby mode' was removed as it is not useful
> > > but
> > > if needed by someone the information is still in the register
> > > value
> > > of 'Source PSR ctl' inside of the brackets, PSR mode and Enabled
> > > was
> > > squashed into Status, some renames and reorders and we have this
> > > cleaner version. This will also make easy to parse debugfs for
> > > IGT
> > > tests.
> > > 
> > > Cc: Rodrigo Vivi 
> > > Cc: Dhinakaran Pandiyan 
> > > Suggested-by: Dhinakaran Pandiyan 
> > > Signed-off-by: José Roberto de Souza 
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c | 96 +++--
> > > 
> > > 
> > >  1 file changed, 49 insertions(+), 47 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 38dcee1ca062..86303ba02666 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2665,7 +2665,8 @@
> > > DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > >  static void
> > >  psr_source_status(struct drm_i915_private *dev_priv, struct
> > > seq_file
> > > *m)
> > >  {
> > > - u32 val, psr_status;
> > > + u32 val, status_val;
> > > + const char *status = "unknown";
> > >  
> > >   if (dev_priv->psr.psr2_enabled) {
> > >   static const char * const live_status[] = {
> > > @@ -2681,14 +2682,11 @@ psr_source_status(struct drm_i915_private
> > > *dev_priv, struct seq_file *m)
> > >   "BUF_ON",
> > >   "TG_ON"
> > >   };
> > > - psr_status = I915_READ(EDP_PSR2_STATUS);
> > > - val = (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
> > > - EDP_PSR2_STATUS_STATE_SHIFT;
> > > - if (val < ARRAY_SIZE(live_status)) {
> > > - seq_printf(m, "Source PSR status: 0x%x [%s]\n",
> > > -psr_status, live_status[val]);
> > > - return;
> > > - }
> > > + val = I915_READ(EDP_PSR2_STATUS);
> > > + status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> > > +   EDP_PSR2_STATUS_STATE_SHIFT;
> > > + if (status_val < ARRAY_SIZE(live_status))
> > > + status = live_status[status_val];
> > >   } else {
> > >   static const char * const live_status[] = {
> > >   "IDLE",
> > > @@ -2700,74 +2698,78 @@ psr_source_status(struct drm_i915_private
> > > *dev_priv, struct seq_file *m)
> > >   "SRDOFFACK",
> > >   "SRDENT_ON",
> > >   };
> > > - psr_status = I915_READ(EDP_PSR_STATUS);
> > > - val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
> > > - EDP_PSR_STATUS_STATE_SHIFT;
> > > - if (val < ARRAY_SIZE(live_status)) {
> > > - seq_printf(m, "Source PSR status: 0x%x [%s]\n",
> > > -psr_status, live_status[val]);
> > > - return;
> > > - }
> > > + val = I915_READ(EDP_PSR_STATUS);
> > > + status_val = (val & EDP_PSR_STATUS_STATE_MASK) >>
> > > +   EDP_PSR_STATUS_STATE_SHIFT;
> > > + if (status_val < ARRAY_SIZE(live_status))
> > > + status = live_status[status_val];
> > >   }
> > >  
> > > - seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status,
> > > "unknown");
> > > + seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val);
> > >  }
> > >  
> > >  static int i915_edp_psr_status(struct seq_file *m, void *data)
> > >  {
> > >   struct drm_i915_private *dev_priv = node_to_i915(m->private);
> > > - u32 psrperf = 0;
> > > - bool enabled = false;
> > > - bool sink_support;
> > > + struct i915_psr *psr = _priv->psr;
> > > + const char *status;
> > > + bool enabled;
> > > + u32 val;
> > >  
> > >   if (!HAS_PSR(dev_priv))
> > >   return -ENODEV;
> > >  
> > > - sink_support = dev_priv->psr.sink_support;
> > > - seq_printf(m, "Sink_Support: %s\n", yesno(sink_support));
> > > - if (!sink_support)
> > > + seq_printf(m, "Sink support: %s", yesno(psr->sink_support));
> > > + 

Re: [Intel-gfx] [PATCH 2/5] drm/i915: Refactor PSR status debugfs

2018-12-11 Thread Souza, Jose
On Mon, 2018-12-10 at 22:51 -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2018-12-04 at 15:00 -0800, José Roberto de Souza wrote:
> > The old debugfs fields was not following a naming partern and it
> > was
> > a bit confusing.
> > 
> > So it went from:
> > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> > Sink_Support: yes
> > PSR mode: PSR1
> > Enabled: yes
> > Busy frontbuffer bits: 0x000
> > Main link in standby mode: no
> > HW Enabled & Active bit: yes
> > Source PSR status: 0x24050006 [SRDONACK]
> > 
> > To:
> > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> > Sink support: yes [0x0003]
> > Status: PSR1 enabled
> > Source PSR ctl: enabled [0x81f00e26]
> > Source PSR status: SRDENT [0x40040006]
> > Busy frontbuffer bits: 0x
> > 
> > The 'Main link in standby mode' was removed as it is not useful but
> > if needed by someone the information is still in the register value
> > of 'Source PSR ctl' inside of the brackets, PSR mode and Enabled
> > was
> > squashed into Status, some renames and reorders and we have this
> > cleaner version. This will also make easy to parse debugfs for IGT
> > tests.
> > 
> > Cc: Rodrigo Vivi 
> > Cc: Dhinakaran Pandiyan 
> > Suggested-by: Dhinakaran Pandiyan 
> > Signed-off-by: José Roberto de Souza 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 96 +++--
> > 
> >  1 file changed, 49 insertions(+), 47 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 38dcee1ca062..86303ba02666 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2665,7 +2665,8 @@ DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> >  static void
> >  psr_source_status(struct drm_i915_private *dev_priv, struct
> > seq_file
> > *m)
> >  {
> > -   u32 val, psr_status;
> > +   u32 val, status_val;
> > +   const char *status = "unknown";
> >  
> > if (dev_priv->psr.psr2_enabled) {
> > static const char * const live_status[] = {
> > @@ -2681,14 +2682,11 @@ psr_source_status(struct drm_i915_private
> > *dev_priv, struct seq_file *m)
> > "BUF_ON",
> > "TG_ON"
> > };
> > -   psr_status = I915_READ(EDP_PSR2_STATUS);
> > -   val = (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
> > -   EDP_PSR2_STATUS_STATE_SHIFT;
> > -   if (val < ARRAY_SIZE(live_status)) {
> > -   seq_printf(m, "Source PSR status: 0x%x [%s]\n",
> > -  psr_status, live_status[val]);
> > -   return;
> > -   }
> > +   val = I915_READ(EDP_PSR2_STATUS);
> > +   status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> > + EDP_PSR2_STATUS_STATE_SHIFT;
> > +   if (status_val < ARRAY_SIZE(live_status))
> > +   status = live_status[status_val];
> > } else {
> > static const char * const live_status[] = {
> > "IDLE",
> > @@ -2700,74 +2698,78 @@ psr_source_status(struct drm_i915_private
> > *dev_priv, struct seq_file *m)
> > "SRDOFFACK",
> > "SRDENT_ON",
> > };
> > -   psr_status = I915_READ(EDP_PSR_STATUS);
> > -   val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
> > -   EDP_PSR_STATUS_STATE_SHIFT;
> > -   if (val < ARRAY_SIZE(live_status)) {
> > -   seq_printf(m, "Source PSR status: 0x%x [%s]\n",
> > -  psr_status, live_status[val]);
> > -   return;
> > -   }
> > +   val = I915_READ(EDP_PSR_STATUS);
> > +   status_val = (val & EDP_PSR_STATUS_STATE_MASK) >>
> > + EDP_PSR_STATUS_STATE_SHIFT;
> > +   if (status_val < ARRAY_SIZE(live_status))
> > +   status = live_status[status_val];
> > }
> >  
> > -   seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status,
> > "unknown");
> > +   seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val);
> >  }
> >  
> >  static int i915_edp_psr_status(struct seq_file *m, void *data)
> >  {
> > struct drm_i915_private *dev_priv = node_to_i915(m->private);
> > -   u32 psrperf = 0;
> > -   bool enabled = false;
> > -   bool sink_support;
> > +   struct i915_psr *psr = _priv->psr;
> > +   const char *status;
> > +   bool enabled;
> > +   u32 val;
> >  
> > if (!HAS_PSR(dev_priv))
> > return -ENODEV;
> >  
> > -   sink_support = dev_priv->psr.sink_support;
> > -   seq_printf(m, "Sink_Support: %s\n", yesno(sink_support));
> > -   if (!sink_support)
> > +   seq_printf(m, "Sink support: %s", yesno(psr->sink_support));
> > +   if (!psr->sink_support) {
> > +   seq_puts(m, "\n");
> > return 0;
> > +   }
> >  
> > intel_runtime_pm_get(dev_priv);
> > +   mutex_lock(>lock);
> >  
> > -   mutex_lock(_priv->psr.lock);
> 

Re: [Intel-gfx] [PATCH 2/5] drm/i915: Refactor PSR status debugfs

2018-12-10 Thread Dhinakaran Pandiyan
On Tue, 2018-12-04 at 15:00 -0800, José Roberto de Souza wrote:
> The old debugfs fields was not following a naming partern and it was
> a bit confusing.
> 
> So it went from:
> ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> Sink_Support: yes
> PSR mode: PSR1
> Enabled: yes
> Busy frontbuffer bits: 0x000
> Main link in standby mode: no
> HW Enabled & Active bit: yes
> Source PSR status: 0x24050006 [SRDONACK]
> 
> To:
> ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
> Sink support: yes [0x0003]
> Status: PSR1 enabled
> Source PSR ctl: enabled [0x81f00e26]
> Source PSR status: SRDENT [0x40040006]
> Busy frontbuffer bits: 0x
> 
> The 'Main link in standby mode' was removed as it is not useful but
> if needed by someone the information is still in the register value
> of 'Source PSR ctl' inside of the brackets, PSR mode and Enabled was
> squashed into Status, some renames and reorders and we have this
> cleaner version. This will also make easy to parse debugfs for IGT
> tests.
> 
> Cc: Rodrigo Vivi 
> Cc: Dhinakaran Pandiyan 
> Suggested-by: Dhinakaran Pandiyan 
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 96 +++--
> 
>  1 file changed, 49 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 38dcee1ca062..86303ba02666 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2665,7 +2665,8 @@ DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
>  static void
>  psr_source_status(struct drm_i915_private *dev_priv, struct seq_file
> *m)
>  {
> - u32 val, psr_status;
> + u32 val, status_val;
> + const char *status = "unknown";
>  
>   if (dev_priv->psr.psr2_enabled) {
>   static const char * const live_status[] = {
> @@ -2681,14 +2682,11 @@ psr_source_status(struct drm_i915_private
> *dev_priv, struct seq_file *m)
>   "BUF_ON",
>   "TG_ON"
>   };
> - psr_status = I915_READ(EDP_PSR2_STATUS);
> - val = (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
> - EDP_PSR2_STATUS_STATE_SHIFT;
> - if (val < ARRAY_SIZE(live_status)) {
> - seq_printf(m, "Source PSR status: 0x%x [%s]\n",
> -psr_status, live_status[val]);
> - return;
> - }
> + val = I915_READ(EDP_PSR2_STATUS);
> + status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> +   EDP_PSR2_STATUS_STATE_SHIFT;
> + if (status_val < ARRAY_SIZE(live_status))
> + status = live_status[status_val];
>   } else {
>   static const char * const live_status[] = {
>   "IDLE",
> @@ -2700,74 +2698,78 @@ psr_source_status(struct drm_i915_private
> *dev_priv, struct seq_file *m)
>   "SRDOFFACK",
>   "SRDENT_ON",
>   };
> - psr_status = I915_READ(EDP_PSR_STATUS);
> - val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
> - EDP_PSR_STATUS_STATE_SHIFT;
> - if (val < ARRAY_SIZE(live_status)) {
> - seq_printf(m, "Source PSR status: 0x%x [%s]\n",
> -psr_status, live_status[val]);
> - return;
> - }
> + val = I915_READ(EDP_PSR_STATUS);
> + status_val = (val & EDP_PSR_STATUS_STATE_MASK) >>
> +   EDP_PSR_STATUS_STATE_SHIFT;
> + if (status_val < ARRAY_SIZE(live_status))
> + status = live_status[status_val];
>   }
>  
> - seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status,
> "unknown");
> + seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val);
>  }
>  
>  static int i915_edp_psr_status(struct seq_file *m, void *data)
>  {
>   struct drm_i915_private *dev_priv = node_to_i915(m->private);
> - u32 psrperf = 0;
> - bool enabled = false;
> - bool sink_support;
> + struct i915_psr *psr = _priv->psr;
> + const char *status;
> + bool enabled;
> + u32 val;
>  
>   if (!HAS_PSR(dev_priv))
>   return -ENODEV;
>  
> - sink_support = dev_priv->psr.sink_support;
> - seq_printf(m, "Sink_Support: %s\n", yesno(sink_support));
> - if (!sink_support)
> + seq_printf(m, "Sink support: %s", yesno(psr->sink_support));
> + if (!psr->sink_support) {
> + seq_puts(m, "\n");
>   return 0;
> + }
>  
>   intel_runtime_pm_get(dev_priv);
> + mutex_lock(>lock);
>  
> - mutex_lock(_priv->psr.lock);
> - seq_printf(m, "PSR mode: %s\n",
> -dev_priv->psr.psr2_enabled ? "PSR2" : "PSR1");
> - seq_printf(m, "Enabled: %s\n", yesno(dev_priv->psr.enabled));
> - seq_printf(m, 

[Intel-gfx] [PATCH 2/5] drm/i915: Refactor PSR status debugfs

2018-12-04 Thread José Roberto de Souza
The old debugfs fields was not following a naming partern and it was
a bit confusing.

So it went from:
~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
Sink_Support: yes
PSR mode: PSR1
Enabled: yes
Busy frontbuffer bits: 0x000
Main link in standby mode: no
HW Enabled & Active bit: yes
Source PSR status: 0x24050006 [SRDONACK]

To:
~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status
Sink support: yes [0x0003]
Status: PSR1 enabled
Source PSR ctl: enabled [0x81f00e26]
Source PSR status: SRDENT [0x40040006]
Busy frontbuffer bits: 0x

The 'Main link in standby mode' was removed as it is not useful but
if needed by someone the information is still in the register value
of 'Source PSR ctl' inside of the brackets, PSR mode and Enabled was
squashed into Status, some renames and reorders and we have this
cleaner version. This will also make easy to parse debugfs for IGT
tests.

Cc: Rodrigo Vivi 
Cc: Dhinakaran Pandiyan 
Suggested-by: Dhinakaran Pandiyan 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 96 +++--
 1 file changed, 49 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 38dcee1ca062..86303ba02666 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2665,7 +2665,8 @@ DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
 static void
 psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
 {
-   u32 val, psr_status;
+   u32 val, status_val;
+   const char *status = "unknown";
 
if (dev_priv->psr.psr2_enabled) {
static const char * const live_status[] = {
@@ -2681,14 +2682,11 @@ psr_source_status(struct drm_i915_private *dev_priv, 
struct seq_file *m)
"BUF_ON",
"TG_ON"
};
-   psr_status = I915_READ(EDP_PSR2_STATUS);
-   val = (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
-   EDP_PSR2_STATUS_STATE_SHIFT;
-   if (val < ARRAY_SIZE(live_status)) {
-   seq_printf(m, "Source PSR status: 0x%x [%s]\n",
-  psr_status, live_status[val]);
-   return;
-   }
+   val = I915_READ(EDP_PSR2_STATUS);
+   status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
+ EDP_PSR2_STATUS_STATE_SHIFT;
+   if (status_val < ARRAY_SIZE(live_status))
+   status = live_status[status_val];
} else {
static const char * const live_status[] = {
"IDLE",
@@ -2700,74 +2698,78 @@ psr_source_status(struct drm_i915_private *dev_priv, 
struct seq_file *m)
"SRDOFFACK",
"SRDENT_ON",
};
-   psr_status = I915_READ(EDP_PSR_STATUS);
-   val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
-   EDP_PSR_STATUS_STATE_SHIFT;
-   if (val < ARRAY_SIZE(live_status)) {
-   seq_printf(m, "Source PSR status: 0x%x [%s]\n",
-  psr_status, live_status[val]);
-   return;
-   }
+   val = I915_READ(EDP_PSR_STATUS);
+   status_val = (val & EDP_PSR_STATUS_STATE_MASK) >>
+ EDP_PSR_STATUS_STATE_SHIFT;
+   if (status_val < ARRAY_SIZE(live_status))
+   status = live_status[status_val];
}
 
-   seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status, "unknown");
+   seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val);
 }
 
 static int i915_edp_psr_status(struct seq_file *m, void *data)
 {
struct drm_i915_private *dev_priv = node_to_i915(m->private);
-   u32 psrperf = 0;
-   bool enabled = false;
-   bool sink_support;
+   struct i915_psr *psr = _priv->psr;
+   const char *status;
+   bool enabled;
+   u32 val;
 
if (!HAS_PSR(dev_priv))
return -ENODEV;
 
-   sink_support = dev_priv->psr.sink_support;
-   seq_printf(m, "Sink_Support: %s\n", yesno(sink_support));
-   if (!sink_support)
+   seq_printf(m, "Sink support: %s", yesno(psr->sink_support));
+   if (!psr->sink_support) {
+   seq_puts(m, "\n");
return 0;
+   }
 
intel_runtime_pm_get(dev_priv);
+   mutex_lock(>lock);
 
-   mutex_lock(_priv->psr.lock);
-   seq_printf(m, "PSR mode: %s\n",
-  dev_priv->psr.psr2_enabled ? "PSR2" : "PSR1");
-   seq_printf(m, "Enabled: %s\n", yesno(dev_priv->psr.enabled));
-   seq_printf(m, "Busy frontbuffer bits: 0x%03x\n",
-  dev_priv->psr.busy_frontbuffer_bits);
+   seq_printf(m, " [0x%08x]\n", psr->dp->psr_dpcd[0]);
 
-   if