[PATCH v5 4/6] drm: bridge/dw_hdmi-i2s-audio: add audio driver

[Mark Brown]

On Sat, Aug 08, 2015 at 04:35:41PM +0100, Russell King - ARM Linux wrote:
> On Sat, Jun 20, 2015 at 12:28:15AM +0800, Yakir Yang wrote:
> > Add ALSA based HDMI I2S audio driver for dw_hdmi. Sound card
> > driver could connect to this codec through the codec dai name
> > "dw-hdmi-i2s-audio".

> I'm applying this patch to my tree with some changes.  I haven't seen
> anyone ack it, nor review it any further.  I'm hoping someone can ack
> this patch with the following changes below - I'll be posting it as
> part of my dw-hdmi devel updates later today.

I'm pretty sure I deleted this patch unread because what shows up in my
inbox is:

->  197   C 08/08 Russell King -  (1.1K) │ └─>Re: [PATCH v5 4/6] drm: 
bridge/dw_

so there's no indication that it's anything to do with audio :(
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: 

[PATCH v5 4/6] drm: bridge/dw_hdmi-i2s-audio: add audio driver

[Yakir Yang]

Hi Russell,

在 2015/8/8 23:35, Russell King - ARM Linux 写道:
> On Sat, Jun 20, 2015 at 12:28:15AM +0800, Yakir Yang wrote:
>> Add ALSA based HDMI I2S audio driver for dw_hdmi. Sound card
>> driver could connect to this codec through the codec dai name
>> "dw-hdmi-i2s-audio".
> I'm applying this patch to my tree with some changes.  I haven't seen
> anyone ack it, nor review it any further.  I'm hoping someone can ack
> this patch with the following changes below - I'll be posting it as
> part of my dw-hdmi devel updates later today.
Aha, wow !!!
I'm so happy now :-D, many thanks for your improved and applay.

- Yakir

>> +ret = request_irq(dw->data.irq, snd_dw_hdmi_irq, IRQF_SHARED,
>> +  "dw-dw-audio", dw);
> DRIVER_NAME, dw);
>
>> +MODULE_AUTHOR("Yakir Yang ");
>> +MODULE_DESCRIPTION("DW dw Audio ASoC Interface");
> Fixing this to be "Synopsis DesignWare HDMI I2S ASoC interface".
>
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS(PLATFORM_MODULE_PREFIX DRIVER_NAME);
> Replacing PLATFORM_MODULE_PREFIX with the "platform:" string.
>
>> +if (hdmi_readb(hdmi, HDMI_CONFIG1_ID) & HDMI_CONFIG1_AHB) {
>>  pdevinfo.name = "dw-hdmi-ahb-audio";
> Fixing this to be "dw-hdmi-i2s-audio".
>

-- next part --
An HTML attachment was scrubbed...
URL: 

[PATCH v5 4/6] drm: bridge/dw_hdmi-i2s-audio: add audio driver

[Russell King - ARM Linux]

On Sat, Jun 20, 2015 at 12:28:15AM +0800, Yakir Yang wrote:
> Add ALSA based HDMI I2S audio driver for dw_hdmi. Sound card
> driver could connect to this codec through the codec dai name
> "dw-hdmi-i2s-audio".

I'm applying this patch to my tree with some changes.  I haven't seen
anyone ack it, nor review it any further.  I'm hoping someone can ack
this patch with the following changes below - I'll be posting it as
part of my dw-hdmi devel updates later today.

> + ret = request_irq(dw->data.irq, snd_dw_hdmi_irq, IRQF_SHARED,
> +   "dw-dw-audio", dw);

  DRIVER_NAME, dw);

> +MODULE_AUTHOR("Yakir Yang ");
> +MODULE_DESCRIPTION("DW dw Audio ASoC Interface");

Fixing this to be "Synopsis DesignWare HDMI I2S ASoC interface".

> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS(PLATFORM_MODULE_PREFIX DRIVER_NAME);

Replacing PLATFORM_MODULE_PREFIX with the "platform:" string.

> + if (hdmi_readb(hdmi, HDMI_CONFIG1_ID) & HDMI_CONFIG1_AHB) {
>   pdevinfo.name = "dw-hdmi-ahb-audio";

Fixing this to be "dw-hdmi-i2s-audio".

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH v5 4/6] drm: bridge/dw_hdmi-i2s-audio: add audio driver

[Yakir Yang]

Russell,

在 2015/6/22 18:10, Russell King - ARM Linux 写道:
> On Mon, Jun 22, 2015 at 12:06:04PM +0200, Paul Bolle wrote:
>> But I think there's no platform_device with a "dw-hdmi-i2s-audio" name.
>> So I wonder whether this MODULE_ALIAS() is actually needed. What breaks
>> if you leave it out?
> +   } else if (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S) {
> +   pdevinfo.name = "dw-hdmi-ahb-audio";
>
> This should be "dw-hdmi-i2s-audio" to avoid picking up on my AHB audio
> driver.
Yep, thanks a lot ;)

- Yakir

[PATCH v5 4/6] drm: bridge/dw_hdmi-i2s-audio: add audio driver

[Yakir Yang]

Paul,

在 2015/6/22 18:06, Paul Bolle 写道:
> Something I didn't notice in v4, sorry.
>
> On Sat, 2015-06-20 at 00:28 +0800, Yakir Yang wrote:
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c
>> +#define DRIVER_NAME "dw-hdmi-i2s-audio"
>> +MODULE_ALIAS(PLATFORM_MODULE_PREFIX DRIVER_NAME);
> 0) Side note: this is the first time that PLATFORM_MODULE_PREFIX is used
> inside MODULE_ALIAS(). But none of the 1000+ other "platform:" aliases
> do that. And neither does 5/6 of this series! That suggests, I think,
> that this shouldn't be done.
>
> You could consider adding something like
>  #define MODULE_ALIAS_PLATFORM(NAME) MODULE_ALIAS(PLATFORM_MODULE_PREFIX 
> NAME)
>
> But then, I think, all the current 1000+ platform: aliases should be
> converted to that macro. Would that be worth it?
Yeah, It would be horrible to coverted to that macro  ;)

> 1) Now on to my remark: this alias seems to be only useful if there also
> is a struct platform_device with a "dw-hdmi-i2s-audio" name. Because
> that platform_device would, badly summarized, fire of a
> "MODALIAS=platform:dw-hdmi-i2s-audio" uevent when created. Which, in its
> turn, would trigger userspace to load this module, correct?
>
> But I think there's no platform_device with a "dw-hdmi-i2s-audio" name.
> So I wonder whether this MODULE_ALIAS() is actually needed. What breaks
> if you leave it out?
Thanks for your attentive comment .  Actually there should be an 
platform device that named "dw-hdmi-i2s-audio" which I misspelt in 
dw_hdmi.c driver.

thanks,
- Yakir

> (Likewise for 5/6, but there the platform_device should have a
> "rockchip-hdmi-audio" name.)
>
> Thanks,
>
>
> Paul Bolle
>
>
>
>

[PATCH v5 4/6] drm: bridge/dw_hdmi-i2s-audio: add audio driver

[Paul Bolle]

Something I didn't notice in v4, sorry.

On Sat, 2015-06-20 at 00:28 +0800, Yakir Yang wrote:
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c

> +#define DRIVER_NAME "dw-hdmi-i2s-audio"

> +MODULE_ALIAS(PLATFORM_MODULE_PREFIX DRIVER_NAME);

0) Side note: this is the first time that PLATFORM_MODULE_PREFIX is used
inside MODULE_ALIAS(). But none of the 1000+ other "platform:" aliases
do that. And neither does 5/6 of this series! That suggests, I think,
that this shouldn't be done.

You could consider adding something like
#define MODULE_ALIAS_PLATFORM(NAME) MODULE_ALIAS(PLATFORM_MODULE_PREFIX 
NAME)

But then, I think, all the current 1000+ platform: aliases should be
converted to that macro. Would that be worth it?

1) Now on to my remark: this alias seems to be only useful if there also
is a struct platform_device with a "dw-hdmi-i2s-audio" name. Because
that platform_device would, badly summarized, fire of a
"MODALIAS=platform:dw-hdmi-i2s-audio" uevent when created. Which, in its
turn, would trigger userspace to load this module, correct?

But I think there's no platform_device with a "dw-hdmi-i2s-audio" name.
So I wonder whether this MODULE_ALIAS() is actually needed. What breaks
if you leave it out?

(Likewise for 5/6, but there the platform_device should have a
"rockchip-hdmi-audio" name.)

Thanks,


Paul Bolle

[PATCH v5 4/6] drm: bridge/dw_hdmi-i2s-audio: add audio driver

[Russell King - ARM Linux]

On Mon, Jun 22, 2015 at 12:06:04PM +0200, Paul Bolle wrote:
> But I think there's no platform_device with a "dw-hdmi-i2s-audio" name.
> So I wonder whether this MODULE_ALIAS() is actually needed. What breaks
> if you leave it out?

+   } else if (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S) {
+   pdevinfo.name = "dw-hdmi-ahb-audio";

This should be "dw-hdmi-i2s-audio" to avoid picking up on my AHB audio
driver.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH v5 4/6] drm: bridge/dw_hdmi-i2s-audio: add audio driver

[Yakir Yang]

Add ALSA based HDMI I2S audio driver for dw_hdmi. Sound card
driver could connect to this codec through the codec dai name
"dw-hdmi-i2s-audio".

Signed-off-by: Yakir Yang 
---
Changes in v5:
- Take Mark Brown suggest that remove jack_status recorded, report
  jack status directly when hdmi plug happend, and remove devm_kfree
  and devm_free_irq.
- Correct the MODULE_LICENSE to "GPL v2"
- Move source from sound/soc/codecs to drivers/gpu/drm/bridge/
- Rename dai driver name to "dw-hdmi-i2s-hifi"

Changes in v4:
- Replace delaywork with irq thread, and add suspend/resume interfaces,
  replace "dw-hdmi-audio" with consecutive strings.

Changes in v3:
- Keep audio format config function in dw-hdmi-audio driver
  and remove audio_config & get_connect_status functions,
  move jack control to dw-hdmi-audio completely.

Changes in v2:
- Update dw_hdmi audio control interfaces, and adjust jack report process

 drivers/gpu/drm/bridge/Kconfig |   9 +
 drivers/gpu/drm/bridge/Makefile|   1 +
 drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c | 398 +
 drivers/gpu/drm/bridge/dw_hdmi.c   |  24 +-
 drivers/gpu/drm/bridge/dw_hdmi.h   |   3 +
 5 files changed, 426 insertions(+), 9 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 204861b..59e3f24 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -14,6 +14,15 @@ config DRM_DW_HDMI_AHB_AUDIO
  Designware HDMI block.  This is used in conjunction with
  the i.MX6 HDMI driver.

+config DRM_DW_HDMI_I2S_AUDIO
+   tristate "Synopsis Designware I2S Audio interface"
+   depends on DRM_DW_HDMI && SND
+   select SND_PCM
+   help
+ Support the I2S Audio interface which is part of the Synopsis
+ Designware HDMI block.  This is used in conjunction with the
+ RK3288 HDMI driver.
+
 config DRM_PTN3460
tristate "PTN3460 DP/LVDS bridge"
depends on DRM
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index eb80dbb..65a1239 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_PS8622) += ps8622.o
 obj-$(CONFIG_DRM_PTN3460) += ptn3460.o
 obj-$(CONFIG_DRM_DW_HDMI) += dw_hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw_hdmi-ahb-audio.o
+obj-$(CONFIG_DRM_DW_HDMI_I2S_AUDIO) += dw_hdmi-i2s-audio.o
diff --git a/drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c 
b/drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c
new file mode 100644
index 000..341ab97
--- /dev/null
+++ b/drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c
@@ -0,0 +1,398 @@
+/*
+ * DesignWare HDMI audio driver
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Written and tested against the Designware HDMI Tx found in RK3288.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "dw_hdmi-audio.h"
+
+#define DRIVER_NAME "dw-hdmi-i2s-audio"
+
+enum {
+   AUDIO_CONF1_DATWIDTH_MSK = 0x1F,
+   AUDIO_CONF1_DATAMODE_MSK = 0xE0,
+   AUDIO_DAIFMT_IIS = 0x0,
+   AUDIO_DAIFMT_RIGHT_J = 0x20,
+   AUDIO_DAIFMT_LEFT_J = 0x40,
+   AUDIO_DAIFMT_BURST_1 = 0x60,
+   AUDIO_DAIFMT_BURST_2 = 0x80,
+   AUDIO_CONF0_INTERFACE_MSK = BIT(5),
+   AUDIO_INPUTTYPE_IIS = 0x20,
+   AUDIO_INPUTTYPE_SPDIF = 0x00,
+   AUDIO_CONF0_I2SINEN_MSK = 0x0F,
+   AUDIO_CHANNELNUM_2 = 0x01,
+   AUDIO_CHANNELNUM_4 = 0x03,
+   AUDIO_CHANNELNUM_6 = 0x07,
+   AUDIO_CHANNELNUM_8 = 0x0F,
+   HDMI_PHY_HPD = BIT(1),
+   HDMI_PHY_STAT0 = 0x3004,
+   HDMI_AUD_CONF0 = 0x3100,
+   HDMI_AUD_CONF1 = 0x3101,
+   HDMI_AUD_INPUTCLKFS = 0x3206,
+};
+
+struct dw_audio_fmt {
+   int input_type;
+   int chan_num;
+   int sample_rate;
+   int word_length;
+   int dai_fmt;
+};
+
+struct snd_dw_hdmi {
+   struct device *dev;
+   struct dw_hdmi_audio_data data;
+
+   bool is_jack_ready;
+   struct snd_soc_jack jack;
+
+   bool is_playback_status;
+   struct dw_audio_fmt fmt;
+};
+
+static void hdmi_writel(struct snd_dw_hdmi *dw, u8 val, int offset)
+{
+   writel(val, dw->data.base + (offset << 2));
+}
+
+static u8 hdmi_readl(struct snd_dw_hdmi *dw, int offset)
+{
+   return readl(dw->data.base + (offset << 2));
+}
+
+static void hdmi_modl(struct snd_dw_hdmi *dw, u8 data,
+ u8 mask, unsigned reg)
+{
+   u8 val = hdmi_readl(dw, reg) & ~mask;
+
+   val |= data & mask;
+   hdmi_writel(dw, val, reg);
+}
+
+int snd_dw_hdmi_jack_detect(struct snd_dw_hdmi *dw)
+{
+   u8 jack_status;
+
+   if (!dw->is_jack_ready)
+   return -EINVAL;
+