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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <878racd5nx.fsf@intel.com>
Date:   Tue, 15 Aug 2023 12:19:14 +0300
From:   Jani Nikula <jani.nikula@...ux.intel.com>
To:     Maxime Ripard <mripard@...nel.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>, Emma Anholt <emma@...olt.net>
Cc:     Hans Verkuil <hverkuil@...all.nl>, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, Maxime Ripard <mripard@...nel.org>
Subject: Re: [PATCH RFC 01/13] drm/connector: Introduce an HDMI connector

On Mon, 14 Aug 2023, Maxime Ripard <mripard@...nel.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@...nel.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