[v4,3/3] drm/i915/dp: Expose connector VRR info via debugfs
diff mbox series

Message ID 20200414050807.13531-3-manasi.d.navare@intel.com
State New
Headers show
Series
  • [v4,1/3] drm/dp: DRM DP helper for reading Ignore MSA from DPCD
Related show

Commit Message

Navare, Manasi April 14, 2020, 5:08 a.m. UTC
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>
---
 .../drm/i915/display/intel_display_debugfs.c  | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Daniel Vetter April 14, 2020, 12:44 p.m. UTC | #1
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
Michel Dänzer April 15, 2020, 10:33 a.m. UTC | #2
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)
Daniel Vetter April 15, 2020, 11:58 a.m. UTC | #3
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

Patch
diff mbox series

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;
 }