Re: [Intel-gfx] [PATCH v7 3/3] drm/i915/dp: Expose connector VRR monitor range via debugfs

[Manasi Navare <manasi.d.navare@intel.com>]

On Mon, Jun 15, 2020 at 10:36:28PM +0100, Emil Velikov wrote:
> Hi Manasi,
> 
> On Sat, 13 Jun 2020 at 00:55, Manasi Navare <manasi.d.navare@intel.com> wrote:
> >
> > From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> >
> > [Why]
> > It's useful to know the min and max vrr range for IGT testing.
> >
> > [How]
> > Expose the min and max vfreq for the connector via a debugfs file
> > on the connector, "vrr_range".
> >
> > Example usage: cat /sys/kernel/debug/dri/0/DP-1/vrr_range
> >
> > v7:
> > * Fix cmpilation due to rebase
> > v6:
> > * Rebase (manasi)
> > v5:
> > * Rename to vrr_range to match AMD debugfs
> > v4:
> > * Rebase
> > v3:
> > * Remove the unnecessary debug print (Manasi)
> > v2:
> > * Fix the typo in max_vfreq (Manasi)
> > * Change the name of node to i915_vrr_info so we can add
> > other vrr info for more debug info (Manasi)
> > * Change the VRR capable to display Yes or No (Manasi)
> > * Fix indentation checkpatch errors (Manasi)
> >
> Nit: generally revision log is listed in v2 -> v6 order.

Okay point noted. Will update this in the next rev

> 
> > Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_debugfs.c  | 22 ++++++++++++++++++-
> >  1 file changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index 28dd717e943a..2921f7d2a26e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -2185,6 +2185,21 @@ static const struct file_operations i915_dsc_fec_support_fops = {
> >         .write = i915_dsc_fec_support_write
> >  };
> >
> > +static int vrr_range_show(struct seq_file *m, void *data)
> > +{
> > +       struct drm_connector *connector = m->private;
> > +
> > +       if (connector->status != connector_status_connected)
> > +               return -ENODEV;
> > +
> > +       seq_printf(m, "Vrr_capable: %s\n", yesno(intel_dp_is_vrr_capable(connector)));
> > +       seq_printf(m, "Min: %u\n", (u8)connector->display_info.monitor_range.min_vfreq);
> > +       seq_printf(m, "Max: %u\n", (u8)connector->display_info.monitor_range.max_vfreq);
> > +
> > +       return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(vrr_range);
> > +
> >  /**
> >   * intel_connector_debugfs_add - add i915 specific connector debugfs files
> >   * @connector: pointer to a registered drm_connector
> > @@ -2220,10 +2235,15 @@ int intel_connector_debugfs_add(struct drm_connector *connector)
> >         if (INTEL_GEN(dev_priv) >= 10 &&
> >             ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort &&
> >               !to_intel_connector(connector)->mst_port) ||
> > -            connector->connector_type == DRM_MODE_CONNECTOR_eDP))
> > +            connector->connector_type == DRM_MODE_CONNECTOR_eDP)) {
> >                 debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
> >                                     connector, &i915_dsc_fec_support_fops);
> >
> > +               if (INTEL_GEN(dev_priv) >= 12)
> > +                       debugfs_create_file("vrr_range", S_IRUGO,
> > +                                           root, connector, &vrr_range_fops);
> > +       }
> > +
> 
> I think this should be added by core drm. Ideally drm will add it
> automatically for each connector that the driver has called
> drm_connector_attach_vrr_capable_property() upon.
>

But in this case drm_connector_attach_vrr_capable_property() is called by individual
driver since its an optional connector property. So we call this inside i915.
Also currently AMD sets this debugfs inside AMD IMO, so setting this here for now.
But I agree that can be moved to drm core may be when drm_display_info gets populated
with min and max, thats where drm can add this?

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