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

[Ramalingam C <ramalingam.c@intel.com>]

On Thursday 05 March 2015 04:48 PM, Daniel Vetter wrote:
> 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@intel.com> wrote:
>>> From: Vandana Kannan <vandana.kannan@intel.com>
>>>
>>> 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@intel.com>
>>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>>> ---
>>>   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);
Front buffer bits are added here.
>>> +
>>> +               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.
When the front buffer bit indicates Idle and the DRRS state is 
DRRS_HIGH_RR then we should have the downclock_work  scheduled.
I was double checking that here. If you are removing that I am fine with 
that.
> 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.
Already we have added the front buffer bits in the string.
>
> 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@intel.com>
>>
>>> +                               } 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@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>> -- 
>> Rodrigo Vivi
>> Blog: http://blog.vivi.eng.br

-- 
Ram

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