On Fri, 2018-06-22 at 09:29 +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.
> 
> v2: rebase
> v3: DK
>     Rename psr2_live_status to psr_source_status.
> v4: DK
>     Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
>     Pass seq to psr_source_status, handle source status prints in
>     psr_source_status.
> v5: Fixed CI warning messages
> v6:
>     Remove extra space in the title before the colon.(DK)
>     Rebase. (Jani)
> v7: use tabs for indenting the values.(Jani)

This made me go through the patch again, I'm afraid I see more
formatting (mostly) issues. Sorry for not noticing earlier.



> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
> 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagar...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 72 ++++++++++++++++++++++++---
> ----------
>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>  2 files changed, 49 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index c400f42..3941d85 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2597,27 +2597,55 @@ 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"
> -     };
> +static void
> +psr_source_status(struct drm_i915_private *dev_priv, struct seq_file
> *m)
> +{
> +     u32 val, psr_status = 0;
                 unnecessary initialization.

>  
> -     val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> EDP_PSR2_STATUS_STATE_SHIFT;
> -     if (val < ARRAY_SIZE(live_status))
> -             return live_status[val];
> +     if (dev_priv->psr.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"
> +             };
> +             psr_status = I915_READ(EDP_PSR2_STATUS);
> +             val =  (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
                      ^extra space here.

> +                     EDP_PSR2_STATUS_STATE_SHIFT;
> +             if (val < ARRAY_SIZE(live_status)) {
> +                     seq_printf(m, "Source PSR status: %x[%s]\n",
                                                          0x%x [%s]
i.e., you're missing the 0x prefix to denote hex numerals and a space.

> psr_status,
> +                                live_status[val]);
> +                     return;
> +             }
> +     } else {
> +             static const char * const live_status[] = {
> +                     "IDLE",
> +                     "SRDONACK",
> +                     "SRDENT",
> +                     "BUFOFF",
> +                     "BUFON",
> +                     "AUXACK",
> +                     "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: %x[%s]\n",
> psr_status,
> +                                live_status[val]);
> +                     return;
> +             }
> +     }
>  
> -     return "unknown";
> +     seq_printf(m, "Source psr status: %x[%s]\n", psr_status, 
                              ^lower case is inconsistent with the
other debugs you are printing.

> "unknown");
>  }
>  
>  static const char *psr_sink_status(u8 val)
> @@ -2681,12 +2709,8 @@ 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_source_status(dev_priv, m);
>  
>       if (dev_priv->psr.enabled) {
>               struct drm_dp_aux *aux = &dev_priv->psr.enabled-
> >aux;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index caad19f..4efad4d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4072,6 +4072,7 @@ enum {
>  
>  #define EDP_PSR_STATUS                               _MMIO(dev_priv
> ->psr_mmio_base + 0x40)
>  #define   EDP_PSR_STATUS_STATE_MASK          (7 << 29)
> +#define   EDP_PSR_STATUS_STATE_SHIFT         29
>  #define   EDP_PSR_STATUS_STATE_IDLE          (0 << 29)
>  #define   EDP_PSR_STATUS_STATE_SRDONACK              (1 << 29)
>  #define   EDP_PSR_STATUS_STATE_SRDENT                (2 << 29)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to