On Fri, 2018-04-20 at 17:14 +0000, Souza, Jose wrote:
> On Fri, 2018-04-20 at 15:06 +0530, vathsala nagaraju wrote:
> > From: Vathsala Nagaraju <vathsala.nagar...@intel.com>
> > 
> > Prints live state of psr1.Extending the existing
> > PSR2 live state function to cover psr1.
> > 
> > Tested on KBL with psr2 and psr1 panel.
> > 
> > Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
> > 
> > Signed-off-by: Vathsala Nagaraju <vathsala.nagar...@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 68 ++++++++++++++++++++++++---
> > ----------
> >  drivers/gpu/drm/i915/i915_reg.h     |  1 +
> >  2 files changed, 45 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index e0274f4..3056f04 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2580,25 +2580,42 @@ static int i915_guc_log_relay_release(struct
> > inode *inode, struct file *file)
> >     .release = i915_guc_log_relay_release,
> >  };
> >  
> > -static const char *psr2_live_status(u32 val)
> > -{
> > -   static const char * const live_status[] = {
> > -           "IDLE",
> > -           "CAPTURE",
> > -           "CAPTURE_FS",
> > -           "SLEEP",
> > -           "BUFON_FW",
> > -           "ML_UP",
> > -           "SU_STANDBY",
> > -           "FAST_SLEEP",
> > -           "DEEP_SLEEP",
> > -           "BUF_ON",
> > -           "TG_ON"
> > -   };
> > -
> > -   val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> > EDP_PSR2_STATUS_STATE_SHIFT;
> > -   if (val < ARRAY_SIZE(live_status))
> > -           return live_status[val];
> > +static const char *psr_live_status(bool is_psr2_enabled, u32 val)
> > +{
> > +   if (is_psr2_enabled) {
> > +           static const char * const live_status[] = {
> > +                   "IDLE",
> > +                   "CAPTURE",
> > +                   "CAPTURE_FS",
> > +                   "SLEEP",
> > +                   "BUFON_FW",
> > +                   "ML_UP",
> > +                   "SU_STANDBY",
> > +                   "FAST_SLEEP",
> > +                   "DEEP_SLEEP",
> > +                   "BUF_ON",
> > +                   "TG_ON"
> > +           };
> > +           val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> > +                   EDP_PSR2_STATUS_STATE_SHIFT;
> > +           if (val < ARRAY_SIZE(live_status))
> > +                   return live_status[val];
> > +   } else {
> > +           static const char * const live_status[] = {
> > +                   "IDLE",
> > +                   "SRDONACK",
> > +                   "SRDENT",
> > +                   "BUFOFF",
> > +                   "BUFON",
> > +                   "AUXACK",
> > +                   "SRDOFFACK",
> > +                   "SRDENT_ON",
> > +           };
> > +           val = (val & EDP_PSR_STATUS_STATE_MASK) >>
> > +                   EDP_PSR_STATUS_STATE_SHIFT;
> > +           if (val < ARRAY_SIZE(live_status))
> > +                   return live_status[val];
> > +   }
> >  
> >     return "unknown";
> >  }
> > @@ -2611,6 +2628,7 @@ static int i915_edp_psr_status(struct seq_file
> > *m, void *data)
> >     enum pipe pipe;
> >     bool enabled = false;
> >     bool sink_support;
> > +   u32 psr_status;
> >  
> >     if (!HAS_PSR(dev_priv))
> >             return -ENODEV;
> > @@ -2678,12 +2696,14 @@ static int i915_edp_psr_status(struct
> > seq_file *m, void *data)
> >  
> >             seq_printf(m, "Performance_Counter: %u\n", psrperf);
> >     }
> > -   if (dev_priv->psr.psr2_enabled) {
> > -           u32 psr2 = I915_READ(EDP_PSR2_STATUS);
> >  
> > -           seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> > -                      psr2, psr2_live_status(psr2));
> > -   }
> > +   psr_status = (dev_priv->psr.psr2_enabled) ?
> > I915_READ(EDP_PSR2_STATUS) :
> > +                                               I915_READ(EDP_PS
> > R_STATUS);
> 
> Maybe move the read of the PSR or PSR2 status to inside of
> psr_live_status()

I am thinking we could reduce some clutter by changing both the status
functions to have this signature. 


static void psr_source_status(dev_priv, m) 
{

}

static void psr_sink_status(dev_priv, m)
{

}


> 
> 
> Other than that looks good to me.
> 
> > +   seq_printf(m, "EDP_PSR%s_STATUS: %x [%s]\n",
                        ^source_status or whatever the correct parallel to sink 
status that
Jose is using.


> > +                 dev_priv->psr.psr2_enabled ? "2" : "1",
> > +                 psr_status,
> > +                 psr_live_status(dev_priv->psr.psr2_enabled,
> > psr_status));
> > +
> >     mutex_unlock(&dev_priv->psr.lock);
> >  
> >     intel_runtime_pm_put(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index fb10602..c9598b4 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4058,6 +4058,7 @@ enum {
> >  #define   EDP_PSR_STATUS_SENDING_TP2_TP3   (1<<8)
> >  #define   EDP_PSR_STATUS_SENDING_TP1               (1<<4)
> >  #define   EDP_PSR_STATUS_IDLE_MASK         0xf
> > +#define   EDP_PSR_STATUS_STATE_SHIFT               29
> >  
> >  #define EDP_PSR_PERF_CNT           _MMIO(dev_priv-
> > >psr_mmio_base + 0x44)
> >  #define   EDP_PSR_PERF_CNT_MASK            0xffffff

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

Reply via email to