Re: [PATCH] drm/i915: Added debugfs support for PSR Status

From: Paulo Zanoni <przanoni@gmail.com>
To: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH] drm/i915: Added debugfs support for PSR Status
Date: Fri, 28 Jun 2013 17:14:54 -0300	[thread overview]
Message-ID: <CA+gsUGTfVOAFBC3gZH9Nyu=Lm2jRf=uPw3jEHG+HvnZJd5WDtg@mail.gmail.com> (raw)
In-Reply-To: <CA+gsUGSa0Zj4BmM29iZvgrccfYNNgcZo2eDX4ELk9-z86=r2eg@mail.gmail.com>

2013/6/28 Paulo Zanoni <przanoni@gmail.com>:
> 2013/6/28 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
>> Adding support for PSR Status, PSR entry counter and performance counters.
>> Heavily based on initial work from Shobhit.
>>
>> v2: Fix PSR Status Link bits by Paulo Zanoni.
>> v3: Prefer seq_puts to seq_printf by Paulo Zanoni.
>>
>> CC: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Credits-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c | 90 +++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/i915_reg.h     | 24 ++++++++++
>>  2 files changed, 114 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index a188624..67c777f 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1877,6 +1877,95 @@ static int i915_dpio_info(struct seq_file *m, void *data)
>>         return 0;
>>  }
>>
>> +static int i915_edp_psr_status(struct seq_file *m, void *data)
>> +{
>> +       struct drm_info_node *node = m->private;
>> +       struct drm_device *dev = node->minor->dev;
>> +       struct drm_i915_private *dev_priv = dev->dev_private;
>> +       u32 psrctl, psrstat, psrperf;
>> +
>> +       psrctl = I915_READ(EDP_PSR_CTL);
>> +       seq_printf(m, "PSR Enabled: %s\n",
>> +                  yesno(psrctl & EDP_PSR_ENABLE));
>> +
>> +       psrstat = I915_READ(EDP_PSR_STATUS_CTL);
>> +
>> +       seq_puts(m, "PSR Current State: ");
>> +       switch (psrstat & EDP_PSR_STATUS_STATE_MASK) {
>> +       case EDP_PSR_STATUS_STATE_IDLE:
>> +               seq_puts(m, "Reset state\n");
>> +               break;
>> +       case EDP_PSR_STATUS_STATE_SRDONACK:
>> +               seq_puts(m, "Wait for TG/Stream to send on frame of data after SRD conditions are met\n");
>> +               break;
>> +       case EDP_PSR_STATUS_STATE_SRDENT:
>> +               seq_puts(m, "SRD entry\n");
>> +               break;
>> +       case EDP_PSR_STATUS_STATE_BUFOFF:
>> +               seq_puts(m, "Wait for buffer turn off\n");
>> +               break;
>> +       case EDP_PSR_STATUS_STATE_BUFON:
>> +                       seq_puts(m, "Wait for buffer turn on\n");
>
> Wrong indentation above.
>
>> +               break;
>> +       case EDP_PSR_STATUS_STATE_AUXACK:
>> +                       seq_puts(m, "Wait for AUX to acknowledge on SRD exit\n");
>
> Wrong indentation above.
>
>> +               break;
>> +       case EDP_PSR_STATUS_STATE_SRDOFFACK:
>> +                       seq_puts(m, "Wait for TG/Stream to acknowledge the SRD VDM exit\n");
>
> Wrong indentation above.
>
> Everything else looks correct. With that fixed:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Actually no. With this patch, we'll read non-existing registers on
platforms that don't support PSR. We need an early return for that
case.

>
>> +               break;
>> +       default:
>> +               seq_puts(m, "Unknown\n");
>> +               break;
>> +       }
>> +
>> +       seq_puts(m, "Link Status: ");
>> +       switch (psrstat & EDP_PSR_STATUS_LINK_MASK) {
>> +       case EDP_PSR_STATUS_LINK_FULL_OFF:
>> +               seq_puts(m, "Link is fully off\n");
>> +               break;
>> +       case EDP_PSR_STATUS_LINK_FULL_ON:
>> +               seq_puts(m, "Link is fully on\n");
>> +               break;
>> +       case EDP_PSR_STATUS_LINK_STANDBY:
>> +               seq_puts(m, "Link is in standby\n");
>> +               break;
>> +       default:
>> +               seq_puts(m, "Unknown\n");
>> +               break;
>> +       }
>> +
>> +       seq_printf(m, "PSR Entry Count: %u\n",
>> +                  psrstat >> EDP_PSR_STATUS_COUNT_SHIFT &
>> +                  EDP_PSR_STATUS_COUNT_MASK);
>> +
>> +       seq_printf(m, "Max Sleep Timer Counter: %u\n",
>> +                  psrstat >> EDP_PSR_STATUS_MAX_SLEEP_TIMER_SHIFT &
>> +                  EDP_PSR_STATUS_MAX_SLEEP_TIMER_MASK);
>> +
>> +       seq_printf(m, "Had AUX error: %s\n",
>> +                  yesno(psrstat & EDP_PSR_STATUS_AUX_ERROR));
>> +
>> +       seq_printf(m, "Sending AUX: %s\n",
>> +                  yesno(psrstat & EDP_PSR_STATUS_AUX_SENDING));
>> +
>> +       seq_printf(m, "Sending Idle: %s\n",
>> +                  yesno(psrstat & EDP_PSR_STATUS_SENDING_IDLE));
>> +
>> +       seq_printf(m, "Sending TP2 TP3: %s\n",
>> +                  yesno(psrstat & EDP_PSR_STATUS_SENDING_TP2_TP3));
>> +
>> +       seq_printf(m, "Sending TP1: %s\n",
>> +                  yesno(psrstat & EDP_PSR_STATUS_SENDING_TP1));
>> +
>> +       seq_printf(m, "Idle Count: %u\n",
>> +                  psrstat & EDP_PSR_STATUS_IDLE_MASK);
>> +
>> +       psrperf = (I915_READ(EDP_PSR_PERF_CNT)) & EDP_PSR_PERF_CNT_MASK;
>> +       seq_printf(m, "Performance Counter: %u\n", psrperf);
>> +
>> +       return 0;
>> +}
>> +
>>  static int
>>  i915_wedged_get(void *data, u64 *val)
>>  {
>> @@ -2306,6 +2395,7 @@ static struct drm_info_list i915_debugfs_list[] = {
>>         {"i915_swizzle_info", i915_swizzle_info, 0},
>>         {"i915_ppgtt_info", i915_ppgtt_info, 0},
>>         {"i915_dpio", i915_dpio_info, 0},
>> +       {"i915_edp_psr_status", i915_edp_psr_status, 0},
>>  };
>>  #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index caf57d8..833cc97 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1812,6 +1812,30 @@
>>
>>  #define EDP_PSR_STATUS_CTL                     0x64840
>>  #define   EDP_PSR_STATUS_STATE_MASK            (7<<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)
>> +#define   EDP_PSR_STATUS_STATE_BUFOFF          (3<<29)
>> +#define   EDP_PSR_STATUS_STATE_BUFON           (4<<29)
>> +#define   EDP_PSR_STATUS_STATE_AUXACK          (5<<29)
>> +#define   EDP_PSR_STATUS_STATE_SRDOFFACK       (6<<29)
>> +#define   EDP_PSR_STATUS_LINK_MASK             (3<<26)
>> +#define   EDP_PSR_STATUS_LINK_FULL_OFF         (0<<26)
>> +#define   EDP_PSR_STATUS_LINK_FULL_ON          (1<<26)
>> +#define   EDP_PSR_STATUS_LINK_STANDBY          (2<<26)
>> +#define   EDP_PSR_STATUS_MAX_SLEEP_TIMER_SHIFT 20
>> +#define   EDP_PSR_STATUS_MAX_SLEEP_TIMER_MASK  0x1f
>> +#define   EDP_PSR_STATUS_COUNT_SHIFT           16
>> +#define   EDP_PSR_STATUS_COUNT_MASK            0xf
>> +#define   EDP_PSR_STATUS_AUX_ERROR             (1<<15)
>> +#define   EDP_PSR_STATUS_AUX_SENDING           (1<<12)
>> +#define   EDP_PSR_STATUS_SENDING_IDLE          (1<<9)
>> +#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_PERF_CNT               0x64844
>> +#define   EDP_PSR_PERF_CNT_MASK                0xffffff
>>
>>  #define EDP_PSR_DEBUG_CTL              0x64860
>>  #define   EDP_PSR_DEBUG_MASK_LPSP      (1<<27)
>> --
>> 1.8.1.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni

-- 
Paulo Zanoni