| Message ID | 20181108144353.31363-3-nicholas.kazlauskas@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | A DRM API for adaptive sync and variable refresh rate support | expand |
On Thu, Nov 8, 2018 at 3:44 PM Nicholas Kazlauskas <nicholas.kazlauskas@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@amd.com> > Reviewed-by: Harry Wentland <harry.wentland@amd.com> > Cc: Manasi Navare <manasi.d.navare@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? 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Jan 30, 2019 at 11:42 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Thu, Nov 8, 2018 at 3:44 PM Nicholas Kazlauskas > <nicholas.kazlauskas@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@amd.com> > > Reviewed-by: Harry Wentland <harry.wentland@amd.com> > > Cc: Manasi Navare <manasi.d.navare@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 > > 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@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
On 1/30/19 6:02 AM, Daniel Vetter wrote: > On Wed, Jan 30, 2019 at 11:42 AM Daniel Vetter <daniel@ffwll.ch> wrote: >> >> On Thu, Nov 8, 2018 at 3:44 PM Nicholas Kazlauskas >> <nicholas.kazlauskas@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@amd.com> >>> Reviewed-by: Harry Wentland <harry.wentland@amd.com> >>> Cc: Manasi Navare <manasi.d.navare@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@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 > > >
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"); + 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