Re: [Intel-gfx] [PATCH v4 3/3] drm/i915/dp: Expose connector VRR info via debugfs
On Wed, Apr 15, 2020 at 12:33 PM Michel Dänzer 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 > >> > >> [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 > >> Signed-off-by: Manasi Navare > >> Cc: Jani Nikula > >> Cc: Ville Syrjälä > >> Tested-by: Manasi Navare > > > > 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 3/3] drm/i915/dp: Expose connector VRR info via debugfs
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 >> >> [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 >> Signed-off-by: Manasi Navare >> Cc: Jani Nikula >> Cc: Ville Syrjälä >> Tested-by: Manasi Navare > > 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) -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 3/3] drm/i915/dp: Expose connector VRR info via debugfs
On Mon, Apr 13, 2020 at 10:08:07PM -0700, Manasi Navare wrote:
> From: Bhanuprakash Modem
>
> [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
> Signed-off-by: Manasi Navare
> Cc: Jani Nikula
> Cc: Ville Syrjälä
> Tested-by: Manasi Navare
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, _dsc_fec_support_fops);
>
> + if (INTEL_GEN(dev_priv) >= 12)
> + debugfs_create_file("i915_vrr_info", S_IRUGO,
> + root, connector,
> _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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 3/3] drm/i915/dp: Expose connector VRR info via debugfs
From: Bhanuprakash Modem
[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
Signed-off-by: Manasi Navare
Cc: Jani Nikula
Cc: Ville Syrjälä
Tested-by: Manasi Navare
---
.../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, _dsc_fec_support_fops);
+ if (INTEL_GEN(dev_priv) >= 12)
+ debugfs_create_file("i915_vrr_info", S_IRUGO,
+ root, connector,
_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
