Re: [PATCH RFC 01/13] drm/connector: Introduce an HDMI connector

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 14 Aug 2023, Maxime Ripard <mripard@kernel.org> wrote:
> A lot of the various HDMI drivers duplicate some logic that depends on
> the HDMI spec itself and not really a particular hardware
> implementation.
>
> Output BPC or format selection, infoframe generation are good examples
> of such areas.
>
> This creates a lot of boilerplate, with a lot of variations, which makes
> it hard for userspace to rely on, and makes it difficult to get it right
> for drivers.
>
> Let's create a new connector variant specifically dedicated to HDMI
> controllers that will allow to abstract away the duplicated logic.
>
> Hopefully, this will make drivers simpler to handle, and their behaviour
> more consistent.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/Makefile             |  1 +
>  drivers/gpu/drm/drm_hdmi_connector.c | 45 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h          | 16 +++++++++++++
>  3 files changed, 62 insertions(+)
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 7a09a89b493b..1520d4ccd3d7 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -27,6 +27,7 @@ drm-y := \
>  	drm_fourcc.o \
>  	drm_framebuffer.o \
>  	drm_gem.o \
> +	drm_hdmi_connector.o \
>  	drm_ioctl.o \
>  	drm_lease.o \
>  	drm_managed.o \
> diff --git a/drivers/gpu/drm/drm_hdmi_connector.c b/drivers/gpu/drm/drm_hdmi_connector.c
> new file mode 100644
> index 000000000000..62f01dd2e6df
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_hdmi_connector.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <drm/drm_connector.h>
> +#include <drm/drm_mode.h>
> +
> +#include <linux/export.h>
> +
> +/**
> + * drmm_hdmi_connector_init - Init a preallocated HDMI connector
> + * @dev: DRM device
> + * @hdmi_connector: A pointer to the HDMI connector to init
> + * @connector_type: user visible type of the connector
> + * @ddc: optional pointer to the associated ddc adapter
> + *
> + * Initialises a preallocated HDMI connector. Connectors can be
> + * subclassed as part of driver connector objects.
> + *
> + * Cleanup is automatically handled with a call to
> + * drm_connector_cleanup() in a DRM-managed action.
> + *
> + * The connector structure should be allocated with drmm_kzalloc().
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drmm_hdmi_connector_init(struct drm_device *dev,
> +			     struct drm_hdmi_connector *hdmi_connector,
> +			     const struct drm_connector_funcs *funcs,
> +			     int connector_type,
> +			     struct i2c_adapter *ddc)
> +{
> +	struct drm_connector *connector = &hdmi_connector->base;
> +	int ret;
> +
> +	if (connector_type != DRM_MODE_CONNECTOR_HDMIA ||
> +	    connector_type != DRM_MODE_CONNECTOR_HDMIB)
> +		return -EINVAL;
> +
> +	ret = drmm_connector_init(dev, connector, funcs, connector_type, ddc);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drmm_hdmi_connector_init);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index d300fde6c1a4..1859b74083f5 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -2042,6 +2042,22 @@ void drm_connector_attach_privacy_screen_provider(
>  	struct drm_connector *connector, struct drm_privacy_screen *priv);
>  void drm_connector_update_privacy_screen(const struct drm_connector_state *connector_state);
>  
> +struct drm_hdmi_connector {
> +	/**
> +	 * @base: Base Connector
> +	 */
> +	struct drm_connector base;
> +};
> +
> +#define connector_to_hdmi_connector(connector) \
> +	container_of_const(connector, struct drm_hdmi_connector, base)
> +
> +int drmm_hdmi_connector_init(struct drm_device *dev,
> +			     struct drm_hdmi_connector *hdmi_connector,
> +			     const struct drm_connector_funcs *funcs,
> +			     int connector_type,
> +			     struct i2c_adapter *ddc);
> +

Pure bikeshedding, maybe add drm_hdmi_connector.h from the start so we
don't have to split this up later.

BR,
Jani.

>  /**
>   * struct drm_tile_group - Tile group metadata
>   * @refcount: reference count

-- 
Jani Nikula, Intel Open Source Graphics Center