| Message ID | 20200414050807.13531-3-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/3] drm/dp: DRM DP helper for reading Ignore MSA from DPCD | expand |
On Mon, Apr 13, 2020 at 10:08:07PM -0700, Manasi Navare 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, "i915_vrr_info". > > Example usage: cat /sys/kernel/debug/dri/0/DP-1/i915_vrr_info > > 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) > > 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> So if I'm understanding things correctly AMD butchered the VRR stuff and only exposes it when: - VRR_ENABLED is set - _and_ you're using the legacy page_flip path, atomic flip doesn't support it - _and_ the PAGE_FLIP_ASYNC flag is set. That's pretty bonkers uapi, and I think before we add a new driver we need to figure out how to make this work. Works case we might need a revert hammer for the amdgpu side and new VRR properties so we're not tricking any existing userspace into something where stuff then would break. But I'm hoping we can do a bit more clever than that. Or maybe I'm just wrong on how amdgpu vrr works. Adding amd folks. -Daniel > --- > .../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 bdeea2e02642..35b229ab4d19 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -2096,6 +2096,21 @@ static const struct file_operations i915_dsc_fec_support_fops = { > .write = i915_dsc_fec_support_write > }; > > +static int i915_vrr_info_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(i915_vrr_info); > + > /** > * intel_connector_debugfs_add - add i915 specific connector debugfs files > * @connector: pointer to a registered drm_connector > @@ -2130,9 +2145,14 @@ int intel_connector_debugfs_add(struct drm_connector *connector) > > if (INTEL_GEN(dev_priv) >= 10 && > (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort || > - 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("i915_vrr_info", S_IRUGO, > + root, connector, &i915_vrr_info_fops); > + } > + > return 0; > } > -- > 2.19.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 2020-04-14 2:44 p.m., Daniel Vetter wrote: > On Mon, Apr 13, 2020 at 10:08:07PM -0700, Manasi Navare 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, "i915_vrr_info". >> >> Example usage: cat /sys/kernel/debug/dri/0/DP-1/i915_vrr_info >> >> 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) >> >> 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> > > So if I'm understanding things correctly AMD butchered the VRR stuff and > only exposes it when: > > - VRR_ENABLED is set Not really surprising? :) > - _and_ you're using the legacy page_flip path, atomic flip doesn't > support it Simon Ser has VRR working with sway using the atomic KMS API. > - _and_ the PAGE_FLIP_ASYNC flag is set. AFAIK it works both without and with PAGE_FLIP_ASYNC. (Async just means tearing if the flip is programmed outside of vertical blank)
On Wed, Apr 15, 2020 at 12:33 PM Michel Dänzer <michel@daenzer.net> wrote: > > On 2020-04-14 2:44 p.m., Daniel Vetter wrote: > > On Mon, Apr 13, 2020 at 10:08:07PM -0700, Manasi Navare 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, "i915_vrr_info". > >> > >> Example usage: cat /sys/kernel/debug/dri/0/DP-1/i915_vrr_info > >> > >> 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) > >> > >> 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> > > > > So if I'm understanding things correctly AMD butchered the VRR stuff and > > only exposes it when: > > > > - VRR_ENABLED is set > > Not really surprising? :) > > > - _and_ you're using the legacy page_flip path, atomic flip doesn't > > support it > > Simon Ser has VRR working with sway using the atomic KMS API. > > > - _and_ the PAGE_FLIP_ASYNC flag is set. > > AFAIK it works both without and with PAGE_FLIP_ASYNC. (Async just means > tearing if the flip is programmed outside of vertical blank) Yeah Nicolas already explained it all on the igt thread, conclusion is that the igt needs some work to improve it (need to test the atomic path too, and try to be a bit less hackish with the timing tests). So all good, just me who panicked and got led astray by the comment in the igt and that amdgpu still implements its own page_flip callback (in completely separate paths, it doesn't seem to converge even in the low-level chip functions), so wasn't obvious to me from reading code that the atomic path would also work. -Daniel
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c index bdeea2e02642..35b229ab4d19 100644 --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c @@ -2096,6 +2096,21 @@ static const struct file_operations i915_dsc_fec_support_fops = { .write = i915_dsc_fec_support_write }; +static int i915_vrr_info_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(i915_vrr_info); + /** * intel_connector_debugfs_add - add i915 specific connector debugfs files * @connector: pointer to a registered drm_connector @@ -2130,9 +2145,14 @@ int intel_connector_debugfs_add(struct drm_connector *connector) if (INTEL_GEN(dev_priv) >= 10 && (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort || - 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("i915_vrr_info", S_IRUGO, + root, connector, &i915_vrr_info_fops); + } + return 0; }