Nouveau - Nov 2022 - [PATCH v7 16/23] drm/probe-helper: Provide a TV get_modes helper

From: Noralf Tr?nnes <noralf at tronnes.org>

Most of the TV connectors will need a similar get_modes implementation
that will, depending on the drivers' capabilities, register the 480i and
576i modes.

That implementation will also need to set the preferred flag and order
the modes based on the driver and users preferrence.

This is especially important to guarantee that a userspace stack such as
Xorg can start and pick up the preferred mode while maintaining a
working output.

Signed-off-by: Noralf Tr?nnes <noralf at tronnes.org>
Signed-off-by: Maxime Ripard <maxime at cerno.tech>

---
Changes in v7:
- Used Noralf's implementation

Changes in v6:
- New patch
---
 drivers/gpu/drm/drm_probe_helper.c | 97 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_probe_helper.h     |  1 +
 2 files changed, 98 insertions(+)

diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index 2fc21df709bc..edb2e4c4530a 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -1147,3 +1147,100 @@ int drm_connector_helper_get_modes(struct drm_connector
*connector)
 	return count;
 }
 EXPORT_SYMBOL(drm_connector_helper_get_modes);
+
+static bool tv_mode_supported(struct drm_connector *connector,
+			      enum drm_connector_tv_mode mode)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *property = dev->mode_config.tv_mode_property;
+
+	unsigned int i;
+
+	for (i = 0; i < property->num_values; i++)
+		if (property->values[i] == mode)
+			return true;
+
+	return false;
+}
+
+/**
+ * drm_connector_helper_tv_get_modes - Fills the modes availables to a TV
connector
+ * @connector: The connector
+ *
+ * Fills the available modes for a TV connector based on the supported
+ * TV modes, and the default mode expressed by the kernel command line.
+ *
+ * This can be used as the default TV connector helper .get_modes() hook
+ * if the driver does not need any special processing.
+ *
+ * Returns:
+ * The number of modes added to the connector.
+ */
+int drm_connector_helper_tv_get_modes(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *tv_mode_property +		dev->mode_config.tv_mode_property;
+	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
+	unsigned int ntsc_modes = BIT(DRM_MODE_TV_MODE_NTSC) |
+		BIT(DRM_MODE_TV_MODE_NTSC_443) |
+		BIT(DRM_MODE_TV_MODE_NTSC_J) |
+		BIT(DRM_MODE_TV_MODE_PAL_M);
+	unsigned int pal_modes = BIT(DRM_MODE_TV_MODE_PAL) |
+		BIT(DRM_MODE_TV_MODE_PAL_N) |
+		BIT(DRM_MODE_TV_MODE_SECAM);
+	unsigned int tv_modes[2] = { UINT_MAX, UINT_MAX };
+	unsigned int i, supported_tv_modes = 0;
+
+	if (!tv_mode_property)
+		return 0;
+
+	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)) {
+		uint64_t default_mode;
+
+		if (drm_object_property_get_default_value(&connector->base,
+							  tv_mode_property,
+							  &default_mode))
+			return 0;
+
+		if (cmdline->tv_mode_specified)
+			default_mode = cmdline->tv_mode;
+
+		if (BIT(default_mode) & ntsc_modes) {
+			tv_modes[0] = DRM_MODE_TV_MODE_NTSC;
+			tv_modes[1] = DRM_MODE_TV_MODE_PAL;
+		} else {
+			tv_modes[0] = DRM_MODE_TV_MODE_PAL;
+			tv_modes[1] = DRM_MODE_TV_MODE_NTSC;
+		}
+	} else if (supported_tv_modes & ntsc_modes) {
+		tv_modes[0] = DRM_MODE_TV_MODE_NTSC;
+	} else if (supported_tv_modes & pal_modes) {
+		tv_modes[0] = DRM_MODE_TV_MODE_PAL;
+	} else {
+		return 0;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
+		struct drm_display_mode *mode;
+
+		if (tv_modes[i] == DRM_MODE_TV_MODE_NTSC)
+			mode = drm_mode_analog_ntsc_480i(dev);
+		else if (tv_modes[i] == DRM_MODE_TV_MODE_PAL)
+			mode = drm_mode_analog_pal_576i(dev);
+		else
+			break;
+		if (!mode)
+			return i;
+		if (!i)
+			mode->type |= DRM_MODE_TYPE_PREFERRED;
+		drm_mode_probed_add(connector, mode);
+	}
+
+	return i;
+}
+EXPORT_SYMBOL(drm_connector_helper_tv_get_modes);
diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h
index 5880daa14624..4977e0ab72db 100644
--- a/include/drm/drm_probe_helper.h
+++ b/include/drm/drm_probe_helper.h
@@ -35,5 +35,6 @@ int drm_connector_helper_get_modes_from_ddc(struct
drm_connector *connector);
 int drm_connector_helper_get_modes_fixed(struct drm_connector *connector,
 					 const struct drm_display_mode *fixed_mode);
 int drm_connector_helper_get_modes(struct drm_connector *connector);
+int drm_connector_helper_tv_get_modes(struct drm_connector *connector);
 
 #endif

-- 
b4 0.11.0-dev-99e3a

Den 07.11.2022 15.16, skrev Maxime Ripard:
> From: Noralf Tr?nnes <noralf at tronnes.org>
> 
> Most of the TV connectors will need a similar get_modes implementation
> that will, depending on the drivers' capabilities, register the 480i
and
> 576i modes.
> 
> That implementation will also need to set the preferred flag and order
> the modes based on the driver and users preferrence.
> 
> This is especially important to guarantee that a userspace stack such as
> Xorg can start and pick up the preferred mode while maintaining a
> working output.
> 
> Signed-off-by: Noralf Tr?nnes <noralf at tronnes.org>
> Signed-off-by: Maxime Ripard <maxime at cerno.tech>
> 
> ---
> Changes in v7:
> - Used Noralf's implementation
> 
> Changes in v6:
> - New patch
> ---
>  drivers/gpu/drm/drm_probe_helper.c | 97
++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_probe_helper.h     |  1 +
>  2 files changed, 98 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
> index 2fc21df709bc..edb2e4c4530a 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -1147,3 +1147,100 @@ int drm_connector_helper_get_modes(struct
drm_connector *connector)
>  	return count;
>  }
>  EXPORT_SYMBOL(drm_connector_helper_get_modes);
> +
> +static bool tv_mode_supported(struct drm_connector *connector,
> +			      enum drm_connector_tv_mode mode)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *property = dev->mode_config.tv_mode_property;
> +
> +	unsigned int i;
> +
> +	for (i = 0; i < property->num_values; i++)
> +		if (property->values[i] == mode)
> +			return true;
> +
> +	return false;
> +}
This function is not used in the new implementation.

I hope you have tested this patch since I didn't even compile test my
implementation (probably should have said so...)

Noralf.
> +
> +/**
> + * drm_connector_helper_tv_get_modes - Fills the modes availables to a TV
connector
> + * @connector: The connector
> + *
> + * Fills the available modes for a TV connector based on the supported
> + * TV modes, and the default mode expressed by the kernel command line.
> + *
> + * This can be used as the default TV connector helper .get_modes() hook
> + * if the driver does not need any special processing.
> + *
> + * Returns:
> + * The number of modes added to the connector.
> + */
> +int drm_connector_helper_tv_get_modes(struct drm_connector *connector)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *tv_mode_property > +	
dev->mode_config.tv_mode_property;
> +	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
> +	unsigned int ntsc_modes = BIT(DRM_MODE_TV_MODE_NTSC) |
> +		BIT(DRM_MODE_TV_MODE_NTSC_443) |
> +		BIT(DRM_MODE_TV_MODE_NTSC_J) |
> +		BIT(DRM_MODE_TV_MODE_PAL_M);
> +	unsigned int pal_modes = BIT(DRM_MODE_TV_MODE_PAL) |
> +		BIT(DRM_MODE_TV_MODE_PAL_N) |
> +		BIT(DRM_MODE_TV_MODE_SECAM);
> +	unsigned int tv_modes[2] = { UINT_MAX, UINT_MAX };
> +	unsigned int i, supported_tv_modes = 0;
> +
> +	if (!tv_mode_property)
> +		return 0;
> +
> +	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)) {
> +		uint64_t default_mode;
> +
> +		if (drm_object_property_get_default_value(&connector->base,
> +							  tv_mode_property,
> +							  &default_mode))
> +			return 0;
> +
> +		if (cmdline->tv_mode_specified)
> +			default_mode = cmdline->tv_mode;
> +
> +		if (BIT(default_mode) & ntsc_modes) {
> +			tv_modes[0] = DRM_MODE_TV_MODE_NTSC;
> +			tv_modes[1] = DRM_MODE_TV_MODE_PAL;
> +		} else {
> +			tv_modes[0] = DRM_MODE_TV_MODE_PAL;
> +			tv_modes[1] = DRM_MODE_TV_MODE_NTSC;
> +		}
> +	} else if (supported_tv_modes & ntsc_modes) {
> +		tv_modes[0] = DRM_MODE_TV_MODE_NTSC;
> +	} else if (supported_tv_modes & pal_modes) {
> +		tv_modes[0] = DRM_MODE_TV_MODE_PAL;
> +	} else {
> +		return 0;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
> +		struct drm_display_mode *mode;
> +
> +		if (tv_modes[i] == DRM_MODE_TV_MODE_NTSC)
> +			mode = drm_mode_analog_ntsc_480i(dev);
> +		else if (tv_modes[i] == DRM_MODE_TV_MODE_PAL)
> +			mode = drm_mode_analog_pal_576i(dev);
> +		else
> +			break;
> +		if (!mode)
> +			return i;
> +		if (!i)
> +			mode->type |= DRM_MODE_TYPE_PREFERRED;
> +		drm_mode_probed_add(connector, mode);
> +	}
> +
> +	return i;
> +}
> +EXPORT_SYMBOL(drm_connector_helper_tv_get_modes);
> diff --git a/include/drm/drm_probe_helper.h
b/include/drm/drm_probe_helper.h
> index 5880daa14624..4977e0ab72db 100644
> --- a/include/drm/drm_probe_helper.h
> +++ b/include/drm/drm_probe_helper.h
> @@ -35,5 +35,6 @@ int drm_connector_helper_get_modes_from_ddc(struct
drm_connector *connector);
>  int drm_connector_helper_get_modes_fixed(struct drm_connector *connector,
>  					 const struct drm_display_mode *fixed_mode);
>  int drm_connector_helper_get_modes(struct drm_connector *connector);
> +int drm_connector_helper_tv_get_modes(struct drm_connector *connector);
>  
>  #endif
>