[PATCH v5 0/6] Add dw_hdmi i2s audio support

[PATCH v5 0/6] Add dw_hdmi i2s audio support

[Yakir Yang]

As Russell have send an series patches for dw_hdmi audio, and most important
it works well on rockchip platform. Cause some patches have been achieve in
Russell's series, so I abondon some patches in this verison.

Here are the abondon patches that already be achieved:
* drm: bridge/dw_hdmi: adjust n/cts setting order
* drm: bridge/dw_hdmi: set ncts_atomic_write & cts_manual
* drm: bridge/dw_hdmi: combine hdmi_set_clock_regenerator_n() and hdmi_regenerate_cts()
* drm: bridge/dw_hdmi: add enable/disable to dw_hdmi_audio callbacks

And I also abondon some interrupt patches that is not a common bug in dw_hdmi
driver, but is a rockchip platform bug which HDMI registers would reset by CRU
module after system suspend/resume (in this case HPD interrupt mask would reset,
then driver can't respond hot plug event after system resume).

Here are the abondon patches that used to "fix" dw_hdmi interrupt bug:
* drm: bridge/dw_hdmi: add irq control to suspend/resume
* drm: bridge/dw_hdmi: wrap irq control in fucntions
* drm: rockchip/dw_hdmi_rockchip: add resume/suspend support

Here are the leave list that I have make in my series:
- add n/cts combinations for more display modes
- enable audio support when sink device is HDMI monitor and has audio
- rename dw_hdmi-ahb-audio.h to dw_hdmi-audio.h
- add dw_hdmi i2s audio driver
- add rockchip_hdmi_audio sound card driver
- add dt-bindings for documentation for rockchip_hdmi_audio driver


Changes in v5:
- make more words in commit message.
- Take Russell suggest that create "sink_has_audio" and "sink_is_hdmi" in
  struct hdmi, and keep vmode's mdvi changeless.
- Config fc_invidconf to HDMI mode when sink device is HDMI monitor.
- 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"
- Take Mark Brown suggest that remove the no useful code in probe func,
  and remove the snd_soc_dai_set_fmt() snd_soc_unregister_card(), remove
  those noisy dev_info()
- Rename codec_dai_name to "dw-hdmi-i2s-hifi"

Changes in v4:
- Take Dainel suggest that introduce drm_detect_monitor_audio() to judge
  whether source should config audio.
- Replace delaywork with irq thread, and add suspend/resume interfaces,
  replace "dw-hdmi-audio" with consecutive strings.
- Add ".pm = &snd_soc_pm_ops,"

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.
- Delete the operation of jack in rockchip-hdmi-audio driver,
  get ready to switch to simple-audio-card driver.
- modify cpu-of-node to i2s-controller

Changes in v2:
- Update dw_hdmi audio control interfaces, and adjust jack report process
- give "codec-name" & "codec-dai-name" an const name
- remove codec-name and codec-dai-name
- rename rockchip,rockchip-hdmi-audio.txt to rockchip,rockchip-dw-hdmi-audio.txt

Yakir Yang (6):
  drm: bridge/dw_hdmi: add audio support for more display resolutions
  drm: bridge/dw_hdmi: enable audio when sink device is HDMI and has audio
  drm: bridge/dw_hdmi: rename dw_hdmi-ahb-audio.h to dw_hdmi-audio.h
  drm: bridge/dw_hdmi-i2s-audio: add audio driver
  ASoC: rockchip/rockchip-hdmi-audio: add sound driver for hdmi audio
  dt-bindings: Add documentation for rockchip_hdmi_audio driver

 .../sound/rockchip,rockchip-dw-hdmi-audio.txt      |  12 +
 drivers/gpu/drm/bridge/Kconfig                     |   9 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c         |   2 +-
 .../{dw_hdmi-ahb-audio.h => dw_hdmi-audio.h}       |   0
 drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c         | 398 +++++++++++++++++++++
 drivers/gpu/drm/bridge/dw_hdmi.c                   | 100 +++++-
 drivers/gpu/drm/bridge/dw_hdmi.h                   |   3 +
 sound/soc/rockchip/Kconfig                         |   9 +
 sound/soc/rockchip/Makefile                        |   2 +
 sound/soc/rockchip/rockchip_hdmi_audio.c           | 138 +++++++
 11 files changed, 658 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip,rockchip-dw-hdmi-audio.txt
 rename drivers/gpu/drm/bridge/{dw_hdmi-ahb-audio.h => dw_hdmi-audio.h} (100%)
 create mode 100644 drivers/gpu/drm/bridge/dw_hdmi-i2s-audio.c
 create mode 100644 sound/soc/rockchip/rockchip_hdmi_audio.c

-- 
2.1.2


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v5 1/6] drm: bridge/dw_hdmi: add audio support for more display resolutions

[Yakir Yang]

Just like HDMISpecification 1.4 document descripted, the soure shall
determine the fractional relationship between the TMDS clock an an
audio reference clock, the sink may then recreate the audio clock from
the TMDS clock by using an clock divider. So if we can make sink generate
the correct samplerate, then we can say those display resolutions with
this pixelclock could support audio play.

The exact relationship between the two clocks will be:
	128 * SampleRate = TmdsClock * N / CTS.
So this patch would generate the correct N/CTS values, add audio support
for the below  tmds clocks:
25.175MHz, 40MHz, 54MHz, 65MHz, 74.25MHz, 83.5MHz, 106.5MHz, 108Mhz

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v5:
- make more words in commit message.

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/gpu/drm/bridge/dw_hdmi.c | 56 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index dc0aed1..f717a2a 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -222,8 +222,24 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk,
 	case 44100:
 		if (pixel_clk == 25170000)
 			n = 7007;
+		else if (pixel_clk == 25175000)
+			n = 28224;
+		else if (pixel_clk == 40000000)
+			n = 7056;
+		else if (pixel_clk == 54000000)
+			n = 6272;
+		else if (pixel_clk == 65000000)
+			n = 7056;
 		else if (pixel_clk == 74170000)
 			n = 17836;
+		else if (pixel_clk == 74250000)
+			n = 6272;
+		else if (pixel_clk == 83500000)
+			n = 7056;
+		else if (pixel_clk == 106500000)
+			n = 4074;
+		else if (pixel_clk == 108000000)
+			n = 4018;
 		else if (pixel_clk == 148350000)
 			n = (ratio == 150) ? 17836 : 8918;
 		else
@@ -233,10 +249,26 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk,
 	case 48000:
 		if (pixel_clk == 25170000)
 			n = (ratio == 150) ? 9152 : 6864;
+		else if (pixel_clk == 25175000)
+			n = 6144;
 		else if (pixel_clk == 27020000)
 			n = (ratio == 150) ? 8192 : 6144;
+		else if (pixel_clk == 40000000)
+			n = 6144;
+		else if (pixel_clk == 54000000)
+			n = 6144;
+		else if (pixel_clk == 65000000)
+			n = 6144;
 		else if (pixel_clk == 74170000)
 			n = 11648;
+		else if (pixel_clk == 74250000)
+			n = 6144;
+		else if (pixel_clk == 83500000)
+			n = 6144;
+		else if (pixel_clk == 106500000)
+			n = 6144;
+		else if (pixel_clk == 108000000)
+			n = 6144;
 		else if (pixel_clk == 148350000)
 			n = (ratio == 150) ? 11648 : 5824;
 		else
@@ -284,10 +316,16 @@ static unsigned int hdmi_compute_cts(unsigned int freq, unsigned long pixel_clk,
 	case 96000:
 	case 192000:
 		switch (pixel_clk) {
+		case 25175000:
 		case 25200000:
 		case 27000000:
+		case 40000000:
 		case 54000000:
+		case 65000000:
 		case 74250000:
+		case 83500000:
+		case 106500000:
+		case 108000000:
 		case 148500000:
 			cts = pixel_clk / 1000;
 			break;
@@ -308,18 +346,36 @@ static unsigned int hdmi_compute_cts(unsigned int freq, unsigned long pixel_clk,
 	case 88200:
 	case 176400:
 		switch (pixel_clk) {
+		case 25175000:
+			cts = 125875;
+			break;
 		case 25200000:
 			cts = 28000;
 			break;
 		case 27000000:
 			cts = 30000;
 			break;
+		case 40000000:
+			cts = 50000;
+			break;
 		case 54000000:
 			cts = 60000;
 			break;
+		case 65000000:
+			cts = 81250;
+			break;
 		case 74250000:
 			cts = 82500;
 			break;
+		case 83500000:
+			cts = 104375;
+			break;
+		case 106500000:
+			cts = 88750;
+			break;
+		case 108000000:
+			cts = 76875;
+			break;
 		case 148500000:
 			cts = 165000;
 			break;
-- 
2.1.2


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 1/6] drm: bridge/dw_hdmi: add audio support for more display resolutions

[Russell King - ARM Linux]

On Sat, Jun 20, 2015 at 12:19:12AM +0800, Yakir Yang wrote:
> Just like HDMISpecification 1.4 document descripted, the soure shall
> determine the fractional relationship between the TMDS clock an an
> audio reference clock, the sink may then recreate the audio clock from
> the TMDS clock by using an clock divider. So if we can make sink generate
> the correct samplerate, then we can say those display resolutions with
> this pixelclock could support audio play.
> 
> The exact relationship between the two clocks will be:
> 	128 * SampleRate = TmdsClock * N / CTS.
> So this patch would generate the correct N/CTS values, add audio support
> for the below  tmds clocks:
> 25.175MHz, 40MHz, 54MHz, 65MHz, 74.25MHz, 83.5MHz, 106.5MHz, 108Mhz

I think I've said this before.  The documentation for iMX6 (which is
freely available, unlike Rock-chips documentation, so you can look at
it yourself) specifies that only certain audio and video rates are
supported.  See the iMX6 manuals, the section on the HDMI Tx, sub-
section "CTS calculation".

It clearly states that values which do not appear in the table there
are "not supported".

Adding more rates to this code puts iMX6 outside of its specification.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 1/6] drm: bridge/dw_hdmi: add audio support for more display resolutions

[Russell King - ARM Linux]

On Sat, Jun 20, 2015 at 12:19:12AM +0800, Yakir Yang wrote:
> The exact relationship between the two clocks will be:
> 	128 * SampleRate = TmdsClock * N / CTS.
> So this patch would generate the correct N/CTS values, add audio support
> for the below  tmds clocks:
> 25.175MHz, 40MHz, 54MHz, 65MHz, 74.25MHz, 83.5MHz, 106.5MHz, 108Mhz

Having played around with the iMX6 devices, I can now say that the
statement in the iMX6 manuals is basically wrong.  There's two things
which leads me to that conclusion.

1. They list the standard values for CTS/N at standard TMDS frequencies
   as listed in the HDMI specification (with the /1.001 and *1.001 omitted.)
   In other words, there is nothing non-standard about these values.

2. The dw_hdmi driver as from Freescale as N values for all standard
   TMDS frequencies given in the HDMI specification, including the
   /1.001 and *1.001 frequencies.  However, the CTS calculation errors
   these out.  (Note that under DRM, these can never match, because DRM
   uses frequencies of kHz resolution, not 10kHz.)

3. They then say that when using deep colour modes, the CTS values have
   to be scaled with the TMDS clock.  So, from that we can ascertain that
   other CTS values which are not listed in the table are permissible.
   This statement implies that *1.25, *1.5, *2 factors on the listed
   TMDS clock and CTS value must also be supported.

4. Practical experimentation on iMX6 Solo and Quad hardware (which have
   two different revisions of the Designware IP) show that the use of any
   CTS/N values derived from the standard ones work.  By that, I mean
   using the "other" N value and calculating CTS for non-listed TMDS
   clocks does indeed work.  Relaxing the restriction in the driver (as
   below) and running through all modes which my TV supports results in
   audio playback working, from 1080p@60 down to 640x480@59.94Hz.

So, I'm now of the opinion that the statement in iMX6 manuals concerning
restrictions on N/CTS values is incorrect, and no such restriction on
these exists.

In light of that, I now have code which removes this restriction from
dw_hdmi - we generate a standard-specified N value for standard clocks,
and then calculate the CTS value from that via:

        unsigned long ftdms = pixel_clk;
        u64 tmp;

        /*
         * Compute the CTS value from the N value.  Note that CTS and N
         * can be up to 20 bits in total, so we need 64-bit math.  Also
         * note that our TDMS clock is not fully accurate; it is accurate
         * to kHz.  This can introduce an unnecessary remainder in the
         * calculation below, so we don't try to warn about that.
         */
        tmp = (u64)ftdms * n;
        do_div(tmp, 128 * sample_rate);
        cts = tmp;

Note: I've removed all the deep colour and pixel repetition support here
because that's basically broken - the "ratio" was always 100.  If we need
pixel repetition (which is unsupported right now) it should be a matter of
scaling the ftmds appropriately which then scales the CTS value by the
same ratio.  "ftmds" above should be as close as possible to the _real_
TMDS clock rate, not the pixel clock.  In the case of deep colour, that
may influence the N value too (as 1.25x doesn't end up producing an
integer CTS value every time) so that would need to be re-addressed.

As for this specific patch, the part which remains relevant:

> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
> index dc0aed1..f717a2a 100644
> --- a/drivers/gpu/drm/bridge/dw_hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c
> @@ -222,8 +222,24 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk,
>  	case 44100:
>  		if (pixel_clk == 25170000)
>  			n = 7007;
> +		else if (pixel_clk == 25175000)
> +			n = 28224;

That value is wrong.  HDMI requires:

	128 * fs / 1500Hz <= n <= 128 * fs / 300Hz

For 44.1kHz, that gives a range of 3763.2 to 18816.  Presumably this
restriction is to limit the range of the feedback path in the clock
regenerator in the sink so that designers have some parameters to work
with.  Going outside these limits is unwise, and while it may work with
some sinks, it's definitely outside of the HDMI spec.

> +		else if (pixel_clk == 40000000)
> +			n = 7056;
> +		else if (pixel_clk == 54000000)
> +			n = 6272;
> +		else if (pixel_clk == 65000000)
> +			n = 7056;

Rather than a big if () else if () else if (), can't we have:

		if (pixel_clk == 25175000)
			n = 7007;
		else if (pixel_clk == 40000000 || pixel_clk == 65000000 ||
			 pixel_clk == 83500000)
			n = 7056;
		else ...

etc?

I'm also thinking that these values are pretty standard, and they
should not be internal to anyone's driver - maybe they should live
in drivers/video/hdmi.c.

I'll be posting my patch set which lifts the restriction later this
evening.

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

[PATCH v5 2/6] drm: bridge/dw_hdmi: enable audio when sink device is HDMI and has audio

[Yakir Yang]

The original code only provide audio func when video code is a CEA mode,
but I found that No-CEA mode could also play sound when sink is HDMI mode
and has audio. So I think if sink device has the ability to play audio,
then source should enable audio support that set fc_invidconf to HDMI mode
and config n/cts rightly.

And actually we could know those information through parsing sink edid.
drm_detect_hdmi_monitor() could report whether sink is a HDMI monitor,
drm_detect_monitor_audio() could return whether sink has audio.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v5:
- Take Russell suggest that create "sink_has_audio" and "sink_is_hdmi" in
  struct hdmi, and keep vmode's mdvi changeless.
- Config fc_invidconf to HDMI mode when sink device is HDMI monitor.

Changes in v4:
- Take Dainel suggest that introduce drm_detect_monitor_audio() to judge
  whether source should config audio.

Changes in v3: None
Changes in v2: None

 drivers/gpu/drm/bridge/dw_hdmi.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index f717a2a..417cc82 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -119,6 +119,8 @@ struct dw_hdmi {
 
 	u8 edid[HDMI_EDID_LEN];
 	bool cable_plugin;
+	bool sink_is_hdmi;
+	bool sink_has_audio;
 
 	bool phy_enabled;
 	struct drm_display_mode previous_mode;
@@ -1152,7 +1154,11 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
 		HDMI_FC_INVIDCONF_IN_I_P_INTERLACED :
 		HDMI_FC_INVIDCONF_IN_I_P_PROGRESSIVE;
 
-	inv_val |= (vmode->mdvi ?
+	/*
+	 * when sink device is hdmi monitor, then we can config
+	 * this to hdmi_mode even if the video mode is No-CEA mode.
+	 */
+	inv_val |= (hdmi->sink_is_hdmi ?
 		HDMI_FC_INVIDCONF_DVI_MODEZ_DVI_MODE :
 		HDMI_FC_INVIDCONF_DVI_MODEZ_HDMI_MODE);
 
@@ -1330,11 +1336,10 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 	/* HDMI Initialization Step B.3 */
 	dw_hdmi_enable_video_path(hdmi);
 
-	/* not for DVI mode */
-	if (hdmi->hdmi_data.video_mode.mdvi) {
-		dev_dbg(hdmi->dev, "%s DVI mode\n", __func__);
+	if (!hdmi->sink_has_audio) {
+		dev_info(hdmi->dev, "sink device does not support audio\n");
 	} else {
-		dev_dbg(hdmi->dev, "%s CEA mode\n", __func__);
+		dev_dbg(hdmi->dev, "sink device support audio\n");
 
 		/* HDMI Initialization Step E - Configure audio */
 		hdmi_clk_regenerator_update_pixel_clock(hdmi);
@@ -1500,6 +1505,9 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
 		dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
 			edid->width_cm, edid->height_cm);
 
+		hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
+		hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
+
 		drm_mode_connector_update_edid_property(connector, edid);
 		ret = drm_add_edid_modes(connector, edid);
 		/* Store the ELD */
-- 
2.1.2


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v5 3/6] drm: bridge/dw_hdmi: rename dw_hdmi-ahb-audio.h to dw_hdmi-audio.h

[Yakir Yang]

First step to make ahb audio driver and i2s audio driver code exist,
share the audio data struct in dw_hdmi-audio.h

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c                      | 2 +-
 drivers/gpu/drm/bridge/{dw_hdmi-ahb-audio.h => dw_hdmi-audio.h} | 0
 drivers/gpu/drm/bridge/dw_hdmi.c                                | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename drivers/gpu/drm/bridge/{dw_hdmi-ahb-audio.h => dw_hdmi-audio.h} (100%)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c
index 2bb68bd..0a270ea 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c
@@ -21,7 +21,7 @@
 #include <sound/pcm_drm_eld.h>
 #include <sound/pcm_iec958.h>
 
-#include "dw_hdmi-ahb-audio.h"
+#include "dw_hdmi-audio.h"
 
 #define DRIVER_NAME "dw-hdmi-ahb-audio"
 
diff --git a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h b/drivers/gpu/drm/bridge/dw_hdmi-audio.h
similarity index 100%
rename from drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h
rename to drivers/gpu/drm/bridge/dw_hdmi-audio.h
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 417cc82..cbb55ae 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -28,7 +28,7 @@
 #include <drm/bridge/dw_hdmi.h>
 
 #include "dw_hdmi.h"
-#include "dw_hdmi-ahb-audio.h"
+#include "dw_hdmi-audio.h"
 
 #define HDMI_EDID_LEN		512
 
-- 
2.1.2


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[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 <ykk@rock-chips.com>
---
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 0000000..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 <linux/io.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/moduleparam.h>
+
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/core.h>
+#include <sound/jack.h>
+#include <sound/initval.h>
+#include <sound/pcm_params.h>
+#include <drm/bridge/dw_hdmi.h>
+
+#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;
+
+	jack_status = !!(hdmi_readl(dw, HDMI_PHY_STAT0) & HDMI_PHY_HPD) ?
+			SND_JACK_LINEOUT : 0;
+
+	snd_soc_jack_report(&dw->jack, jack_status, SND_JACK_LINEOUT);
+
+	return 0;
+}
+
+static irqreturn_t snd_dw_hdmi_irq(int irq, void *dev_id)
+{
+	struct snd_dw_hdmi *dw = dev_id;
+
+	snd_dw_hdmi_jack_detect(dw);
+
+	return IRQ_HANDLED;
+}
+
+static void dw_hdmi_audio_set_fmt(struct snd_dw_hdmi *dw,
+				  const struct dw_audio_fmt *fmt)
+{
+	hdmi_modl(dw, fmt->input_type, AUDIO_CONF0_INTERFACE_MSK,
+		  HDMI_AUD_CONF0);
+
+	hdmi_modl(dw, fmt->chan_num, AUDIO_CONF0_I2SINEN_MSK,
+		  HDMI_AUD_CONF0);
+
+	hdmi_modl(dw, fmt->word_length, AUDIO_CONF1_DATWIDTH_MSK,
+		  HDMI_AUD_CONF1);
+
+	hdmi_modl(dw, fmt->dai_fmt, AUDIO_CONF1_DATAMODE_MSK,
+		  HDMI_AUD_CONF1);
+
+	hdmi_writel(dw, 0, HDMI_AUD_INPUTCLKFS);
+
+	dw_hdmi_set_sample_rate(dw->data.hdmi, fmt->sample_rate);
+}
+
+static void dw_audio_set_fmt(struct snd_dw_hdmi *dw,
+			     const struct dw_audio_fmt *fmt)
+{
+	if (fmt)
+		dw->fmt = *fmt;
+	dw_hdmi_audio_set_fmt(dw, &dw->fmt);
+}
+
+static int snd_dw_hdmi_dai_startup(struct snd_pcm_substream *substream,
+				   struct snd_soc_dai *codec_dai)
+{
+	struct snd_dw_hdmi *dw = snd_soc_dai_get_drvdata(codec_dai);
+
+	dw->is_playback_status = true;
+	dw_hdmi_audio_enable(dw->data.hdmi);
+
+	return 0;
+}
+
+static int snd_dw_hdmi_dai_hw_params(struct snd_pcm_substream *substream,
+				     struct snd_pcm_hw_params *params,
+				     struct snd_soc_dai *codec_dai)
+{
+	struct snd_dw_hdmi *dw = snd_soc_dai_get_drvdata(codec_dai);
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct dw_audio_fmt dw_fmt;
+	unsigned int fmt, rate, chan, width;
+
+	fmt = rtd->dai_link->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK;
+	switch (fmt) {
+	case SND_SOC_DAIFMT_I2S:
+		dw_fmt.dai_fmt = AUDIO_DAIFMT_IIS;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		dw_fmt.dai_fmt = AUDIO_DAIFMT_LEFT_J;
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		dw_fmt.dai_fmt = AUDIO_DAIFMT_RIGHT_J;
+		break;
+	default:
+		dev_err(codec_dai->dev, "DAI format unsupported");
+		return -EINVAL;
+	}
+
+	width = params_width(params);
+	switch (width) {
+	case 16:
+	case 24:
+		dw_fmt.word_length = width;
+		break;
+	default:
+		dev_err(codec_dai->dev, "width[%d] not support!\n", width);
+		return -EINVAL;
+	}
+
+	chan = params_channels(params);
+	switch (chan) {
+	case 2:
+		dw_fmt.chan_num = AUDIO_CHANNELNUM_2;
+		break;
+	case 4:
+		dw_fmt.chan_num = AUDIO_CHANNELNUM_4;
+		break;
+	case 6:
+		dw_fmt.chan_num = AUDIO_CHANNELNUM_6;
+		break;
+	case 8:
+		dw_fmt.chan_num = AUDIO_CHANNELNUM_8;
+		break;
+	default:
+		dev_err(codec_dai->dev, "channel[%d] not support!\n", chan);
+		return -EINVAL;
+	}
+
+	rate = params_rate(params);
+	switch (rate) {
+	case 32000:
+	case 44100:
+	case 48000:
+	case 88200:
+	case 96000:
+	case 176400:
+	case 192000:
+		dw_fmt.sample_rate = rate;
+		break;
+	default:
+		dev_err(codec_dai->dev, "rate[%d] not support!\n", rate);
+		return -EINVAL;
+	}
+
+	dw_fmt.input_type = AUDIO_INPUTTYPE_IIS;
+
+	dw_audio_set_fmt(dw, &dw_fmt);
+
+	return 0;
+}
+
+static int snd_dw_hdmi_dai_trigger(struct snd_pcm_substream *substream,
+				   int cmd, struct snd_soc_dai *codec_dai)
+{
+	struct snd_dw_hdmi *dw = snd_soc_dai_get_drvdata(codec_dai);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		dw_hdmi_audio_enable(dw->data.hdmi);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		dw_hdmi_audio_disable(dw->data.hdmi);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void snd_dw_hdmi_dai_shutdown(struct snd_pcm_substream *substream,
+				     struct snd_soc_dai *codec_dai)
+{
+	struct snd_dw_hdmi *dw = snd_soc_dai_get_drvdata(codec_dai);
+
+	dw->is_playback_status = false;
+	dw_hdmi_audio_disable(dw->data.hdmi);
+}
+
+static int snd_dw_hdmi_audio_probe(struct snd_soc_codec *codec)
+{
+	struct snd_dw_hdmi *dw = snd_soc_codec_get_drvdata(codec);
+	int ret;
+
+	ret = snd_soc_jack_new(codec, "dw Jack", SND_JACK_LINEOUT,
+			       &dw->jack);
+	if (ret) {
+		dev_err(dw->dev, "jack new failed (%d)\n", ret);
+		dw->is_jack_ready = false;
+		return ret;
+	}
+
+	dw->is_jack_ready = true;
+
+	return snd_dw_hdmi_jack_detect(dw);
+}
+
+static const struct snd_soc_dapm_widget snd_dw_hdmi_audio_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("TX"),
+};
+
+static const struct snd_soc_dapm_route snd_dw_hdmi_audio_routes[] = {
+	{ "TX", NULL, "Playback" },
+};
+
+static const struct snd_soc_dai_ops dw_hdmi_dai_ops = {
+	.startup = snd_dw_hdmi_dai_startup,
+	.hw_params = snd_dw_hdmi_dai_hw_params,
+	.trigger = snd_dw_hdmi_dai_trigger,
+	.shutdown = snd_dw_hdmi_dai_shutdown,
+};
+
+static struct snd_soc_dai_driver dw_hdmi_audio_dai = {
+	.name = "dw-hdmi-i2s-hifi",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 8,
+		.rates = SNDRV_PCM_RATE_32000 |
+			 SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |
+			 SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 |
+			 SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+	.ops = &dw_hdmi_dai_ops,
+};
+
+static const struct snd_soc_codec_driver dw_hdmi_audio = {
+	.probe = snd_dw_hdmi_audio_probe,
+	.dapm_widgets = snd_dw_hdmi_audio_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(snd_dw_hdmi_audio_widgets),
+	.dapm_routes = snd_dw_hdmi_audio_routes,
+	.num_dapm_routes = ARRAY_SIZE(snd_dw_hdmi_audio_routes),
+};
+
+static int dw_hdmi_audio_probe(struct platform_device *pdev)
+{
+	struct dw_hdmi_audio_data *data = pdev->dev.platform_data;
+	struct snd_dw_hdmi *dw;
+	int ret;
+
+	dw = devm_kzalloc(&pdev->dev, sizeof(*dw), GFP_KERNEL);
+	if (!dw)
+		return -ENOMEM;
+
+	dw->data = *data;
+	dw->dev = &pdev->dev;
+	dw->is_jack_ready = false;
+	platform_set_drvdata(pdev, dw);
+
+	ret = request_irq(dw->data.irq, snd_dw_hdmi_irq, IRQF_SHARED,
+			  "dw-dw-audio", dw);
+	if (ret) {
+		dev_err(&pdev->dev, "request irq failed (%d)\n", ret);
+		return -EINVAL;
+	}
+
+	ret = snd_soc_register_codec(&pdev->dev, &dw_hdmi_audio,
+				     &dw_hdmi_audio_dai, 1);
+	if (ret) {
+		dev_err(&pdev->dev, "register codec failed (%d)\n", ret);
+		return -EINVAL;
+	}
+
+	dev_dbg(&pdev->dev, "dw audio init success.\n");
+
+	return 0;
+}
+
+static int dw_hdmi_audio_remove(struct platform_device *pdev)
+{
+	struct snd_dw_hdmi *dw = platform_get_drvdata(pdev);
+
+	snd_soc_unregister_codec(&pdev->dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int dw_hdmi_audio_resume(struct device *dev)
+{
+	struct snd_dw_hdmi *dw = dev_get_drvdata(dev);
+
+	snd_dw_hdmi_jack_detect(dw);
+
+	if (dw->is_playback_status)
+		dw_hdmi_audio_set_fmt(dw, &dw->fmt);
+
+	return 0;
+}
+
+static int dw_hdmi_audio_suspend(struct device *dev)
+{
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops dw_hdmi_audio_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(dw_hdmi_audio_suspend, dw_hdmi_audio_resume)
+};
+
+static struct platform_driver dw_hdmi_audio_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.pm = &dw_hdmi_audio_pm,
+	},
+	.probe = dw_hdmi_audio_probe,
+	.remove = dw_hdmi_audio_remove,
+};
+module_platform_driver(dw_hdmi_audio_driver);
+
+MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
+MODULE_DESCRIPTION("DW dw Audio ASoC Interface");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS(PLATFORM_MODULE_PREFIX DRIVER_NAME);
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index cbb55ae..5356126 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -1784,20 +1784,26 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 
 	memset(&pdevinfo, 0, sizeof(pdevinfo));
 	pdevinfo.parent = dev;
-	pdevinfo.id = PLATFORM_DEVID_AUTO;
 
-	if (hdmi_readb(hdmi, HDMI_CONFIG1_ID) & HDMI_CONFIG1_AHB) {
-		audio.phys = iores->start;
-		audio.base = hdmi->regs;
-		audio.irq = irq;
-		audio.hdmi = hdmi;
-		audio.eld = hdmi->connector.eld;
+	audio.phys = iores->start;
+	audio.base = hdmi->regs;
+	audio.irq = irq;
+	audio.hdmi = hdmi;
+	audio.eld = hdmi->connector.eld;
+
+	pdevinfo.data = &audio;
+	pdevinfo.size_data = sizeof(audio);
 
+	if (hdmi_readb(hdmi, HDMI_CONFIG1_ID) & HDMI_CONFIG1_AHB) {
 		pdevinfo.name = "dw-hdmi-ahb-audio";
-		pdevinfo.data = &audio;
-		pdevinfo.size_data = sizeof(audio);
+		pdevinfo.id = PLATFORM_DEVID_AUTO;
 		pdevinfo.dma_mask = DMA_BIT_MASK(32);
 		hdmi->audio = platform_device_register_full(&pdevinfo);
+
+	} else if (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S) {
+		pdevinfo.name = "dw-hdmi-ahb-audio";
+		pdevinfo.id = PLATFORM_DEVID_NONE;
+		hdmi->audio = platform_device_register_full(&pdevinfo);
 	}
 
 	dev_set_drvdata(dev, hdmi);
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.h b/drivers/gpu/drm/bridge/dw_hdmi.h
index 78e54e8..9c22377 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.h
+++ b/drivers/gpu/drm/bridge/dw_hdmi.h
@@ -545,6 +545,9 @@
 #define HDMI_I2CM_FS_SCL_LCNT_0_ADDR            0x7E12
 
 enum {
+/* CONFIG0_ID field values */
+	HDMI_CONFIG0_I2S = 0x01,
+
 /* CONFIG1_ID field values */
 	HDMI_CONFIG1_AHB = 0x01,
 
-- 
2.1.2


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [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
>
>
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [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 <ykk@rock-chips.com>");
> +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.

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 808 bytes --]

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 :(

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH v5 5/6] ASoC: rockchip/rockchip-hdmi-audio: add sound driver for hdmi audio

[Yakir Yang]

Add a sound driver that combines rockchip-i2s cpu_dai and dw-hdmi-codec
as codec_dai to provide hdmi audio output on rk3288 platforms.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v5:
- Take Mark Brown suggest that remove the no useful code in probe func,
  and remove the snd_soc_dai_set_fmt() snd_soc_unregister_card(), remove
  those noisy dev_info()
- Rename codec_dai_name to "dw-hdmi-i2s-hifi"

Changes in v4:
- Add ".pm = &snd_soc_pm_ops,"

Changes in v3:
- Delete the operation of jack in rockchip-hdmi-audio driver,
  get ready to switch to simple-audio-card driver.

Changes in v2:
- give "codec-name" & "codec-dai-name" an const name

 sound/soc/rockchip/Kconfig               |   9 ++
 sound/soc/rockchip/Makefile              |   2 +
 sound/soc/rockchip/rockchip_hdmi_audio.c | 138 +++++++++++++++++++++++++++++++
 3 files changed, 149 insertions(+)
 create mode 100644 sound/soc/rockchip/rockchip_hdmi_audio.c

diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index e181826..ed2b7f0 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -14,3 +14,12 @@ config SND_SOC_ROCKCHIP_I2S
 	  Say Y or M if you want to add support for I2S driver for
 	  Rockchip I2S device. The device supports upto maximum of
 	  8 channels each for play and record.
+
+config SND_SOC_ROCKCHIP_HDMI_AUDIO
+	tristate "ASoC support for Rockchip HDMI audio"
+	depends on SND_SOC_ROCKCHIP
+	select SND_SOC_ROCKCHIP_I2S
+	select SND_SOC_DW_HDMI_AUDIO
+	help
+	  Say Y or M here if you want to add support for SoC audio on Rockchip
+	  HDMI, such as rk3288 hdmi.
diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
index b921909..b9185b3 100644
--- a/sound/soc/rockchip/Makefile
+++ b/sound/soc/rockchip/Makefile
@@ -1,4 +1,6 @@
 # ROCKCHIP Platform Support
 snd-soc-i2s-objs := rockchip_i2s.o
+snd-soc-rockchip-hdmi-audio-objs := rockchip_hdmi_audio.o
 
 obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-i2s.o
+obj-$(CONFIG_SND_SOC_ROCKCHIP_HDMI_AUDIO) += snd-soc-rockchip-hdmi-audio.o
diff --git a/sound/soc/rockchip/rockchip_hdmi_audio.c b/sound/soc/rockchip/rockchip_hdmi_audio.c
new file mode 100644
index 0000000..53ecd60
--- /dev/null
+++ b/sound/soc/rockchip/rockchip_hdmi_audio.c
@@ -0,0 +1,138 @@
+/*
+ * ROCKCHIP ALSA SoC DAI driver for HDMI audio on rockchip processors.
+ * Copyright (c) 2014, ROCKCHIP CORPORATION. All rights reserved.
+ * Authors: Yakir Yang <ykk@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.*
+ *
+ */
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <sound/soc.h>
+#include <sound/pcm.h>
+#include <sound/core.h>
+#include <sound/pcm_params.h>
+
+#include "rockchip_i2s.h"
+
+#define DRV_NAME "rockchip-hdmi-audio"
+
+static int rockchip_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
+					struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	unsigned int dai_fmt = rtd->dai_link->dai_fmt;
+	int mclk, ret;
+
+	switch (params_rate(params)) {
+	case 8000:
+	case 16000:
+	case 24000:
+	case 32000:
+	case 48000:
+	case 64000:
+	case 96000:
+		mclk = 12288000;
+		break;
+	case 11025:
+	case 22050:
+	case 44100:
+	case 88200:
+		mclk = 11289600;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk, SND_SOC_CLOCK_OUT);
+	if (ret < 0) {
+		dev_err(cpu_dai->dev, "failed to set cpu_dai sysclk.\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct snd_soc_ops hdmi_audio_dai_ops = {
+	.hw_params = rockchip_hdmi_audio_hw_params,
+};
+
+static struct snd_soc_dai_link hdmi_audio_dai = {
+	.name = "RockchipHDMI",
+	.stream_name = "RockchipHDMI",
+	.codec_name = "dw-hdmi-i2s-audio",
+	.codec_dai_name = "dw-hdmi-i2s-hifi",
+	.ops = &hdmi_audio_dai_ops,
+	.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+		   SND_SOC_DAIFMT_CBS_CFS,
+};
+
+static struct snd_soc_card rockchip_hdmi_audio_card = {
+	.name = "RockchipHDMI",
+	.owner = THIS_MODULE,
+	.dai_link = &hdmi_audio_dai,
+	.num_links = 1,
+};
+
+static int rockchip_hdmi_audio_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &rockchip_hdmi_audio_card;
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+
+	card->dev = &pdev->dev;
+	platform_set_drvdata(pdev, card);
+
+	hdmi_audio_dai.cpu_of_node = of_parse_phandle(np, "i2s-controller", 0);
+	if (!hdmi_audio_dai.cpu_of_node) {
+		dev_err(&pdev->dev, "Property 'i2s-controller' missing !\n");
+		return -EINVAL;
+	}
+
+	hdmi_audio_dai.platform_of_node = hdmi_audio_dai.cpu_of_node;
+
+	ret = snd_soc_register_card(card);
+	if (ret) {
+		dev_err(&pdev->dev, "register card failed (%d)\n", ret);
+		card->dev = NULL;
+		return -EINVAL;
+	}
+
+	dev_dbg(&pdev->dev, "hdmi audio init success.\n");
+
+	return 0;
+}
+
+static const struct of_device_id rockchip_hdmi_audio_of_match[] = {
+	{ .compatible = "rockchip,rk3288-hdmi-audio", },
+	{},
+};
+
+static struct platform_driver rockchip_hdmi_audio_driver = {
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.pm = &snd_soc_pm_ops,
+		.of_match_table = rockchip_hdmi_audio_of_match,
+	},
+	.probe = rockchip_hdmi_audio_probe,
+};
+module_platform_driver(rockchip_hdmi_audio_driver);
+
+MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
+MODULE_DESCRIPTION("Rockchip HDMI Audio ASoC Interface");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_DEVICE_TABLE(of, rockchip_hdmi_audio_of_match);
-- 
2.1.2


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v5 6/6] dt-bindings: Add documentation for rockchip-hdmi-audio driver

[Yakir Yang]

Required properties:
- compatible: platform specific
- i2s-controller: the i2s controller device node

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v5: None
Changes in v4: None
Changes in v3:
- modify cpu-of-node to i2s-controller

Changes in v2:
- remove codec-name and codec-dai-name
- rename rockchip,rockchip-hdmi-audio.txt to rockchip,rockchip-dw-hdmi-audio.txt

 .../bindings/sound/rockchip,rockchip-dw-hdmi-audio.txt       | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip,rockchip-dw-hdmi-audio.txt

diff --git a/Documentation/devicetree/bindings/sound/rockchip,rockchip-dw-hdmi-audio.txt b/Documentation/devicetree/bindings/sound/rockchip,rockchip-dw-hdmi-audio.txt
new file mode 100644
index 0000000..f0d23c5
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rockchip,rockchip-dw-hdmi-audio.txt
@@ -0,0 +1,12 @@
+Rockchip hdmi audio bindings
+
+Required properties:
+- compatible: platform specific
+- i2s-controller: the i2s controller device node
+
+Example:
+
+sound {
+	compatible = "rockchip,rk3288-hdmi-audio";
+	i2s-controller = <&i2s>;
+};
-- 
2.1.2


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/