Re: [RFC V3 1/2] drm/vrr: Attach vrr_enabled property to the drm crtc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue-31-05-2022 10:42 pm, Navare, Manasi wrote:
On Tue, May 17, 2022 at 12:56:35PM +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().

We are not attaching the prop anymore, please remove this from the
commit message.

Thanks for the reply,

I'll update this in next rev.



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
V3: Drop helper to attach vrr_enabled prop, since it is already
attached (Manasi)

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

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 26a77a735905..8bb8b4bf4199 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -239,6 +239,9 @@ struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc)
   * 		Driver's default scaling filter
   * 	Nearest Neighbor:
   * 		Nearest Neighbor scaling filter
+ * VRR_ENABLED:
+ *	Atomic property for setting the VRR state of the CRTC.
+ *	To enable the VRR on CRTC, user-space must set this property to 1.

This prop was primarily a userspace Write only and driver read only
property which would be used only by the userspace to request VRR on
that CRTC,

Are we now modifying this to be used as a bidirectional property to also
indicate the status of VRR on that CRTC which will be updated by the
driver?

Precisely YES.


We need to add this accordingly and update the DRM documentation and
also get acks from other vendors since AMD and other folks mght be using
this as a write only prop.

Sure, I'll update the documentation.

@Harry/@Nicholas, can you please comment on this approach?

I would like to modify "vrr_enabled" prop to be used as a bidirectional property. So, that userspace can request to enable the VRR and also to get the status of VRR on that CRTC.

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

Example:
* If userspace has accidentally set the "vrr_enabled" prop to true without checking connector's "vrr_capable" prop, driver should reset this "vrr_enabled" property to false.

* Userspace can read "vrr_enabled" prop anytime to get the current status of VRR on that crtc.

- Bhanu


Manasi

   */
__printf(6, 0)
@@ -883,3 +886,26 @@ int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
  	return 0;
  }
  EXPORT_SYMBOL(drm_crtc_create_scaling_filter_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..906787398f40 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1333,4 +1333,7 @@ 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_set_vrr_enabled_property(struct drm_crtc *crtc,
+					    bool vrr_enabled);
+
  #endif /* __DRM_CRTC_H__ */
--
2.35.1





[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux