[PATCH v1 0/2] ASoC/tda998x: Fix reporting of nonexistent capture streams

[PATCH v1 0/2] ASoC/tda998x: Fix reporting of nonexistent capture streams

[Mark Brown]

The recently added pcm-test selftest has pointed out that systems with
the tda998x driver end up advertising that they support capture when in
reality as far as I can see the tda998x devices are transmit only.  The
DAIs registered through hdmi-codec are bidirectional, meaning that for
I2S systems when combined with a typical bidrectional CPU DAI the
overall capability of the PCM is bidirectional.  In most cases the I2S
links will clock OK but no useful audio will be returned which isn't so
bad but we should still not advertise the useless capability, and some
systems may notice problems for example due to pinmux management.

This is happening due to the hdmi-codec helpers not providing any
mechanism for indicating unidirectional audio so add one and use it in
the tda998x driver.  It is likely other hdmi-codec users are also
affected but I don't have those systems to hand.

Mark Brown (2):
  ASoC: hdmi-codec: Allow playback and capture to be disabled
  drm: tda99x: Don't advertise non-existent capture support

 drivers/gpu/drm/i2c/tda998x_drv.c |  2 ++
 include/sound/hdmi-codec.h        |  4 ++++
 sound/soc/codecs/hdmi-codec.c     | 30 +++++++++++++++++++++++++-----
 3 files changed, 31 insertions(+), 5 deletions(-)


base-commit: f0c4d9fc9cc9462659728d168387191387e903cc
-- 
2.30.2

[PATCH v1 1/2] ASoC: hdmi-codec: Allow playback and capture to be disabled

[Mark Brown]

Currently the hdmi-codec driver always registers both playback and capture
capabilities but for most systems there's no actual capture capability,
usually HDMI is transmit only. Provide platform data which allows the users
to indicate what is supported so that we don't end up advertising things
to userspace that we can't actually support.

In order to avoid breaking existing users the flags in platform data are
a bit awkward and specify what should be disabled rather than doing the
perhaps more expected thing and defaulting to not supporting capture.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/sound/hdmi-codec.h    |  4 ++++
 sound/soc/codecs/hdmi-codec.c | 30 +++++++++++++++++++++++++-----
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index 48ad33aba393..9b162ac1e08e 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -124,7 +124,11 @@ struct hdmi_codec_ops {
 struct hdmi_codec_pdata {
 	const struct hdmi_codec_ops *ops;
 	uint i2s:1;
+	uint no_i2s_playback:1;
+	uint no_i2s_capture:1;
 	uint spdif:1;
+	uint no_spdif_playback:1;
+	uint no_spdif_capture:1;
 	int max_i2s_channels;
 	void *data;
 };
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 0b1cdb2d6049..74cbbe16f9ae 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -816,12 +816,19 @@ static int hdmi_dai_probe(struct snd_soc_dai *dai)
 			.source = "RX",
 		},
 	};
-	int ret;
+	int ret, i;
 
 	dapm = snd_soc_component_get_dapm(dai->component);
-	ret = snd_soc_dapm_add_routes(dapm, route, 2);
-	if (ret)
-		return ret;
+
+	/* One of the directions might be omitted for unidirectional DAIs */
+	for (i = 0; i < ARRAY_SIZE(route); i++) {
+		if (!route[i].source || !route[i].sink)
+			continue;
+
+		ret = snd_soc_dapm_add_routes(dapm, &route[i], 1);
+		if (ret)
+			return ret;
+	}
 
 	daifmt = devm_kzalloc(dai->dev, sizeof(*daifmt), GFP_KERNEL);
 	if (!daifmt)
@@ -1009,11 +1016,24 @@ static int hdmi_codec_probe(struct platform_device *pdev)
 	if (hcd->i2s) {
 		daidrv[i] = hdmi_i2s_dai;
 		daidrv[i].playback.channels_max = hcd->max_i2s_channels;
+		if (hcd->no_i2s_playback)
+			memset(&daidrv[i].playback, 0,
+			       sizeof(daidrv[i].playback));
+		if (hcd->no_i2s_capture)
+			memset(&daidrv[i].capture, 0,
+			       sizeof(daidrv[i].capture));
 		i++;
 	}
 
-	if (hcd->spdif)
+	if (hcd->spdif) {
 		daidrv[i] = hdmi_spdif_dai;
+		if (hcd->no_spdif_playback)
+			memset(&daidrv[i].playback, 0,
+			       sizeof(daidrv[i].playback));
+		if (hcd->no_spdif_capture)
+			memset(&daidrv[i].capture, 0,
+			       sizeof(daidrv[i].capture));
+	}
 
 	dev_set_drvdata(dev, hcp);
 
-- 
2.30.2

[PATCH v1 2/2] drm: tda99x: Don't advertise non-existent capture support

[Mark Brown]

As far as I can tell none of the tda998x devices support audio capture so
don't advertise support for it, ensuring that we don't confuse userspace.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index d444e7fffb54..a14d2896aebb 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1174,6 +1174,8 @@ static int tda998x_audio_codec_init(struct tda998x_priv *priv,
 	struct hdmi_codec_pdata codec_data = {
 		.ops = &audio_codec_ops,
 		.max_i2s_channels = 2,
+		.no_i2s_capture = 1,
+		.no_spdif_capture = 1,
 	};
 
 	if (priv->audio_port_enable[AUDIO_ROUTE_I2S])
-- 
2.30.2

Re: [PATCH v1 0/2] ASoC/tda998x: Fix reporting of nonexistent capture streams

[Russell King (Oracle)]

On Wed, Nov 30, 2022 at 06:46:42PM +0000, Mark Brown wrote:
> The recently added pcm-test selftest has pointed out that systems with
> the tda998x driver end up advertising that they support capture when in
> reality as far as I can see the tda998x devices are transmit only.  The
> DAIs registered through hdmi-codec are bidirectional, meaning that for
> I2S systems when combined with a typical bidrectional CPU DAI the
> overall capability of the PCM is bidirectional.  In most cases the I2S
> links will clock OK but no useful audio will be returned which isn't so
> bad but we should still not advertise the useless capability, and some
> systems may notice problems for example due to pinmux management.
> 
> This is happening due to the hdmi-codec helpers not providing any
> mechanism for indicating unidirectional audio so add one and use it in
> the tda998x driver.  It is likely other hdmi-codec users are also
> affected but I don't have those systems to hand.
> 
> Mark Brown (2):
>   ASoC: hdmi-codec: Allow playback and capture to be disabled
>   drm: tda99x: Don't advertise non-existent capture support
> 
>  drivers/gpu/drm/i2c/tda998x_drv.c |  2 ++
>  include/sound/hdmi-codec.h        |  4 ++++
>  sound/soc/codecs/hdmi-codec.c     | 30 +++++++++++++++++++++++++-----
>  3 files changed, 31 insertions(+), 5 deletions(-)

Looks sane.

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v1 0/2] ASoC/tda998x: Fix reporting of nonexistent capture streams

[Russell King (Oracle)]

On Wed, Nov 30, 2022 at 06:46:42PM +0000, Mark Brown wrote:
> The recently added pcm-test selftest has pointed out that systems with
> the tda998x driver end up advertising that they support capture when in
> reality as far as I can see the tda998x devices are transmit only.  The
> DAIs registered through hdmi-codec are bidirectional, meaning that for
> I2S systems when combined with a typical bidrectional CPU DAI the
> overall capability of the PCM is bidirectional.  In most cases the I2S
> links will clock OK but no useful audio will be returned which isn't so
> bad but we should still not advertise the useless capability, and some
> systems may notice problems for example due to pinmux management.
> 
> This is happening due to the hdmi-codec helpers not providing any
> mechanism for indicating unidirectional audio so add one and use it in
> the tda998x driver.  It is likely other hdmi-codec users are also
> affected but I don't have those systems to hand.
> 
> Mark Brown (2):
>   ASoC: hdmi-codec: Allow playback and capture to be disabled
>   drm: tda99x: Don't advertise non-existent capture support
> 
>  drivers/gpu/drm/i2c/tda998x_drv.c |  2 ++
>  include/sound/hdmi-codec.h        |  4 ++++
>  sound/soc/codecs/hdmi-codec.c     | 30 +++++++++++++++++++++++++-----
>  3 files changed, 31 insertions(+), 5 deletions(-)

Looks sane.

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v1 0/2] ASoC/tda998x: Fix reporting of nonexistent capture streams

[Mark Brown]

On Wed, 30 Nov 2022 18:46:42 +0000, Mark Brown wrote:
> The recently added pcm-test selftest has pointed out that systems with
> the tda998x driver end up advertising that they support capture when in
> reality as far as I can see the tda998x devices are transmit only.  The
> DAIs registered through hdmi-codec are bidirectional, meaning that for
> I2S systems when combined with a typical bidrectional CPU DAI the
> overall capability of the PCM is bidirectional.  In most cases the I2S
> links will clock OK but no useful audio will be returned which isn't so
> bad but we should still not advertise the useless capability, and some
> systems may notice problems for example due to pinmux management.
> 
> [...]

Applied to

   broonie/sound.git for-next

Thanks!

[1/2] ASoC: hdmi-codec: Allow playback and capture to be disabled
      commit: f77a066f4ed307db93aafee621e2683c3bda98ce
[2/2] drm: tda99x: Don't advertise non-existent capture support
      commit: a04f1c81316d27e140c3df5561e5ef87794cd4bc

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark