[RFC v2 1/2] drm/vrr: Attach vrr_enabled property to the drm crtc
Modem, Bhanuprakash
bhanuprakash.modem at intel.com
Thu Apr 28 11:51:28 UTC 2022
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 at linux.intel.com>
>> Cc: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>> Cc: Manasi Navare <manasi.d.navare at intel.com>
>> Cc: Harry Wentland <harry.wentland at amd.com>
>> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at 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
>>
More information about the dri-devel
mailing list