On 1/30/19 6:02 AM, Daniel Vetter wrote: > On Wed, Jan 30, 2019 at 11:42 AM Daniel Vetter <daniel@xxxxxxxx> wrote: >> >> On Thu, Nov 8, 2018 at 3:44 PM Nicholas Kazlauskas >> <nicholas.kazlauskas@xxxxxxx> 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@xxxxxxx> >>> Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx> >>> Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx> >>> --- >>> 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@xxxxxxxxxxxxxxxxxxxxx >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx