Re: [PATCH] drm: Try to document legacy DPMS uapi a bit better

[Eric Anholt <eric@anholt.net>]

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

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> Laurent asked for this.
>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index ba9f36cef68c..b458eb488128 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -720,6 +720,25 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
>   * 	callback. For atomic drivers the remapping to the "ACTIVE" property is
>   * 	implemented in the DRM core.  This is the only standard connector
>   * 	property that userspace can change.
> + *
> + * 	WARNING:
> + *
> + * 	For userspace also running on legacy drivers the "DPMS" semantics are a
> + * 	lot more complicated. First, userspace cannot rely on the "DPMS" value
> + * 	returned by the GETCONNECTOR actually reflecting reality, because many
> + * 	drivers fail to update it. For atomic drivers this is taken care of in
> + * 	drm_atomic_helper_update_legacy_modeset_state().
> + *
> + * 	The second issue is that the DPMS state is only relevant when the

s/relevant/defined/ maybe?

> + * 	connector is connected to a CRTC. In atomic the DRM core enforces that
> + * 	"ACTIVE" is off in such a case, no such checks exists for "DPMS".

Maybe another newline here?

> + * 	Finally, when enabling an output using the legacy SETCONFIG ioctl then
> + * 	"DPMS" is forced to ON. But see above, that might not be reflected in
> + * 	the software value.

Maybe add " on legacy drivers"?

> + *
> + * 	Summarizing: Only set "DPMS" when the connector is known to be enabled,
> + * 	assume that a successful SETCONFIG call also set "DPMS" to on, and never

"also sets"

> + * 	read back the value of "DPMS" because it can be incorrect.

With whatever set of my suggestions you like,

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel