On Fri, Oct 26, 2018 at 02:49:31PM +0000, Kazlauskas, Nicholas wrote: > On 10/26/18 7:37 AM, Pekka Paalanen wrote: > > On Thu, 11 Oct 2018 12:39:41 -0400 > > Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx> wrote: > > > >> These include the drm_connector 'vrr_capable' and the drm_crtc > >> 'vrr_enabled' properties. > >> > >> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx> > >> --- > >> Documentation/gpu/drm-kms.rst | 7 +++++++ > >> drivers/gpu/drm/drm_connector.c | 22 ++++++++++++++++++++++ > >> 2 files changed, 29 insertions(+) > >> > >> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > >> index 4b1501b4835b..8da2a178cf85 100644 > >> --- a/Documentation/gpu/drm-kms.rst > >> +++ b/Documentation/gpu/drm-kms.rst > >> @@ -575,6 +575,13 @@ Explicit Fencing Properties > >> .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > >> :doc: explicit fencing properties > >> > >> + > >> +Variable Refresh Properties > >> +--------------------------- > >> + > >> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > >> + :doc: Variable refresh properties > >> + > >> Existing KMS Properties > >> ----------------------- > >> > >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > >> index f0deeb7298d0..2a12853ca917 100644 > >> --- a/drivers/gpu/drm/drm_connector.c > >> +++ b/drivers/gpu/drm/drm_connector.c > >> @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev) > >> } > >> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > >> > >> +/** > >> + * DOC: Variable refresh properties > >> + * > >> + * Variable refresh rate control is supported via properties on the > >> + * &drm_connector and &drm_crtc objects. > >> + * > >> + * "vrr_capable": > >> + * Optional &drm_connector boolean property that drivers should attach > >> + * with drm_connector_attach_vrr_capable_property() on connectors that > >> + * could support variable refresh rates. Drivers should update the > >> + * property value by calling drm_connector_set_vrr_capable_property(). > >> + * > >> + * Absence of the property should indicate absence of support. > >> + * > >> + * "vrr_enabled": > >> + * Default &drm_crtc boolean property that notifies the driver that the > >> + * variable refresh rate adjustment should be enabled for the CRTC. > > > > Hi, > > > > where is the documentation that explains how drivers must implement > > "variable refresh rate adjustment"? > > > > What should and could userspace expect to get if it flicks this switch? > > > > I also think the kernel documentation should include a description of > > what VRR actually is and how it conceptually works as far as userspace > > is concerned. > > > > That is, the kernel documentation should describe what this thing does, > > so that we avoid every driver implementing a different thing. For > > example, one driver could prevent the luminance flickering itself by > > tuning the timings while another driver might not do that, which means > > that an application tested on the former driver will look just fine > > while it is unbearable to watch on the latter. > > > > > > Thanks, > > pq > > I felt it was best to leave this more on the vague side to not impose > restrictions yet on what a driver must do. > > If you think it's worth defining what the "baseline" expectation is for > userspace, I would probably describe it as "utilizing the monitor's > variable refresh rate to reduce stuttering or tearing that can occur > during flipping". This is currently what the amdgpu driver has enabled > for support. The implementation also isn't much more complex than just > passing the variable refresh range to the hardware. > > I wouldn't want every driver to be forced to implement some sort of > luminance flickering by default. It's not noticeable on many panels and > any tuning would inherently add latency to the output. It would probably > be better left as a new property or a driver specific feature. > > In general I would imagine that most future VRR features would end up as > new properties. Anything that's purely software could be implemented as > a drm helper that every driver can use. I think the target presentation > timestamp feature is a good example for that. Speaking of timestamps. What is the expected behaviour of vblank timestamps when vrr is enabled? -- Ville Syrjälä Intel _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx