All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v2 0/2] Attach and Set vrr_enabled property
@ 2022-04-25  6:46 ` Bhanuprakash Modem
  0 siblings, 0 replies; 12+ messages in thread
From: Bhanuprakash Modem @ 2022-04-25  6:46 UTC (permalink / raw)
  To: intel-gfx, dri-devel, ville.syrjala, manasi.d.navare; +Cc: Bhanuprakash Modem

This series will add a support to attach & set the vrr_enabled property
for crtc based on the platform support and the request from userspace.
And userspace can also query to get the status of "vrr_enabled".

Test-with: 20220422075223.2792586-2-bhanuprakash.modem@intel.com

Bhanuprakash Modem (2):
  drm/vrr: Attach vrr_enabled property to the drm crtc
  drm/i915/vrr: Attach and set drm crtc vrr_enabled property

 drivers/gpu/drm/drm_crtc.c                | 44 +++++++++++++++++++++++
 drivers/gpu/drm/drm_mode_config.c         |  2 +-
 drivers/gpu/drm/i915/display/intel_crtc.c |  3 ++
 drivers/gpu/drm/i915/display/intel_vrr.c  |  8 +++++
 include/drm/drm_crtc.h                    |  4 +++
 5 files changed, 60 insertions(+), 1 deletion(-)

--
2.35.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] [RFC v2 0/2] Attach and Set vrr_enabled property
@ 2022-04-25  6:46 ` Bhanuprakash Modem
  0 siblings, 0 replies; 12+ messages in thread
From: Bhanuprakash Modem @ 2022-04-25  6:46 UTC (permalink / raw)
  To: intel-gfx, dri-devel, ville.syrjala, manasi.d.navare

This series will add a support to attach & set the vrr_enabled property
for crtc based on the platform support and the request from userspace.
And userspace can also query to get the status of "vrr_enabled".

Test-with: 20220422075223.2792586-2-bhanuprakash.modem@intel.com

Bhanuprakash Modem (2):
  drm/vrr: Attach vrr_enabled property to the drm crtc
  drm/i915/vrr: Attach and set drm crtc vrr_enabled property

 drivers/gpu/drm/drm_crtc.c                | 44 +++++++++++++++++++++++
 drivers/gpu/drm/drm_mode_config.c         |  2 +-
 drivers/gpu/drm/i915/display/intel_crtc.c |  3 ++
 drivers/gpu/drm/i915/display/intel_vrr.c  |  8 +++++
 include/drm/drm_crtc.h                    |  4 +++
 5 files changed, 60 insertions(+), 1 deletion(-)

--
2.35.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC v2 1/2] drm/vrr: Attach vrr_enabled property to the drm crtc
  2022-04-25  6:46 ` [Intel-gfx] " Bhanuprakash Modem
@ 2022-04-25  6:46   ` Bhanuprakash Modem
  -1 siblings, 0 replies; 12+ messages in thread
From: Bhanuprakash Modem @ 2022-04-25  6:46 UTC (permalink / raw)
  To: intel-gfx, dri-devel, ville.syrjala, manasi.d.navare
  Cc: Nicholas Kazlauskas, Bhanuprakash Modem

Modern display hardware is capable of supporting variable refresh rates.
This patch introduces helpers to attach and set "vrr_enabled" property
on the crtc to allow userspace to query VRR enabled status on that crtc.

Atomic drivers should attach this property to crtcs those are capable of
driving variable refresh rates using
drm_mode_crtc_attach_vrr_enabled_property().

The value should be updated based on driver and hardware capability
by using drm_mode_crtc_set_vrr_enabled_property().

V2: Use property flag as atomic

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/drm_crtc.c        | 44 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_mode_config.c |  2 +-
 include/drm/drm_crtc.h            |  4 +++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 26a77a735905..95b4a0c7ecb3 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -883,3 +883,47 @@ int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
 	return 0;
 }
 EXPORT_SYMBOL(drm_crtc_create_scaling_filter_property);
+
+/**
+ * drm_mode_crtc_attach_vrr_enabled_property - attaches the vrr_enabled property
+ * @crtc: drm CRTC to attach the vrr_enabled property on.
+ *
+ * This is used by atomic drivers to add support for querying
+ * VRR enabled status for a crtc.
+ */
+void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (!config->prop_vrr_enabled)
+		return;
+
+	drm_object_attach_property(&crtc->base,
+				   config->prop_vrr_enabled,
+				   0);
+}
+EXPORT_SYMBOL(drm_mode_crtc_attach_vrr_enabled_property);
+
+/**
+ * drm_mode_crtc_set_vrr_enabled_property - sets the vrr enabled property for
+ * a crtc.
+ * @crtc: drm CRTC
+ * @vrr_enabled: True to enable the VRR on CRTC
+ *
+ * Should be used by atomic drivers to update the VRR enabled status on a CRTC
+ */
+void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
+					    bool vrr_enabled)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (!config->prop_vrr_enabled)
+		return;
+
+	drm_object_property_set_value(&crtc->base,
+				      config->prop_vrr_enabled,
+				      vrr_enabled);
+}
+EXPORT_SYMBOL(drm_mode_crtc_set_vrr_enabled_property);
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 37b4b9f0e468..b7cde73d5586 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -323,7 +323,7 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.prop_mode_id = prop;
 
-	prop = drm_property_create_bool(dev, 0,
+	prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
 			"VRR_ENABLED");
 	if (!prop)
 		return -ENOMEM;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index a70baea0636c..bde657cb0f9e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1333,4 +1333,8 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
 int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
 					    unsigned int supported_filters);
 
+void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc);
+void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
+					    bool vrr_enabled);
+
 #endif /* __DRM_CRTC_H__ */
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] [RFC v2 1/2] drm/vrr: Attach vrr_enabled property to the drm crtc
@ 2022-04-25  6:46   ` Bhanuprakash Modem
  0 siblings, 0 replies; 12+ messages in thread
From: Bhanuprakash Modem @ 2022-04-25  6:46 UTC (permalink / raw)
  To: intel-gfx, dri-devel, ville.syrjala, manasi.d.navare
  Cc: Harry Wentland, Nicholas Kazlauskas

Modern display hardware is capable of supporting variable refresh rates.
This patch introduces helpers to attach and set "vrr_enabled" property
on the crtc to allow userspace to query VRR enabled status on that crtc.

Atomic drivers should attach this property to crtcs those are capable of
driving variable refresh rates using
drm_mode_crtc_attach_vrr_enabled_property().

The value should be updated based on driver and hardware capability
by using drm_mode_crtc_set_vrr_enabled_property().

V2: Use property flag as atomic

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/drm_crtc.c        | 44 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_mode_config.c |  2 +-
 include/drm/drm_crtc.h            |  4 +++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 26a77a735905..95b4a0c7ecb3 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -883,3 +883,47 @@ int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
 	return 0;
 }
 EXPORT_SYMBOL(drm_crtc_create_scaling_filter_property);
+
+/**
+ * drm_mode_crtc_attach_vrr_enabled_property - attaches the vrr_enabled property
+ * @crtc: drm CRTC to attach the vrr_enabled property on.
+ *
+ * This is used by atomic drivers to add support for querying
+ * VRR enabled status for a crtc.
+ */
+void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (!config->prop_vrr_enabled)
+		return;
+
+	drm_object_attach_property(&crtc->base,
+				   config->prop_vrr_enabled,
+				   0);
+}
+EXPORT_SYMBOL(drm_mode_crtc_attach_vrr_enabled_property);
+
+/**
+ * drm_mode_crtc_set_vrr_enabled_property - sets the vrr enabled property for
+ * a crtc.
+ * @crtc: drm CRTC
+ * @vrr_enabled: True to enable the VRR on CRTC
+ *
+ * Should be used by atomic drivers to update the VRR enabled status on a CRTC
+ */
+void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
+					    bool vrr_enabled)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (!config->prop_vrr_enabled)
+		return;
+
+	drm_object_property_set_value(&crtc->base,
+				      config->prop_vrr_enabled,
+				      vrr_enabled);
+}
+EXPORT_SYMBOL(drm_mode_crtc_set_vrr_enabled_property);
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 37b4b9f0e468..b7cde73d5586 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -323,7 +323,7 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.prop_mode_id = prop;
 
-	prop = drm_property_create_bool(dev, 0,
+	prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
 			"VRR_ENABLED");
 	if (!prop)
 		return -ENOMEM;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index a70baea0636c..bde657cb0f9e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1333,4 +1333,8 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
 int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
 					    unsigned int supported_filters);
 
+void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc);
+void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
+					    bool vrr_enabled);
+
 #endif /* __DRM_CRTC_H__ */
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v2 2/2] drm/i915/vrr: Attach and set drm crtc vrr_enabled property
  2022-04-25  6:46 ` [Intel-gfx] " Bhanuprakash Modem
@ 2022-04-25  6:46   ` Bhanuprakash Modem
  -1 siblings, 0 replies; 12+ messages in thread
From: Bhanuprakash Modem @ 2022-04-25  6:46 UTC (permalink / raw)
  To: intel-gfx, dri-devel, ville.syrjala, manasi.d.navare; +Cc: Bhanuprakash Modem

This function attaches & sets the vrr_enabled property for crtc
based on the platform support and the request from userspace.

V2: Check for platform support before updating the prop.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc.c | 3 +++
 drivers/gpu/drm/i915/display/intel_vrr.c  | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 4442aa355f86..36deaca9af66 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -366,6 +366,9 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 						BIT(DRM_SCALING_FILTER_DEFAULT) |
 						BIT(DRM_SCALING_FILTER_NEAREST_NEIGHBOR));
 
+	if (HAS_VRR(dev_priv))
+		drm_mode_crtc_attach_vrr_enabled_property(&crtc->base);
+
 	intel_color_init(crtc);
 
 	intel_crtc_drrs_init(crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 396f2f994fa0..7263b35550de 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -160,6 +160,10 @@ void intel_vrr_enable(struct intel_encoder *encoder,
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	u32 trans_vrr_ctl;
 
+	if (HAS_VRR(dev_priv))
+		drm_mode_crtc_set_vrr_enabled_property(crtc_state->uapi.crtc,
+						       crtc_state->vrr.enable);
+
 	if (!crtc_state->vrr.enable)
 		return;
 
@@ -211,6 +215,10 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
 
+	if (HAS_VRR(dev_priv))
+		drm_mode_crtc_set_vrr_enabled_property(old_crtc_state->uapi.crtc,
+						       false);
+
 	if (!old_crtc_state->vrr.enable)
 		return;
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] [RFC v2 2/2] drm/i915/vrr: Attach and set drm crtc vrr_enabled property
@ 2022-04-25  6:46   ` Bhanuprakash Modem
  0 siblings, 0 replies; 12+ messages in thread
From: Bhanuprakash Modem @ 2022-04-25  6:46 UTC (permalink / raw)
  To: intel-gfx, dri-devel, ville.syrjala, manasi.d.navare

This function attaches & sets the vrr_enabled property for crtc
based on the platform support and the request from userspace.

V2: Check for platform support before updating the prop.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc.c | 3 +++
 drivers/gpu/drm/i915/display/intel_vrr.c  | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 4442aa355f86..36deaca9af66 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -366,6 +366,9 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 						BIT(DRM_SCALING_FILTER_DEFAULT) |
 						BIT(DRM_SCALING_FILTER_NEAREST_NEIGHBOR));
 
+	if (HAS_VRR(dev_priv))
+		drm_mode_crtc_attach_vrr_enabled_property(&crtc->base);
+
 	intel_color_init(crtc);
 
 	intel_crtc_drrs_init(crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 396f2f994fa0..7263b35550de 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -160,6 +160,10 @@ void intel_vrr_enable(struct intel_encoder *encoder,
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	u32 trans_vrr_ctl;
 
+	if (HAS_VRR(dev_priv))
+		drm_mode_crtc_set_vrr_enabled_property(crtc_state->uapi.crtc,
+						       crtc_state->vrr.enable);
+
 	if (!crtc_state->vrr.enable)
 		return;
 
@@ -211,6 +215,10 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
 
+	if (HAS_VRR(dev_priv))
+		drm_mode_crtc_set_vrr_enabled_property(old_crtc_state->uapi.crtc,
+						       false);
+
 	if (!old_crtc_state->vrr.enable)
 		return;
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Attach and Set vrr_enabled property (rev2)
  2022-04-25  6:46 ` [Intel-gfx] " Bhanuprakash Modem
                   ` (2 preceding siblings ...)
  (?)
@ 2022-04-25  7:07 ` Patchwork
  -1 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2022-04-25  7:07 UTC (permalink / raw)
  To: Bhanuprakash Modem; +Cc: intel-gfx

== Series Details ==

Series: Attach and Set vrr_enabled property (rev2)
URL   : https://patchwork.freedesktop.org/series/102978/
State : warning

== Summary ==

Error: dim checkpatch failed
74f8ea70155d drm/vrr: Attach vrr_enabled property to the drm crtc
-:90: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#90: FILE: drivers/gpu/drm/drm_mode_config.c:327:
+	prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
 			"VRR_ENABLED");

total: 0 errors, 0 warnings, 1 checks, 63 lines checked
ecc537c1ea75 drm/i915/vrr: Attach and set drm crtc vrr_enabled property



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for Attach and Set vrr_enabled property (rev2)
  2022-04-25  6:46 ` [Intel-gfx] " Bhanuprakash Modem
                   ` (3 preceding siblings ...)
  (?)
@ 2022-04-25  7:30 ` Patchwork
  -1 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2022-04-25  7:30 UTC (permalink / raw)
  To: Bhanuprakash Modem; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 15377 bytes --]

== Series Details ==

Series: Attach and Set vrr_enabled property (rev2)
URL   : https://patchwork.freedesktop.org/series/102978/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11548 -> Patchwork_102978v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_102978v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_102978v2, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/index.html

Participating hosts (42 -> 44)
------------------------------

  Additional (5): fi-kbl-soraka fi-bdw-5557u bat-adls-5 bat-dg1-6 bat-dg2-8 
  Missing    (3): bat-adlm-1 fi-bsw-cyan bat-dg1-5 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_102978v2:

### IGT changes ###

#### Possible regressions ####

  * {igt@kms_vrr@parse-vrr-range@pipe-a-dsi-1} (NEW):
    - {fi-tgl-dsi}:       NOTRUN -> [SKIP][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-tgl-dsi/igt@kms_vrr@parse-vrr-range@pipe-a-dsi-1.html

  * igt@runner@aborted:
    - fi-rkl-11600:       NOTRUN -> [FAIL][2]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-rkl-11600/igt@runner@aborted.html
    - fi-rkl-guc:         NOTRUN -> [FAIL][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-rkl-guc/igt@runner@aborted.html
    - fi-adl-ddr5:        NOTRUN -> [FAIL][4]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-adl-ddr5/igt@runner@aborted.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_suspend@basic-s0@smem:
    - {fi-ehl-2}:         [PASS][5] -> [DMESG-WARN][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11548/fi-ehl-2/igt@gem_exec_suspend@basic-s0@smem.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-ehl-2/igt@gem_exec_suspend@basic-s0@smem.html
    - {fi-jsl-1}:         [PASS][7] -> [DMESG-WARN][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11548/fi-jsl-1/igt@gem_exec_suspend@basic-s0@smem.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-jsl-1/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@kms_force_connector_basic@force-connector-state:
    - {bat-dg2-8}:        NOTRUN -> [DMESG-WARN][9]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/bat-dg2-8/igt@kms_force_connector_basic@force-connector-state.html

  * igt@kms_vrr@negative-basic:
    - {fi-tgl-dsi}:       NOTRUN -> [SKIP][10]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-tgl-dsi/igt@kms_vrr@negative-basic.html

  * igt@runner@aborted:
    - {bat-adls-5}:       NOTRUN -> [FAIL][11]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/bat-adls-5/igt@runner@aborted.html
    - {bat-adlp-6}:       NOTRUN -> [FAIL][12]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/bat-adlp-6/igt@runner@aborted.html
    - {bat-jsl-2}:        NOTRUN -> [FAIL][13]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/bat-jsl-2/igt@runner@aborted.html
    - {bat-jsl-1}:        NOTRUN -> [FAIL][14]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/bat-jsl-1/igt@runner@aborted.html

  
New tests
---------

  New tests have been introduced between CI_DRM_11548 and Patchwork_102978v2:

### New IGT tests (9) ###

  * igt@kms_vrr@parse-vrr-range@pipe-a-dp-1:
    - Statuses : 3 pass(s)
    - Exec time: [0.04, 0.17] s

  * igt@kms_vrr@parse-vrr-range@pipe-a-dp-2:
    - Statuses : 2 pass(s) 2 skip(s)
    - Exec time: [0.0, 0.43] s

  * igt@kms_vrr@parse-vrr-range@pipe-a-dsi-1:
    - Statuses : 2 skip(s)
    - Exec time: [0.0, 0.00] s

  * igt@kms_vrr@parse-vrr-range@pipe-a-edp-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.42, 0.49] s

  * igt@kms_vrr@parse-vrr-range@pipe-a-hdmi-a-1:
    - Statuses : 5 pass(s)
    - Exec time: [0.03, 0.20] s

  * igt@kms_vrr@parse-vrr-range@pipe-a-hdmi-a-2:
    - Statuses : 3 pass(s)
    - Exec time: [0.05, 0.27] s

  * igt@kms_vrr@parse-vrr-range@pipe-a-lvds-1:
    - Statuses : 1 skip(s)
    - Exec time: [0.0] s

  * igt@kms_vrr@parse-vrr-range@pipe-a-vga-1:
    - Statuses : 5 skip(s)
    - Exec time: [0.0] s

  * igt@kms_vrr@parse-vrr-range@pipe-c-hdmi-a-2:
    - Statuses : 1 pass(s)
    - Exec time: [0.10] s

  

Known issues
------------

  Here are the changes found in Patchwork_102978v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fence@basic-busy@bcs0:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][15] ([fdo#109271]) +10 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-kbl-soraka/igt@gem_exec_fence@basic-busy@bcs0.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#2190])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#4613]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][18] ([i915#1886])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        [PASS][19] -> [INCOMPLETE][20] ([i915#4785])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11548/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
    - fi-bdw-5557u:       NOTRUN -> [INCOMPLETE][21] ([i915#3921])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-bdw-5557u/igt@i915_selftest@live@hangcheck.html
    - fi-snb-2600:        [PASS][22] -> [INCOMPLETE][23] ([i915#3921])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11548/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][24] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-kbl-soraka/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][25] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-bdw-5557u/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][26] ([fdo#109271] / [i915#533])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-kbl-soraka/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][27] ([fdo#109271]) +15 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-bdw-5557u/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@kms_vrr@negative-basic:
    - fi-bsw-nick:        NOTRUN -> [SKIP][28] ([fdo#109271]) +1 similar issue
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-bsw-nick/igt@kms_vrr@negative-basic.html
    - fi-kbl-7500u:       NOTRUN -> [SKIP][29] ([fdo#109271])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-kbl-7500u/igt@kms_vrr@negative-basic.html
    - fi-cfl-8109u:       NOTRUN -> [SKIP][30] ([fdo#109271])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-cfl-8109u/igt@kms_vrr@negative-basic.html
    - fi-bwr-2160:        NOTRUN -> [SKIP][31] ([fdo#109271]) +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-bwr-2160/igt@kms_vrr@negative-basic.html
    - fi-snb-2600:        NOTRUN -> [SKIP][32] ([fdo#109271]) +1 similar issue
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-snb-2600/igt@kms_vrr@negative-basic.html
    - fi-hsw-g3258:       NOTRUN -> [SKIP][33] ([fdo#109271]) +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-hsw-g3258/igt@kms_vrr@negative-basic.html
    - fi-hsw-4770:        NOTRUN -> [SKIP][34] ([fdo#109271]) +1 similar issue
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-hsw-4770/igt@kms_vrr@negative-basic.html
    - fi-bxt-dsi:         NOTRUN -> [SKIP][35] ([fdo#109271]) +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-bxt-dsi/igt@kms_vrr@negative-basic.html
    - fi-skl-6700k2:      NOTRUN -> [SKIP][36] ([fdo#109271]) +1 similar issue
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-skl-6700k2/igt@kms_vrr@negative-basic.html
    - fi-elk-e7500:       NOTRUN -> [SKIP][37] ([fdo#109271]) +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-elk-e7500/igt@kms_vrr@negative-basic.html
    - fi-glk-j4005:       NOTRUN -> [SKIP][38] ([fdo#109271])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-glk-j4005/igt@kms_vrr@negative-basic.html
    - fi-ivb-3770:        NOTRUN -> [SKIP][39] ([fdo#109271]) +1 similar issue
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-ivb-3770/igt@kms_vrr@negative-basic.html
    - fi-ilk-650:         NOTRUN -> [SKIP][40] ([fdo#109271]) +1 similar issue
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-ilk-650/igt@kms_vrr@negative-basic.html
    - fi-bsw-n3050:       NOTRUN -> [SKIP][41] ([fdo#109271])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-bsw-n3050/igt@kms_vrr@negative-basic.html
    - fi-cfl-guc:         NOTRUN -> [SKIP][42] ([fdo#109271])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-cfl-guc/igt@kms_vrr@negative-basic.html
    - fi-pnv-d510:        NOTRUN -> [SKIP][43] ([fdo#109271]) +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-pnv-d510/igt@kms_vrr@negative-basic.html
    - fi-bsw-kefka:       NOTRUN -> [SKIP][44] ([fdo#109271])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-bsw-kefka/igt@kms_vrr@negative-basic.html
    - fi-cfl-8700k:       NOTRUN -> [SKIP][45] ([fdo#109271])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-cfl-8700k/igt@kms_vrr@negative-basic.html

  * igt@kms_vrr@parse-vrr-range:
    - fi-blb-e6850:       NOTRUN -> [SKIP][46] ([fdo#109271]) +1 similar issue
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-blb-e6850/igt@kms_vrr@parse-vrr-range.html
    - fi-kbl-8809g:       NOTRUN -> [SKIP][47] ([fdo#109271]) +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-kbl-8809g/igt@kms_vrr@parse-vrr-range.html
    - fi-kbl-7567u:       NOTRUN -> [SKIP][48] ([fdo#109271]) +1 similar issue
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-kbl-7567u/igt@kms_vrr@parse-vrr-range.html

  * {igt@kms_vrr@parse-vrr-range@pipe-a-dp-2} (NEW):
    - fi-skl-guc:         NOTRUN -> [SKIP][49] ([fdo#109271]) +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-skl-guc/igt@kms_vrr@parse-vrr-range@pipe-a-dp-2.html

  * {igt@kms_vrr@parse-vrr-range@pipe-a-lvds-1} (NEW):
    - fi-snb-2520m:       NOTRUN -> [SKIP][50] ([fdo#109271]) +1 similar issue
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-snb-2520m/igt@kms_vrr@parse-vrr-range@pipe-a-lvds-1.html

  * igt@runner@aborted:
    - fi-hsw-4770:        NOTRUN -> [FAIL][51] ([fdo#109271] / [i915#4312] / [i915#5594])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-hsw-4770/igt@runner@aborted.html
    - fi-tgl-1115g4:      NOTRUN -> [FAIL][52] ([i915#5257])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/fi-tgl-1115g4/igt@runner@aborted.html
    - bat-dg1-6:          NOTRUN -> [FAIL][53] ([i915#5257])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/bat-dg1-6/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#4897]: https://gitlab.freedesktop.org/drm/intel/issues/4897
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5594]: https://gitlab.freedesktop.org/drm/intel/issues/5594
  [i915#5617]: https://gitlab.freedesktop.org/drm/intel/issues/5617
  [i915#5763]: https://gitlab.freedesktop.org/drm/intel/issues/5763


Build changes
-------------

  * IGT: IGT_6449 -> IGTPW_6978
  * Linux: CI_DRM_11548 -> Patchwork_102978v2

  CI-20190529: 20190529
  CI_DRM_11548: 4163e73aa7c34d043f2c7ed1567b6e21b6f1119a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_6978: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6978/index.html
  IGT_6449: 704da775abb83faa9324a665fe2992ab90f4ab03 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_102978v2: 4163e73aa7c34d043f2c7ed1567b6e21b6f1119a @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

af2f75203c23 drm/i915/vrr: Attach and set drm crtc vrr_enabled property
bb41fb54b167 drm/vrr: Attach vrr_enabled property to the drm crtc

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102978v2/index.html

[-- Attachment #2: Type: text/html, Size: 18492 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v2 1/2] drm/vrr: Attach vrr_enabled property to the drm crtc
  2022-04-25  6:46   ` [Intel-gfx] " Bhanuprakash Modem
@ 2022-04-26 22:01     ` Navare, Manasi
  -1 siblings, 0 replies; 12+ messages in thread
From: Navare, Manasi @ 2022-04-26 22:01 UTC (permalink / raw)
  To: Bhanuprakash Modem; +Cc: intel-gfx, dri-devel, Nicholas Kazlauskas

On Mon, Apr 25, 2022 at 12:16:11PM +0530, Bhanuprakash Modem wrote:
> Modern display hardware is capable of supporting variable refresh rates.
> This patch introduces helpers to attach and set "vrr_enabled" property
> on the crtc to allow userspace to query VRR enabled status on that crtc.
> 
> Atomic drivers should attach this property to crtcs those are capable of
> driving variable refresh rates using
> drm_mode_crtc_attach_vrr_enabled_property().
> 
> The value should be updated based on driver and hardware capability
> by using drm_mode_crtc_set_vrr_enabled_property().
> 
> V2: Use property flag as atomic

We already have userspace making us of the CRTC vrr_enabled property to
enable VRR for the CRTC like in case of full screen gaming.

This can already be done through:
drm_atomic_crtc_set_property call. Why do we need additonal helpers
for setting the same per CRTC property?

This is a default CRTC property so it will be created annd attached for
CRTC as per the DRM doc:
"VRR_ENABLED":
 *      Default &drm_crtc boolean property that notifies the driver that the
 *      content on the CRTC is suitable for variable refresh rate presentation.
 *      The driver will take this property as a hint to enable variable
 *      refresh rate support if the receiver supports it, ie. if the
 *      "vrr_capable" property is true on the &drm_connector object. The
 *      vertical front porch duration will be extended until page-flip or
 *      timeout when enabled.

Then we can use the atomic_crtc_set/get_property helpers to set it
Am I missing some other use case here?

Manasi

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>  drivers/gpu/drm/drm_crtc.c        | 44 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_mode_config.c |  2 +-
>  include/drm/drm_crtc.h            |  4 +++
>  3 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 26a77a735905..95b4a0c7ecb3 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -883,3 +883,47 @@ int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_crtc_create_scaling_filter_property);
> +
> +/**
> + * drm_mode_crtc_attach_vrr_enabled_property - attaches the vrr_enabled property
> + * @crtc: drm CRTC to attach the vrr_enabled property on.
> + *
> + * This is used by atomic drivers to add support for querying
> + * VRR enabled status for a crtc.
> + */
> +void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (!config->prop_vrr_enabled)
> +		return;
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->prop_vrr_enabled,
> +				   0);
> +}
> +EXPORT_SYMBOL(drm_mode_crtc_attach_vrr_enabled_property);
> +
> +/**
> + * drm_mode_crtc_set_vrr_enabled_property - sets the vrr enabled property for
> + * a crtc.
> + * @crtc: drm CRTC
> + * @vrr_enabled: True to enable the VRR on CRTC
> + *
> + * Should be used by atomic drivers to update the VRR enabled status on a CRTC
> + */
> +void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
> +					    bool vrr_enabled)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (!config->prop_vrr_enabled)
> +		return;
> +
> +	drm_object_property_set_value(&crtc->base,
> +				      config->prop_vrr_enabled,
> +				      vrr_enabled);
> +}
> +EXPORT_SYMBOL(drm_mode_crtc_set_vrr_enabled_property);
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 37b4b9f0e468..b7cde73d5586 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -323,7 +323,7 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.prop_mode_id = prop;
>  
> -	prop = drm_property_create_bool(dev, 0,
> +	prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
>  			"VRR_ENABLED");
>  	if (!prop)
>  		return -ENOMEM;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a70baea0636c..bde657cb0f9e 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1333,4 +1333,8 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
>  int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
>  					    unsigned int supported_filters);
>  
> +void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc);
> +void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
> +					    bool vrr_enabled);
> +
>  #endif /* __DRM_CRTC_H__ */
> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [RFC v2 1/2] drm/vrr: Attach vrr_enabled property to the drm crtc
@ 2022-04-26 22:01     ` Navare, Manasi
  0 siblings, 0 replies; 12+ messages in thread
From: Navare, Manasi @ 2022-04-26 22:01 UTC (permalink / raw)
  To: Bhanuprakash Modem
  Cc: intel-gfx, Harry Wentland, dri-devel, Nicholas Kazlauskas

On Mon, Apr 25, 2022 at 12:16:11PM +0530, Bhanuprakash Modem wrote:
> Modern display hardware is capable of supporting variable refresh rates.
> This patch introduces helpers to attach and set "vrr_enabled" property
> on the crtc to allow userspace to query VRR enabled status on that crtc.
> 
> Atomic drivers should attach this property to crtcs those are capable of
> driving variable refresh rates using
> drm_mode_crtc_attach_vrr_enabled_property().
> 
> The value should be updated based on driver and hardware capability
> by using drm_mode_crtc_set_vrr_enabled_property().
> 
> V2: Use property flag as atomic

We already have userspace making us of the CRTC vrr_enabled property to
enable VRR for the CRTC like in case of full screen gaming.

This can already be done through:
drm_atomic_crtc_set_property call. Why do we need additonal helpers
for setting the same per CRTC property?

This is a default CRTC property so it will be created annd attached for
CRTC as per the DRM doc:
"VRR_ENABLED":
 *      Default &drm_crtc boolean property that notifies the driver that the
 *      content on the CRTC is suitable for variable refresh rate presentation.
 *      The driver will take this property as a hint to enable variable
 *      refresh rate support if the receiver supports it, ie. if the
 *      "vrr_capable" property is true on the &drm_connector object. The
 *      vertical front porch duration will be extended until page-flip or
 *      timeout when enabled.

Then we can use the atomic_crtc_set/get_property helpers to set it
Am I missing some other use case here?

Manasi

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>  drivers/gpu/drm/drm_crtc.c        | 44 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_mode_config.c |  2 +-
>  include/drm/drm_crtc.h            |  4 +++
>  3 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 26a77a735905..95b4a0c7ecb3 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -883,3 +883,47 @@ int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_crtc_create_scaling_filter_property);
> +
> +/**
> + * drm_mode_crtc_attach_vrr_enabled_property - attaches the vrr_enabled property
> + * @crtc: drm CRTC to attach the vrr_enabled property on.
> + *
> + * This is used by atomic drivers to add support for querying
> + * VRR enabled status for a crtc.
> + */
> +void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (!config->prop_vrr_enabled)
> +		return;
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->prop_vrr_enabled,
> +				   0);
> +}
> +EXPORT_SYMBOL(drm_mode_crtc_attach_vrr_enabled_property);
> +
> +/**
> + * drm_mode_crtc_set_vrr_enabled_property - sets the vrr enabled property for
> + * a crtc.
> + * @crtc: drm CRTC
> + * @vrr_enabled: True to enable the VRR on CRTC
> + *
> + * Should be used by atomic drivers to update the VRR enabled status on a CRTC
> + */
> +void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
> +					    bool vrr_enabled)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (!config->prop_vrr_enabled)
> +		return;
> +
> +	drm_object_property_set_value(&crtc->base,
> +				      config->prop_vrr_enabled,
> +				      vrr_enabled);
> +}
> +EXPORT_SYMBOL(drm_mode_crtc_set_vrr_enabled_property);
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 37b4b9f0e468..b7cde73d5586 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -323,7 +323,7 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.prop_mode_id = prop;
>  
> -	prop = drm_property_create_bool(dev, 0,
> +	prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
>  			"VRR_ENABLED");
>  	if (!prop)
>  		return -ENOMEM;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a70baea0636c..bde657cb0f9e 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1333,4 +1333,8 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
>  int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
>  					    unsigned int supported_filters);
>  
> +void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc);
> +void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
> +					    bool vrr_enabled);
> +
>  #endif /* __DRM_CRTC_H__ */
> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v2 1/2] drm/vrr: Attach vrr_enabled property to the drm crtc
  2022-04-26 22:01     ` [Intel-gfx] " Navare, Manasi
@ 2022-04-28 11:51       ` Modem, Bhanuprakash
  -1 siblings, 0 replies; 12+ messages in thread
From: Modem, Bhanuprakash @ 2022-04-28 11:51 UTC (permalink / raw)
  To: Navare, Manasi; +Cc: intel-gfx, Nicholas Kazlauskas, dri-devel

On Wed-27-04-2022 03:31 am, Navare, Manasi wrote:
> On Mon, Apr 25, 2022 at 12:16:11PM +0530, Bhanuprakash Modem wrote:
>> Modern display hardware is capable of supporting variable refresh rates.
>> This patch introduces helpers to attach and set "vrr_enabled" property
>> on the crtc to allow userspace to query VRR enabled status on that crtc.
>>
>> Atomic drivers should attach this property to crtcs those are capable of
>> driving variable refresh rates using
>> drm_mode_crtc_attach_vrr_enabled_property().
>>
>> The value should be updated based on driver and hardware capability
>> by using drm_mode_crtc_set_vrr_enabled_property().
>>
>> V2: Use property flag as atomic
> 
> We already have userspace making us of the CRTC vrr_enabled property to
> enable VRR for the CRTC like in case of full screen gaming.
> 
> This can already be done through:
> drm_atomic_crtc_set_property call. Why do we need additonal helpers
> for setting the same per CRTC property?

Thanks for the Review, Manasi.

Yeah, I just realized that we are attaching "vrr_enabled" as part of 
crtc_init()

drm_crtc_init_with_planes() --> __drm_crtc_init_with_planes() --> 
drm_object_attach_property(vrr_enabled);

I'll drop the drm_mode_crtc_attach_vrr_enabled_property() helper.

> 
> This is a default CRTC property so it will be created annd attached for
> CRTC as per the DRM doc:
> "VRR_ENABLED":
>   *      Default &drm_crtc boolean property that notifies the driver that the
>   *      content on the CRTC is suitable for variable refresh rate presentation.
>   *      The driver will take this property as a hint to enable variable
>   *      refresh rate support if the receiver supports it, ie. if the
>   *      "vrr_capable" property is true on the &drm_connector object. The
>   *      vertical front porch duration will be extended until page-flip or
>   *      timeout when enabled.
> 
> Then we can use the atomic_crtc_set/get_property helpers to set it
> Am I missing some other use case here?

My intention is to set/clear "VRR_ENABLED" prop based on the driver and 
hardware capabilities. And user can request anytime to get the status of 
VRR on that CRTC.

Example:
Consider we have an Non-VRR panel is connected, but driver supports VRR 
(hence vrr_capable = 0)
  * Request from user-space to enable the VRR on CRTC.
  * Driver should take the decision to set/clear the "VRR_ENABLED".
  * Request from use-space to read back the "VRR_ENABLED". It must be 0, 
since "vrr_capable" is 0.

Without this series, in above scenario I am getting "VRR_ENABLED" as 1 
which is not true.

IGT to validate the same:  https://patchwork.freedesktop.org/series/100539/

I think, still we need a helper to set/clear the prop 
drm_mode_crtc_set_vrr_enabled_property(). I am not sure that we can use 
atomic_crtc_set/get_property helpers here, as these helpers are used by 
atomic_ioctls only.

- Bhanu

> 
> Manasi
> 
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> ---
>>   drivers/gpu/drm/drm_crtc.c        | 44 +++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/drm_mode_config.c |  2 +-
>>   include/drm/drm_crtc.h            |  4 +++
>>   3 files changed, 49 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 26a77a735905..95b4a0c7ecb3 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -883,3 +883,47 @@ int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
>>   	return 0;
>>   }
>>   EXPORT_SYMBOL(drm_crtc_create_scaling_filter_property);
>> +
>> +/**
>> + * drm_mode_crtc_attach_vrr_enabled_property - attaches the vrr_enabled property
>> + * @crtc: drm CRTC to attach the vrr_enabled property on.
>> + *
>> + * This is used by atomic drivers to add support for querying
>> + * VRR enabled status for a crtc.
>> + */
>> +void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc)
>> +{
>> +	struct drm_device *dev = crtc->dev;
>> +	struct drm_mode_config *config = &dev->mode_config;
>> +
>> +	if (!config->prop_vrr_enabled)
>> +		return;
>> +
>> +	drm_object_attach_property(&crtc->base,
>> +				   config->prop_vrr_enabled,
>> +				   0);
>> +}
>> +EXPORT_SYMBOL(drm_mode_crtc_attach_vrr_enabled_property);
>> +
>> +/**
>> + * drm_mode_crtc_set_vrr_enabled_property - sets the vrr enabled property for
>> + * a crtc.
>> + * @crtc: drm CRTC
>> + * @vrr_enabled: True to enable the VRR on CRTC
>> + *
>> + * Should be used by atomic drivers to update the VRR enabled status on a CRTC
>> + */
>> +void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
>> +					    bool vrr_enabled)
>> +{
>> +	struct drm_device *dev = crtc->dev;
>> +	struct drm_mode_config *config = &dev->mode_config;
>> +
>> +	if (!config->prop_vrr_enabled)
>> +		return;
>> +
>> +	drm_object_property_set_value(&crtc->base,
>> +				      config->prop_vrr_enabled,
>> +				      vrr_enabled);
>> +}
>> +EXPORT_SYMBOL(drm_mode_crtc_set_vrr_enabled_property);
>> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>> index 37b4b9f0e468..b7cde73d5586 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -323,7 +323,7 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>>   		return -ENOMEM;
>>   	dev->mode_config.prop_mode_id = prop;
>>   
>> -	prop = drm_property_create_bool(dev, 0,
>> +	prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
>>   			"VRR_ENABLED");
>>   	if (!prop)
>>   		return -ENOMEM;
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index a70baea0636c..bde657cb0f9e 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -1333,4 +1333,8 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
>>   int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
>>   					    unsigned int supported_filters);
>>   
>> +void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc);
>> +void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
>> +					    bool vrr_enabled);
>> +
>>   #endif /* __DRM_CRTC_H__ */
>> -- 
>> 2.35.1
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [RFC v2 1/2] drm/vrr: Attach vrr_enabled property to the drm crtc
@ 2022-04-28 11:51       ` Modem, Bhanuprakash
  0 siblings, 0 replies; 12+ messages in thread
From: Modem, Bhanuprakash @ 2022-04-28 11:51 UTC (permalink / raw)
  To: Navare, Manasi; +Cc: intel-gfx, Nicholas Kazlauskas, dri-devel

On Wed-27-04-2022 03:31 am, Navare, Manasi wrote:
> On Mon, Apr 25, 2022 at 12:16:11PM +0530, Bhanuprakash Modem wrote:
>> Modern display hardware is capable of supporting variable refresh rates.
>> This patch introduces helpers to attach and set "vrr_enabled" property
>> on the crtc to allow userspace to query VRR enabled status on that crtc.
>>
>> Atomic drivers should attach this property to crtcs those are capable of
>> driving variable refresh rates using
>> drm_mode_crtc_attach_vrr_enabled_property().
>>
>> The value should be updated based on driver and hardware capability
>> by using drm_mode_crtc_set_vrr_enabled_property().
>>
>> V2: Use property flag as atomic
> 
> We already have userspace making us of the CRTC vrr_enabled property to
> enable VRR for the CRTC like in case of full screen gaming.
> 
> This can already be done through:
> drm_atomic_crtc_set_property call. Why do we need additonal helpers
> for setting the same per CRTC property?

Thanks for the Review, Manasi.

Yeah, I just realized that we are attaching "vrr_enabled" as part of 
crtc_init()

drm_crtc_init_with_planes() --> __drm_crtc_init_with_planes() --> 
drm_object_attach_property(vrr_enabled);

I'll drop the drm_mode_crtc_attach_vrr_enabled_property() helper.

> 
> This is a default CRTC property so it will be created annd attached for
> CRTC as per the DRM doc:
> "VRR_ENABLED":
>   *      Default &drm_crtc boolean property that notifies the driver that the
>   *      content on the CRTC is suitable for variable refresh rate presentation.
>   *      The driver will take this property as a hint to enable variable
>   *      refresh rate support if the receiver supports it, ie. if the
>   *      "vrr_capable" property is true on the &drm_connector object. The
>   *      vertical front porch duration will be extended until page-flip or
>   *      timeout when enabled.
> 
> Then we can use the atomic_crtc_set/get_property helpers to set it
> Am I missing some other use case here?

My intention is to set/clear "VRR_ENABLED" prop based on the driver and 
hardware capabilities. And user can request anytime to get the status of 
VRR on that CRTC.

Example:
Consider we have an Non-VRR panel is connected, but driver supports VRR 
(hence vrr_capable = 0)
  * Request from user-space to enable the VRR on CRTC.
  * Driver should take the decision to set/clear the "VRR_ENABLED".
  * Request from use-space to read back the "VRR_ENABLED". It must be 0, 
since "vrr_capable" is 0.

Without this series, in above scenario I am getting "VRR_ENABLED" as 1 
which is not true.

IGT to validate the same:  https://patchwork.freedesktop.org/series/100539/

I think, still we need a helper to set/clear the prop 
drm_mode_crtc_set_vrr_enabled_property(). I am not sure that we can use 
atomic_crtc_set/get_property helpers here, as these helpers are used by 
atomic_ioctls only.

- Bhanu

> 
> Manasi
> 
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> ---
>>   drivers/gpu/drm/drm_crtc.c        | 44 +++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/drm_mode_config.c |  2 +-
>>   include/drm/drm_crtc.h            |  4 +++
>>   3 files changed, 49 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 26a77a735905..95b4a0c7ecb3 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -883,3 +883,47 @@ int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
>>   	return 0;
>>   }
>>   EXPORT_SYMBOL(drm_crtc_create_scaling_filter_property);
>> +
>> +/**
>> + * drm_mode_crtc_attach_vrr_enabled_property - attaches the vrr_enabled property
>> + * @crtc: drm CRTC to attach the vrr_enabled property on.
>> + *
>> + * This is used by atomic drivers to add support for querying
>> + * VRR enabled status for a crtc.
>> + */
>> +void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc)
>> +{
>> +	struct drm_device *dev = crtc->dev;
>> +	struct drm_mode_config *config = &dev->mode_config;
>> +
>> +	if (!config->prop_vrr_enabled)
>> +		return;
>> +
>> +	drm_object_attach_property(&crtc->base,
>> +				   config->prop_vrr_enabled,
>> +				   0);
>> +}
>> +EXPORT_SYMBOL(drm_mode_crtc_attach_vrr_enabled_property);
>> +
>> +/**
>> + * drm_mode_crtc_set_vrr_enabled_property - sets the vrr enabled property for
>> + * a crtc.
>> + * @crtc: drm CRTC
>> + * @vrr_enabled: True to enable the VRR on CRTC
>> + *
>> + * Should be used by atomic drivers to update the VRR enabled status on a CRTC
>> + */
>> +void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
>> +					    bool vrr_enabled)
>> +{
>> +	struct drm_device *dev = crtc->dev;
>> +	struct drm_mode_config *config = &dev->mode_config;
>> +
>> +	if (!config->prop_vrr_enabled)
>> +		return;
>> +
>> +	drm_object_property_set_value(&crtc->base,
>> +				      config->prop_vrr_enabled,
>> +				      vrr_enabled);
>> +}
>> +EXPORT_SYMBOL(drm_mode_crtc_set_vrr_enabled_property);
>> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>> index 37b4b9f0e468..b7cde73d5586 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -323,7 +323,7 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>>   		return -ENOMEM;
>>   	dev->mode_config.prop_mode_id = prop;
>>   
>> -	prop = drm_property_create_bool(dev, 0,
>> +	prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
>>   			"VRR_ENABLED");
>>   	if (!prop)
>>   		return -ENOMEM;
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index a70baea0636c..bde657cb0f9e 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -1333,4 +1333,8 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
>>   int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
>>   					    unsigned int supported_filters);
>>   
>> +void drm_mode_crtc_attach_vrr_enabled_property(struct drm_crtc *crtc);
>> +void drm_mode_crtc_set_vrr_enabled_property(struct drm_crtc *crtc,
>> +					    bool vrr_enabled);
>> +
>>   #endif /* __DRM_CRTC_H__ */
>> -- 
>> 2.35.1
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-04-28 11:51 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-25  6:46 [RFC v2 0/2] Attach and Set vrr_enabled property Bhanuprakash Modem
2022-04-25  6:46 ` [Intel-gfx] " Bhanuprakash Modem
2022-04-25  6:46 ` [RFC v2 1/2] drm/vrr: Attach vrr_enabled property to the drm crtc Bhanuprakash Modem
2022-04-25  6:46   ` [Intel-gfx] " Bhanuprakash Modem
2022-04-26 22:01   ` Navare, Manasi
2022-04-26 22:01     ` [Intel-gfx] " Navare, Manasi
2022-04-28 11:51     ` Modem, Bhanuprakash
2022-04-28 11:51       ` [Intel-gfx] " Modem, Bhanuprakash
2022-04-25  6:46 ` [RFC v2 2/2] drm/i915/vrr: Attach and set drm crtc vrr_enabled property Bhanuprakash Modem
2022-04-25  6:46   ` [Intel-gfx] " Bhanuprakash Modem
2022-04-25  7:07 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Attach and Set vrr_enabled property (rev2) Patchwork
2022-04-25  7:30 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.