UAPI Re: [PATCH 1/3] drm: Add DRM_MODE_TV_MODE_MONOCHROME

[Pekka Paalanen <pekka.paalanen@haloniitty.fi>]

[-- Attachment #1: Type: text/plain, Size: 5121 bytes --]

On Fri, 16 Feb 2024 18:48:55 +0000
Dave Stevenson <dave.stevenson@raspberrypi.com> wrote:

> From: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
> 
> Add this as a value for enum_drm_connector_tv_mode, represented
> by the string "Mono", to generate video with no colour encoding
> or bursts. Define it to have no pedestal (since only NTSC-M calls
> for a pedestal).
> 
> Change default mode creation to acommodate the new tv_mode value
> which comprises both 525-line and 625-line formats.
> 
> Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

Hi Dave and Nick,

since no-one else commented yet, I'd like to remind of the new UAPI
requirements:
https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements

AFAIU, adding a new value to an existing enum still counts as new UAPI.

Maybe there is no need for the full treatment here, or maybe there is,
I'm not sure. I think you should make some statement about how the new
UAPI requirements have been addressed.

Btw. no-one has submitted a record with "TV mode" to
https://drmdb.emersion.fr/
It only lists the radeon-specific "tv standard" property. I first
thought you had mistaken the property name in the cover letter.


Thanks,
pq

> ---
>  drivers/gpu/drm/drm_connector.c    | 7 +++++++
>  drivers/gpu/drm/drm_modes.c        | 5 ++++-
>  drivers/gpu/drm/drm_probe_helper.c | 5 +++--
>  include/drm/drm_connector.h        | 7 +++++++
>  4 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b0516505f7ae..fe05d27f3404 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1005,6 +1005,7 @@ static const struct drm_prop_enum_list drm_tv_mode_enum_list[] = {
>  	{ DRM_MODE_TV_MODE_PAL_M, "PAL-M" },
>  	{ DRM_MODE_TV_MODE_PAL_N, "PAL-N" },
>  	{ DRM_MODE_TV_MODE_SECAM, "SECAM" },
> +	{ DRM_MODE_TV_MODE_MONOCHROME, "Mono" },
>  };
>  DRM_ENUM_NAME_FN(drm_get_tv_mode_name, drm_tv_mode_enum_list)
>  
> @@ -1697,6 +1698,12 @@ EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property);
>   *		TV Mode is CCIR System B (aka 625-lines) together with
>   *		the SECAM Color Encoding.
>   *
> + *	Mono:
> + *
> + *		Use timings appropriate to the DRM mode, including
> + *		equalizing pulses for a 525-line or 625-line mode,
> + *		with no pedestal or color encoding.
> + *
>   *	Drivers can set up this property by calling
>   *	drm_mode_create_tv_properties().
>   */
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index c4f88c3a93b7..d274e7b00b79 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -530,7 +530,8 @@ static int fill_analog_mode(struct drm_device *dev,
>   * @interlace: whether to compute an interlaced mode
>   *
>   * This function creates a struct drm_display_mode instance suited for
> - * an analog TV output, for one of the usual analog TV mode.
> + * an analog TV output, for one of the usual analog TV modes. Where
> + * this is DRM_MODE_TV_MODE_MONOCHROME, a 625-line mode will be created.
>   *
>   * Note that @hdisplay is larger than the usual constraints for the PAL
>   * and NTSC timings, and we'll choose to ignore most timings constraints
> @@ -568,6 +569,8 @@ struct drm_display_mode *drm_analog_tv_mode(struct drm_device *dev,
>  	case DRM_MODE_TV_MODE_PAL_N:
>  		fallthrough;
>  	case DRM_MODE_TV_MODE_SECAM:
> +		fallthrough;
> +	case DRM_MODE_TV_MODE_MONOCHROME:
>  		analog = DRM_MODE_ANALOG_PAL;
>  		break;
>  
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index d1e1ade66f81..9254dc2af873 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -1211,8 +1211,9 @@ int drm_connector_helper_tv_get_modes(struct drm_connector *connector)
>  	for (i = 0; i < tv_mode_property->num_values; i++)
>  		supported_tv_modes |= BIT(tv_mode_property->values[i]);
>  
> -	if ((supported_tv_modes & ntsc_modes) &&
> -	    (supported_tv_modes & pal_modes)) {
> +	if (((supported_tv_modes & ntsc_modes) &&
> +	     (supported_tv_modes & pal_modes)) ||
> +	    (supported_tv_modes & BIT(DRM_MODE_TV_MODE_MONOCHROME))) {
>  		uint64_t default_mode;
>  
>  		if (drm_object_property_get_default_value(&connector->base,
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index fe88d7fc6b8f..90fd0ea0ca09 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -200,6 +200,13 @@ enum drm_connector_tv_mode {
>  	 */
>  	DRM_MODE_TV_MODE_SECAM,
>  
> +	/**
> +	 * @DRM_MODE_TV_MODE_MONOCHROME: Use timings appropriate to
> +	 * the DRM mode, including equalizing pulses for a 525-line
> +	 * or 625-line mode, with no pedestal or color encoding.
> +	 */
> +	DRM_MODE_TV_MODE_MONOCHROME,
> +
>  	/**
>  	 * @DRM_MODE_TV_MODE_MAX: Number of analog TV output modes.
>  	 *


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