[PATCH v7 2/5] drm: Add vrr_enabled property to drm CRTC
Kazlauskas, Nicholas
Nicholas.Kazlauskas at amd.com
Wed Jan 30 13:37:12 UTC 2019
On 1/30/19 6:02 AM, Daniel Vetter wrote:
> On Wed, Jan 30, 2019 at 11:42 AM Daniel Vetter <daniel at ffwll.ch> wrote:
>>
>> On Thu, Nov 8, 2018 at 3:44 PM Nicholas Kazlauskas
>> <nicholas.kazlauskas at amd.com> wrote:
>>>
>>> This patch introduces the 'vrr_enabled' CRTC property to allow
>>> dynamic control over variable refresh rate support for a CRTC.
>>>
>>> This property should be treated like a content hint to the driver -
>>> if the hardware or driver is not capable of driving variable refresh
>>> timings then this is not considered an error.
>>>
>>> Capability for variable refresh rate support should be determined
>>> by querying the vrr_capable drm connector property.
>>>
>>> It is worth noting that while the property is intended for atomic use
>>> it isn't filtered from legacy userspace queries. This allows for Xorg
>>> userspace drivers to implement support.
>>>
>>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>>> Reviewed-by: Harry Wentland <harry.wentland at amd.com>
>>> Cc: Manasi Navare <manasi.d.navare at intel.com>
>>> ---
>>> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++
>>> drivers/gpu/drm/drm_crtc.c | 2 ++
>>> drivers/gpu/drm/drm_mode_config.c | 6 ++++++
>>> include/drm/drm_crtc.h | 9 +++++++++
>>> include/drm/drm_mode_config.h | 5 +++++
>>> 5 files changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>>> index d5b7f315098c..eec396a57b88 100644
>>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>>> @@ -433,6 +433,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>> ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
>>> drm_property_blob_put(mode);
>>> return ret;
>>> + } else if (property == config->prop_vrr_enabled) {
>>> + state->vrr_enabled = val;
>>> } else if (property == config->degamma_lut_property) {
>>> ret = drm_atomic_replace_property_blob_from_id(dev,
>>> &state->degamma_lut,
>>> @@ -491,6 +493,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>>> *val = state->active;
>>> else if (property == config->prop_mode_id)
>>> *val = (state->mode_blob) ? state->mode_blob->base.id : 0;
>>> + else if (property == config->prop_vrr_enabled)
>>> + *val = state->vrr_enabled;
>>> else if (property == config->degamma_lut_property)
>>> *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
>>> else if (property == config->ctm_property)
>>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>>> index 268a182ae189..6f8ddfcfaba5 100644
>>> --- a/drivers/gpu/drm/drm_crtc.c
>>> +++ b/drivers/gpu/drm/drm_crtc.c
>>> @@ -340,6 +340,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
>>> drm_object_attach_property(&crtc->base, config->prop_mode_id, 0);
>>> drm_object_attach_property(&crtc->base,
>>> config->prop_out_fence_ptr, 0);
>>> + drm_object_attach_property(&crtc->base,
>>> + config->prop_vrr_enabled, 0);
>>> }
>>>
>>> return 0;
>>> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>>> index ee80788f2c40..5670c67f28d4 100644
>>> --- a/drivers/gpu/drm/drm_mode_config.c
>>> +++ b/drivers/gpu/drm/drm_mode_config.c
>>> @@ -310,6 +310,12 @@ 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,
>>> + "VRR_ENABLED");
>>
>> VRR_ENABLED doesn't match vrr_enabled in the docs. But the connector
>> one seems to indeed be lowercase. And casing matters for properties
>>
>> Can you pls figure out who's right here and fix this confusion up?
>
> I checked the igt quickly, patch is typed, will send out today or so.
> -Daniel
Yeah, it should be "VRR_ENABLED" in the docs (like the other CRTC
default properties, eg. explicit fencing "IN_FENCE_FD” and "OUT_FENCE_PTR".
It looks a bit weird next to "vrr_capable" but it makes sense in the
usage context for CRTC vs connector at least.
This should be fixed in the docs and it sounds like you have a patch so
I'll drop my R-B when it's up.
Thanks!
Nicholas Kazlauskas
>
>>
>> Thanks, Daniel
>>
>>> + if (!prop)
>>> + return -ENOMEM;
>>> + dev->mode_config.prop_vrr_enabled = prop;
>>> +
>>> prop = drm_property_create(dev,
>>> DRM_MODE_PROP_BLOB,
>>> "DEGAMMA_LUT", 0);
>>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>>> index b21437bc95bf..39c3900aab3c 100644
>>> --- a/include/drm/drm_crtc.h
>>> +++ b/include/drm/drm_crtc.h
>>> @@ -290,6 +290,15 @@ struct drm_crtc_state {
>>> */
>>> u32 pageflip_flags;
>>>
>>> + /**
>>> + * @vrr_enabled:
>>> + *
>>> + * Indicates if variable refresh rate should be enabled for the CRTC.
>>> + * Support for the requested vrr state will depend on driver and
>>> + * hardware capabiltiy - lacking support is not treated as failure.
>>> + */
>>> + bool vrr_enabled;
>>> +
>>> /**
>>> * @event:
>>> *
>>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>>> index 928e4172a0bb..49f2fcfdb5fc 100644
>>> --- a/include/drm/drm_mode_config.h
>>> +++ b/include/drm/drm_mode_config.h
>>> @@ -639,6 +639,11 @@ struct drm_mode_config {
>>> * connectors must be of and active must be set to disabled, too.
>>> */
>>> struct drm_property *prop_mode_id;
>>> + /**
>>> + * @prop_vrr_enabled: Default atomic CRTC property to indicate
>>> + * whether variable refresh rate should be enabled on the CRTC.
>>> + */
>>> + struct drm_property *prop_vrr_enabled;
>>>
>>> /**
>>> * @dvi_i_subconnector_property: Optional DVI-I property to
>>> --
>>> 2.19.1
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
>
>
More information about the amd-gfx
mailing list