[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240621-polite-ruby-hyrax-c4020e@houat>
Date: Fri, 21 Jun 2024 11:30:29 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman <jonas@...boo.se>,
Jernej Skrabec <jernej.skrabec@...il.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>, Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, linux-sound@...r.kernel.org
Subject: Re: [PATCH RFC 4/5] drm/bridge: connector: add support for HDMI
codec framework
On Sat, Jun 15, 2024 at 08:53:33PM GMT, Dmitry Baryshkov wrote:
> Add necessary glue code to be able to use new HDMI codec framework from
> the DRM bridge drivers. The drm_bridge implements a limited set of the
> hdmi_codec_ops interface, with the functions accepting both
> drm_connector and drm_bridge instead of just a generic void pointer.
>
> This framework is integrated with the DRM HDMI Connector framework, but
> can also be used for DisplayPort connectors.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> ---
> drivers/gpu/drm/drm_bridge_connector.c | 130 ++++++++++++++++++++++++++++++++-
> include/drm/drm_bridge.h | 46 ++++++++++++
> 2 files changed, 174 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c
> index 0869b663f17e..71d6fdc2391f 100644
> --- a/drivers/gpu/drm/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/drm_bridge_connector.c
> @@ -20,6 +20,8 @@
> #include <drm/drm_probe_helper.h>
> #include <drm/display/drm_hdmi_state_helper.h>
>
> +#include <sound/hdmi-codec.h>
> +
> /**
> * DOC: overview
> *
> @@ -95,6 +97,14 @@ struct drm_bridge_connector {
> * HDMI connector infrastructure, if any (see &DRM_BRIDGE_OP_HDMI).
> */
> struct drm_bridge *bridge_hdmi;
> + /**
> + * @bridge_hdmi_codec:
> + *
> + * The bridge in the chain that implements necessary support for the
> + * HDMI Audio Codec infrastructure, if any (see
> + * &DRM_BRIDGE_OP_HDMI_CODEC).
> + */
> + struct drm_bridge *bridge_hdmi_codec;
Can we have a setup where one bridge would support the video stream and
another one the audio?
I think for now I'd rather make them both provided by the same bridge,
and we can always change that later on if we need to.
> };
>
> #define to_drm_bridge_connector(x) \
> @@ -343,10 +353,104 @@ static int drm_bridge_connector_write_infoframe(struct drm_connector *connector,
> return bridge->funcs->hdmi_write_infoframe(bridge, type, buffer, len);
> }
>
> +static int drm_bridge_connector_audio_startup(struct device *dev, void *data)
> +{
> + struct drm_connector *connector = data;
> + struct drm_bridge_connector *bridge_connector =
> + to_drm_bridge_connector(connector);
> + struct drm_bridge *bridge;
> +
> + bridge = bridge_connector->bridge_hdmi_codec;
> + if (!bridge)
> + return -EINVAL;
> +
> + if (bridge->funcs->hdmi_codec_audio_startup)
> + return bridge->funcs->hdmi_codec_audio_startup(dev, connector, bridge);
> + else
> + return 0;
> +}
> +
> +static int drm_bridge_connector_prepare(struct device *dev, void *data,
> + struct hdmi_codec_daifmt *fmt,
> + struct hdmi_codec_params *hparms)
> +{
> + struct drm_connector *connector = data;
> + struct drm_bridge_connector *bridge_connector =
> + to_drm_bridge_connector(connector);
> + struct drm_bridge *bridge;
> +
> + bridge = bridge_connector->bridge_hdmi_codec;
> + if (!bridge)
> + return -EINVAL;
> +
> + return bridge->funcs->hdmi_codec_prepare(dev, connector, bridge, fmt, hparms);
> +}
> +
> +static void drm_bridge_connector_audio_shutdown(struct device *dev, void *data)
> +{
> + struct drm_connector *connector = data;
> + struct drm_bridge_connector *bridge_connector =
> + to_drm_bridge_connector(connector);
> + struct drm_bridge *bridge;
> +
> + bridge = bridge_connector->bridge_hdmi_codec;
> + if (!bridge)
> + return;
> +
> + bridge->funcs->hdmi_codec_audio_shutdown(dev, connector, bridge);
> +}
> +
> +static int drm_bridge_connector_mute_stream(struct device *dev, void *data,
> + bool enable, int direction)
> +{
> + struct drm_connector *connector = data;
> + struct drm_bridge_connector *bridge_connector =
> + to_drm_bridge_connector(connector);
> + struct drm_bridge *bridge;
> +
> + bridge = bridge_connector->bridge_hdmi_codec;
> + if (!bridge)
> + return -EINVAL;
> +
> + if (bridge->funcs->hdmi_codec_mute_stream)
> + return bridge->funcs->hdmi_codec_mute_stream(dev, connector, bridge,
> + enable, direction);
> + else
> + return -ENOTSUPP;
> +}
> +
> +static int drm_bridge_connector_get_dai_id(struct snd_soc_component *comment,
> + struct device_node *endpoint,
> + void *data)
> +{
> + struct drm_connector *connector = data;
> + struct drm_bridge_connector *bridge_connector =
> + to_drm_bridge_connector(connector);
> + struct drm_bridge *bridge;
> +
> + bridge = bridge_connector->bridge_hdmi_codec;
> + if (!bridge)
> + return -EINVAL;
> +
> + if (bridge->funcs->hdmi_codec_get_dai_id)
> + return bridge->funcs->hdmi_codec_get_dai_id(connector, bridge, endpoint);
> + else
> + return -ENOTSUPP;
> +}
> +
> +static const struct hdmi_codec_ops drm_bridge_connector_hdmi_codec_ops = {
> + .audio_startup = drm_bridge_connector_audio_startup,
> + .prepare = drm_bridge_connector_prepare,
> + .audio_shutdown = drm_bridge_connector_audio_shutdown,
> + .mute_stream = drm_bridge_connector_mute_stream,
> + .get_dai_id = drm_bridge_connector_get_dai_id,
> +};
> +
> static const struct drm_connector_hdmi_funcs drm_bridge_connector_hdmi_funcs = {
> .tmds_char_rate_valid = drm_bridge_connector_tmds_char_rate_valid,
> .clear_infoframe = drm_bridge_connector_clear_infoframe,
> .write_infoframe = drm_bridge_connector_write_infoframe,
> + .codec_ops = &drm_bridge_connector_hdmi_codec_ops,
> };
>
> /* -----------------------------------------------------------------------------
> @@ -427,6 +531,23 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> max_bpc = bridge->max_bpc;
> }
>
> + if (bridge->ops & DRM_BRIDGE_OP_HDMI_CODEC) {
> + if (bridge_connector->bridge_hdmi_codec)
> + return ERR_PTR(-EBUSY);
> + if (!bridge->funcs->hdmi_codec_prepare ||
> + !bridge->funcs->hdmi_codec_audio_shutdown)
> + return ERR_PTR(-EINVAL);
> +
> + bridge_connector->bridge_hdmi_codec = bridge;
> +
> + connector->hdmi_codec.parent_dev = bridge->parent;
> + connector->hdmi_codec.i2s = bridge->hdmi_codec_i2s;
> + connector->hdmi_codec.spdif = bridge->hdmi_codec_spdif;
> + connector->hdmi_codec.max_i2s_channels = bridge->max_i2s_channels;
> +
> + bridge->hdmi_codec = &connector->hdmi_codec;
> + }
> +
> if (!drm_bridge_get_next_bridge(bridge))
> connector_type = bridge->type;
>
> @@ -448,7 +569,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> return ERR_PTR(-EINVAL);
> }
>
> - if (bridge_connector->bridge_hdmi)
> + if (bridge_connector->bridge_hdmi) {
> ret = drmm_connector_hdmi_init(drm, connector,
> bridge_connector->bridge_hdmi->vendor,
> bridge_connector->bridge_hdmi->product,
> @@ -457,10 +578,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> connector_type, ddc,
> supported_formats,
> max_bpc);
> - else
> + } else {
> ret = drmm_connector_init(drm, connector,
> &drm_bridge_connector_funcs,
> connector_type, ddc);
> + if (!ret && bridge_connector->bridge_hdmi_codec) {
> + ret = drmm_connector_hdmi_codec_alloc(drm, connector,
> + &drm_bridge_connector_hdmi_codec_ops);
> + }
> + }
> if (ret) {
> kfree(bridge_connector);
> return ERR_PTR(ret);
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 75019d16be64..c4a95c489b00 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -41,6 +41,8 @@ struct drm_display_info;
> struct drm_minor;
> struct drm_panel;
> struct edid;
> +struct hdmi_codec_daifmt;
> +struct hdmi_codec_params;
> struct i2c_adapter;
>
> /**
> @@ -676,6 +678,29 @@ struct drm_bridge_funcs {
> enum hdmi_infoframe_type type,
> const u8 *buffer, size_t len);
>
> + int (*hdmi_codec_audio_startup)(struct device *dev,
> + struct drm_connector *connector,
> + struct drm_bridge *bridge);
> +
> + int (*hdmi_codec_prepare)(struct device *dev,
> + struct drm_connector *connector,
> + struct drm_bridge *bridge,
> + struct hdmi_codec_daifmt *fmt,
> + struct hdmi_codec_params *hparms);
> +
> + void (*hdmi_codec_audio_shutdown)(struct device *dev,
> + struct drm_connector *connector,
> + struct drm_bridge *bridge);
> +
> + int (*hdmi_codec_mute_stream)(struct device *dev,
> + struct drm_connector *connector,
> + struct drm_bridge *bridge,
> + bool enable, int direction);
> +
> + int (*hdmi_codec_get_dai_id)(struct drm_connector *connector,
> + struct drm_bridge *bridge,
> + struct device_node *endpoint);
> +
> /**
> * @debugfs_init:
> *
> @@ -761,6 +786,20 @@ enum drm_bridge_ops {
> * drivers.
> */
> DRM_BRIDGE_OP_HDMI = BIT(4),
> + /**
> + * @DRM_BRIDGE_OP_HDMI_CODEC: The bridge provides HDMI Audio Codec
> + * operations. Bridges that set this flag must implement the
> + * &drm_bridge_funcs->hdmi_codec_prepare and
> + * &drm_bridge_funcs->hdmi_codec_audio_shutdown callbacks and set the
> + * relevant field in the &drm_bridge structure.
> + *
> + * This framework can be used by both HDMI and DisplayPort bridges.
> + *
> + * Note: currently there can be at most one bridge in a chain that sets
> + * this bit. This is to simplify corresponding glue code in connector
> + * drivers.
> + */
> + DRM_BRIDGE_OP_HDMI_CODEC = BIT(5),
I think I'd go and make it one step further and make HDMI_CODEC imply HDMI.
Maxime
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists