Re: [PATCH v4 3/4] drm: Document variable refresh properties

[Pekka Paalanen <ppaalanen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3186 bytes --]

On Thu, 11 Oct 2018 12:39:41 -0400
Nicholas Kazlauskas <nicholas.kazlauskas-5C7GfCeVMHo@public.gmane.org> wrote:

> These include the drm_connector 'vrr_capable' and the drm_crtc
> 'vrr_enabled' properties.
> 
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas-5C7GfCeVMHo@public.gmane.org>
> ---
>  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

> + *
> + *	Support for variable refresh rate will depend on the "vrr_capable"
> + *	property exposed on the &drm_connector object.
> + */
> +
>  /**
>   * drm_connector_attach_vrr_capable_property - creates the
>   * vrr_capable property


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx