Re: [PATCH] drm/i915: Add debugfs entry for DRRS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 04, 2015 at 03:00:17PM -0800, Rodrigo Vivi wrote:
> On Tue, Mar 3, 2015 at 7:23 AM, Ramalingam C <ramalingam.c@xxxxxxxxx> wrote:
> > From: Vandana Kannan <vandana.kannan@xxxxxxxxx>
> >
> > Adding a debugfs entry to determine if DRRS is supported or not
> >
> > V2: [By Ram]: Following details about the active crtc will be filled
> >         in seq-file of the debugfs
> >         1. Encoder output type
> >         2. DRRS Support on this CRTC
> >         3. DRRS current state
> >         4. Current Vrefresh
> > Format is as follows:
> > CRTC 1:  Output: eDP, DRRS Supported: Yes (Seamless), DRRS_State: DRRS_HIGH_RR, Vrefresh: 60
> > CRTC 2:  Output: HDMI, DRRS Supported : No, VBT DRRS_type: Seamless
> > CRTC 1:  Output: eDP, DRRS Supported: Yes (Seamless), DRRS_State: DRRS_LOW_RR, Vrefresh: 40
> > CRTC 2:  Output: HDMI, DRRS Supported : No, VBT DRRS_type: Seamless
> >
> > V3: [By Ram]: Readability is improved.
> >         Another error case is covered [Daniel]
> >
> > V4: [By Ram]: Current status of the Idleness DRRS along with
> >         the Front buffer bits are added to the debugfs. [Rodrigo]
> >
> > V5: [By Ram]: Rephrased to make it easy to understand.
> >         And format is modified. [Rodrigo]
> >
> > V6: [By Ram]: Modeset mutex are acquired for each crtc along with
> >         renaming the Idleness detection states  [Daniel]
> >
> > Signed-off-by: Vandana Kannan <vandana.kannan@xxxxxxxxx>
> > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c |  141 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 141 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 94b3984..90e56ca 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2870,6 +2870,146 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
> >         return 0;
> >  }
> >
> > +static void drrs_status_per_crtc(struct seq_file *m,
> > +               struct drm_device *dev, struct intel_crtc *intel_crtc)
> > +{
> > +       struct intel_encoder *intel_encoder;
> > +       struct drm_i915_private *dev_priv = dev->dev_private;
> > +       struct i915_drrs *drrs = &dev_priv->drrs;
> > +       int vrefresh = 0;
> > +       u32 work_status;
> > +
> > +       for_each_encoder_on_crtc(dev, &intel_crtc->base, intel_encoder) {
> > +               /* Encoder connected on this CRTC */
> > +               switch (intel_encoder->type) {
> > +               case INTEL_OUTPUT_EDP:
> > +                       seq_puts(m, "eDP:\n");
> > +                       break;
> > +               case INTEL_OUTPUT_DSI:
> > +                       seq_puts(m, "DSI:\n");
> > +                       break;
> > +               case INTEL_OUTPUT_HDMI:
> > +                       seq_puts(m, "HDMI:\n");
> > +                       break;
> > +               case INTEL_OUTPUT_DISPLAYPORT:
> > +                       seq_puts(m, "DP:\n");
> > +                       break;
> > +               default:
> > +                       seq_printf(m, "Other encoder (id=%d).\n",
> > +                                               intel_encoder->type);
> > +                       return;
> > +               }
> > +       }
> > +
> > +       if (dev_priv->vbt.drrs_type == STATIC_DRRS_SUPPORT)
> > +               seq_puts(m, "\tVBT: DRRS_type: Static");
> > +       else if (dev_priv->vbt.drrs_type == SEAMLESS_DRRS_SUPPORT)
> > +               seq_puts(m, "\tVBT: DRRS_type: Seamless");
> > +       else if (dev_priv->vbt.drrs_type == DRRS_NOT_SUPPORTED)
> > +               seq_puts(m, "\tVBT: DRRS_type: None");
> > +       else
> > +               seq_puts(m, "\tVBT: DRRS_type: FIXME: Unrecognized Value");
> > +
> > +       seq_puts(m, "\n\n");
> > +
> > +       /*
> > +        * Idleness DRRS detection states:
> > +        *      Enabled   :     Idleness detection is active. When system is
> > +        *                      Idle for the defined duration DRRS_LOW_RR
> > +        *                      will be set. Or Idleness is already detected
> > +        *                      and DRRS_LOW_RR is applied.
> > +        *      Suspended :     Due to frontbuffer's busy state, Idleness
> > +        *                      detection is suspended.
> > +        *      Disabled  :     Idleness detection is disabled until a call is
> > +        *                      made to enable. No encoder pointer will be
> > +        *                      available.
> > +        */
> > +       if (intel_crtc->config->has_drrs) {
> > +               struct intel_panel *panel;
> > +
> > +               mutex_lock(&drrs->mutex);
> > +               /* DRRS Supported */
> > +               seq_puts(m, "\tDRRS Supported: Yes\n");
> > +
> > +               /* disable_drrs() will make drrs->dp NULL */
> > +               if (!drrs->dp) {
> > +                       seq_puts(m, "Idleness DRRS: Disabled");
> > +                       mutex_unlock(&drrs->mutex);
> > +                       return;
> > +               }
> > +
> > +               panel = &drrs->dp->attached_connector->panel;
> > +               seq_printf(m, "\t\tBusy_frontbuffer_bits: 0x%X",
> > +                                       drrs->busy_frontbuffer_bits);
> > +
> > +               seq_puts(m, "\n\t\t");
> > +               if (drrs->refresh_rate_type == DRRS_HIGH_RR) {
> > +                       seq_puts(m, "DRRS_State: DRRS_HIGH_RR\n");
> > +                       vrefresh = panel->fixed_mode->vrefresh;
> > +               } else if (drrs->refresh_rate_type == DRRS_LOW_RR) {
> > +                       seq_puts(m, "DRRS_State: DRRS_LOW_RR\n");
> > +                       vrefresh = panel->downclock_mode->vrefresh;
> > +               } else {
> > +                       seq_printf(m, "DRRS_State: Unknown(%d)\n",
> > +                                               drrs->refresh_rate_type);
> > +                       mutex_unlock(&drrs->mutex);
> > +                       return;
> > +               }
> > +               seq_printf(m, "\t\tVrefresh: %d", vrefresh);
> > +
> > +               seq_puts(m, "\n\t\t");
> > +               work_status = work_busy(&drrs->work.work);
> > +               if (drrs->busy_frontbuffer_bits) {
> > +                       seq_puts(m, "Front buffer: Busy.\n");
> > +                       seq_puts(m, "\t\tIdleness DRRS: Suspended");
> > +               } else {
> > +                       seq_puts(m, "Front buffer: Idle");
> > +                       seq_puts(m, "\n\t\t");
> > +                       if (drrs->refresh_rate_type == DRRS_HIGH_RR) {
> > +                               if (work_status) {
> > +                                       seq_puts(m, "Idleness DRRS: Enabled");
> 
> I couldn't understand yet why the work busy means DRRS enabled necessarily.
> Isn't there a more reliable way to check for DRRS enabled?

Yeah that seems wrong really and will just confuse people. I'll remove
this code. Also it's usually much more informative to dump all the
frontbuffer bits instead of just a busy/idle answer - if there's a bug in
the frontbuffer tracking it's good to know where the busy bits come from.

Applied the patch with those changes.
-Daniel

> Nothing that can't be done in a followup patch...  Also I'd like to
> see drrs status now so feel free to use
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> 
> > +                               } else {
> > +                                       seq_puts(m, "Idleness DRRS: Suspended.");
> > +                                       seq_puts(m, " FIXME: Shouldn't be here");
> > +                               }
> > +                       } else if (drrs->refresh_rate_type == DRRS_LOW_RR) {
> > +                               seq_puts(m, "Idleness DRRS: Enabled");
> > +                       }
> > +               }
> > +               mutex_unlock(&drrs->mutex);
> > +       } else {
> > +               /* DRRS not supported. Print the VBT parameter*/
> > +               seq_puts(m, "\tDRRS Supported : No");
> > +       }
> > +       seq_puts(m, "\n");
> > +}
> > +
> > +static int i915_drrs_status(struct seq_file *m, void *unused)
> > +{
> > +       struct drm_info_node *node = m->private;
> > +       struct drm_device *dev = node->minor->dev;
> > +       struct intel_crtc *intel_crtc;
> > +       int active_crtc_cnt = 0;
> > +
> > +       for_each_intel_crtc(dev, intel_crtc) {
> > +               drm_modeset_lock(&intel_crtc->base.mutex, NULL);
> > +
> > +               if (intel_crtc->active) {
> > +                       active_crtc_cnt++;
> > +                       seq_printf(m, "\nCRTC %d:  ", active_crtc_cnt);
> > +
> > +                       drrs_status_per_crtc(m, dev, intel_crtc);
> > +               }
> > +
> > +               drm_modeset_unlock(&intel_crtc->base.mutex);
> > +       }
> > +
> > +       if (!active_crtc_cnt)
> > +               seq_puts(m, "No active crtc found\n");
> > +
> > +       return 0;
> > +}
> > +
> >  struct pipe_crc_info {
> >         const char *name;
> >         struct drm_device *dev;
> > @@ -4548,6 +4688,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
> >         {"i915_wa_registers", i915_wa_registers, 0},
> >         {"i915_ddb_info", i915_ddb_info, 0},
> >         {"i915_sseu_status", i915_sseu_status, 0},
> > +       {"i915_drrs_status", i915_drrs_status, 0},
> >  };
> >  #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
> >
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux