[PATCH 2/2] drm: add docs for standard CRTC properties

[Daniel Vetter]

On Mon, May 18, 2020 at 02:22:47PM +0000, Simon Ser wrote:
> This patch adds docs for the ACTIVE and MODE_ID CRTC properties.
> 
> Signed-off-by: Simon Ser <contact at emersion.fr>
> Cc: Daniel Vetter <daniel.vetter at intel.com>
> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> Cc: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> Cc: Michel Dänzer <michel at daenzer.net>
> ---
> 
> Let me know if you think of other things to add.
> 
> Should we refer to ALLOW_MODESET in the MODE_ID docs?
> 
>  Documentation/gpu/drm-kms.rst |  5 +++++
>  drivers/gpu/drm/drm_crtc.c    | 16 ++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 397314d08f77..4289b7205696 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -460,6 +460,11 @@ HDMI Specific Connector Properties
>  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
>     :doc: HDMI connector properties
>  
> +Standard CRTC Properties

Missing the 
--------------------------
line here so this reanders neatly as a title and works with the sidebar
toc. Please check the output to make sure that all looks good.


> +
> +.. kernel-doc:: drivers/gpu/drm/drm_crtc.c
> +   :doc: standard CRTC properties
> +
>  Plane Composition Properties
>  ----------------------------
>  
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 4936e1080e41..c28e662c1e93 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -204,6 +204,22 @@ struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc)
>  	return fence;
>  }
>  
> +/**
> + * DOC: standard CRTC properties
> + *
> + * DRM CRTCs have a few standardized properties:
> + *
> + * ACTIVE:
> + * 	Atomic property for setting the power state of the CRTC. When set to 1 the
> + * 	CRTC will actively display content. When set to 0 the CRTC will be powered
> + * 	off. There is no expectation that user-space will reset CRTC resources like
> + * 	the mode and planes when setting ACTIVE to 0.

Maybe add "Note that the legacy DPMS property on connectors is internally
routed to control this property for atomic drivers."


Also I think we should add the dpms guarantee here.

"Userspace can rely on an change to ACTIVE always suceeding as long as no
other property has changed. If a change to ACTIVE only fails an atomic
test, this is a driver bug."

Kinda important part of the uapi here :-) Also the dpms emulation relies
on this.

> + * MODE_ID:
> + * 	Atomic property for setting the CRTC display timings. The value is the ID
> + * 	of a blob containing the DRM mode info. To disable the CRTC, user-space
> + * 	must set this property to 0.

Maybe we should add: "Note that only disabling by setting MODE_ID to 0
will release all internal resources (like reserved memory bandwidth or
clock generators). Only setting ACTIVE to 0 does not release reserved
resources, but only shuts down the hardware."

With these clarifications all added:

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Thanks for doing this, I think really important we start properly
documenting these uapi details at least somewhere!

Cheers, Daniel

> + */
> +
>  /**
>   * drm_crtc_init_with_planes - Initialise a new CRTC object with
>   *    specified primary and cursor planes.
> -- 
> 2.26.2
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch