[PATCH] drm/bridge: ite-it66121: Convert to DRM HDMI Audio Helper

Sen Wang posted 1 patch 2 weeks, 6 days ago
There is a newer version of this series
drivers/gpu/drm/bridge/ite-it66121.c | 135 +++++++++------------------
1 file changed, 45 insertions(+), 90 deletions(-)
[PATCH] drm/bridge: ite-it66121: Convert to DRM HDMI Audio Helper
Posted by Sen Wang 2 weeks, 6 days ago
Convert the IT66121 HDMI bridge driver from manually registering an
hdmi-codec platform device to using the DRM HDMI Audio Helper framework
via DRM_BRIDGE_OP_HDMI_AUDIO instead.

The previous implementation manually allocated hdmi_codec_pdata,
registered the platform device, and implemented hdmi_codec_ops callbacks
including get_eld. The new approach sets DRM_BRIDGE_OP_HDMI_AUDIO on the
bridge, letting the framework handle the codec registration. This also
resolves some non-compliance issues with the current audio implementation,
such as HDMI audio advertising a non-functional capture stream to userspace.

The audio callbacks are converted from hdmi_codec_ops signatures to
drm_bridge_funcs hdmi_audio callbacks:
  - it66121_audio_hw_params   -> it66121_hdmi_audio_prepare
  - it66121_audio_startup     -> it66121_hdmi_audio_startup
  - it66121_audio_shutdown    -> it66121_hdmi_audio_shutdown
  - it66121_audio_mute        -> it66121_hdmi_audio_mute_stream

The it66121_audio_get_eld, it66121_audio_codec_ops, and
it66121_audio_codec_init functions are removed as the framework handles
these responsibilities.

Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Sen Wang <sen@ti.com>
---
Tested in beagleplay ensuring HDMI/ HDMI audio works correctly with
the new implementation, including proper ELD data and audio.

 drivers/gpu/drm/bridge/ite-it66121.c | 135 +++++++++------------------
 1 file changed, 45 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 9246e9c15a6e..6ac90315b822 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -306,7 +306,6 @@ struct it66121_ctx {
 	struct mutex lock; /* Protects fields below and device registers */
 	struct hdmi_avi_infoframe hdmi_avi_infoframe;
 	struct {
-		struct platform_device *pdev;
 		u8 ch_enable;
 		u8 fs;
 		u8 swl;
@@ -902,24 +901,6 @@ static const struct drm_edid *it66121_bridge_edid_read(struct drm_bridge *bridge
 	return drm_edid;
 }
 
-static const struct drm_bridge_funcs it66121_bridge_funcs = {
-	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
-	.atomic_reset = drm_atomic_helper_bridge_reset,
-	.attach = it66121_bridge_attach,
-	.atomic_get_output_bus_fmts = it66121_bridge_atomic_get_output_bus_fmts,
-	.atomic_get_input_bus_fmts = it66121_bridge_atomic_get_input_bus_fmts,
-	.atomic_enable = it66121_bridge_enable,
-	.atomic_disable = it66121_bridge_disable,
-	.atomic_check = it66121_bridge_check,
-	.mode_set = it66121_bridge_mode_set,
-	.mode_valid = it66121_bridge_mode_valid,
-	.detect = it66121_bridge_detect,
-	.edid_read = it66121_bridge_edid_read,
-	.hpd_enable = it66121_bridge_hpd_enable,
-	.hpd_disable = it66121_bridge_hpd_disable,
-};
-
 static irqreturn_t it66121_irq_threaded_handler(int irq, void *dev_id)
 {
 	int ret;
@@ -1225,14 +1206,16 @@ static int it661221_audio_ch_enable(struct it66121_ctx *ctx, bool enable)
 	return ret;
 }
 
-static int it66121_audio_hw_params(struct device *dev, void *data,
-				   struct hdmi_codec_daifmt *daifmt,
-				   struct hdmi_codec_params *params)
+static int it66121_hdmi_audio_prepare(struct drm_bridge *bridge,
+				      struct drm_connector *connector,
+				      struct hdmi_codec_daifmt *daifmt,
+				      struct hdmi_codec_params *params)
 {
 	u8 fs;
 	u8 swl;
 	int ret;
-	struct it66121_ctx *ctx = dev_get_drvdata(dev);
+	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
+	struct device *dev = ctx->dev;
 	static u8 iec60958_chstat[5];
 	unsigned int channels = params->channels;
 	unsigned int sample_rate = params->sample_rate;
@@ -1379,41 +1362,44 @@ static int it66121_audio_hw_params(struct device *dev, void *data,
 	return ret;
 }
 
-static int it66121_audio_startup(struct device *dev, void *data)
+static int it66121_hdmi_audio_startup(struct drm_bridge *bridge,
+				      struct drm_connector *connector)
 {
 	int ret;
-	struct it66121_ctx *ctx = dev_get_drvdata(dev);
+	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
 
 	mutex_lock(&ctx->lock);
 	ret = it661221_audio_output_enable(ctx, true);
 	if (ret)
-		dev_err(dev, "Failed to enable audio output: %d\n", ret);
+		dev_err(ctx->dev, "Failed to enable audio output: %d\n", ret);
 
 	mutex_unlock(&ctx->lock);
 
 	return ret;
 }
 
-static void it66121_audio_shutdown(struct device *dev, void *data)
+static void it66121_hdmi_audio_shutdown(struct drm_bridge *bridge,
+					struct drm_connector *connector)
 {
 	int ret;
-	struct it66121_ctx *ctx = dev_get_drvdata(dev);
+	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
 
 	mutex_lock(&ctx->lock);
 	ret = it661221_audio_output_enable(ctx, false);
 	if (ret)
-		dev_err(dev, "Failed to disable audio output: %d\n", ret);
+		dev_err(ctx->dev, "Failed to disable audio output: %d\n", ret);
 
 	mutex_unlock(&ctx->lock);
 }
 
-static int it66121_audio_mute(struct device *dev, void *data,
-			      bool enable, int direction)
+static int it66121_hdmi_audio_mute_stream(struct drm_bridge *bridge,
+					  struct drm_connector *connector,
+					  bool enable, int direction)
 {
 	int ret;
-	struct it66121_ctx *ctx = dev_get_drvdata(dev);
+	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
 
-	dev_dbg(dev, "%s: enable=%s, direction=%d\n",
+	dev_dbg(ctx->dev, "%s: enable=%s, direction=%d\n",
 		__func__, enable ? "true" : "false", direction);
 
 	mutex_lock(&ctx->lock);
@@ -1436,64 +1422,28 @@ static int it66121_audio_mute(struct device *dev, void *data,
 	return ret;
 }
 
-static int it66121_audio_get_eld(struct device *dev, void *data,
-				 u8 *buf, size_t len)
-{
-	struct it66121_ctx *ctx = dev_get_drvdata(dev);
-
-	mutex_lock(&ctx->lock);
-	if (!ctx->connector) {
-		/* Pass en empty ELD if connector not available */
-		dev_dbg(dev, "No connector present, passing empty EDID data");
-		memset(buf, 0, len);
-	} else {
-		mutex_lock(&ctx->connector->eld_mutex);
-		memcpy(buf, ctx->connector->eld,
-		       min(sizeof(ctx->connector->eld), len));
-		mutex_unlock(&ctx->connector->eld_mutex);
-	}
-	mutex_unlock(&ctx->lock);
-
-	return 0;
-}
-
-static const struct hdmi_codec_ops it66121_audio_codec_ops = {
-	.hw_params = it66121_audio_hw_params,
-	.audio_startup = it66121_audio_startup,
-	.audio_shutdown = it66121_audio_shutdown,
-	.mute_stream = it66121_audio_mute,
-	.get_eld = it66121_audio_get_eld,
+static const struct drm_bridge_funcs it66121_bridge_funcs = {
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
+	.attach = it66121_bridge_attach,
+	.atomic_get_output_bus_fmts = it66121_bridge_atomic_get_output_bus_fmts,
+	.atomic_get_input_bus_fmts = it66121_bridge_atomic_get_input_bus_fmts,
+	.atomic_enable = it66121_bridge_enable,
+	.atomic_disable = it66121_bridge_disable,
+	.atomic_check = it66121_bridge_check,
+	.mode_set = it66121_bridge_mode_set,
+	.mode_valid = it66121_bridge_mode_valid,
+	.detect = it66121_bridge_detect,
+	.edid_read = it66121_bridge_edid_read,
+	.hpd_enable = it66121_bridge_hpd_enable,
+	.hpd_disable = it66121_bridge_hpd_disable,
+	.hdmi_audio_startup = it66121_hdmi_audio_startup,
+	.hdmi_audio_prepare = it66121_hdmi_audio_prepare,
+	.hdmi_audio_shutdown = it66121_hdmi_audio_shutdown,
+	.hdmi_audio_mute_stream = it66121_hdmi_audio_mute_stream,
 };
 
-static int it66121_audio_codec_init(struct it66121_ctx *ctx, struct device *dev)
-{
-	struct hdmi_codec_pdata codec_data = {
-		.ops = &it66121_audio_codec_ops,
-		.i2s = 1, /* Only i2s support for now */
-		.spdif = 0,
-		.max_i2s_channels = 8,
-		.no_capture_mute = 1,
-	};
-
-	if (!of_property_present(dev->of_node, "#sound-dai-cells")) {
-		dev_info(dev, "No \"#sound-dai-cells\", no audio\n");
-		return 0;
-	}
-
-	ctx->audio.pdev = platform_device_register_data(dev,
-							HDMI_CODEC_DRV_NAME,
-							PLATFORM_DEVID_AUTO,
-							&codec_data,
-							sizeof(codec_data));
-
-	if (IS_ERR(ctx->audio.pdev)) {
-		dev_err(dev, "Failed to initialize HDMI audio codec: %d\n",
-			PTR_ERR_OR_ZERO(ctx->audio.pdev));
-	}
-
-	return PTR_ERR_OR_ZERO(ctx->audio.pdev);
-}
-
 static const char * const it66121_supplies[] = {
 	"vcn33", "vcn18", "vrf12"
 };
@@ -1602,7 +1552,12 @@ static int it66121_probe(struct i2c_client *client)
 		}
 	}
 
-	it66121_audio_codec_init(ctx, dev);
+	if (of_property_present(dev->of_node, "#sound-dai-cells")) {
+		ctx->bridge.ops |= DRM_BRIDGE_OP_HDMI_AUDIO;
+		ctx->bridge.hdmi_audio_dev = dev;
+		ctx->bridge.hdmi_audio_max_i2s_playback_channels = 8;
+		ctx->bridge.hdmi_audio_dai_port = -1;
+	}
 
 	drm_bridge_add(&ctx->bridge);
 
-- 
2.43.0