alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support
@ 2015-03-30 19:39 Russell King - ARM Linux
  2015-03-30 19:40 ` [PATCH RFC 01/11] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator() Russell King
                   ` (10 more replies)
  0 siblings, 11 replies; 30+ messages in thread
From: Russell King - ARM Linux @ 2015-03-30 19:39 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, Jaroslav Kysela, Mark Brown, Yakir Yang

I'm sending this series for comments, and to allow people to update
their code bases (for those who are using my AHB audio driver on
iMX6).  This is really several series, and I'm not expecting this
to be ready for the upcoming merge window.  That said, if anyone
wants to say that they're happy with some of the initial four patches,
I'm happy to queue those for David, if he's willing to take them.

This series applies on top of my previous series, which finished with
the patch "drm: bridge/dw_hdmi: adjust n/cts setting order".

The first four patches are a few cleanups to the dw_hdmi driver.

The next two patches introduce new interfaces to the dw_hdmi driver
to support AHB audio, including the errata found on iMX6 which
requires N to be programmed to zero when the audio FIFO is not full.
I'm expecting some discussion with these last two as we try to work
out how to deal with the two variants of audio support for this part
(AHB audio vs I2S audio).

The following three add various helpers to the DRM and ALSA subsystems
for audio support which is non-specific to the AHB audio driver.  For
DRM, this is a helper macro to obtain the pointer into the short audio
descriptors.  For ALSA, it's a set of helpers to restrict an audio PCM
device's capabilities according to the ELD, and to generate the IEC958
channel status data.  If these helpers are acceptable, we can convert
a number of drivers to them.

The ELD helper isn't quite finished, but should be sufficient for
initial testing.

The last two patches add support for the dw_hdmi audio.  This is an
ALSA driver rather than an ASoC driver as it's a pretty simple affair.
I couldn't find a suitable location in the sound/ subtree to place it,
so I've placed it next to the bridge - yes, we probably need to find
it a better home in the sound/ subtree, but it currently makes
use of dw_hdmi.h in the directory it lives.

 drivers/gpu/drm/bridge/Kconfig             |  11 +
 drivers/gpu/drm/bridge/Makefile            |   1 +
 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c | 566 +++++++++++++++++++++++++++++
 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h |  14 +
 drivers/gpu/drm/bridge/dw_hdmi.c           | 250 ++++++++-----
 drivers/gpu/drm/bridge/dw_hdmi.h           |   3 +
 include/drm/bridge/dw_hdmi.h               |   7 +
 include/drm/drm_edid.h                     |  19 +
 include/sound/pcm_drm_eld.h                |   6 +
 include/sound/pcm_iec958.h                 |   9 +
 sound/core/Kconfig                         |   6 +
 sound/core/Makefile                        |   3 +
 sound/core/pcm_drm_eld.c                   |  91 +++++
 sound/core/pcm_iec958.c                    |  70 ++++
 14 files changed, 964 insertions(+), 92 deletions(-)

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 01/11] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator()
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
@ 2015-03-30 19:40 ` Russell King
  2015-03-31  6:55   ` Yang Kuankuan
  2015-03-30 19:40 ` [PATCH RFC 02/11] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode() Russell King
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel, Yakir Yang

Clean up hdmi_set_clk_regenerator() by allowing it to take the audio
sample rate and ratio directly, rather than hiding it inside the
function.  Raise the unsupported pixel clock/sample rate message from
debug to error level as this results in audio not working correctly.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/bridge/dw_hdmi.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index cca1c3d165e2..49df6c8c4ea8 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -335,39 +335,37 @@ static unsigned int hdmi_compute_cts(unsigned int freq, unsigned long pixel_clk,
 }
 
 static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
-				     unsigned long pixel_clk)
+	unsigned long pixel_clk, unsigned int sample_rate, unsigned int ratio)
 {
-	unsigned int clk_n, clk_cts;
+	unsigned int n, cts;
 
-	clk_n = hdmi_compute_n(hdmi->sample_rate, pixel_clk,
-			       hdmi->ratio);
-	clk_cts = hdmi_compute_cts(hdmi->sample_rate, pixel_clk,
-				   hdmi->ratio);
-
-	if (!clk_cts) {
-		dev_dbg(hdmi->dev, "%s: pixel clock not supported: %lu\n",
-			__func__, pixel_clk);
-		return;
+	n = hdmi_compute_n(sample_rate, pixel_clk, ratio);
+	cts = hdmi_compute_cts(sample_rate, pixel_clk, ratio);
+	if (!cts) {
+		dev_err(hdmi->dev,
+			"%s: pixel clock/sample rate not supported: %luMHz / %ukHz\n",
+			__func__, pixel_clk, sample_rate);
 	}
 
-	dev_dbg(hdmi->dev, "%s: samplerate=%d  ratio=%d  pixelclk=%lu  N=%d cts=%d\n",
-		__func__, hdmi->sample_rate, hdmi->ratio,
-		pixel_clk, clk_n, clk_cts);
+	dev_dbg(hdmi->dev, "%s: samplerate=%ukHz ratio=%d pixelclk=%luMHz N=%d cts=%d\n",
+		__func__, sample_rate, ratio, pixel_clk, n, cts);
 
-	hdmi_set_cts_n(hdmi, clk_cts, clk_n);
+	hdmi_set_cts_n(hdmi, cts, n);
 }
 
 static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
 {
 	mutex_lock(&hdmi->audio_mutex);
-	hdmi_set_clk_regenerator(hdmi, 74250000);
+	hdmi_set_clk_regenerator(hdmi, 74250000, hdmi->sample_rate,
+				 hdmi->ratio);
 	mutex_unlock(&hdmi->audio_mutex);
 }
 
 static void hdmi_clk_regenerator_update_pixel_clock(struct dw_hdmi *hdmi)
 {
 	mutex_lock(&hdmi->audio_mutex);
-	hdmi_set_clk_regenerator(hdmi, hdmi->hdmi_data.video_mode.mpixelclock);
+	hdmi_set_clk_regenerator(hdmi, hdmi->hdmi_data.video_mode.mpixelclock,
+				 hdmi->sample_rate, hdmi->ratio);
 	mutex_unlock(&hdmi->audio_mutex);
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH RFC 02/11] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode()
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
  2015-03-30 19:40 ` [PATCH RFC 01/11] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator() Russell King
@ 2015-03-30 19:40 ` Russell King
  2015-03-31  9:02   ` Yang Kuankuan
  2015-03-30 19:40 ` [PATCH RFC 03/11] drm: bridge/dw_hdmi: simplify hdmi_config_AVI() a little Russell King
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel, Yakir Yang

Use drm_hdmi_avi_infoframe_from_display_mode() to compose the AVI
frame.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/bridge/dw_hdmi.c | 126 +++++++++++++++++++++------------------
 1 file changed, 67 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 49df6c8c4ea8..44c63883db19 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -919,74 +919,79 @@ static void hdmi_tx_hdcp_config(struct dw_hdmi *hdmi)
 		  HDMI_A_HDCPCFG1_ENCRYPTIONDISABLE_MASK, HDMI_A_HDCPCFG1);
 }
 
-static void hdmi_config_AVI(struct dw_hdmi *hdmi)
+static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 {
-	u8 val, pix_fmt, under_scan;
-	u8 act_ratio, coded_ratio, colorimetry, ext_colorimetry;
-	bool aspect_16_9;
+	struct hdmi_avi_infoframe frame;
+	u8 val;
 
-	aspect_16_9 = false; /* FIXME */
+	/* Initialise info frame from DRM mode */
+	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
 
-	/* AVI Data Byte 1 */
 	if (hdmi->hdmi_data.enc_out_format == YCBCR444)
-		pix_fmt = HDMI_FC_AVICONF0_PIX_FMT_YCBCR444;
+		frame.colorspace = HDMI_COLORSPACE_YUV444;
 	else if (hdmi->hdmi_data.enc_out_format == YCBCR422_8BITS)
-		pix_fmt = HDMI_FC_AVICONF0_PIX_FMT_YCBCR422;
+		frame.colorspace = HDMI_COLORSPACE_YUV422;
 	else
-		pix_fmt = HDMI_FC_AVICONF0_PIX_FMT_RGB;
-
-		under_scan =  HDMI_FC_AVICONF0_SCAN_INFO_NODATA;
-
-	/*
-	 * Active format identification data is present in the AVI InfoFrame.
-	 * Under scan info, no bar data
-	 */
-	val = pix_fmt | under_scan |
-		HDMI_FC_AVICONF0_ACTIVE_FMT_INFO_PRESENT |
-		HDMI_FC_AVICONF0_BAR_DATA_NO_DATA;
-
-	hdmi_writeb(hdmi, val, HDMI_FC_AVICONF0);
-
-	/* AVI Data Byte 2 -Set the Aspect Ratio */
-	if (aspect_16_9) {
-		act_ratio = HDMI_FC_AVICONF1_ACTIVE_ASPECT_RATIO_16_9;
-		coded_ratio = HDMI_FC_AVICONF1_CODED_ASPECT_RATIO_16_9;
-	} else {
-		act_ratio = HDMI_FC_AVICONF1_ACTIVE_ASPECT_RATIO_4_3;
-		coded_ratio = HDMI_FC_AVICONF1_CODED_ASPECT_RATIO_4_3;
-	}
+		frame.colorspace = HDMI_COLORSPACE_RGB;
 
 	/* Set up colorimetry */
 	if (hdmi->hdmi_data.enc_out_format == XVYCC444) {
-		colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_EXTENDED_INFO;
+		frame.colorimetry = HDMI_COLORIMETRY_EXTENDED;
 		if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601)
-			ext_colorimetry =
-				HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601;
+			frame.extended_colorimetry =
+				HDMI_EXTENDED_COLORIMETRY_XV_YCC_601;
 		else /*hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_709*/
-			ext_colorimetry =
-				HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC709;
+			frame.extended_colorimetry =
+				HDMI_EXTENDED_COLORIMETRY_XV_YCC_709;
 	} else if (hdmi->hdmi_data.enc_out_format != RGB) {
 		if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601)
-			colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_SMPTE;
+			frame.colorimetry = HDMI_COLORIMETRY_ITU_601;
 		else /*hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_709*/
-			colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_ITUR;
-		ext_colorimetry = HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601;
+			frame.colorimetry = HDMI_COLORIMETRY_ITU_709;
+		frame.extended_colorimetry = HDMI_EXTENDED_COLORIMETRY_XV_YCC_601;
 	} else { /* Carries no data */
-		colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_NO_DATA;
-		ext_colorimetry = HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601;
+		frame.colorimetry = HDMI_COLORIMETRY_NONE;
+		frame.extended_colorimetry = HDMI_EXTENDED_COLORIMETRY_XV_YCC_601;
 	}
 
-	val = colorimetry | coded_ratio | act_ratio;
+	frame.scan_mode = HDMI_SCAN_MODE_NONE;
+
+	/*
+	 * The Designware IP uses a different byte format from standard
+	 * AVI info frames, though generally the bits are in the correct
+	 * bytes.
+	 */
+
+	/*
+	 * AVI data byte 1 differences: Colorspace in bits 4,5 rather than 5,6,
+	 * active aspect present in bit 6 rather than 4.
+	 */
+	val = (frame.colorspace & 3) << 4 | (frame.scan_mode & 0x3);
+	if (frame.active_aspect & 15)
+		val |= HDMI_FC_AVICONF0_ACTIVE_FMT_INFO_PRESENT;
+	if (frame.top_bar || frame.bottom_bar)
+		val |= HDMI_FC_AVICONF0_BAR_DATA_HORIZ_BAR;
+	if (frame.left_bar || frame.right_bar)
+		val |= HDMI_FC_AVICONF0_BAR_DATA_VERT_BAR;
+	hdmi_writeb(hdmi, val, HDMI_FC_AVICONF0);
+
+	/* AVI data byte 2 differences: none */
+	val = ((frame.colorimetry & 0x3) << 6) |
+	      ((frame.picture_aspect & 0x3) << 4) |
+	      (frame.active_aspect & 0xf);
 	hdmi_writeb(hdmi, val, HDMI_FC_AVICONF1);
 
-	/* AVI Data Byte 3 */
-	val = HDMI_FC_AVICONF2_IT_CONTENT_NO_DATA | ext_colorimetry |
-		HDMI_FC_AVICONF2_RGB_QUANT_DEFAULT |
-		HDMI_FC_AVICONF2_SCALING_NONE;
+	/* AVI data byte 3 differences: none */
+	val = ((frame.extended_colorimetry & 0x7) << 4) |
+	      ((frame.quantization_range & 0x3) << 2) |
+	      (frame.nups & 0x3);
+	if (frame.itc)
+		val |= HDMI_FC_AVICONF2_IT_CONTENT_VALID;
 	hdmi_writeb(hdmi, val, HDMI_FC_AVICONF2);
 
-	/* AVI Data Byte 4 */
-	hdmi_writeb(hdmi, hdmi->vic, HDMI_FC_AVIVID);
+	/* AVI data byte 4 differences: none */
+	val = frame.video_code & 0x7f;
+	hdmi_writeb(hdmi, val, HDMI_FC_AVIVID);
 
 	/* AVI Data Byte 5- set up input and output pixel repetition */
 	val = (((hdmi->hdmi_data.video_mode.mpixelrepetitioninput + 1) <<
@@ -997,20 +1002,23 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi)
 		HDMI_FC_PRCONF_OUTPUT_PR_FACTOR_MASK);
 	hdmi_writeb(hdmi, val, HDMI_FC_PRCONF);
 
-	/* IT Content and quantization range = don't care */
-	val = HDMI_FC_AVICONF3_IT_CONTENT_TYPE_GRAPHICS |
-		HDMI_FC_AVICONF3_QUANT_RANGE_LIMITED;
+	/*
+	 * AVI data byte 5 differences: content type in 0,1 rather than 4,5,
+	 * ycc range in bits 2,3 rather than 6,7
+	 */
+	val = ((frame.ycc_quantization_range & 0x3) << 2) |
+	      (frame.content_type & 0x3);
 	hdmi_writeb(hdmi, val, HDMI_FC_AVICONF3);
 
 	/* AVI Data Bytes 6-13 */
-	hdmi_writeb(hdmi, 0, HDMI_FC_AVIETB0);
-	hdmi_writeb(hdmi, 0, HDMI_FC_AVIETB1);
-	hdmi_writeb(hdmi, 0, HDMI_FC_AVISBB0);
-	hdmi_writeb(hdmi, 0, HDMI_FC_AVISBB1);
-	hdmi_writeb(hdmi, 0, HDMI_FC_AVIELB0);
-	hdmi_writeb(hdmi, 0, HDMI_FC_AVIELB1);
-	hdmi_writeb(hdmi, 0, HDMI_FC_AVISRB0);
-	hdmi_writeb(hdmi, 0, HDMI_FC_AVISRB1);
+	hdmi_writeb(hdmi, frame.top_bar & 0xff, HDMI_FC_AVIETB0);
+	hdmi_writeb(hdmi, (frame.top_bar >> 8) & 0xff, HDMI_FC_AVIETB1);
+	hdmi_writeb(hdmi, frame.bottom_bar & 0xff, HDMI_FC_AVISBB0);
+	hdmi_writeb(hdmi, (frame.bottom_bar >> 8) & 0xff, HDMI_FC_AVISBB1);
+	hdmi_writeb(hdmi, frame.left_bar & 0xff, HDMI_FC_AVIELB0);
+	hdmi_writeb(hdmi, (frame.left_bar >> 8) & 0xff, HDMI_FC_AVIELB1);
+	hdmi_writeb(hdmi, frame.right_bar & 0xff, HDMI_FC_AVISRB0);
+	hdmi_writeb(hdmi, (frame.right_bar >> 8) & 0xff, HDMI_FC_AVISRB1);
 }
 
 static void hdmi_av_composer(struct dw_hdmi *hdmi,
@@ -1244,7 +1252,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 		hdmi_enable_audio_clk(hdmi);
 
 		/* HDMI Initialization Step F - Configure AVI InfoFrame */
-		hdmi_config_AVI(hdmi);
+		hdmi_config_AVI(hdmi, mode);
 	}
 
 	hdmi_video_packetize(hdmi);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH RFC 03/11] drm: bridge/dw_hdmi: simplify hdmi_config_AVI() a little
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
  2015-03-30 19:40 ` [PATCH RFC 01/11] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator() Russell King
  2015-03-30 19:40 ` [PATCH RFC 02/11] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode() Russell King
@ 2015-03-30 19:40 ` Russell King
  2015-03-30 19:40 ` [PATCH RFC 04/11] drm: bridge/dw_hdmi: remove mhsyncpolarity/mvsyncpolarity/minterlaced Russell King
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel, Yakir Yang

When a YCBCR format is selected, we can merely copy the colorimetry
information directly as we use the same definitions for both the
unpacked AVI info frame and the hdmi_data_info structure.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/bridge/dw_hdmi.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 44c63883db19..fd02a220ad5b 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -944,10 +944,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 			frame.extended_colorimetry =
 				HDMI_EXTENDED_COLORIMETRY_XV_YCC_709;
 	} else if (hdmi->hdmi_data.enc_out_format != RGB) {
-		if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601)
-			frame.colorimetry = HDMI_COLORIMETRY_ITU_601;
-		else /*hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_709*/
-			frame.colorimetry = HDMI_COLORIMETRY_ITU_709;
+		frame.colorimetry = hdmi->hdmi_data.colorimetry;
 		frame.extended_colorimetry = HDMI_EXTENDED_COLORIMETRY_XV_YCC_601;
 	} else { /* Carries no data */
 		frame.colorimetry = HDMI_COLORIMETRY_NONE;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH RFC 04/11] drm: bridge/dw_hdmi: remove mhsyncpolarity/mvsyncpolarity/minterlaced
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
                   ` (2 preceding siblings ...)
  2015-03-30 19:40 ` [PATCH RFC 03/11] drm: bridge/dw_hdmi: simplify hdmi_config_AVI() a little Russell King
@ 2015-03-30 19:40 ` Russell King
  2015-03-30 19:40 ` [PATCH RFC 05/11] drm: bridge/dw_hdmi: introduce interface to setting sample rate Russell King
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel, Yakir Yang

Remove the struct hdmi_vmode mhsyncpolarity/mvsyncpolarity/minterlaced
members, which are only used within a single function.  We can directly
reference the appropriate mode->flags instead.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/bridge/dw_hdmi.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index fd02a220ad5b..06b5583140a8 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -82,9 +82,6 @@ static const u16 csc_coeff_rgb_in_eitu709[3][4] = {
 
 struct hdmi_vmode {
 	bool mdvi;
-	bool mhsyncpolarity;
-	bool mvsyncpolarity;
-	bool minterlaced;
 	bool mdataenablepolarity;
 
 	unsigned int mpixelclock;
@@ -1025,9 +1022,6 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
 	struct hdmi_vmode *vmode = &hdmi->hdmi_data.video_mode;
 	int hblank, vblank, h_de_hs, v_de_vs, hsync_len, vsync_len;
 
-	vmode->mhsyncpolarity = !!(mode->flags & DRM_MODE_FLAG_PHSYNC);
-	vmode->mvsyncpolarity = !!(mode->flags & DRM_MODE_FLAG_PVSYNC);
-	vmode->minterlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE);
 	vmode->mpixelclock = mode->clock * 1000;
 
 	dev_dbg(hdmi->dev, "final pixclk = %d\n", vmode->mpixelclock);
@@ -1037,13 +1031,13 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
 		HDMI_FC_INVIDCONF_HDCP_KEEPOUT_ACTIVE :
 		HDMI_FC_INVIDCONF_HDCP_KEEPOUT_INACTIVE);
 
-	inv_val |= (vmode->mvsyncpolarity ?
+	inv_val |= mode->flags & DRM_MODE_FLAG_PVSYNC ?
 		HDMI_FC_INVIDCONF_VSYNC_IN_POLARITY_ACTIVE_HIGH :
-		HDMI_FC_INVIDCONF_VSYNC_IN_POLARITY_ACTIVE_LOW);
+		HDMI_FC_INVIDCONF_VSYNC_IN_POLARITY_ACTIVE_LOW;
 
-	inv_val |= (vmode->mhsyncpolarity ?
+	inv_val |= mode->flags & DRM_MODE_FLAG_PHSYNC ?
 		HDMI_FC_INVIDCONF_HSYNC_IN_POLARITY_ACTIVE_HIGH :
-		HDMI_FC_INVIDCONF_HSYNC_IN_POLARITY_ACTIVE_LOW);
+		HDMI_FC_INVIDCONF_HSYNC_IN_POLARITY_ACTIVE_LOW;
 
 	inv_val |= (vmode->mdataenablepolarity ?
 		HDMI_FC_INVIDCONF_DE_IN_POLARITY_ACTIVE_HIGH :
@@ -1052,13 +1046,13 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
 	if (hdmi->vic == 39)
 		inv_val |= HDMI_FC_INVIDCONF_R_V_BLANK_IN_OSC_ACTIVE_HIGH;
 	else
-		inv_val |= (vmode->minterlaced ?
+		inv_val |= mode->flags & DRM_MODE_FLAG_INTERLACE ?
 			HDMI_FC_INVIDCONF_R_V_BLANK_IN_OSC_ACTIVE_HIGH :
-			HDMI_FC_INVIDCONF_R_V_BLANK_IN_OSC_ACTIVE_LOW);
+			HDMI_FC_INVIDCONF_R_V_BLANK_IN_OSC_ACTIVE_LOW;
 
-	inv_val |= (vmode->minterlaced ?
+	inv_val |= mode->flags & DRM_MODE_FLAG_INTERLACE ?
 		HDMI_FC_INVIDCONF_IN_I_P_INTERLACED :
-		HDMI_FC_INVIDCONF_IN_I_P_PROGRESSIVE);
+		HDMI_FC_INVIDCONF_IN_I_P_PROGRESSIVE;
 
 	inv_val |= (vmode->mdvi ?
 		HDMI_FC_INVIDCONF_DVI_MODEZ_DVI_MODE :
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH RFC 05/11] drm: bridge/dw_hdmi: introduce interface to setting sample rate
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
                   ` (3 preceding siblings ...)
  2015-03-30 19:40 ` [PATCH RFC 04/11] drm: bridge/dw_hdmi: remove mhsyncpolarity/mvsyncpolarity/minterlaced Russell King
@ 2015-03-30 19:40 ` Russell King
  2015-03-30 19:40 ` [PATCH RFC 06/11] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio Russell King
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel, Yakir Yang

Introduce dw_hdmi_set_sample_rate(), which allows us to configure the
audio sample rate, setting the CTS/N values appropriately.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/bridge/dw_hdmi.c | 10 ++++++++++
 include/drm/bridge/dw_hdmi.h     |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 06b5583140a8..245d17e58dec 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -366,6 +366,16 @@ static void hdmi_clk_regenerator_update_pixel_clock(struct dw_hdmi *hdmi)
 	mutex_unlock(&hdmi->audio_mutex);
 }
 
+void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
+{
+	mutex_lock(&hdmi->audio_mutex);
+	hdmi->sample_rate = rate;
+	hdmi_set_clk_regenerator(hdmi, hdmi->hdmi_data.video_mode.mpixelclock,
+				 hdmi->sample_rate, hdmi->ratio);
+	mutex_unlock(&hdmi->audio_mutex);
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
+
 /*
  * this submodule is responsible for the video data synchronization.
  * for example, for RGB 4:4:4 input, the data map is defined as
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 5a4f49005169..424b51f6a215 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -12,6 +12,8 @@
 
 #include <drm/drmP.h>
 
+struct dw_hdmi;
+
 enum {
 	DW_HDMI_RES_8,
 	DW_HDMI_RES_10,
@@ -58,4 +60,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 		 void *data, struct drm_encoder *encoder,
 		 struct resource *iores, int irq,
 		 const struct dw_hdmi_plat_data *plat_data);
+
+void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
+
 #endif /* __IMX_HDMI_H__ */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH RFC 06/11] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
                   ` (4 preceding siblings ...)
  2015-03-30 19:40 ` [PATCH RFC 05/11] drm: bridge/dw_hdmi: introduce interface to setting sample rate Russell King
@ 2015-03-30 19:40 ` Russell King
  2015-03-31  7:45   ` Yang Kuankuan
  2015-03-31  9:15   ` Philipp Zabel
  2015-03-30 19:40 ` [PATCH RFC 07/11] drm/edid: add function to help find SADs Russell King
                   ` (4 subsequent siblings)
  10 siblings, 2 replies; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel, Yakir Yang

iMX6 devices from an errata (ERR005174) where the audio FIFO can be
emptied while it is partially full, resulting in misalignment of the
audio samples.

To prevent this, the errata workaround recommends writing N as zero
until the audio FIFO has been loaded by DMA.  Writing N=0 prevents the
HDMI bridge from reading from the audio FIFO, effectively disabling
audio.

This means we need to provide the audio driver with a pair of functions
to enable/disable audio.  These are dw_hdmi_audio_enable() and
dw_hdmi_audio_disable().

A spinlock is introduced to ensure that setting the CTS/N values can't
race, ensuring that the audio driver calling the enable/disable
functions (which are called in an atomic context) can't race with a
modeset.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/bridge/dw_hdmi.c | 34 +++++++++++++++++++++++++++++++++-
 include/drm/bridge/dw_hdmi.h     |  2 ++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 245d17e58dec..67f07d15c12b 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -18,6 +18,7 @@
 #include <linux/hdmi.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
+#include <linux/spinlock.h>
 
 #include <drm/drm_of.h>
 #include <drm/drmP.h>
@@ -124,8 +125,12 @@ struct dw_hdmi {
 	struct i2c_adapter *ddc;
 	void __iomem *regs;
 
+	spinlock_t audio_lock;
 	struct mutex audio_mutex;
 	unsigned int sample_rate;
+	unsigned int audio_cts;
+	unsigned int audio_n;
+	bool audio_enable;
 	int ratio;
 
 	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
@@ -347,7 +352,11 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
 	dev_dbg(hdmi->dev, "%s: samplerate=%ukHz ratio=%d pixelclk=%luMHz N=%d cts=%d\n",
 		__func__, sample_rate, ratio, pixel_clk, n, cts);
 
-	hdmi_set_cts_n(hdmi, cts, n);
+	spin_lock_irq(&hdmi->audio_lock);
+	hdmi->audio_n = n;
+	hdmi->audio_cts = cts;
+	hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
+	spin_unlock_irq(&hdmi->audio_lock);
 }
 
 static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
@@ -376,6 +385,28 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
 
+void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&hdmi->audio_lock, flags);
+	hdmi->audio_enable = true;
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable);
+
+void dw_hdmi_audio_disable(struct dw_hdmi *hdmi)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&hdmi->audio_lock, flags);
+	hdmi->audio_enable = false;
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
+	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);
+
 /*
  * this submodule is responsible for the video data synchronization.
  * for example, for RGB 4:4:4 input, the data map is defined as
@@ -1579,6 +1610,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 	hdmi->encoder = encoder;
 
 	mutex_init(&hdmi->audio_mutex);
+	spin_lock_init(&hdmi->audio_lock);
 
 	of_property_read_u32(np, "reg-io-width", &val);
 
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 424b51f6a215..ab5d36e83ea2 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -62,5 +62,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 		 const struct dw_hdmi_plat_data *plat_data);
 
 void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
+void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
+void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
 
 #endif /* __IMX_HDMI_H__ */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH RFC 07/11] drm/edid: add function to help find SADs
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
                   ` (5 preceding siblings ...)
  2015-03-30 19:40 ` [PATCH RFC 06/11] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio Russell King
@ 2015-03-30 19:40 ` Russell King
  2015-04-01 11:47   ` Jani Nikula
  2015-03-30 19:40 ` [PATCH RFC 08/11] sound/core: add DRM ELD helper Russell King
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel, Yakir Yang

Add a function to find the start of the SADs in the ELD.  This
complements the helper to retrieve the SAD count.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 include/drm/drm_edid.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 87d85e81d3a7..c44ad513e0f7 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -346,6 +346,25 @@ static inline int drm_eld_mnl(const uint8_t *eld)
 }
 
 /**
+ * drm_eld_sad - Get ELD SAD structures.
+ * @eld: pointer to an eld memory structure with sad_count set
+ */
+static inline const uint8_t *drm_eld_sad(const uint8_t *eld)
+{
+	unsigned int ver, mnl;
+
+	ver = (eld[DRM_ELD_VER] & DRM_ELD_VER_MASK) >> DRM_ELD_VER_SHIFT;
+	if (ver != 2 && ver != 31)
+		return NULL;
+
+	mnl = drm_eld_mnl(eld);
+	if (mnl > 16)
+		return NULL;
+
+	return eld + DRM_ELD_CEA_SAD(mnl, 0);
+}
+
+/**
  * drm_eld_sad_count - Get ELD SAD count.
  * @eld: pointer to an eld memory structure with sad_count set
  */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH RFC 08/11] sound/core: add DRM ELD helper
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
                   ` (6 preceding siblings ...)
  2015-03-30 19:40 ` [PATCH RFC 07/11] drm/edid: add function to help find SADs Russell King
@ 2015-03-30 19:40 ` Russell King
  2015-03-31  9:12   ` Philipp Zabel
  2015-03-30 19:40 ` [PATCH RFC 09/11] sound/core: add IEC958 channel status helper Russell King
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, Takashi Iwai, Mark Brown, Philipp Zabel, Yakir Yang

Add a helper for the EDID like data structure, which is typically passed
from a HDMI adapter to its associated audio driver.  This informs the
audio driver of the capabilities of the attached HDMI sink.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 include/sound/pcm_drm_eld.h |  6 +++
 sound/core/Kconfig          |  3 ++
 sound/core/Makefile         |  1 +
 sound/core/pcm_drm_eld.c    | 91 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 101 insertions(+)
 create mode 100644 include/sound/pcm_drm_eld.h
 create mode 100644 sound/core/pcm_drm_eld.c

diff --git a/include/sound/pcm_drm_eld.h b/include/sound/pcm_drm_eld.h
new file mode 100644
index 000000000000..93357b25d2e2
--- /dev/null
+++ b/include/sound/pcm_drm_eld.h
@@ -0,0 +1,6 @@
+#ifndef __SOUND_PCM_DRM_ELD_H
+#define __SOUND_PCM_DRM_ELD_H
+
+int snd_pcm_hw_constraint_eld(struct snd_pcm_runtime *runtime, void *eld);
+
+#endif
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index 313f22e9d929..b534c8a6046b 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -6,6 +6,9 @@ config SND_PCM
 	tristate
 	select SND_TIMER
 
+config SND_PCM_ELD
+	bool
+
 config SND_DMAENGINE_PCM
 	tristate
 
diff --git a/sound/core/Makefile b/sound/core/Makefile
index 4daf2f58261c..591b49157b4d 100644
--- a/sound/core/Makefile
+++ b/sound/core/Makefile
@@ -13,6 +13,7 @@ snd-$(CONFIG_SND_JACK)	  += jack.o
 snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
 		pcm_memory.o memalloc.o
 snd-pcm-$(CONFIG_SND_DMA_SGBUF) += sgbuf.o
+snd-pcm-$(CONFIG_SND_PCM_ELD) += pcm_drm_eld.o
 
 # for trace-points
 CFLAGS_pcm_lib.o := -I$(src)
diff --git a/sound/core/pcm_drm_eld.c b/sound/core/pcm_drm_eld.c
new file mode 100644
index 000000000000..78d551a1b05f
--- /dev/null
+++ b/sound/core/pcm_drm_eld.c
@@ -0,0 +1,91 @@
+/*
+ *  PCM DRM helpers
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License.
+ */
+#include <linux/export.h>
+#include <drm/drm_edid.h>
+#include <sound/pcm.h>
+#include <sound/pcm_drm_eld.h>
+
+static const unsigned int eld_rates[] = {
+	32000,
+	44100,
+	48000,
+	88200,
+	96000,
+	176400,
+	192000,
+};
+
+static int eld_limit_rates(struct snd_pcm_hw_params *params,
+			   struct snd_pcm_hw_rule *rule)
+{
+	struct snd_interval *r = hw_param_interval(params, rule->var);
+	struct snd_interval *c;
+	unsigned int rate_mask = 7, i;
+	const u8 *sad, *eld = rule->private;
+
+	sad = drm_eld_sad(eld);
+	if (sad) {
+		c = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+
+		for (i = drm_eld_sad_count(eld); i > 0; i--, sad += 3) {
+			unsigned channels = 1 + (sad[0] & 7);
+
+			/*
+			 * Exclude SADs which do not include the
+			 * requested number of channels.
+			 */
+			if (c->min <= channels)
+				rate_mask |= sad[1];
+		}
+	}
+
+	return snd_interval_list(r, ARRAY_SIZE(eld_rates), eld_rates,
+				 rate_mask);
+}
+
+static int eld_limit_channels(struct snd_pcm_hw_params *params,
+			      struct snd_pcm_hw_rule *rule)
+{
+	struct snd_interval *var = hw_param_interval(params, rule->var);
+	struct snd_interval t = { .min = 1, .max = 2, .integer = 1, };
+	unsigned int i;
+	const u8 *sad, *eld = rule->private;
+
+	sad = drm_eld_sad(eld);
+	if (sad) {
+		for (i = drm_eld_sad_count(eld); i > 0; i--, sad += 3) {
+			switch (sad[0] & 0x78) {
+			case 0x08:
+				t.max = max(t.max, (sad[0] & 7) + 1u);
+				break;
+			}
+		}
+	}
+
+	return snd_interval_refine(var, &t);
+}
+
+int snd_pcm_hw_constraint_eld(struct snd_pcm_runtime *runtime, void *eld)
+{
+	int ret;
+
+	ret = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+				  eld_limit_rates, eld,
+				  SNDRV_PCM_HW_PARAM_RATE,
+				  SNDRV_PCM_HW_PARAM_CHANNELS, -1);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+				  eld_limit_channels, eld,
+				  SNDRV_PCM_HW_PARAM_CHANNELS,
+				  SNDRV_PCM_HW_PARAM_RATE, -1);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_pcm_hw_constraint_eld);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH RFC 09/11] sound/core: add IEC958 channel status helper
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
                   ` (7 preceding siblings ...)
  2015-03-30 19:40 ` [PATCH RFC 08/11] sound/core: add DRM ELD helper Russell King
@ 2015-03-30 19:40 ` Russell King
  2015-03-31  8:30   ` Yang Kuankuan
  2015-03-31  9:10   ` Philipp Zabel
  2015-03-30 19:40 ` [PATCH RFC 10/11] drm: bridge/dw_hdmi-ahb-audio: add audio driver Russell King
  2015-03-30 19:40 ` [PATCH RFC 11/11] drm: bridge/dw_hdmi-ahb-audio: parse ELD from HDMI driver Russell King
  10 siblings, 2 replies; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, Takashi Iwai, Mark Brown, Philipp Zabel, Yakir Yang

Add a helper to create the IEC958 channel status from an ALSA
snd_pcm_runtime structure, taking account of the sample rate.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 include/sound/pcm_iec958.h |  9 ++++++
 sound/core/Kconfig         |  3 ++
 sound/core/Makefile        |  2 ++
 sound/core/pcm_iec958.c    | 70 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 84 insertions(+)
 create mode 100644 include/sound/pcm_iec958.h
 create mode 100644 sound/core/pcm_iec958.c

diff --git a/include/sound/pcm_iec958.h b/include/sound/pcm_iec958.h
new file mode 100644
index 000000000000..0eed397aca8e
--- /dev/null
+++ b/include/sound/pcm_iec958.h
@@ -0,0 +1,9 @@
+#ifndef __SOUND_PCM_IEC958_H
+#define __SOUND_PCM_IEC958_H
+
+#include <linux/types.h>
+
+int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
+	size_t len);
+
+#endif
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index b534c8a6046b..1507469425ec 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -9,6 +9,9 @@ config SND_PCM
 config SND_PCM_ELD
 	bool
 
+config SND_PCM_IEC958
+	bool
+
 config SND_DMAENGINE_PCM
 	tristate
 
diff --git a/sound/core/Makefile b/sound/core/Makefile
index 591b49157b4d..70ea06712ec2 100644
--- a/sound/core/Makefile
+++ b/sound/core/Makefile
@@ -14,10 +14,12 @@ snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
 		pcm_memory.o memalloc.o
 snd-pcm-$(CONFIG_SND_DMA_SGBUF) += sgbuf.o
 snd-pcm-$(CONFIG_SND_PCM_ELD) += pcm_drm_eld.o
+snd-pcm-$(CONFIG_SND_PCM_IEC958) += snd-pcm-iec958.o
 
 # for trace-points
 CFLAGS_pcm_lib.o := -I$(src)
 
+snd-pcm-iec958-objs := pcm_iec958.o
 snd-pcm-dmaengine-objs := pcm_dmaengine.o
 
 snd-rawmidi-objs  := rawmidi.o
diff --git a/sound/core/pcm_iec958.c b/sound/core/pcm_iec958.c
new file mode 100644
index 000000000000..e1ff88a17dde
--- /dev/null
+++ b/sound/core/pcm_iec958.c
@@ -0,0 +1,70 @@
+/*
+ *  PCM DRM helpers
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License.
+ */
+#include <linux/export.h>
+#include <linux/types.h>
+#include <sound/asoundef.h>
+#include <sound/pcm.h>
+#include <sound/pcm_iec958.h>
+
+/**
+ * snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status
+ * @runtime: pcm runtime structure with ->rate filled in
+ * @cs: channel status buffer, at least four bytes
+ * @len: length of channel status buffer
+ *
+ * Create the consumer format channel status data in @cs of maximum size
+ * @len corresponding to the parameters of the PCM runtime @runtime.
+ *
+ * Drivers may wish to tweak the contents of the buffer after creation.
+ *
+ * Returns: length of buffer, or negative error code if something failed.
+ */
+int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
+	size_t len)
+{
+	unsigned int fs;
+
+	if (len < 4)
+		return -EINVAL;
+
+	switch (runtime->rate) {
+	case 32000:
+		fs = IEC958_AES3_CON_FS_32000;
+		break;
+	case 44100:
+		fs = IEC958_AES3_CON_FS_44100;
+		break;
+	case 48000:
+		fs = IEC958_AES3_CON_FS_48000;
+		break;
+	case 88200:
+		fs = IEC958_AES3_CON_FS_88200;
+		break;
+	case 96000:
+		fs = IEC958_AES3_CON_FS_96000;
+		break;
+	case 176400:
+		fs = IEC958_AES3_CON_FS_176400;
+		break;
+	case 192000:
+		fs = IEC958_AES3_CON_FS_192000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	memset(cs, 0, len);
+
+	cs[0] = IEC958_AES0_CON_NOT_COPYRIGHT | IEC958_AES0_CON_EMPHASIS_NONE;
+	cs[1] = IEC958_AES1_CON_GENERAL;
+	cs[2] = IEC958_AES2_CON_SOURCE_UNSPEC | IEC958_AES2_CON_CHANNEL_UNSPEC;
+	cs[3] = IEC958_AES3_CON_CLOCK_1000PPM | fs;
+
+	return len;
+}
+EXPORT_SYMBOL(snd_pcm_create_iec958_consumer);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH RFC 10/11] drm: bridge/dw_hdmi-ahb-audio: add audio driver
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
                   ` (8 preceding siblings ...)
  2015-03-30 19:40 ` [PATCH RFC 09/11] sound/core: add IEC958 channel status helper Russell King
@ 2015-03-30 19:40 ` Russell King
  2015-03-30 19:40 ` [PATCH RFC 11/11] drm: bridge/dw_hdmi-ahb-audio: parse ELD from HDMI driver Russell King
  10 siblings, 0 replies; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel, Yakir Yang

Add ALSA based HDMI AHB audio driver for dw_hdmi.  The only buffer
format supported by the hardware is its own special IEC958 based format,
which is not compatible with any ALSA format.  To avoid doing too much
data manipulation within the driver, we support only ALSAs IEC958 LE and
24-bit PCM formats for 2 to 6 channels, which we convert to its hardware
format.

A more desirable solution would be to have this conversion in userspace,
but ALSA does not appear to allow such transformations outside of
libasound itself.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/bridge/Kconfig             |  10 +
 drivers/gpu/drm/bridge/Makefile            |   1 +
 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c | 560 +++++++++++++++++++++++++++++
 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h |  13 +
 drivers/gpu/drm/bridge/dw_hdmi.c           |  24 ++
 drivers/gpu/drm/bridge/dw_hdmi.h           |   3 +
 6 files changed, 611 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c
 create mode 100644 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index f38bbcdf929b..557962a67fc5 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -3,6 +3,16 @@ config DRM_DW_HDMI
 	depends on DRM
 	select DRM_KMS_HELPER
 
+config DRM_DW_HDMI_AHB_AUDIO
+	tristate "Synopsis Designware AHB Audio interface"
+	depends on DRM_DW_HDMI && SND
+	select SND_PCM
+	select SND_PCM_IEC958
+	help
+	  Support the AHB Audio interface which is part of the Synopsis
+	  Designware HDMI block.  This is used in conjunction with
+	  the i.MX6 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 d8a8cfd12fbb..ebc51905f1df 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -2,3 +2,4 @@ ccflags-y := -Iinclude/drm
 
 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
diff --git a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c
new file mode 100644
index 000000000000..e98c291268f4
--- /dev/null
+++ b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c
@@ -0,0 +1,560 @@
+/*
+ * 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 iMX6.
+ */
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <drm/bridge/dw_hdmi.h>
+
+#include <sound/asoundef.h>
+#include <sound/core.h>
+#include <sound/initval.h>
+#include <sound/pcm.h>
+#include <sound/pcm_iec958.h>
+
+#include "dw_hdmi-ahb-audio.h"
+
+#define DRIVER_NAME "dw-hdmi-ahb-audio"
+
+/* Provide some bits rather than bit offsets */
+enum {
+	HDMI_AHB_DMA_CONF0_SW_FIFO_RST = BIT(7),
+	HDMI_AHB_DMA_CONF0_EN_HLOCK = BIT(3),
+	HDMI_AHB_DMA_START_START = BIT(0),
+	HDMI_AHB_DMA_STOP_STOP = BIT(0),
+	HDMI_IH_MUTE_AHBDMAAUD_STAT0_ERROR = BIT(5),
+	HDMI_IH_MUTE_AHBDMAAUD_STAT0_LOST = BIT(4),
+	HDMI_IH_MUTE_AHBDMAAUD_STAT0_RETRY = BIT(3),
+	HDMI_IH_MUTE_AHBDMAAUD_STAT0_DONE = BIT(2),
+	HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFFULL = BIT(1),
+	HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFEMPTY = BIT(0),
+	HDMI_IH_MUTE_AHBDMAAUD_STAT0_ALL =
+		HDMI_IH_MUTE_AHBDMAAUD_STAT0_ERROR |
+		HDMI_IH_MUTE_AHBDMAAUD_STAT0_LOST |
+		HDMI_IH_MUTE_AHBDMAAUD_STAT0_RETRY |
+		HDMI_IH_MUTE_AHBDMAAUD_STAT0_DONE |
+		HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFFULL |
+		HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFEMPTY,
+	HDMI_IH_AHBDMAAUD_STAT0_ERROR = BIT(5),
+	HDMI_IH_AHBDMAAUD_STAT0_LOST = BIT(4),
+	HDMI_IH_AHBDMAAUD_STAT0_RETRY = BIT(3),
+	HDMI_IH_AHBDMAAUD_STAT0_DONE = BIT(2),
+	HDMI_IH_AHBDMAAUD_STAT0_BUFFFULL = BIT(1),
+	HDMI_IH_AHBDMAAUD_STAT0_BUFFEMPTY = BIT(0),
+	HDMI_IH_AHBDMAAUD_STAT0_ALL =
+		HDMI_IH_AHBDMAAUD_STAT0_ERROR |
+		HDMI_IH_AHBDMAAUD_STAT0_LOST |
+		HDMI_IH_AHBDMAAUD_STAT0_RETRY |
+		HDMI_IH_AHBDMAAUD_STAT0_DONE |
+		HDMI_IH_AHBDMAAUD_STAT0_BUFFFULL |
+		HDMI_IH_AHBDMAAUD_STAT0_BUFFEMPTY,
+	HDMI_AHB_DMA_CONF0_INCR16 = 2 << 1,
+	HDMI_AHB_DMA_CONF0_INCR8 = 1 << 1,
+	HDMI_AHB_DMA_CONF0_INCR4 = 0,
+	HDMI_AHB_DMA_CONF0_BURST_MODE = BIT(0),
+	HDMI_AHB_DMA_MASK_DONE = BIT(7),
+	HDMI_REVISION_ID = 0x0001,
+	HDMI_IH_AHBDMAAUD_STAT0 = 0x0109,
+	HDMI_IH_MUTE_AHBDMAAUD_STAT0 = 0x0189,
+	HDMI_AHB_DMA_CONF0 = 0x3600,
+	HDMI_AHB_DMA_START = 0x3601,
+	HDMI_AHB_DMA_STOP = 0x3602,
+	HDMI_AHB_DMA_THRSLD = 0x3603,
+	HDMI_AHB_DMA_STRADDR0 = 0x3604,
+	HDMI_AHB_DMA_STPADDR0 = 0x3608,
+	HDMI_AHB_DMA_MASK = 0x3614,
+	HDMI_AHB_DMA_POL = 0x3615,
+	HDMI_AHB_DMA_CONF1 = 0x3616,
+	HDMI_AHB_DMA_BUFFPOL = 0x361a,
+};
+
+struct snd_dw_hdmi {
+	struct snd_card *card;
+	struct snd_pcm *pcm;
+	struct dw_hdmi_audio_data data;
+	struct snd_pcm_substream *substream;
+	void (*reformat)(struct snd_dw_hdmi *, size_t, size_t);
+	void *buf_src;
+	void *buf_dst;
+	dma_addr_t buf_addr;
+	unsigned buf_offset;
+	unsigned buf_period;
+	unsigned buf_size;
+	unsigned channels;
+	u8 revision;
+	u8 iec_offset;
+	u8 cs[192][8];
+};
+
+static void dw_hdmi_writel(unsigned long val, void __iomem *ptr)
+{
+	writeb_relaxed(val, ptr);
+	writeb_relaxed(val >> 8, ptr + 1);
+	writeb_relaxed(val >> 16, ptr + 2);
+	writeb_relaxed(val >> 24, ptr + 3);
+}
+
+/*
+ * Convert to hardware format: The userspace buffer contains IEC958 samples,
+ * with the PCUV bits in bits 31..28 and audio samples in bits 27..4.  We
+ * need these to be in bits 27..24, with the IEC B bit in bit 28, and audio
+ * samples in 23..0.
+ *
+ * Default preamble in bits 3..0: 8 = block start, 4 = even 2 = odd
+ *
+ * Ideally, we could do with having the data properly formatted in userspace.
+ */
+static void dw_hdmi_reformat_iec958(struct snd_dw_hdmi *dw,
+	size_t offset, size_t bytes)
+{
+	u32 *src = dw->buf_src + offset;
+	u32 *dst = dw->buf_dst + offset;
+	u32 *end = dw->buf_src + offset + bytes;
+
+	do {
+		u32 b, sample = *src++;
+
+		b = (sample & 8) << (28 - 3);
+
+		sample >>= 4;
+
+		*dst++ = sample | b;
+	} while (src < end);
+}
+
+static u32 parity(u32 sample)
+{
+	sample ^= sample >> 16;
+	sample ^= sample >> 8;
+	sample ^= sample >> 4;
+	sample ^= sample >> 2;
+	sample ^= sample >> 1;
+	return (sample & 1) << 27;
+}
+
+static void dw_hdmi_reformat_s24(struct snd_dw_hdmi *dw,
+	size_t offset, size_t bytes)
+{
+	u32 *src = dw->buf_src + offset;
+	u32 *dst = dw->buf_dst + offset;
+	u32 *end = dw->buf_src + offset + bytes;
+
+	do {
+		unsigned i;
+		u8 *cs;
+
+		cs = dw->cs[dw->iec_offset++];
+		if (dw->iec_offset >= 192)
+			dw->iec_offset = 0;
+
+		i = dw->channels;
+		do {
+			u32 sample = *src++;
+
+			sample &= ~0xff000000;
+			sample |= *cs++ << 24;
+			sample |= parity(sample & ~0xf8000000);
+
+			*dst++ = sample;
+		} while (--i);
+	} while (src < end);
+}
+
+static void dw_hdmi_create_cs(struct snd_dw_hdmi *dw,
+	struct snd_pcm_runtime *runtime)
+{
+	u8 cs[4];
+	unsigned ch, i, j;
+
+	snd_pcm_create_iec958_consumer(runtime, cs, sizeof(cs));
+
+	memset(dw->cs, 0, sizeof(dw->cs));
+
+	for (ch = 0; ch < 8; ch++) {
+		cs[2] &= ~IEC958_AES2_CON_CHANNEL;
+		cs[2] |= (ch + 1) << 4;
+
+		for (i = 0; i < ARRAY_SIZE(cs); i++) {
+			unsigned c = cs[i];
+
+			for (j = 0; j < 8; j++, c >>= 1)
+				dw->cs[i * 8 + j][ch] = (c & 1) << 2;
+		}
+	}
+	dw->cs[0][0] |= BIT(4);
+}
+
+static void dw_hdmi_start_dma(struct snd_dw_hdmi *dw)
+{
+	void __iomem *base = dw->data.base;
+	unsigned offset = dw->buf_offset;
+	unsigned period = dw->buf_period;
+	u32 start, stop;
+
+	dw->reformat(dw, offset, period);
+
+	/* Clear all irqs before enabling irqs and starting DMA */
+	writeb_relaxed(HDMI_IH_AHBDMAAUD_STAT0_ALL,
+		       base + HDMI_IH_AHBDMAAUD_STAT0);
+
+	start = dw->buf_addr + offset;
+	stop = start + period - 1;
+
+	/* Setup the hardware start/stop addresses */
+	dw_hdmi_writel(start, base + HDMI_AHB_DMA_STRADDR0);
+	dw_hdmi_writel(stop, base + HDMI_AHB_DMA_STPADDR0);
+
+	writeb_relaxed((u8)~HDMI_AHB_DMA_MASK_DONE, base + HDMI_AHB_DMA_MASK);
+	writeb(HDMI_AHB_DMA_START_START, base + HDMI_AHB_DMA_START);
+
+	offset += period;
+	if (offset >= dw->buf_size)
+		offset = 0;
+	dw->buf_offset = offset;
+}
+
+static void dw_hdmi_stop_dma(struct snd_dw_hdmi *dw)
+{
+	dw->substream = NULL;
+
+	/* Disable interrupts before disabling DMA */
+	writeb_relaxed(~0, dw->data.base + HDMI_AHB_DMA_MASK);
+	writeb_relaxed(HDMI_AHB_DMA_STOP_STOP, dw->data.base + HDMI_AHB_DMA_STOP);
+}
+
+static irqreturn_t snd_dw_hdmi_irq(int irq, void *data)
+{
+	struct snd_dw_hdmi *dw = data;
+	struct snd_pcm_substream *substream;
+	unsigned stat;
+
+	stat = readb_relaxed(dw->data.base + HDMI_IH_AHBDMAAUD_STAT0);
+	if (!stat)
+		return IRQ_NONE;
+
+	writeb_relaxed(stat, dw->data.base + HDMI_IH_AHBDMAAUD_STAT0);
+
+	substream = dw->substream;
+	if (stat & HDMI_IH_AHBDMAAUD_STAT0_DONE && substream) {
+		snd_pcm_period_elapsed(substream);
+		if (dw->substream)
+			dw_hdmi_start_dma(dw);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static struct snd_pcm_hardware dw_hdmi_hw = {
+	.info = SNDRV_PCM_INFO_INTERLEAVED |
+		SNDRV_PCM_INFO_BLOCK_TRANSFER |
+		SNDRV_PCM_INFO_MMAP |
+		SNDRV_PCM_INFO_MMAP_VALID,
+	.formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE |
+		   SNDRV_PCM_FMTBIT_S24_LE,
+	.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,
+	.channels_min = 2,
+	.channels_max = 8,
+	.buffer_bytes_max = 64 * 1024,
+	.period_bytes_min = 256,
+	.period_bytes_max = 8192,	/* ERR004323: must limit to 8k */
+	.periods_min = 2,
+	.periods_max = 16,
+	.fifo_size = 0,
+};
+
+static int dw_hdmi_open(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct snd_dw_hdmi *dw = substream->private_data;
+	void __iomem *base = dw->data.base;
+	int ret;
+
+	runtime->hw = dw_hdmi_hw;
+
+	ret = snd_pcm_limit_hw_rates(runtime);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
+	if (ret < 0)
+		return ret;
+
+	/* Clear FIFO */
+	writeb_relaxed(HDMI_AHB_DMA_CONF0_SW_FIFO_RST,
+		       base + HDMI_AHB_DMA_CONF0);
+
+	/* Configure interrupt polarities */
+	writeb_relaxed(~0, base + HDMI_AHB_DMA_POL);
+	writeb_relaxed(~0, base + HDMI_AHB_DMA_BUFFPOL);
+
+	/* Keep interrupts masked, and clear any pending */
+	writeb_relaxed(~0, base + HDMI_AHB_DMA_MASK);
+	writeb_relaxed(~0, base + HDMI_IH_AHBDMAAUD_STAT0);
+
+	ret = request_irq(dw->data.irq, snd_dw_hdmi_irq, IRQF_SHARED,
+			  "dw-hdmi-audio", dw);
+	if (ret)
+		return ret;
+
+	/* Un-mute done interrupt */
+	writeb_relaxed(HDMI_IH_MUTE_AHBDMAAUD_STAT0_ALL &
+		       ~HDMI_IH_MUTE_AHBDMAAUD_STAT0_DONE,
+		       base + HDMI_IH_MUTE_AHBDMAAUD_STAT0);
+
+	return 0;
+}
+
+static int dw_hdmi_close(struct snd_pcm_substream *substream)
+{
+	struct snd_dw_hdmi *dw = substream->private_data;
+
+	/* Mute all interrupts */
+	writeb_relaxed(HDMI_IH_MUTE_AHBDMAAUD_STAT0_ALL,
+		       dw->data.base + HDMI_IH_MUTE_AHBDMAAUD_STAT0);
+
+	free_irq(dw->data.irq, dw);
+
+	return 0;
+}
+
+static int dw_hdmi_hw_free(struct snd_pcm_substream *substream)
+{
+	return snd_pcm_lib_free_vmalloc_buffer(substream);
+}
+
+static int dw_hdmi_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params)
+{
+	return snd_pcm_lib_alloc_vmalloc_buffer(substream,
+						params_buffer_bytes(params));
+}
+
+static int dw_hdmi_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct snd_dw_hdmi *dw = substream->private_data;
+	u8 threshold, conf0, conf1;
+
+	/* Setup as per 3.0.5 FSL 4.1.0 BSP */
+	switch (dw->revision) {
+	case 0x0a:
+		conf0 = HDMI_AHB_DMA_CONF0_BURST_MODE |
+			HDMI_AHB_DMA_CONF0_INCR4;
+		if (runtime->channels == 2)
+			threshold = 126;
+		else
+			threshold = 124;
+		break;
+	case 0x1a:
+		conf0 = HDMI_AHB_DMA_CONF0_BURST_MODE |
+			HDMI_AHB_DMA_CONF0_INCR8;
+		threshold = 128;
+		break;
+	default:
+		/* NOTREACHED */
+		return -EINVAL;
+	}
+
+	dw_hdmi_set_sample_rate(dw->data.hdmi, runtime->rate);
+
+	/* Minimum number of bytes in the fifo. */
+	runtime->hw.fifo_size = threshold * 32;
+
+	conf0 |= HDMI_AHB_DMA_CONF0_EN_HLOCK;
+	conf1 = (1 << runtime->channels) - 1;
+
+	writeb_relaxed(threshold, dw->data.base + HDMI_AHB_DMA_THRSLD);
+	writeb_relaxed(conf0, dw->data.base + HDMI_AHB_DMA_CONF0);
+	writeb_relaxed(conf1, dw->data.base + HDMI_AHB_DMA_CONF1);
+
+	switch (runtime->format) {
+	case SNDRV_PCM_FORMAT_IEC958_SUBFRAME_LE:
+		dw->reformat = dw_hdmi_reformat_iec958;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		dw_hdmi_create_cs(dw, runtime);
+		dw->reformat = dw_hdmi_reformat_s24;
+		break;
+	}
+	dw->iec_offset = 0;
+	dw->channels = runtime->channels;
+	dw->buf_src  = runtime->dma_area;
+	dw->buf_dst  = substream->dma_buffer.area;
+	dw->buf_addr = substream->dma_buffer.addr;
+	dw->buf_period = snd_pcm_lib_period_bytes(substream);
+	dw->buf_size = snd_pcm_lib_buffer_bytes(substream);
+
+	return 0;
+}
+
+static int dw_hdmi_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_dw_hdmi *dw = substream->private_data;
+	int ret = 0;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		dw->buf_offset = 0;
+		dw->substream = substream;
+		dw_hdmi_start_dma(dw);
+		dw_hdmi_audio_enable(dw->data.hdmi);
+		substream->runtime->delay = substream->runtime->period_size;
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
+		dw_hdmi_stop_dma(dw);
+		dw_hdmi_audio_disable(dw->data.hdmi);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static snd_pcm_uframes_t dw_hdmi_pointer(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct snd_dw_hdmi *dw = substream->private_data;
+
+	return bytes_to_frames(runtime, dw->buf_offset);
+}
+
+static struct snd_pcm_ops snd_dw_hdmi_ops = {
+	.open = dw_hdmi_open,
+	.close = dw_hdmi_close,
+	.ioctl = snd_pcm_lib_ioctl,
+	.hw_params = dw_hdmi_hw_params,
+	.hw_free = dw_hdmi_hw_free,
+	.prepare = dw_hdmi_prepare,
+	.trigger = dw_hdmi_trigger,
+	.pointer = dw_hdmi_pointer,
+	.page = snd_pcm_lib_get_vmalloc_page,
+};
+
+static int snd_dw_hdmi_probe(struct platform_device *pdev)
+{
+	const struct dw_hdmi_audio_data *data = pdev->dev.platform_data;
+	struct device *dev = pdev->dev.parent;
+	struct snd_dw_hdmi *dw;
+	struct snd_card *card;
+	struct snd_pcm *pcm;
+	unsigned revision;
+	int ret;
+
+	writeb_relaxed(HDMI_IH_MUTE_AHBDMAAUD_STAT0_ALL,
+		       data->base + HDMI_IH_MUTE_AHBDMAAUD_STAT0);
+	revision = readb_relaxed(data->base + HDMI_REVISION_ID);
+	if (revision != 0x0a && revision != 0x1a) {
+		dev_err(dev, "dw-hdmi-audio: unknown revision 0x%02x\n",
+			revision);
+		return -ENXIO;
+	}
+
+	ret = snd_card_new(dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
+			      THIS_MODULE, sizeof(struct snd_dw_hdmi), &card);
+	if (ret < 0)
+		return ret;
+
+	strlcpy(card->driver, DRIVER_NAME, sizeof(card->driver));
+	strlcpy(card->shortname, "DW-HDMI", sizeof(card->shortname));
+	snprintf(card->longname, sizeof(card->longname),
+		 "%s rev 0x%02x, irq %d", card->shortname, revision,
+		 data->irq);
+
+	dw = card->private_data;
+	dw->card = card;
+	dw->data = *data;
+	dw->revision = revision;
+
+	ret = snd_pcm_new(card, "DW HDMI", 0, 1, 0, &pcm);
+	if (ret < 0)
+		goto err;
+
+	dw->pcm = pcm;
+	pcm->private_data = dw;
+	strlcpy(pcm->name, DRIVER_NAME, sizeof(pcm->name));
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &snd_dw_hdmi_ops);
+
+	snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV,
+			dev, 64 * 1024, 64 * 1024);
+
+	ret = snd_card_register(card);
+	if (ret < 0)
+		goto err;
+
+	platform_set_drvdata(pdev, dw);
+
+	return 0;
+
+err:
+	snd_card_free(card);
+	return ret;
+}
+
+static int snd_dw_hdmi_remove(struct platform_device *pdev)
+{
+	struct snd_dw_hdmi *dw = platform_get_drvdata(pdev);
+
+	snd_card_free(dw->card);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int snd_dw_hdmi_suspend(struct device *dev)
+{
+	struct snd_dw_hdmi *dw = dev_get_drvdata(dev);
+
+	snd_power_change_state(dw->card, SNDRV_CTL_POWER_D3cold);
+	snd_pcm_suspend_all(dw->pcm);
+
+	return 0;
+}
+
+static int snd_dw_hdmi_resume(struct device *dev)
+{
+	struct snd_dw_hdmi *dw = dev_get_drvdata(dev);
+
+	snd_power_change_state(dw->card, SNDRV_CTL_POWER_D0);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(snd_dw_hdmi_pm, snd_dw_hdmi_suspend,
+			 snd_dw_hdmi_resume);
+#define PM_OPS &snd_dw_hdmi_pm
+#else
+#define PM_OPS NULL
+#endif
+
+static struct platform_driver snd_dw_hdmi_driver = {
+	.probe	= snd_dw_hdmi_probe,
+	.remove	= snd_dw_hdmi_remove,
+	.driver	= {
+		.name = DRIVER_NAME,
+		.owner = THIS_MODULE,
+		.pm = PM_OPS,
+	},
+};
+
+module_platform_driver(snd_dw_hdmi_driver);
+
+MODULE_AUTHOR("Russell King <rmk+kernel@arm.linux.org.uk>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS(PLATFORM_MODULE_PREFIX DRIVER_NAME);
diff --git a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h
new file mode 100644
index 000000000000..1e840118d90a
--- /dev/null
+++ b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h
@@ -0,0 +1,13 @@
+#ifndef DW_HDMI_AUDIO_H
+#define DW_HDMI_AUDIO_H
+
+struct dw_hdmi;
+
+struct dw_hdmi_audio_data {
+	phys_addr_t phys;
+	void __iomem *base;
+	int irq;
+	struct dw_hdmi *hdmi;
+};
+
+#endif
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 67f07d15c12b..94a355f435bd 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -28,6 +28,7 @@
 #include <drm/bridge/dw_hdmi.h>
 
 #include "dw_hdmi.h"
+#include "dw_hdmi-ahb-audio.h"
 
 #define HDMI_EDID_LEN		512
 
@@ -105,6 +106,7 @@ struct dw_hdmi {
 	struct drm_encoder *encoder;
 	struct drm_bridge *bridge;
 
+	struct platform_device *audio;
 	enum dw_hdmi_devtype dev_type;
 	struct device *dev;
 	struct clk *isfr_clk;
@@ -1593,7 +1595,9 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 {
 	struct drm_device *drm = data;
 	struct device_node *np = dev->of_node;
+	struct platform_device_info pdevinfo;
 	struct device_node *ddc_node;
+	struct dw_hdmi_audio_data audio;
 	struct dw_hdmi *hdmi;
 	int ret;
 	u32 val = 1;
@@ -1713,6 +1717,23 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 	/* Unmute interrupts */
 	hdmi_writeb(hdmi, ~HDMI_IH_PHY_STAT0_HPD, HDMI_IH_MUTE_PHY_STAT0);
 
+	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;
+
+		pdevinfo.name = "dw-hdmi-ahb-audio";
+		pdevinfo.data = &audio;
+		pdevinfo.size_data = sizeof(audio);
+		pdevinfo.dma_mask = DMA_BIT_MASK(32);
+		hdmi->audio = platform_device_register_full(&pdevinfo);
+	}
+
 	dev_set_drvdata(dev, hdmi);
 
 	return 0;
@@ -1730,6 +1751,9 @@ void dw_hdmi_unbind(struct device *dev, struct device *master, void *data)
 {
 	struct dw_hdmi *hdmi = dev_get_drvdata(dev);
 
+	if (hdmi->audio && !IS_ERR(hdmi->audio))
+		platform_device_unregister(hdmi->audio);
+
 	/* Disable all interrupts */
 	hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0);
 
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.h b/drivers/gpu/drm/bridge/dw_hdmi.h
index 175dbc89a824..78e54e813212 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 {
+/* CONFIG1_ID field values */
+	HDMI_CONFIG1_AHB = 0x01,
+
 /* IH_FC_INT2 field values */
 	HDMI_IH_FC_INT2_OVERFLOW_MASK = 0x03,
 	HDMI_IH_FC_INT2_LOW_PRIORITY_OVERFLOW = 0x02,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH RFC 11/11] drm: bridge/dw_hdmi-ahb-audio: parse ELD from HDMI driver
  2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
                   ` (9 preceding siblings ...)
  2015-03-30 19:40 ` [PATCH RFC 10/11] drm: bridge/dw_hdmi-ahb-audio: add audio driver Russell King
@ 2015-03-30 19:40 ` Russell King
  10 siblings, 0 replies; 30+ messages in thread
From: Russell King @ 2015-03-30 19:40 UTC (permalink / raw)
  To: alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel, Yakir Yang

Parse the ELD (EDID like data) stored from the HDMI driver to restrict
the sample rates and channels which are available to ALSA.  This causes
the ALSA device to reflect the capabilities of the overall audio path,
not just what is supported at the HDMI source interface level.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/bridge/Kconfig             | 1 +
 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c | 6 ++++++
 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h | 1 +
 drivers/gpu/drm/bridge/dw_hdmi.c           | 3 +++
 4 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 557962a67fc5..9fff41e76940 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -7,6 +7,7 @@ config DRM_DW_HDMI_AHB_AUDIO
 	tristate "Synopsis Designware AHB Audio interface"
 	depends on DRM_DW_HDMI && SND
 	select SND_PCM
+	select SND_PCM_ELD
 	select SND_PCM_IEC958
 	help
 	  Support the AHB Audio interface which is part of the Synopsis
diff --git a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c
index e98c291268f4..2bb68bda3cb0 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c
@@ -12,11 +12,13 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <drm/bridge/dw_hdmi.h>
+#include <drm/drm_edid.h>
 
 #include <sound/asoundef.h>
 #include <sound/core.h>
 #include <sound/initval.h>
 #include <sound/pcm.h>
+#include <sound/pcm_drm_eld.h>
 #include <sound/pcm_iec958.h>
 
 #include "dw_hdmi-ahb-audio.h"
@@ -284,6 +286,10 @@ static int dw_hdmi_open(struct snd_pcm_substream *substream)
 
 	runtime->hw = dw_hdmi_hw;
 
+	ret = snd_pcm_hw_constraint_eld(runtime, dw->data.eld);
+	if (ret < 0)
+		return ret;
+
 	ret = snd_pcm_limit_hw_rates(runtime);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h
index 1e840118d90a..91f631beecc7 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h
+++ b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.h
@@ -8,6 +8,7 @@ struct dw_hdmi_audio_data {
 	void __iomem *base;
 	int irq;
 	struct dw_hdmi *hdmi;
+	u8 *eld;
 };
 
 #endif
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 94a355f435bd..653b221b220a 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -1447,6 +1447,8 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
 
 		drm_mode_connector_update_edid_property(connector, edid);
 		ret = drm_add_edid_modes(connector, edid);
+		/* Store the ELD */
+		drm_edid_to_eld(connector, edid);
 		kfree(edid);
 	} else {
 		dev_dbg(hdmi->dev, "failed to get edid\n");
@@ -1726,6 +1728,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 		audio.base = hdmi->regs;
 		audio.irq = irq;
 		audio.hdmi = hdmi;
+		audio.eld = hdmi->connector.eld;
 
 		pdevinfo.name = "dw-hdmi-ahb-audio";
 		pdevinfo.data = &audio;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 01/11] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator()
  2015-03-30 19:40 ` [PATCH RFC 01/11] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator() Russell King
@ 2015-03-31  6:55   ` Yang Kuankuan
  2015-03-31 10:35     ` Russell King - ARM Linux
  0 siblings, 1 reply; 30+ messages in thread
From: Yang Kuankuan @ 2015-03-31  6:55 UTC (permalink / raw)
  To: Russell King, alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel

Hi Russell,

On 03/30/2015 03:40 PM, Russell King wrote:
> Clean up hdmi_set_clk_regenerator() by allowing it to take the audio
> sample rate and ratio directly, rather than hiding it inside the
> function.  Raise the unsupported pixel clock/sample rate message from
> debug to error level as this results in audio not working correctly.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>   drivers/gpu/drm/bridge/dw_hdmi.c | 32 +++++++++++++++-----------------
>   1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
> index cca1c3d165e2..49df6c8c4ea8 100644
> --- a/drivers/gpu/drm/bridge/dw_hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c
> @@ -335,39 +335,37 @@ static unsigned int hdmi_compute_cts(unsigned int freq, unsigned long pixel_clk,
>   }
>   
>   static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> -				     unsigned long pixel_clk)
> +	unsigned long pixel_clk, unsigned int sample_rate, unsigned int ratio)
>   {
> -	unsigned int clk_n, clk_cts;
> +	unsigned int n, cts;
>   
> -	clk_n = hdmi_compute_n(hdmi->sample_rate, pixel_clk,
> -			       hdmi->ratio);
> -	clk_cts = hdmi_compute_cts(hdmi->sample_rate, pixel_clk,
> -				   hdmi->ratio);
> -
> -	if (!clk_cts) {
> -		dev_dbg(hdmi->dev, "%s: pixel clock not supported: %lu\n",
> -			__func__, pixel_clk);
> -		return;
> +	n = hdmi_compute_n(sample_rate, pixel_clk, ratio);
> +	cts = hdmi_compute_cts(sample_rate, pixel_clk, ratio);
> +	if (!cts) {
> +		dev_err(hdmi->dev,
> +			"%s: pixel clock/sample rate not supported: %luMHz / %ukHz\n",
> +			__func__, pixel_clk, sample_rate);
>   	}
>   
> -	dev_dbg(hdmi->dev, "%s: samplerate=%d  ratio=%d  pixelclk=%lu  N=%d cts=%d\n",
> -		__func__, hdmi->sample_rate, hdmi->ratio,
> -		pixel_clk, clk_n, clk_cts);
> +	dev_dbg(hdmi->dev, "%s: samplerate=%ukHz ratio=%d pixelclk=%luMHz N=%d cts=%d\n",
> +		__func__, sample_rate, ratio, pixel_clk, n, cts);
>   
> -	hdmi_set_cts_n(hdmi, clk_cts, clk_n);
> +	hdmi_set_cts_n(hdmi, cts, n);
>   }
>   
>   static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
>   {
>   	mutex_lock(&hdmi->audio_mutex);
> -	hdmi_set_clk_regenerator(hdmi, 74250000);
> +	hdmi_set_clk_regenerator(hdmi, 74250000, hdmi->sample_rate,
> +				 hdmi->ratio);
>   	mutex_unlock(&hdmi->audio_mutex);
>   }
>   
>   static void hdmi_clk_regenerator_update_pixel_clock(struct dw_hdmi *hdmi)
>   {
>   	mutex_lock(&hdmi->audio_mutex);
> -	hdmi_set_clk_regenerator(hdmi, hdmi->hdmi_data.video_mode.mpixelclock);
> +	hdmi_set_clk_regenerator(hdmi, hdmi->hdmi_data.video_mode.mpixelclock,
> +				 hdmi->sample_rate, hdmi->ratio);

I'm okay with this change, and also I am preparing that collect N/CTS
setting to an array, like this :

     struct n_cts {
         unsigned int cts;
         unsigned int n;
     };

     struct tmds_n_cts {
         unsigned long tmds;
         /* 1 entry each for 32KHz, 44.1KHz, and 48KHz */
         struct n_cts n_cts[3];
     };

     static const struct tmds_n_cts n_cts_table[] = {
         { 25175000, {{ 28125,  4576}, { 31250,  7007}, { 25175, 6144} } },
     }

But I am confused by the "hdmi->ratio", this variable was modify to 100 
in bind
funciton, then nowhere would change it again. In this case "hdmi->ratio" 
seems
an unused variable, can we remove it ?

Best regards.
Yakir Yang

>   	mutex_unlock(&hdmi->audio_mutex);
>   }
>   

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 06/11] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio
  2015-03-30 19:40 ` [PATCH RFC 06/11] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio Russell King
@ 2015-03-31  7:45   ` Yang Kuankuan
  2015-03-31  9:15   ` Philipp Zabel
  1 sibling, 0 replies; 30+ messages in thread
From: Yang Kuankuan @ 2015-03-31  7:45 UTC (permalink / raw)
  To: Russell King, alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel

Hi Russell,

On 03/30/2015 03:40 PM, Russell King wrote:
> iMX6 devices from an errata (ERR005174) where the audio FIFO can be
> emptied while it is partially full, resulting in misalignment of the
> audio samples.
>
> To prevent this, the errata workaround recommends writing N as zero
> until the audio FIFO has been loaded by DMA.  Writing N=0 prevents the
> HDMI bridge from reading from the audio FIFO, effectively disabling
> audio.
>
> This means we need to provide the audio driver with a pair of functions
> to enable/disable audio.  These are dw_hdmi_audio_enable() and
> dw_hdmi_audio_disable().
>
> A spinlock is introduced to ensure that setting the CTS/N values can't
> race, ensuring that the audio driver calling the enable/disable
> functions (which are called in an atomic context) can't race with a
> modeset.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>   drivers/gpu/drm/bridge/dw_hdmi.c | 34 +++++++++++++++++++++++++++++++++-
>   include/drm/bridge/dw_hdmi.h     |  2 ++
>   2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
> index 245d17e58dec..67f07d15c12b 100644
> --- a/drivers/gpu/drm/bridge/dw_hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c
> @@ -18,6 +18,7 @@
>   #include <linux/hdmi.h>
>   #include <linux/mutex.h>
>   #include <linux/of_device.h>
> +#include <linux/spinlock.h>
>   
>   #include <drm/drm_of.h>
>   #include <drm/drmP.h>
> @@ -124,8 +125,12 @@ struct dw_hdmi {
>   	struct i2c_adapter *ddc;
>   	void __iomem *regs;
>   
> +	spinlock_t audio_lock;
>   	struct mutex audio_mutex;
>   	unsigned int sample_rate;
> +	unsigned int audio_cts;
> +	unsigned int audio_n;
> +	bool audio_enable;
>   	int ratio;
>   
>   	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
> @@ -347,7 +352,11 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
>   	dev_dbg(hdmi->dev, "%s: samplerate=%ukHz ratio=%d pixelclk=%luMHz N=%d cts=%d\n",
>   		__func__, sample_rate, ratio, pixel_clk, n, cts);
>   
> -	hdmi_set_cts_n(hdmi, cts, n);
> +	spin_lock_irq(&hdmi->audio_lock);
> +	hdmi->audio_n = n;
> +	hdmi->audio_cts = cts;
> +	hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> +	spin_unlock_irq(&hdmi->audio_lock);
>   }
>   
>   static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> @@ -376,6 +385,28 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
>   }
>   EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
>   
> +void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&hdmi->audio_lock, flags);
> +	hdmi->audio_enable = true;
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable);
> +
> +void dw_hdmi_audio_disable(struct dw_hdmi *hdmi)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&hdmi->audio_lock, flags);
> +	hdmi->audio_enable = false;
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
> +	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);
> +

It is an smart way to operate audio status that do not relate to the audio
interfaces (AHB or I2S), but I am not sure whether this will work on 
rockchip
platform, I will do some experiment to test it as soon as possible :)

Best regards.
Yakir Yang

>   /*
>    * this submodule is responsible for the video data synchronization.
>    * for example, for RGB 4:4:4 input, the data map is defined as
> @@ -1579,6 +1610,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
>   	hdmi->encoder = encoder;
>   
>   	mutex_init(&hdmi->audio_mutex);
> +	spin_lock_init(&hdmi->audio_lock);
>   
>   	of_property_read_u32(np, "reg-io-width", &val);
>   
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 424b51f6a215..ab5d36e83ea2 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -62,5 +62,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
>   		 const struct dw_hdmi_plat_data *plat_data);
>   
>   void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
> +void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
> +void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
>   
>   #endif /* __IMX_HDMI_H__ */

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 09/11] sound/core: add IEC958 channel status helper
  2015-03-30 19:40 ` [PATCH RFC 09/11] sound/core: add IEC958 channel status helper Russell King
@ 2015-03-31  8:30   ` Yang Kuankuan
  2015-03-31  9:13     ` Russell King - ARM Linux
  2015-03-31  9:10   ` Philipp Zabel
  1 sibling, 1 reply; 30+ messages in thread
From: Yang Kuankuan @ 2015-03-31  8:30 UTC (permalink / raw)
  To: Russell King, alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, Takashi Iwai, Mark Brown, Philipp Zabel

Hi Russell,

On 03/30/2015 03:40 PM, Russell King wrote:
> Add a helper to create the IEC958 channel status from an ALSA
> snd_pcm_runtime structure, taking account of the sample rate.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>   include/sound/pcm_iec958.h |  9 ++++++
>   sound/core/Kconfig         |  3 ++
>   sound/core/Makefile        |  2 ++
>   sound/core/pcm_iec958.c    | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 84 insertions(+)
>   create mode 100644 include/sound/pcm_iec958.h
>   create mode 100644 sound/core/pcm_iec958.c
>
> diff --git a/include/sound/pcm_iec958.h b/include/sound/pcm_iec958.h
> new file mode 100644
> index 000000000000..0eed397aca8e
> --- /dev/null
> +++ b/include/sound/pcm_iec958.h
> @@ -0,0 +1,9 @@
> +#ifndef __SOUND_PCM_IEC958_H
> +#define __SOUND_PCM_IEC958_H
> +
> +#include <linux/types.h>
> +
> +int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
> +	size_t len);
> +
> +#endif
> diff --git a/sound/core/Kconfig b/sound/core/Kconfig
> index b534c8a6046b..1507469425ec 100644
> --- a/sound/core/Kconfig
> +++ b/sound/core/Kconfig
> @@ -9,6 +9,9 @@ config SND_PCM
>   config SND_PCM_ELD
>   	bool
>   
> +config SND_PCM_IEC958
> +	bool
> +
>   config SND_DMAENGINE_PCM
>   	tristate
>   
> diff --git a/sound/core/Makefile b/sound/core/Makefile
> index 591b49157b4d..70ea06712ec2 100644
> --- a/sound/core/Makefile
> +++ b/sound/core/Makefile
> @@ -14,10 +14,12 @@ snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
>   		pcm_memory.o memalloc.o
>   snd-pcm-$(CONFIG_SND_DMA_SGBUF) += sgbuf.o
>   snd-pcm-$(CONFIG_SND_PCM_ELD) += pcm_drm_eld.o
> +snd-pcm-$(CONFIG_SND_PCM_IEC958) += snd-pcm-iec958.o
>   
>   # for trace-points
>   CFLAGS_pcm_lib.o := -I$(src)
>   
> +snd-pcm-iec958-objs := pcm_iec958.o
>   snd-pcm-dmaengine-objs := pcm_dmaengine.o
>   
>   snd-rawmidi-objs  := rawmidi.o
> diff --git a/sound/core/pcm_iec958.c b/sound/core/pcm_iec958.c
> new file mode 100644
> index 000000000000..e1ff88a17dde
> --- /dev/null
> +++ b/sound/core/pcm_iec958.c
> @@ -0,0 +1,70 @@
> +/*
> + *  PCM DRM helpers
> + *
> + *   This program is free software; you can redistribute it and/or modify
> + *   it under the terms of the GNU General Public License as published by
> + *   the Free Software Foundation; either version 2 of the License.
> + */
> +#include <linux/export.h>
> +#include <linux/types.h>
> +#include <sound/asoundef.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_iec958.h>
> +
> +/**
> + * snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status
> + * @runtime: pcm runtime structure with ->rate filled in
> + * @cs: channel status buffer, at least four bytes
> + * @len: length of channel status buffer
> + *
> + * Create the consumer format channel status data in @cs of maximum size
> + * @len corresponding to the parameters of the PCM runtime @runtime.
> + *
> + * Drivers may wish to tweak the contents of the buffer after creation.
> + *
> + * Returns: length of buffer, or negative error code if something failed.
> + */
> +int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
> +	size_t len)
> +{
> +	unsigned int fs;
> +
> +	if (len < 4)
> +		return -EINVAL;
> +
> +	switch (runtime->rate) {
> +	case 32000:
> +		fs = IEC958_AES3_CON_FS_32000;
> +		break;
> +	case 44100:
> +		fs = IEC958_AES3_CON_FS_44100;
> +		break;
> +	case 48000:
> +		fs = IEC958_AES3_CON_FS_48000;
> +		break;
> +	case 88200:
> +		fs = IEC958_AES3_CON_FS_88200;
> +		break;
> +	case 96000:
> +		fs = IEC958_AES3_CON_FS_96000;
> +		break;
> +	case 176400:
> +		fs = IEC958_AES3_CON_FS_176400;
> +		break;
> +	case 192000:
> +		fs = IEC958_AES3_CON_FS_192000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	memset(cs, 0, len);
> +
> +	cs[0] = IEC958_AES0_CON_NOT_COPYRIGHT | IEC958_AES0_CON_EMPHASIS_NONE;
> +	cs[1] = IEC958_AES1_CON_GENERAL;
> +	cs[2] = IEC958_AES2_CON_SOURCE_UNSPEC | IEC958_AES2_CON_CHANNEL_UNSPEC;
> +	cs[3] = IEC958_AES3_CON_CLOCK_1000PPM | fs;
> +

Pretty good, also suitable to rockchip platform, but why not add the
"IEC958_AES2_CON_CHANNEL_MASK" & "IEC958_AES2_CON_WORDLEN" ?

Seems sample frequency & channle number & word length are the basic
message :)

Best regards.
Yakir Yang

> +	return len;
> +}
> +EXPORT_SYMBOL(snd_pcm_create_iec958_consumer);

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 02/11] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode()
  2015-03-30 19:40 ` [PATCH RFC 02/11] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode() Russell King
@ 2015-03-31  9:02   ` Yang Kuankuan
  2015-03-31 11:57     ` Russell King - ARM Linux
  0 siblings, 1 reply; 30+ messages in thread
From: Yang Kuankuan @ 2015-03-31  9:02 UTC (permalink / raw)
  To: Russell King, alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, David Airlie, Mark Brown, Philipp Zabel

Hi Russell,

On 03/30/2015 03:40 PM, Russell King wrote:
> Use drm_hdmi_avi_infoframe_from_display_mode() to compose the AVI
> frame.

Very interesting, I am also preparing the 
drm_hdmi_avi_infoframe_from_display_mode
patches to upstream, seems it is unnecessary now :)

Besides, as you are going an dw_hdmi cleanups, I want to point another 
bugs that relate
to the HDMI CTS test. There are somethings wrong with General Control 
Pack, as for now
the encoder color depth is 8-bit packing mode, so the color depth only 
support 24 bits per
pixel video, In this case the CD filed in GCP should set to "Color Depth 
Not indicated".
In the end we should keep the *csc_color_depth(HDMI_CSC_SCALE)* &
*color_depth(HDMI_VP_PR_CD)* to zero, code should modify like this GCP 
would test pass:

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c 
b/drivers/gpu/drm/bridge/dw_hdmi.c
index bf5be679..c5d9b16 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -695,7 +695,7 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
                 if (!hdmi_data->enc_color_depth) {
                         output_select = 
HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS;
                 } else if (hdmi_data->enc_color_depth == 8) {
-                       color_depth = 4;
+                       color_depth = 0;
                         output_select = 
HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS;
                 } else if (hdmi_data->enc_color_depth == 10) {
                         color_depth = 5;

Best regards.
Yakir Yang
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>   drivers/gpu/drm/bridge/dw_hdmi.c | 126 +++++++++++++++++++++------------------
>   1 file changed, 67 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
> index 49df6c8c4ea8..44c63883db19 100644
> --- a/drivers/gpu/drm/bridge/dw_hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c
> @@ -919,74 +919,79 @@ static void hdmi_tx_hdcp_config(struct dw_hdmi *hdmi)
>   		  HDMI_A_HDCPCFG1_ENCRYPTIONDISABLE_MASK, HDMI_A_HDCPCFG1);
>   }
>   
> -static void hdmi_config_AVI(struct dw_hdmi *hdmi)
> +static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>   {
> -	u8 val, pix_fmt, under_scan;
> -	u8 act_ratio, coded_ratio, colorimetry, ext_colorimetry;
> -	bool aspect_16_9;
> +	struct hdmi_avi_infoframe frame;
> +	u8 val;
>   
> -	aspect_16_9 = false; /* FIXME */
> +	/* Initialise info frame from DRM mode */
> +	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>   
> -	/* AVI Data Byte 1 */
>   	if (hdmi->hdmi_data.enc_out_format == YCBCR444)
> -		pix_fmt = HDMI_FC_AVICONF0_PIX_FMT_YCBCR444;
> +		frame.colorspace = HDMI_COLORSPACE_YUV444;
>   	else if (hdmi->hdmi_data.enc_out_format == YCBCR422_8BITS)
> -		pix_fmt = HDMI_FC_AVICONF0_PIX_FMT_YCBCR422;
> +		frame.colorspace = HDMI_COLORSPACE_YUV422;
>   	else
> -		pix_fmt = HDMI_FC_AVICONF0_PIX_FMT_RGB;
> -
> -		under_scan =  HDMI_FC_AVICONF0_SCAN_INFO_NODATA;
> -
> -	/*
> -	 * Active format identification data is present in the AVI InfoFrame.
> -	 * Under scan info, no bar data
> -	 */
> -	val = pix_fmt | under_scan |
> -		HDMI_FC_AVICONF0_ACTIVE_FMT_INFO_PRESENT |
> -		HDMI_FC_AVICONF0_BAR_DATA_NO_DATA;
> -
> -	hdmi_writeb(hdmi, val, HDMI_FC_AVICONF0);
> -
> -	/* AVI Data Byte 2 -Set the Aspect Ratio */
> -	if (aspect_16_9) {
> -		act_ratio = HDMI_FC_AVICONF1_ACTIVE_ASPECT_RATIO_16_9;
> -		coded_ratio = HDMI_FC_AVICONF1_CODED_ASPECT_RATIO_16_9;
> -	} else {
> -		act_ratio = HDMI_FC_AVICONF1_ACTIVE_ASPECT_RATIO_4_3;
> -		coded_ratio = HDMI_FC_AVICONF1_CODED_ASPECT_RATIO_4_3;
> -	}
> +		frame.colorspace = HDMI_COLORSPACE_RGB;
>   
>   	/* Set up colorimetry */
>   	if (hdmi->hdmi_data.enc_out_format == XVYCC444) {
> -		colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_EXTENDED_INFO;
> +		frame.colorimetry = HDMI_COLORIMETRY_EXTENDED;
>   		if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601)
> -			ext_colorimetry =
> -				HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601;
> +			frame.extended_colorimetry =
> +				HDMI_EXTENDED_COLORIMETRY_XV_YCC_601;
>   		else /*hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_709*/
> -			ext_colorimetry =
> -				HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC709;
> +			frame.extended_colorimetry =
> +				HDMI_EXTENDED_COLORIMETRY_XV_YCC_709;
>   	} else if (hdmi->hdmi_data.enc_out_format != RGB) {
>   		if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601)
> -			colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_SMPTE;
> +			frame.colorimetry = HDMI_COLORIMETRY_ITU_601;
>   		else /*hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_709*/
> -			colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_ITUR;
> -		ext_colorimetry = HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601;
> +			frame.colorimetry = HDMI_COLORIMETRY_ITU_709;
> +		frame.extended_colorimetry = HDMI_EXTENDED_COLORIMETRY_XV_YCC_601;
>   	} else { /* Carries no data */
> -		colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_NO_DATA;
> -		ext_colorimetry = HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601;
> +		frame.colorimetry = HDMI_COLORIMETRY_NONE;
> +		frame.extended_colorimetry = HDMI_EXTENDED_COLORIMETRY_XV_YCC_601;
>   	}
>   
> -	val = colorimetry | coded_ratio | act_ratio;
> +	frame.scan_mode = HDMI_SCAN_MODE_NONE;
> +
> +	/*
> +	 * The Designware IP uses a different byte format from standard
> +	 * AVI info frames, though generally the bits are in the correct
> +	 * bytes.
> +	 */
> +
> +	/*
> +	 * AVI data byte 1 differences: Colorspace in bits 4,5 rather than 5,6,
> +	 * active aspect present in bit 6 rather than 4.
> +	 */
> +	val = (frame.colorspace & 3) << 4 | (frame.scan_mode & 0x3);
> +	if (frame.active_aspect & 15)
> +		val |= HDMI_FC_AVICONF0_ACTIVE_FMT_INFO_PRESENT;
> +	if (frame.top_bar || frame.bottom_bar)
> +		val |= HDMI_FC_AVICONF0_BAR_DATA_HORIZ_BAR;
> +	if (frame.left_bar || frame.right_bar)
> +		val |= HDMI_FC_AVICONF0_BAR_DATA_VERT_BAR;
> +	hdmi_writeb(hdmi, val, HDMI_FC_AVICONF0);
> +
> +	/* AVI data byte 2 differences: none */
> +	val = ((frame.colorimetry & 0x3) << 6) |
> +	      ((frame.picture_aspect & 0x3) << 4) |
> +	      (frame.active_aspect & 0xf);
>   	hdmi_writeb(hdmi, val, HDMI_FC_AVICONF1);
>   
> -	/* AVI Data Byte 3 */
> -	val = HDMI_FC_AVICONF2_IT_CONTENT_NO_DATA | ext_colorimetry |
> -		HDMI_FC_AVICONF2_RGB_QUANT_DEFAULT |
> -		HDMI_FC_AVICONF2_SCALING_NONE;
> +	/* AVI data byte 3 differences: none */
> +	val = ((frame.extended_colorimetry & 0x7) << 4) |
> +	      ((frame.quantization_range & 0x3) << 2) |
> +	      (frame.nups & 0x3);
> +	if (frame.itc)
> +		val |= HDMI_FC_AVICONF2_IT_CONTENT_VALID;
>   	hdmi_writeb(hdmi, val, HDMI_FC_AVICONF2);
>   
> -	/* AVI Data Byte 4 */
> -	hdmi_writeb(hdmi, hdmi->vic, HDMI_FC_AVIVID);
> +	/* AVI data byte 4 differences: none */
> +	val = frame.video_code & 0x7f;
> +	hdmi_writeb(hdmi, val, HDMI_FC_AVIVID);
>   
>   	/* AVI Data Byte 5- set up input and output pixel repetition */
>   	val = (((hdmi->hdmi_data.video_mode.mpixelrepetitioninput + 1) <<
> @@ -997,20 +1002,23 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi)
>   		HDMI_FC_PRCONF_OUTPUT_PR_FACTOR_MASK);
>   	hdmi_writeb(hdmi, val, HDMI_FC_PRCONF);
>   
> -	/* IT Content and quantization range = don't care */
> -	val = HDMI_FC_AVICONF3_IT_CONTENT_TYPE_GRAPHICS |
> -		HDMI_FC_AVICONF3_QUANT_RANGE_LIMITED;
> +	/*
> +	 * AVI data byte 5 differences: content type in 0,1 rather than 4,5,
> +	 * ycc range in bits 2,3 rather than 6,7
> +	 */
> +	val = ((frame.ycc_quantization_range & 0x3) << 2) |
> +	      (frame.content_type & 0x3);
>   	hdmi_writeb(hdmi, val, HDMI_FC_AVICONF3);
>   
>   	/* AVI Data Bytes 6-13 */
> -	hdmi_writeb(hdmi, 0, HDMI_FC_AVIETB0);
> -	hdmi_writeb(hdmi, 0, HDMI_FC_AVIETB1);
> -	hdmi_writeb(hdmi, 0, HDMI_FC_AVISBB0);
> -	hdmi_writeb(hdmi, 0, HDMI_FC_AVISBB1);
> -	hdmi_writeb(hdmi, 0, HDMI_FC_AVIELB0);
> -	hdmi_writeb(hdmi, 0, HDMI_FC_AVIELB1);
> -	hdmi_writeb(hdmi, 0, HDMI_FC_AVISRB0);
> -	hdmi_writeb(hdmi, 0, HDMI_FC_AVISRB1);
> +	hdmi_writeb(hdmi, frame.top_bar & 0xff, HDMI_FC_AVIETB0);
> +	hdmi_writeb(hdmi, (frame.top_bar >> 8) & 0xff, HDMI_FC_AVIETB1);
> +	hdmi_writeb(hdmi, frame.bottom_bar & 0xff, HDMI_FC_AVISBB0);
> +	hdmi_writeb(hdmi, (frame.bottom_bar >> 8) & 0xff, HDMI_FC_AVISBB1);
> +	hdmi_writeb(hdmi, frame.left_bar & 0xff, HDMI_FC_AVIELB0);
> +	hdmi_writeb(hdmi, (frame.left_bar >> 8) & 0xff, HDMI_FC_AVIELB1);
> +	hdmi_writeb(hdmi, frame.right_bar & 0xff, HDMI_FC_AVISRB0);
> +	hdmi_writeb(hdmi, (frame.right_bar >> 8) & 0xff, HDMI_FC_AVISRB1);
>   }
>   
>   static void hdmi_av_composer(struct dw_hdmi *hdmi,
> @@ -1244,7 +1252,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>   		hdmi_enable_audio_clk(hdmi);
>   
>   		/* HDMI Initialization Step F - Configure AVI InfoFrame */
> -		hdmi_config_AVI(hdmi);
> +		hdmi_config_AVI(hdmi, mode);
>   	}
>   
>   	hdmi_video_packetize(hdmi);

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 09/11] sound/core: add IEC958 channel status helper
  2015-03-30 19:40 ` [PATCH RFC 09/11] sound/core: add IEC958 channel status helper Russell King
  2015-03-31  8:30   ` Yang Kuankuan
@ 2015-03-31  9:10   ` Philipp Zabel
  2015-03-31  9:16     ` Russell King - ARM Linux
  1 sibling, 1 reply; 30+ messages in thread
From: Philipp Zabel @ 2015-03-31  9:10 UTC (permalink / raw)
  To: Russell King
  Cc: Fabio Estevam, alsa-devel, dri-devel, Jaroslav Kysela,
	Mark Brown, Yakir Yang, linux-arm-kernel

Hi,

Am Montag, den 30.03.2015, 20:40 +0100 schrieb Russell King:
> Add a helper to create the IEC958 channel status from an ALSA
> snd_pcm_runtime structure, taking account of the sample rate.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  include/sound/pcm_iec958.h |  9 ++++++
>  sound/core/Kconfig         |  3 ++
>  sound/core/Makefile        |  2 ++
>  sound/core/pcm_iec958.c    | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 84 insertions(+)
>  create mode 100644 include/sound/pcm_iec958.h
>  create mode 100644 sound/core/pcm_iec958.c
> 
> diff --git a/include/sound/pcm_iec958.h b/include/sound/pcm_iec958.h
> new file mode 100644
> index 000000000000..0eed397aca8e
> --- /dev/null
> +++ b/include/sound/pcm_iec958.h
> @@ -0,0 +1,9 @@
> +#ifndef __SOUND_PCM_IEC958_H
> +#define __SOUND_PCM_IEC958_H
> +
> +#include <linux/types.h>
> +
> +int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
> +	size_t len);
> +
> +#endif
> diff --git a/sound/core/Kconfig b/sound/core/Kconfig
> index b534c8a6046b..1507469425ec 100644
> --- a/sound/core/Kconfig
> +++ b/sound/core/Kconfig
> @@ -9,6 +9,9 @@ config SND_PCM
>  config SND_PCM_ELD
>  	bool
>  
> +config SND_PCM_IEC958
> +	bool
> +
>  config SND_DMAENGINE_PCM
>  	tristate
>  
> diff --git a/sound/core/Makefile b/sound/core/Makefile
> index 591b49157b4d..70ea06712ec2 100644
> --- a/sound/core/Makefile
> +++ b/sound/core/Makefile
> @@ -14,10 +14,12 @@ snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
>  		pcm_memory.o memalloc.o
>  snd-pcm-$(CONFIG_SND_DMA_SGBUF) += sgbuf.o
>  snd-pcm-$(CONFIG_SND_PCM_ELD) += pcm_drm_eld.o
> +snd-pcm-$(CONFIG_SND_PCM_IEC958) += snd-pcm-iec958.o
>  
>  # for trace-points
>  CFLAGS_pcm_lib.o := -I$(src)
>  
> +snd-pcm-iec958-objs := pcm_iec958.o
>  snd-pcm-dmaengine-objs := pcm_dmaengine.o

This fails to build for me on 4.0-rc6:
make[2]: *** No rule to make target 'sound/core/snd-pcm-iec958.o', needed by 'sound/core/snd-pcm.o'.  Stop.
scripts/Makefile.build:403: recipe for target 'sound/core' failed

unless I change it to either:
--- a/sound/core/Makefile
+++ b/sound/core/Makefile
@@ -14,7 +14,8 @@ snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
                pcm_memory.o memalloc.o
 snd-pcm-$(CONFIG_SND_DMA_SGBUF) += sgbuf.o
 snd-pcm-$(CONFIG_SND_PCM_ELD) += pcm_drm_eld.o
-snd-pcm-$(CONFIG_SND_PCM_IEC958) += snd-pcm-iec958.o
+
+obj-$(CONFIG_SND_PCM_IEC958) += snd-pcm-iec958.o
 
 # for trace-points
 CFLAGS_pcm_lib.o := -I$(src)

or:
--- a/sound/core/Makefile
+++ b/sound/core/Makefile
@@ -14,12 +14,11 @@ snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
                pcm_memory.o memalloc.o
 snd-pcm-$(CONFIG_SND_DMA_SGBUF) += sgbuf.o
 snd-pcm-$(CONFIG_SND_PCM_ELD) += pcm_drm_eld.o
-snd-pcm-$(CONFIG_SND_PCM_IEC958) += snd-pcm-iec958.o
+snd-pcm-$(CONFIG_SND_PCM_IEC958) += pcm_iec958.o
 
 # for trace-points
 CFLAGS_pcm_lib.o := -I$(src)
 
-snd-pcm-iec958-objs := pcm_iec958.o
 snd-pcm-dmaengine-objs := pcm_dmaengine.o
 
 snd-rawmidi-objs  := rawmidi.o

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 08/11] sound/core: add DRM ELD helper
  2015-03-30 19:40 ` [PATCH RFC 08/11] sound/core: add DRM ELD helper Russell King
@ 2015-03-31  9:12   ` Philipp Zabel
  0 siblings, 0 replies; 30+ messages in thread
From: Philipp Zabel @ 2015-03-31  9:12 UTC (permalink / raw)
  To: Russell King
  Cc: Fabio Estevam, alsa-devel, dri-devel, Jaroslav Kysela,
	Mark Brown, Yakir Yang, linux-arm-kernel

Am Montag, den 30.03.2015, 20:40 +0100 schrieb Russell King:
> Add a helper for the EDID like data structure, which is typically passed
> from a HDMI adapter to its associated audio driver.  This informs the
> audio driver of the capabilities of the attached HDMI sink.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  include/sound/pcm_drm_eld.h |  6 +++
>  sound/core/Kconfig          |  3 ++
>  sound/core/Makefile         |  1 +
>  sound/core/pcm_drm_eld.c    | 91 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 101 insertions(+)
>  create mode 100644 include/sound/pcm_drm_eld.h
>  create mode 100644 sound/core/pcm_drm_eld.c
> 
> diff --git a/include/sound/pcm_drm_eld.h b/include/sound/pcm_drm_eld.h
> new file mode 100644
> index 000000000000..93357b25d2e2
> --- /dev/null
> +++ b/include/sound/pcm_drm_eld.h
> @@ -0,0 +1,6 @@
> +#ifndef __SOUND_PCM_DRM_ELD_H
> +#define __SOUND_PCM_DRM_ELD_H
> +
> +int snd_pcm_hw_constraint_eld(struct snd_pcm_runtime *runtime, void *eld);
> +
> +#endif
> diff --git a/sound/core/Kconfig b/sound/core/Kconfig
> index 313f22e9d929..b534c8a6046b 100644
> --- a/sound/core/Kconfig
> +++ b/sound/core/Kconfig
> @@ -6,6 +6,9 @@ config SND_PCM
>  	tristate
>  	select SND_TIMER
>  
> +config SND_PCM_ELD
> +	bool
> +
>  config SND_DMAENGINE_PCM
>  	tristate
>  
> diff --git a/sound/core/Makefile b/sound/core/Makefile
> index 4daf2f58261c..591b49157b4d 100644
> --- a/sound/core/Makefile
> +++ b/sound/core/Makefile
> @@ -13,6 +13,7 @@ snd-$(CONFIG_SND_JACK)	  += jack.o
>  snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
>  		pcm_memory.o memalloc.o
>  snd-pcm-$(CONFIG_SND_DMA_SGBUF) += sgbuf.o
> +snd-pcm-$(CONFIG_SND_PCM_ELD) += pcm_drm_eld.o
>  
>  # for trace-points
>  CFLAGS_pcm_lib.o := -I$(src)
> diff --git a/sound/core/pcm_drm_eld.c b/sound/core/pcm_drm_eld.c
> new file mode 100644
> index 000000000000..78d551a1b05f
> --- /dev/null
> +++ b/sound/core/pcm_drm_eld.c
> @@ -0,0 +1,91 @@
> +/*
> + *  PCM DRM helpers
> + *
> + *   This program is free software; you can redistribute it and/or modify
> + *   it under the terms of the GNU General Public License as published by
> + *   the Free Software Foundation; either version 2 of the License.
> + */

Should this say:

+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.

?

Same for patch 9.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 09/11] sound/core: add IEC958 channel status helper
  2015-03-31  8:30   ` Yang Kuankuan
@ 2015-03-31  9:13     ` Russell King - ARM Linux
  2015-04-01  2:04       ` Yakir
  0 siblings, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2015-03-31  9:13 UTC (permalink / raw)
  To: Yang Kuankuan
  Cc: Fabio Estevam, alsa-devel, dri-devel, Jaroslav Kysela,
	Mark Brown, linux-arm-kernel

On Tue, Mar 31, 2015 at 04:30:39AM -0400, Yang Kuankuan wrote:
> >+	cs[0] = IEC958_AES0_CON_NOT_COPYRIGHT | IEC958_AES0_CON_EMPHASIS_NONE;
> >+	cs[1] = IEC958_AES1_CON_GENERAL;
> >+	cs[2] = IEC958_AES2_CON_SOURCE_UNSPEC | IEC958_AES2_CON_CHANNEL_UNSPEC;
> >+	cs[3] = IEC958_AES3_CON_CLOCK_1000PPM | fs;
> >+
> 
> Pretty good, also suitable to rockchip platform, but why not add the
> "IEC958_AES2_CON_CHANNEL_MASK" & "IEC958_AES2_CON_WORDLEN" ?
> 
> Seems sample frequency & channle number & word length are the basic
> message :)

I was debating about the word length, and that's something I'll add
later to it - but only if length shows that we have the 5th byte
available in the buffer.  Most users seem to only use the first four
bytes.

As for the channel number, this is intentionally left to the driver -
most cases I've found either the driver isn't interested, or where
they are interested (the only case I know of is my dw_hdmi ahb audio
driver), it's more appropriate to generate a baseline channel status,
and let the driver iterate over the channels adding the appropriate
channel number in.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 06/11] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio
  2015-03-30 19:40 ` [PATCH RFC 06/11] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio Russell King
  2015-03-31  7:45   ` Yang Kuankuan
@ 2015-03-31  9:15   ` Philipp Zabel
  1 sibling, 0 replies; 30+ messages in thread
From: Philipp Zabel @ 2015-03-31  9:15 UTC (permalink / raw)
  To: Russell King
  Cc: Fabio Estevam, alsa-devel, dri-devel, Mark Brown, Yakir Yang,
	linux-arm-kernel

Hi Russell,

Am Montag, den 30.03.2015, 20:40 +0100 schrieb Russell King:
> iMX6 devices from an errata (ERR005174) where the audio FIFO can be
              ^ suffer from?

> emptied while it is partially full, resulting in misalignment of the
> audio samples.
[...]

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 09/11] sound/core: add IEC958 channel status helper
  2015-03-31  9:10   ` Philipp Zabel
@ 2015-03-31  9:16     ` Russell King - ARM Linux
  0 siblings, 0 replies; 30+ messages in thread
From: Russell King - ARM Linux @ 2015-03-31  9:16 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Fabio Estevam, alsa-devel, dri-devel, Jaroslav Kysela,
	Mark Brown, Yakir Yang, linux-arm-kernel

On Tue, Mar 31, 2015 at 11:10:34AM +0200, Philipp Zabel wrote:
> This fails to build for me on 4.0-rc6:
> make[2]: *** No rule to make target 'sound/core/snd-pcm-iec958.o', needed by 'sound/core/snd-pcm.o'.  Stop.
> scripts/Makefile.build:403: recipe for target 'sound/core' failed

Yes, I originally had this as a separate module, but decided it was silly,
so put it in the main snd-pcm module instead.

Naturally, it builds for me because the target object exists in my build
tree from the first way...

> --- a/sound/core/Makefile
> +++ b/sound/core/Makefile
> @@ -14,12 +14,11 @@ snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
>                 pcm_memory.o memalloc.o
>  snd-pcm-$(CONFIG_SND_DMA_SGBUF) += sgbuf.o
>  snd-pcm-$(CONFIG_SND_PCM_ELD) += pcm_drm_eld.o
> -snd-pcm-$(CONFIG_SND_PCM_IEC958) += snd-pcm-iec958.o
> +snd-pcm-$(CONFIG_SND_PCM_IEC958) += pcm_iec958.o
>  
>  # for trace-points
>  CFLAGS_pcm_lib.o := -I$(src)
>  
> -snd-pcm-iec958-objs := pcm_iec958.o
>  snd-pcm-dmaengine-objs := pcm_dmaengine.o
>  
>  snd-rawmidi-objs  := rawmidi.o

Yep, that's obviously what I wanted - good catch, thanks!

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 01/11] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator()
  2015-03-31  6:55   ` Yang Kuankuan
@ 2015-03-31 10:35     ` Russell King - ARM Linux
  2015-04-01  1:54       ` Yakir
  0 siblings, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2015-03-31 10:35 UTC (permalink / raw)
  To: Yang Kuankuan
  Cc: Fabio Estevam, alsa-devel, dri-devel, Mark Brown, linux-arm-kernel

On Tue, Mar 31, 2015 at 02:55:53AM -0400, Yang Kuankuan wrote:
> Hi Russell,
> 
> I'm okay with this change, and also I am preparing that collect N/CTS
> setting to an array, like this :
> 
>     struct n_cts {
>         unsigned int cts;
>         unsigned int n;
>     };
> 
>     struct tmds_n_cts {
>         unsigned long tmds;
>         /* 1 entry each for 32KHz, 44.1KHz, and 48KHz */
>         struct n_cts n_cts[3];
>     };
> 
>     static const struct tmds_n_cts n_cts_table[] = {
>         { 25175000, {{ 28125,  4576}, { 31250,  7007}, { 25175, 6144} } },
>     }

I think this is something which should be a common helper rather than
being specific to the driver.  I believe these are the standard values
for coherent audio/video clocks which can be found in the HDMI
specifications.  Such a helper should document that it is only for
use when the audio and video clocks are coherent.

(The HDMI spec gives alternative N values for use with auto-CTS value
generation for non-coherent clocks.)

I'd also prefer the lookup to use the same units as the drm_display_mode
video clock specification, which is kHz, so we can eventually kill the
needless *1000 in dw_hdmi.

One of the issues with using a common helper is that the common helper
would include the 1.001 clocks, which are allegedly unsupported by the
FSL iMX6 implementation, and, if proven that they don't work, we would
need some way to disable audio for those devices.

> But I am confused by the "hdmi->ratio", this variable was modify to
> 100 in bind funciton, then nowhere would change it again. In this
> case "hdmi->ratio" seems an unused variable, can we remove it ?

I guess the FSL sources should be checked to find out what the intended
use for that was, and we then need to decide whether to keep it (and
implement it) or remove it.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 02/11] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode()
  2015-03-31  9:02   ` Yang Kuankuan
@ 2015-03-31 11:57     ` Russell King - ARM Linux
  2015-04-01  1:31       ` Yakir
  0 siblings, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2015-03-31 11:57 UTC (permalink / raw)
  To: Yang Kuankuan
  Cc: Fabio Estevam, alsa-devel, dri-devel, Mark Brown, linux-arm-kernel

On Tue, Mar 31, 2015 at 05:02:20AM -0400, Yang Kuankuan wrote:
> Besides, as you are going an dw_hdmi cleanups, I want to point another bugs
> that relate to the HDMI CTS test. There are somethings wrong with General
> Control Pack, as for now the encoder color depth is 8-bit packing mode,
> so the color depth only support 24 bits per pixel video, In this case the
> CD filed in GCP should set to "Color Depth Not indicated".

I'm not sure I follow.

According to the iMX6 documentation, setting the CD field to either 0000
or 0100 indicates that the color depth is 24 bits per pixel, 8 bits per
component, 8 bit packing mode - there's no documented difference between
these.

Are you saying that you wish to pass something other than 24 bpp video
to your HDMI encoder?

> In the end we should keep the *csc_color_depth(HDMI_CSC_SCALE)* &
> *color_depth(HDMI_VP_PR_CD)* to zero, code should modify like this GCP
> would test pass:

From what you're describing, you want CD field = 0 and CSC_SCALE = 0.
It looks like hdmi_video_packetize() will set the CD field to zero if
hdmi_data.enc_color_depth = 0, but that would cause hdmi_video_sample()
and hdmi_video_csc() to fail.  Maybe those two functions should be fixed
to accept a color depth of zero, and maybe you need to set
enc_color_depth to 0?

Interestingly, HDMI_CSC_SCALE_CSC_COLORDE_PTH_24BPP is defined to be
zero, but again, in the iMX6 data, it could take a value of either
0x00 or 0x40.  I think hdmi_video_csc() should set this to 0x40 if
hdmi_data.enc_color_depth = 0, and 0x40 for hdmi_data.enc_color_depth = 8.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 02/11] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode()
  2015-03-31 11:57     ` Russell King - ARM Linux
@ 2015-04-01  1:31       ` Yakir
  0 siblings, 0 replies; 30+ messages in thread
From: Yakir @ 2015-04-01  1:31 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Fabio Estevam, alsa-devel, David Airlie, dri-devel, Mark Brown,
	Philipp Zabel, linux-arm-kernel

Hi Russell,

在 2015/3/31 19:57, Russell King - ARM Linux 写道:
> On Tue, Mar 31, 2015 at 05:02:20AM -0400, Yang Kuankuan wrote:
>> Besides, as you are going an dw_hdmi cleanups, I want to point another bugs
>> that relate to the HDMI CTS test. There are somethings wrong with General
>> Control Pack, as for now the encoder color depth is 8-bit packing mode,
>> so the color depth only support 24 bits per pixel video, In this case the
>> CD filed in GCP should set to "Color Depth Not indicated".
> I'm not sure I follow.
>
> According to the iMX6 documentation, setting the CD field to either 0000
> or 0100 indicates that the color depth is 24 bits per pixel, 8 bits per
> component, 8 bit packing mode - there's no documented difference between
> these.
Yeah, that is the point. Though there's no designedware datasheet 
difference between
"0000b" & 0100b" in vp_pr_cd, but I think the color_depth in vp_pr_cd 
register are
mapping to CD0-CD3 filed in GCP packet, and acutally "0000b" & "0100b" 
are differnent
in CD filed ("0000b Color Depth not indicated" & "0100b 24 bit per pixel").

If the CD filed is non-zero, that indicate we are support depth color 
mode(but I think
the depth color mode need at least 36bpp). Besides as the HDMI 
Specification descripte,
"/If the Sink dose not receive a GCP with non-zero CD from more than 4 
consecutive video//
//fields, it should exit deep color mode and reverting to 24-bit 
color/"(24-bit color is default).

In the end I think if we only output 24-bit color, we should make the CD 
field to zero :)

> Are you saying that you wish to pass something other than 24 bpp video
> to your HDMI encoder?
>
>> In the end we should keep the *csc_color_depth(HDMI_CSC_SCALE)* &
>> *color_depth(HDMI_VP_PR_CD)* to zero, code should modify like this GCP
>> would test pass:
> >From what you're describing, you want CD field = 0 and CSC_SCALE = 0.
> It looks like hdmi_video_packetize() will set the CD field to zero if
> hdmi_data.enc_color_depth = 0, but that would cause hdmi_video_sample()
> and hdmi_video_csc() to fail.  Maybe those two functions should be fixed
> to accept a color depth of zero, and maybe you need to set
> enc_color_depth to 0?
>
> Interestingly, HDMI_CSC_SCALE_CSC_COLORDE_PTH_24BPP is defined to be
> zero, but again, in the iMX6 data, it could take a value of either
> 0x00 or 0x40.  I think hdmi_video_csc() should set this to 0x40 if
> hdmi_data.enc_color_depth = 0, and 0x40 for hdmi_data.enc_color_depth = 8.
Agree! If we only output 24-bit color, we should let the 
hdmi_data.enc_color_depth = 0,
and hdmi_video_csc() & hdmi_video_packetize() should handle the 
"enc_color_depth = 0".

Best regards.
Yakir Yang

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 01/11] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator()
  2015-03-31 10:35     ` Russell King - ARM Linux
@ 2015-04-01  1:54       ` Yakir
  0 siblings, 0 replies; 30+ messages in thread
From: Yakir @ 2015-04-01  1:54 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Fabio Estevam, alsa-devel, David Airlie, dri-devel, Mark Brown,
	Philipp Zabel, linux-arm-kernel

Hi Rusell,

在 2015/3/31 18:35, Russell King - ARM Linux 写道:
> On Tue, Mar 31, 2015 at 02:55:53AM -0400, Yang Kuankuan wrote:
>> Hi Russell,
>>
>> I'm okay with this change, and also I am preparing that collect N/CTS
>> setting to an array, like this :
>>
>>      struct n_cts {
>>          unsigned int cts;
>>          unsigned int n;
>>      };
>>
>>      struct tmds_n_cts {
>>          unsigned long tmds;
>>          /* 1 entry each for 32KHz, 44.1KHz, and 48KHz */
>>          struct n_cts n_cts[3];
>>      };
>>
>>      static const struct tmds_n_cts n_cts_table[] = {
>>          { 25175000, {{ 28125,  4576}, { 31250,  7007}, { 25175, 6144} } },
>>      }
> I think this is something which should be a common helper rather than
> being specific to the driver.  I believe these are the standard values
> for coherent audio/video clocks which can be found in the HDMI
> specifications.  Such a helper should document that it is only for
> use when the audio and video clocks are coherent.

Yep, it will be logical to add the n/cts calcu to a common helper. And 
actually
the HDMI specifications have give the Revommended N and Expected CTS 
values for
several standard TMDS clock rates(25.2/1.001 ...).

>
> (The HDMI spec gives alternative N values for use with auto-CTS value
> generation for non-coherent clocks.)
>
> I'd also prefer the lookup to use the same units as the drm_display_mode
> video clock specification, which is kHz, so we can eventually kill the
> needless *1000 in dw_hdmi.
>
> One of the issues with using a common helper is that the common helper
> would include the 1.001 clocks, which are allegedly unsupported by the
> FSL iMX6 implementation, and, if proven that they don't work, we would
> need some way to disable audio for those devices.

Okay, but rockchip platform can handle the 1.001 clocks, so maybe we can 
call
some valid function from dw_hdmi-imx & dw_hdmi-rockchip to mark the 
effective
clock that platform support ?

>> But I am confused by the "hdmi->ratio", this variable was modify to
>> 100 in bind funciton, then nowhere would change it again. In this
>> case "hdmi->ratio" seems an unused variable, can we remove it ?
> I guess the FSL sources should be checked to find out what the intended
> use for that was, and we then need to decide whether to keep it (and
> implement it) or remove it.
Need FSL ack...

Best regards.
Yakir Yang


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 09/11] sound/core: add IEC958 channel status helper
  2015-03-31  9:13     ` Russell King - ARM Linux
@ 2015-04-01  2:04       ` Yakir
  2015-04-01  7:58         ` Russell King - ARM Linux
  0 siblings, 1 reply; 30+ messages in thread
From: Yakir @ 2015-04-01  2:04 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Fabio Estevam, alsa-devel, Takashi Iwai, dri-devel, Mark Brown,
	Philipp Zabel, linux-arm-kernel

Hi Russell,

在 2015/3/31 17:13, Russell King - ARM Linux 写道:
> On Tue, Mar 31, 2015 at 04:30:39AM -0400, Yang Kuankuan wrote:
>>> +	cs[0] = IEC958_AES0_CON_NOT_COPYRIGHT | IEC958_AES0_CON_EMPHASIS_NONE;
>>> +	cs[1] = IEC958_AES1_CON_GENERAL;
>>> +	cs[2] = IEC958_AES2_CON_SOURCE_UNSPEC | IEC958_AES2_CON_CHANNEL_UNSPEC;
>>> +	cs[3] = IEC958_AES3_CON_CLOCK_1000PPM | fs;
>>> +
>> Pretty good, also suitable to rockchip platform, but why not add the
>> "IEC958_AES2_CON_CHANNEL_MASK" & "IEC958_AES2_CON_WORDLEN" ?
>>
>> Seems sample frequency & channle number & word length are the basic
>> message :)
> I was debating about the word length, and that's something I'll add
> later to it - but only if length shows that we have the 5th byte
> available in the buffer.  Most users seem to only use the first four
> bytes.
>
> As for the channel number, this is intentionally left to the driver -
> most cases I've found either the driver isn't interested, or where
> they are interested (the only case I know of is my dw_hdmi ahb audio
> driver), it's more appropriate to generate a baseline channel status,
> and let the driver iterate over the channels adding the appropriate
> channel number in.
Okay, agree with you to keep baseline channel status, but seems dw_hdmi
i2s audio are interested in channle number (to fill in schnl resigeters).

Best regards.
Yakir Yang


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 09/11] sound/core: add IEC958 channel status helper
  2015-04-01  2:04       ` Yakir
@ 2015-04-01  7:58         ` Russell King - ARM Linux
  0 siblings, 0 replies; 30+ messages in thread
From: Russell King - ARM Linux @ 2015-04-01  7:58 UTC (permalink / raw)
  To: Yakir
  Cc: Fabio Estevam, alsa-devel, dri-devel, Jaroslav Kysela,
	Mark Brown, linux-arm-kernel

On Wed, Apr 01, 2015 at 10:04:03AM +0800, Yakir wrote:
> Hi Russell,
> 
> 在 2015/3/31 17:13, Russell King - ARM Linux 写道:
> >As for the channel number, this is intentionally left to the driver -
> >most cases I've found either the driver isn't interested, or where
> >they are interested (the only case I know of is my dw_hdmi ahb audio
> >driver), it's more appropriate to generate a baseline channel status,
> >and let the driver iterate over the channels adding the appropriate
> >channel number in.
> Okay, agree with you to keep baseline channel status, but seems dw_hdmi
> i2s audio are interested in channle number (to fill in schnl resigeters).

Correct - but it's pointless having it in this helper as I explained.
Please read my dw_hdmi-ahb-audio code to see why.

It would be wasteful to memset the structure back to zero, only to
re-fill it with exactly the same data except for the channel number.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 07/11] drm/edid: add function to help find SADs
  2015-03-30 19:40 ` [PATCH RFC 07/11] drm/edid: add function to help find SADs Russell King
@ 2015-04-01 11:47   ` Jani Nikula
  2015-04-01 11:56     ` Russell King - ARM Linux
  0 siblings, 1 reply; 30+ messages in thread
From: Jani Nikula @ 2015-04-01 11:47 UTC (permalink / raw)
  To: Russell King, alsa-devel, dri-devel, linux-arm-kernel
  Cc: Fabio Estevam, Mark Brown, Yakir Yang

On Mon, 30 Mar 2015, Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> Add a function to find the start of the SADs in the ELD.  This
> complements the helper to retrieve the SAD count.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

I guess version 31 is specific enough to warrant a #define of its own,
but meh.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  include/drm/drm_edid.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 87d85e81d3a7..c44ad513e0f7 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -346,6 +346,25 @@ static inline int drm_eld_mnl(const uint8_t *eld)
>  }
>  
>  /**
> + * drm_eld_sad - Get ELD SAD structures.
> + * @eld: pointer to an eld memory structure with sad_count set
> + */
> +static inline const uint8_t *drm_eld_sad(const uint8_t *eld)
> +{
> +	unsigned int ver, mnl;
> +
> +	ver = (eld[DRM_ELD_VER] & DRM_ELD_VER_MASK) >> DRM_ELD_VER_SHIFT;
> +	if (ver != 2 && ver != 31)
> +		return NULL;
> +
> +	mnl = drm_eld_mnl(eld);
> +	if (mnl > 16)
> +		return NULL;
> +
> +	return eld + DRM_ELD_CEA_SAD(mnl, 0);
> +}
> +
> +/**
>   * drm_eld_sad_count - Get ELD SAD count.
>   * @eld: pointer to an eld memory structure with sad_count set
>   */
> -- 
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 07/11] drm/edid: add function to help find SADs
  2015-04-01 11:47   ` Jani Nikula
@ 2015-04-01 11:56     ` Russell King - ARM Linux
  2015-04-02 10:52       ` [PATCH] drm/edid: add #defines for ELD versions Jani Nikula
  0 siblings, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2015-04-01 11:56 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Fabio Estevam, alsa-devel, dri-devel, Mark Brown, Yakir Yang,
	linux-arm-kernel

On Wed, Apr 01, 2015 at 02:47:32PM +0300, Jani Nikula wrote:
> On Mon, 30 Mar 2015, Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> > Add a function to find the start of the SADs in the ELD.  This
> > complements the helper to retrieve the SAD count.
> >
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> I guess version 31 is specific enough to warrant a #define of its own,
> but meh.

What about version 2?  We don't have any version definitions in
drm_edid.h at all.  It would be nice to see some...

It would also be nice to see some of this stuff not being DRM specific,
because EDID parsing isn't something limited to DRM - and let's face it,
one of the biggest consumers of graphics on Linux is Android, which I'm
told is pretty much wedded to the fbdev API.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH] drm/edid: add #defines for ELD versions
  2015-04-01 11:56     ` Russell King - ARM Linux
@ 2015-04-02 10:52       ` Jani Nikula
  0 siblings, 0 replies; 30+ messages in thread
From: Jani Nikula @ 2015-04-02 10:52 UTC (permalink / raw)
  To: Russell King - ARM Linux, Jani Nikula
  Cc: Fabio Estevam, alsa-devel, dri-devel, Mark Brown, Yakir Yang,
	linux-arm-kernel

Add ELD versions according to HDA Specification v1.0a.

2 indicates version 2, which supports CEA_Ver 861D or below. Maximum
Baseline ELD size of 80 bytes (15 SAD count).

31 indicates an ELD that has been partially populated through
implementation specific mean of default programming before an external
graphics driver is loaded. Only the field that is called out as "canned"
field will be populated, and audio driver should ignore the non "canned"
field.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

Russell, please feel free to include this patch in your series if this
isn't picked up on its own.
---
 include/drm/drm_edid.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 87d85e81d3a7..799050198323 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -215,6 +215,8 @@ struct detailed_timing {
 #define DRM_ELD_VER			0
 # define DRM_ELD_VER_SHIFT		3
 # define DRM_ELD_VER_MASK		(0x1f << 3)
+# define DRM_ELD_VER_CEA861D		(2 << 3) /* supports 861D or below */
+# define DRM_ELD_VER_CANNED		(0x1f << 3)
 
 #define DRM_ELD_BASELINE_ELD_LEN	2	/* in dwords! */
 
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2015-04-02 10:52 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
2015-03-30 19:40 ` [PATCH RFC 01/11] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator() Russell King
2015-03-31  6:55   ` Yang Kuankuan
2015-03-31 10:35     ` Russell King - ARM Linux
2015-04-01  1:54       ` Yakir
2015-03-30 19:40 ` [PATCH RFC 02/11] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode() Russell King
2015-03-31  9:02   ` Yang Kuankuan
2015-03-31 11:57     ` Russell King - ARM Linux
2015-04-01  1:31       ` Yakir
2015-03-30 19:40 ` [PATCH RFC 03/11] drm: bridge/dw_hdmi: simplify hdmi_config_AVI() a little Russell King
2015-03-30 19:40 ` [PATCH RFC 04/11] drm: bridge/dw_hdmi: remove mhsyncpolarity/mvsyncpolarity/minterlaced Russell King
2015-03-30 19:40 ` [PATCH RFC 05/11] drm: bridge/dw_hdmi: introduce interface to setting sample rate Russell King
2015-03-30 19:40 ` [PATCH RFC 06/11] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio Russell King
2015-03-31  7:45   ` Yang Kuankuan
2015-03-31  9:15   ` Philipp Zabel
2015-03-30 19:40 ` [PATCH RFC 07/11] drm/edid: add function to help find SADs Russell King
2015-04-01 11:47   ` Jani Nikula
2015-04-01 11:56     ` Russell King - ARM Linux
2015-04-02 10:52       ` [PATCH] drm/edid: add #defines for ELD versions Jani Nikula
2015-03-30 19:40 ` [PATCH RFC 08/11] sound/core: add DRM ELD helper Russell King
2015-03-31  9:12   ` Philipp Zabel
2015-03-30 19:40 ` [PATCH RFC 09/11] sound/core: add IEC958 channel status helper Russell King
2015-03-31  8:30   ` Yang Kuankuan
2015-03-31  9:13     ` Russell King - ARM Linux
2015-04-01  2:04       ` Yakir
2015-04-01  7:58         ` Russell King - ARM Linux
2015-03-31  9:10   ` Philipp Zabel
2015-03-31  9:16     ` Russell King - ARM Linux
2015-03-30 19:40 ` [PATCH RFC 10/11] drm: bridge/dw_hdmi-ahb-audio: add audio driver Russell King
2015-03-30 19:40 ` [PATCH RFC 11/11] drm: bridge/dw_hdmi-ahb-audio: parse ELD from HDMI driver Russell King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).