All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5 v4] Add I2S/ADV7511 audio support for ARC AXS10x boards
@ 2016-04-07 16:53 ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: broonie, CARLOS.PALMINHA, Alexey.Brodkin, Vineet.Gupta1, architt,
	airlied, lgirdwood, perex, tiwai, Jose Abreu

ARC AXS10x platforms consist of a mainboard with several peripherals.
One of those peripherals is an HDMI output port controlled by the ADV7511
transmitter.

This patch set adds audio for the ADV7511 transmitter and I2S audio for
the AXS10x platform.

Changes v3 -> v4:
* Reintroduced custom PCM driver (see note below)
* Use DT boolean to switch between ALSA DMA engine PCM or custom PCM
* Use fifo depth to program I2S FCR
* Update I2S documentation

Changes v2 -> v3:
* Removed pll_config functions (as suggested by Alexey Brodkin)
* Removed HDMI start at adv7511_core (as suggested by Archit Taneja)
* Use NOP functions for adv7511_audio (as suggested by Archit Taneja)
* Added adv7511_audio_exit() function (as suggested by Archit Taneja)
* Moved adv7511 to its own folder (as suggested by Archit Taneja)
* Separated file rename of adv7511_core (as suggested by Emil Velikov)
* Compile adv7511 as module if ALSA SoC is compiled as module
* Load adv7511 audio only if declared in device tree (as suggested by Laurent Pinchart)
* Dropped custom platform driver, using now ALSA DMA engine
* Dropped IRQ handler for I2S

Changes v1 -> v2:
* DT bindings moved to separate patch (as suggested by Alexey Brodkin)
* Removed defconfigs entries (as suggested by Alexey Brodkin)


NOTE:
Although the mainline I2S driver uses ALSA DMA engine,
this controller can be built without DMA support so it
was necessary to add this custom platform driver so that
HDMI audio works in AXS boards.

Jose Abreu (5):
  drm/i2c/adv7511: Rename and move to separate folder
  drm/i2c/adv7511: Add audio support
  ASoC: dwc: Use fifo depth to program FCR
  ASoC: dwc: Add custom PCM driver
  ASoC: dwc: Update DOCUMENTATION for I2S Driver

 .../bindings/display/bridge/adi,adv7511.txt        |   3 +
 .../devicetree/bindings/sound/designware-i2s.txt   |   5 +
 drivers/gpu/drm/i2c/Kconfig                        |   6 +-
 drivers/gpu/drm/i2c/Makefile                       |   2 +-
 drivers/gpu/drm/i2c/adv7511/Kconfig                |  18 ++
 drivers/gpu/drm/i2c/adv7511/Makefile               |   3 +
 drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h        |  53 ++++
 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c        | 310 +++++++++++++++++++++
 .../drm/i2c/{adv7511.c => adv7511/adv7511_core.c}  |  43 +--
 include/sound/soc-dai.h                            |   1 +
 sound/soc/dwc/Kconfig                              |   9 +
 sound/soc/dwc/Makefile                             |   1 +
 sound/soc/dwc/designware.h                         |  70 +++++
 sound/soc/dwc/designware_i2s.c                     | 106 +++++--
 sound/soc/dwc/designware_pcm.c                     | 230 +++++++++++++++
 15 files changed, 796 insertions(+), 64 deletions(-)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Kconfig
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Makefile
 rename drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h (90%)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c
 rename drivers/gpu/drm/i2c/{adv7511.c => adv7511/adv7511_core.c} (97%)
 create mode 100644 sound/soc/dwc/designware.h
 create mode 100644 sound/soc/dwc/designware_pcm.c

-- 
1.9.1

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

* [PATCH 0/5 v4] Add I2S/ADV7511 audio support for ARC AXS10x boards
@ 2016-04-07 16:53 ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: Jose Abreu, architt, airlied, Vineet.Gupta1, Alexey.Brodkin,
	CARLOS.PALMINHA, lgirdwood, broonie, tiwai

ARC AXS10x platforms consist of a mainboard with several peripherals.
One of those peripherals is an HDMI output port controlled by the ADV7511
transmitter.

This patch set adds audio for the ADV7511 transmitter and I2S audio for
the AXS10x platform.

Changes v3 -> v4:
* Reintroduced custom PCM driver (see note below)
* Use DT boolean to switch between ALSA DMA engine PCM or custom PCM
* Use fifo depth to program I2S FCR
* Update I2S documentation

Changes v2 -> v3:
* Removed pll_config functions (as suggested by Alexey Brodkin)
* Removed HDMI start at adv7511_core (as suggested by Archit Taneja)
* Use NOP functions for adv7511_audio (as suggested by Archit Taneja)
* Added adv7511_audio_exit() function (as suggested by Archit Taneja)
* Moved adv7511 to its own folder (as suggested by Archit Taneja)
* Separated file rename of adv7511_core (as suggested by Emil Velikov)
* Compile adv7511 as module if ALSA SoC is compiled as module
* Load adv7511 audio only if declared in device tree (as suggested by Laurent Pinchart)
* Dropped custom platform driver, using now ALSA DMA engine
* Dropped IRQ handler for I2S

Changes v1 -> v2:
* DT bindings moved to separate patch (as suggested by Alexey Brodkin)
* Removed defconfigs entries (as suggested by Alexey Brodkin)


NOTE:
Although the mainline I2S driver uses ALSA DMA engine,
this controller can be built without DMA support so it
was necessary to add this custom platform driver so that
HDMI audio works in AXS boards.

Jose Abreu (5):
  drm/i2c/adv7511: Rename and move to separate folder
  drm/i2c/adv7511: Add audio support
  ASoC: dwc: Use fifo depth to program FCR
  ASoC: dwc: Add custom PCM driver
  ASoC: dwc: Update DOCUMENTATION for I2S Driver

 .../bindings/display/bridge/adi,adv7511.txt        |   3 +
 .../devicetree/bindings/sound/designware-i2s.txt   |   5 +
 drivers/gpu/drm/i2c/Kconfig                        |   6 +-
 drivers/gpu/drm/i2c/Makefile                       |   2 +-
 drivers/gpu/drm/i2c/adv7511/Kconfig                |  18 ++
 drivers/gpu/drm/i2c/adv7511/Makefile               |   3 +
 drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h        |  53 ++++
 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c        | 310 +++++++++++++++++++++
 .../drm/i2c/{adv7511.c => adv7511/adv7511_core.c}  |  43 +--
 include/sound/soc-dai.h                            |   1 +
 sound/soc/dwc/Kconfig                              |   9 +
 sound/soc/dwc/Makefile                             |   1 +
 sound/soc/dwc/designware.h                         |  70 +++++
 sound/soc/dwc/designware_i2s.c                     | 106 +++++--
 sound/soc/dwc/designware_pcm.c                     | 230 +++++++++++++++
 15 files changed, 796 insertions(+), 64 deletions(-)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Kconfig
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Makefile
 rename drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h (90%)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c
 rename drivers/gpu/drm/i2c/{adv7511.c => adv7511/adv7511_core.c} (97%)
 create mode 100644 sound/soc/dwc/designware.h
 create mode 100644 sound/soc/dwc/designware_pcm.c

-- 
1.9.1

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

* [PATCH 0/5 v4] Add I2S/ADV7511 audio support for ARC AXS10x boards
@ 2016-04-07 16:53 ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc

ARC AXS10x platforms consist of a mainboard with several peripherals.
One of those peripherals is an HDMI output port controlled by the ADV7511
transmitter.

This patch set adds audio for the ADV7511 transmitter and I2S audio for
the AXS10x platform.

Changes v3 -> v4:
* Reintroduced custom PCM driver (see note below)
* Use DT boolean to switch between ALSA DMA engine PCM or custom PCM
* Use fifo depth to program I2S FCR
* Update I2S documentation

Changes v2 -> v3:
* Removed pll_config functions (as suggested by Alexey Brodkin)
* Removed HDMI start at adv7511_core (as suggested by Archit Taneja)
* Use NOP functions for adv7511_audio (as suggested by Archit Taneja)
* Added adv7511_audio_exit() function (as suggested by Archit Taneja)
* Moved adv7511 to its own folder (as suggested by Archit Taneja)
* Separated file rename of adv7511_core (as suggested by Emil Velikov)
* Compile adv7511 as module if ALSA SoC is compiled as module
* Load adv7511 audio only if declared in device tree (as suggested by Laurent Pinchart)
* Dropped custom platform driver, using now ALSA DMA engine
* Dropped IRQ handler for I2S

Changes v1 -> v2:
* DT bindings moved to separate patch (as suggested by Alexey Brodkin)
* Removed defconfigs entries (as suggested by Alexey Brodkin)


NOTE:
Although the mainline I2S driver uses ALSA DMA engine,
this controller can be built without DMA support so it
was necessary to add this custom platform driver so that
HDMI audio works in AXS boards.

Jose Abreu (5):
  drm/i2c/adv7511: Rename and move to separate folder
  drm/i2c/adv7511: Add audio support
  ASoC: dwc: Use fifo depth to program FCR
  ASoC: dwc: Add custom PCM driver
  ASoC: dwc: Update DOCUMENTATION for I2S Driver

 .../bindings/display/bridge/adi,adv7511.txt        |   3 +
 .../devicetree/bindings/sound/designware-i2s.txt   |   5 +
 drivers/gpu/drm/i2c/Kconfig                        |   6 +-
 drivers/gpu/drm/i2c/Makefile                       |   2 +-
 drivers/gpu/drm/i2c/adv7511/Kconfig                |  18 ++
 drivers/gpu/drm/i2c/adv7511/Makefile               |   3 +
 drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h        |  53 ++++
 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c        | 310 +++++++++++++++++++++
 .../drm/i2c/{adv7511.c => adv7511/adv7511_core.c}  |  43 +--
 include/sound/soc-dai.h                            |   1 +
 sound/soc/dwc/Kconfig                              |   9 +
 sound/soc/dwc/Makefile                             |   1 +
 sound/soc/dwc/designware.h                         |  70 +++++
 sound/soc/dwc/designware_i2s.c                     | 106 +++++--
 sound/soc/dwc/designware_pcm.c                     | 230 +++++++++++++++
 15 files changed, 796 insertions(+), 64 deletions(-)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Kconfig
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Makefile
 rename drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h (90%)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c
 rename drivers/gpu/drm/i2c/{adv7511.c => adv7511/adv7511_core.c} (97%)
 create mode 100644 sound/soc/dwc/designware.h
 create mode 100644 sound/soc/dwc/designware_pcm.c

-- 
1.9.1

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

* [PATCH 1/5 v4] drm/i2c/adv7511: Rename and move to separate folder
  2016-04-07 16:53 ` Jose Abreu
  (?)
@ 2016-04-07 16:53   ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: broonie, CARLOS.PALMINHA, Alexey.Brodkin, Vineet.Gupta1, architt,
	airlied, lgirdwood, perex, tiwai, Jose Abreu

Main file of adv7511 driver was renamed from adv7511.c
to adv7511_core.c and moved to separate folder in order
to prepare the adding of audio support.

Struct adv7511 was moved to adv7511.h and functions
adv7511_packet_enable() and adv7511_packet_disable()
were made public also to prepare the adding of audio
support.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---

No changes v3 -> v4.

This patch was only introduced in v3.

 drivers/gpu/drm/i2c/Kconfig                        |  6 +---
 drivers/gpu/drm/i2c/Makefile                       |  2 +-
 drivers/gpu/drm/i2c/adv7511/Kconfig                |  6 ++++
 drivers/gpu/drm/i2c/adv7511/Makefile               |  2 ++
 drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h        | 31 +++++++++++++++++++++
 .../drm/i2c/{adv7511.c => adv7511/adv7511_core.c}  | 32 ++--------------------
 6 files changed, 43 insertions(+), 36 deletions(-)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Kconfig
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Makefile
 rename drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h (93%)
 rename drivers/gpu/drm/i2c/{adv7511.c => adv7511/adv7511_core.c} (97%)

diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
index 22c7ed6..9258daf 100644
--- a/drivers/gpu/drm/i2c/Kconfig
+++ b/drivers/gpu/drm/i2c/Kconfig
@@ -1,11 +1,7 @@
 menu "I2C encoder or helper chips"
      depends on DRM && DRM_KMS_HELPER && I2C
 
-config DRM_I2C_ADV7511
-	tristate "AV7511 encoder"
-	select REGMAP_I2C
-	help
-	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
+source "drivers/gpu/drm/i2c/adv7511/Kconfig"
 
 config DRM_I2C_CH7006
 	tristate "Chrontel ch7006 TV encoder"
diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile
index 2c72eb5..f144830 100644
--- a/drivers/gpu/drm/i2c/Makefile
+++ b/drivers/gpu/drm/i2c/Makefile
@@ -1,6 +1,6 @@
 ccflags-y := -Iinclude/drm
 
-obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
+obj-y += adv7511/
 
 ch7006-y := ch7006_drv.o ch7006_mode.o
 obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o
diff --git a/drivers/gpu/drm/i2c/adv7511/Kconfig b/drivers/gpu/drm/i2c/adv7511/Kconfig
new file mode 100644
index 0000000..302c8e34
--- /dev/null
+++ b/drivers/gpu/drm/i2c/adv7511/Kconfig
@@ -0,0 +1,6 @@
+config DRM_I2C_ADV7511
+	tristate "AV7511 encoder"
+	select REGMAP_I2C
+	help
+	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
+
diff --git a/drivers/gpu/drm/i2c/adv7511/Makefile b/drivers/gpu/drm/i2c/adv7511/Makefile
new file mode 100644
index 0000000..c13f5a1
--- /dev/null
+++ b/drivers/gpu/drm/i2c/adv7511/Makefile
@@ -0,0 +1,2 @@
+adv7511-y := adv7511_core.o
+obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
diff --git a/drivers/gpu/drm/i2c/adv7511.h b/drivers/gpu/drm/i2c/adv7511/adv7511.h
similarity index 93%
rename from drivers/gpu/drm/i2c/adv7511.h
rename to drivers/gpu/drm/i2c/adv7511/adv7511.h
index 38515b3..fcae1ee 100644
--- a/drivers/gpu/drm/i2c/adv7511.h
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511.h
@@ -286,4 +286,35 @@ struct adv7511_video_config {
 	struct hdmi_avi_infoframe avi_infoframe;
 };
 
+struct adv7511 {
+	struct i2c_client *i2c_main;
+	struct i2c_client *i2c_edid;
+
+	struct regmap *regmap;
+	struct regmap *packet_memory_regmap;
+	enum drm_connector_status status;
+	bool powered;
+
+	unsigned int f_tmds;
+
+	unsigned int current_edid_segment;
+	uint8_t edid_buf[256];
+	bool edid_read;
+
+	wait_queue_head_t wq;
+	struct drm_encoder *encoder;
+
+	bool embedded_sync;
+	enum adv7511_sync_polarity vsync_polarity;
+	enum adv7511_sync_polarity hsync_polarity;
+	bool rgb;
+
+	struct edid *edid;
+
+	struct gpio_desc *gpio_pd;
+};
+
+int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
+int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
+
 #endif /* __DRM_I2C_ADV7511_H__ */
diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
similarity index 97%
rename from drivers/gpu/drm/i2c/adv7511.c
rename to drivers/gpu/drm/i2c/adv7511/adv7511_core.c
index a02112b..2b00581 100644
--- a/drivers/gpu/drm/i2c/adv7511.c
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
@@ -20,34 +20,6 @@
 
 #include "adv7511.h"
 
-struct adv7511 {
-	struct i2c_client *i2c_main;
-	struct i2c_client *i2c_edid;
-
-	struct regmap *regmap;
-	struct regmap *packet_memory_regmap;
-	enum drm_connector_status status;
-	bool powered;
-
-	unsigned int f_tmds;
-
-	unsigned int current_edid_segment;
-	uint8_t edid_buf[256];
-	bool edid_read;
-
-	wait_queue_head_t wq;
-	struct drm_encoder *encoder;
-
-	bool embedded_sync;
-	enum adv7511_sync_polarity vsync_polarity;
-	enum adv7511_sync_polarity hsync_polarity;
-	bool rgb;
-
-	struct edid *edid;
-
-	struct gpio_desc *gpio_pd;
-};
-
 static struct adv7511 *encoder_to_adv7511(struct drm_encoder *encoder)
 {
 	return to_encoder_slave(encoder)->slave_priv;
@@ -194,7 +166,7 @@ static void adv7511_set_colormap(struct adv7511 *adv7511, bool enable,
 			   ADV7511_CSC_UPDATE_MODE, 0);
 }
 
-static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
+int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
 {
 	if (packet & 0xff)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0,
@@ -209,7 +181,7 @@ static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
 	return 0;
 }
 
-static int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
+int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
 {
 	if (packet & 0xff)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0,
-- 
1.9.1

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

* [PATCH 1/5 v4] drm/i2c/adv7511: Rename and move to separate folder
@ 2016-04-07 16:53   ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: Jose Abreu, architt, airlied, Vineet.Gupta1, Alexey.Brodkin,
	CARLOS.PALMINHA, lgirdwood, broonie, tiwai, perex

Main file of adv7511 driver was renamed from adv7511.c
to adv7511_core.c and moved to separate folder in order
to prepare the adding of audio support.

Struct adv7511 was moved to adv7511.h and functions
adv7511_packet_enable() and adv7511_packet_disable()
were made public also to prepare the adding of audio
support.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---

No changes v3 -> v4.

This patch was only introduced in v3.

 drivers/gpu/drm/i2c/Kconfig                        |  6 +---
 drivers/gpu/drm/i2c/Makefile                       |  2 +-
 drivers/gpu/drm/i2c/adv7511/Kconfig                |  6 ++++
 drivers/gpu/drm/i2c/adv7511/Makefile               |  2 ++
 drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h        | 31 +++++++++++++++++++++
 .../drm/i2c/{adv7511.c => adv7511/adv7511_core.c}  | 32 ++--------------------
 6 files changed, 43 insertions(+), 36 deletions(-)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Kconfig
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Makefile
 rename drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h (93%)
 rename drivers/gpu/drm/i2c/{adv7511.c => adv7511/adv7511_core.c} (97%)

diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
index 22c7ed6..9258daf 100644
--- a/drivers/gpu/drm/i2c/Kconfig
+++ b/drivers/gpu/drm/i2c/Kconfig
@@ -1,11 +1,7 @@
 menu "I2C encoder or helper chips"
      depends on DRM && DRM_KMS_HELPER && I2C
 
-config DRM_I2C_ADV7511
-	tristate "AV7511 encoder"
-	select REGMAP_I2C
-	help
-	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
+source "drivers/gpu/drm/i2c/adv7511/Kconfig"
 
 config DRM_I2C_CH7006
 	tristate "Chrontel ch7006 TV encoder"
diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile
index 2c72eb5..f144830 100644
--- a/drivers/gpu/drm/i2c/Makefile
+++ b/drivers/gpu/drm/i2c/Makefile
@@ -1,6 +1,6 @@
 ccflags-y := -Iinclude/drm
 
-obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
+obj-y += adv7511/
 
 ch7006-y := ch7006_drv.o ch7006_mode.o
 obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o
diff --git a/drivers/gpu/drm/i2c/adv7511/Kconfig b/drivers/gpu/drm/i2c/adv7511/Kconfig
new file mode 100644
index 0000000..302c8e34
--- /dev/null
+++ b/drivers/gpu/drm/i2c/adv7511/Kconfig
@@ -0,0 +1,6 @@
+config DRM_I2C_ADV7511
+	tristate "AV7511 encoder"
+	select REGMAP_I2C
+	help
+	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
+
diff --git a/drivers/gpu/drm/i2c/adv7511/Makefile b/drivers/gpu/drm/i2c/adv7511/Makefile
new file mode 100644
index 0000000..c13f5a1
--- /dev/null
+++ b/drivers/gpu/drm/i2c/adv7511/Makefile
@@ -0,0 +1,2 @@
+adv7511-y := adv7511_core.o
+obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
diff --git a/drivers/gpu/drm/i2c/adv7511.h b/drivers/gpu/drm/i2c/adv7511/adv7511.h
similarity index 93%
rename from drivers/gpu/drm/i2c/adv7511.h
rename to drivers/gpu/drm/i2c/adv7511/adv7511.h
index 38515b3..fcae1ee 100644
--- a/drivers/gpu/drm/i2c/adv7511.h
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511.h
@@ -286,4 +286,35 @@ struct adv7511_video_config {
 	struct hdmi_avi_infoframe avi_infoframe;
 };
 
+struct adv7511 {
+	struct i2c_client *i2c_main;
+	struct i2c_client *i2c_edid;
+
+	struct regmap *regmap;
+	struct regmap *packet_memory_regmap;
+	enum drm_connector_status status;
+	bool powered;
+
+	unsigned int f_tmds;
+
+	unsigned int current_edid_segment;
+	uint8_t edid_buf[256];
+	bool edid_read;
+
+	wait_queue_head_t wq;
+	struct drm_encoder *encoder;
+
+	bool embedded_sync;
+	enum adv7511_sync_polarity vsync_polarity;
+	enum adv7511_sync_polarity hsync_polarity;
+	bool rgb;
+
+	struct edid *edid;
+
+	struct gpio_desc *gpio_pd;
+};
+
+int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
+int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
+
 #endif /* __DRM_I2C_ADV7511_H__ */
diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
similarity index 97%
rename from drivers/gpu/drm/i2c/adv7511.c
rename to drivers/gpu/drm/i2c/adv7511/adv7511_core.c
index a02112b..2b00581 100644
--- a/drivers/gpu/drm/i2c/adv7511.c
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
@@ -20,34 +20,6 @@
 
 #include "adv7511.h"
 
-struct adv7511 {
-	struct i2c_client *i2c_main;
-	struct i2c_client *i2c_edid;
-
-	struct regmap *regmap;
-	struct regmap *packet_memory_regmap;
-	enum drm_connector_status status;
-	bool powered;
-
-	unsigned int f_tmds;
-
-	unsigned int current_edid_segment;
-	uint8_t edid_buf[256];
-	bool edid_read;
-
-	wait_queue_head_t wq;
-	struct drm_encoder *encoder;
-
-	bool embedded_sync;
-	enum adv7511_sync_polarity vsync_polarity;
-	enum adv7511_sync_polarity hsync_polarity;
-	bool rgb;
-
-	struct edid *edid;
-
-	struct gpio_desc *gpio_pd;
-};
-
 static struct adv7511 *encoder_to_adv7511(struct drm_encoder *encoder)
 {
 	return to_encoder_slave(encoder)->slave_priv;
@@ -194,7 +166,7 @@ static void adv7511_set_colormap(struct adv7511 *adv7511, bool enable,
 			   ADV7511_CSC_UPDATE_MODE, 0);
 }
 
-static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
+int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
 {
 	if (packet & 0xff)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0,
@@ -209,7 +181,7 @@ static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
 	return 0;
 }
 
-static int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
+int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
 {
 	if (packet & 0xff)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0,
-- 
1.9.1

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

* [PATCH 1/5 v4] drm/i2c/adv7511: Rename and move to separate folder
@ 2016-04-07 16:53   ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc

Main file of adv7511 driver was renamed from adv7511.c
to adv7511_core.c and moved to separate folder in order
to prepare the adding of audio support.

Struct adv7511 was moved to adv7511.h and functions
adv7511_packet_enable() and adv7511_packet_disable()
were made public also to prepare the adding of audio
support.

Signed-off-by: Jose Abreu <joabreu at synopsys.com>
---

No changes v3 -> v4.

This patch was only introduced in v3.

 drivers/gpu/drm/i2c/Kconfig                        |  6 +---
 drivers/gpu/drm/i2c/Makefile                       |  2 +-
 drivers/gpu/drm/i2c/adv7511/Kconfig                |  6 ++++
 drivers/gpu/drm/i2c/adv7511/Makefile               |  2 ++
 drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h        | 31 +++++++++++++++++++++
 .../drm/i2c/{adv7511.c => adv7511/adv7511_core.c}  | 32 ++--------------------
 6 files changed, 43 insertions(+), 36 deletions(-)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Kconfig
 create mode 100644 drivers/gpu/drm/i2c/adv7511/Makefile
 rename drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h (93%)
 rename drivers/gpu/drm/i2c/{adv7511.c => adv7511/adv7511_core.c} (97%)

diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
index 22c7ed6..9258daf 100644
--- a/drivers/gpu/drm/i2c/Kconfig
+++ b/drivers/gpu/drm/i2c/Kconfig
@@ -1,11 +1,7 @@
 menu "I2C encoder or helper chips"
      depends on DRM && DRM_KMS_HELPER && I2C
 
-config DRM_I2C_ADV7511
-	tristate "AV7511 encoder"
-	select REGMAP_I2C
-	help
-	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
+source "drivers/gpu/drm/i2c/adv7511/Kconfig"
 
 config DRM_I2C_CH7006
 	tristate "Chrontel ch7006 TV encoder"
diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile
index 2c72eb5..f144830 100644
--- a/drivers/gpu/drm/i2c/Makefile
+++ b/drivers/gpu/drm/i2c/Makefile
@@ -1,6 +1,6 @@
 ccflags-y := -Iinclude/drm
 
-obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
+obj-y += adv7511/
 
 ch7006-y := ch7006_drv.o ch7006_mode.o
 obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o
diff --git a/drivers/gpu/drm/i2c/adv7511/Kconfig b/drivers/gpu/drm/i2c/adv7511/Kconfig
new file mode 100644
index 0000000..302c8e34
--- /dev/null
+++ b/drivers/gpu/drm/i2c/adv7511/Kconfig
@@ -0,0 +1,6 @@
+config DRM_I2C_ADV7511
+	tristate "AV7511 encoder"
+	select REGMAP_I2C
+	help
+	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
+
diff --git a/drivers/gpu/drm/i2c/adv7511/Makefile b/drivers/gpu/drm/i2c/adv7511/Makefile
new file mode 100644
index 0000000..c13f5a1
--- /dev/null
+++ b/drivers/gpu/drm/i2c/adv7511/Makefile
@@ -0,0 +1,2 @@
+adv7511-y := adv7511_core.o
+obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
diff --git a/drivers/gpu/drm/i2c/adv7511.h b/drivers/gpu/drm/i2c/adv7511/adv7511.h
similarity index 93%
rename from drivers/gpu/drm/i2c/adv7511.h
rename to drivers/gpu/drm/i2c/adv7511/adv7511.h
index 38515b3..fcae1ee 100644
--- a/drivers/gpu/drm/i2c/adv7511.h
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511.h
@@ -286,4 +286,35 @@ struct adv7511_video_config {
 	struct hdmi_avi_infoframe avi_infoframe;
 };
 
+struct adv7511 {
+	struct i2c_client *i2c_main;
+	struct i2c_client *i2c_edid;
+
+	struct regmap *regmap;
+	struct regmap *packet_memory_regmap;
+	enum drm_connector_status status;
+	bool powered;
+
+	unsigned int f_tmds;
+
+	unsigned int current_edid_segment;
+	uint8_t edid_buf[256];
+	bool edid_read;
+
+	wait_queue_head_t wq;
+	struct drm_encoder *encoder;
+
+	bool embedded_sync;
+	enum adv7511_sync_polarity vsync_polarity;
+	enum adv7511_sync_polarity hsync_polarity;
+	bool rgb;
+
+	struct edid *edid;
+
+	struct gpio_desc *gpio_pd;
+};
+
+int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
+int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
+
 #endif /* __DRM_I2C_ADV7511_H__ */
diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
similarity index 97%
rename from drivers/gpu/drm/i2c/adv7511.c
rename to drivers/gpu/drm/i2c/adv7511/adv7511_core.c
index a02112b..2b00581 100644
--- a/drivers/gpu/drm/i2c/adv7511.c
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
@@ -20,34 +20,6 @@
 
 #include "adv7511.h"
 
-struct adv7511 {
-	struct i2c_client *i2c_main;
-	struct i2c_client *i2c_edid;
-
-	struct regmap *regmap;
-	struct regmap *packet_memory_regmap;
-	enum drm_connector_status status;
-	bool powered;
-
-	unsigned int f_tmds;
-
-	unsigned int current_edid_segment;
-	uint8_t edid_buf[256];
-	bool edid_read;
-
-	wait_queue_head_t wq;
-	struct drm_encoder *encoder;
-
-	bool embedded_sync;
-	enum adv7511_sync_polarity vsync_polarity;
-	enum adv7511_sync_polarity hsync_polarity;
-	bool rgb;
-
-	struct edid *edid;
-
-	struct gpio_desc *gpio_pd;
-};
-
 static struct adv7511 *encoder_to_adv7511(struct drm_encoder *encoder)
 {
 	return to_encoder_slave(encoder)->slave_priv;
@@ -194,7 +166,7 @@ static void adv7511_set_colormap(struct adv7511 *adv7511, bool enable,
 			   ADV7511_CSC_UPDATE_MODE, 0);
 }
 
-static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
+int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
 {
 	if (packet & 0xff)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0,
@@ -209,7 +181,7 @@ static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
 	return 0;
 }
 
-static int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
+int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
 {
 	if (packet & 0xff)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0,
-- 
1.9.1

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

* [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-07 16:53 ` Jose Abreu
@ 2016-04-07 16:53   ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: broonie, CARLOS.PALMINHA, Alexey.Brodkin, Vineet.Gupta1, architt,
	airlied, lgirdwood, perex, tiwai, Jose Abreu

This patch adds audio support for the ADV7511 HDMI transmitter
using ALSA SoC.

The code was ported from Analog Devices linux tree from
commit 1770c4a1e32b ("Merge remote-tracking branch
'xilinx/master' into xcomm_zynq"), which is available at:
	- https://github.com/analogdevicesinc/linux/

The audio can be disabled using menu-config and/or device tree
so it is possible to use only video mode. The audio
(when enabled) registers as a codec into ALSA.

SPDIF DAI format was also added to ASoC as it is required
by adv7511 audio.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---

No changes v3 -> v4.

Changes v2 -> v3:
* Removed HDMI start at adv7511_core (as suggested by Archit Taneja)
* Use NOP functions for adv7511_audio (as suggested by Archit Taneja)
* Added adv7511_audio_exit() function (as suggested by Archit Taneja)
* Moved adv7511 to its own folder (as suggested by Archit Taneja)
* Compile adv7511 as module if ALSA SoC is compiled as module
* Load adv7511 audio only if declared in device tree (as suggested by Laurent Pinchart)

No changes v1 -> v2.

 .../bindings/display/bridge/adi,adv7511.txt        |   3 +
 drivers/gpu/drm/i2c/adv7511/Kconfig                |  12 +
 drivers/gpu/drm/i2c/adv7511/Makefile               |   1 +
 drivers/gpu/drm/i2c/adv7511/adv7511.h              |  22 ++
 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c        | 310 +++++++++++++++++++++
 drivers/gpu/drm/i2c/adv7511/adv7511_core.c         |  11 +
 include/sound/soc-dai.h                            |   1 +
 7 files changed, 360 insertions(+)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 96c25ee..920e542 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -43,6 +43,9 @@ Optional properties:
   data stream (similar to BT.656). Defaults to separate H/V synchronization
   signals.
 
+- adi,enable-audio: If set the ADV7511 driver will register a codec interface
+  into ALSA SoC.
+
 Required nodes:
 
 The ADV7511 has two video ports. Their connections are modelled using the OF
diff --git a/drivers/gpu/drm/i2c/adv7511/Kconfig b/drivers/gpu/drm/i2c/adv7511/Kconfig
index 302c8e34..900f3e9 100644
--- a/drivers/gpu/drm/i2c/adv7511/Kconfig
+++ b/drivers/gpu/drm/i2c/adv7511/Kconfig
@@ -4,3 +4,15 @@ config DRM_I2C_ADV7511
 	help
 	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
 
+config DRM_I2C_ADV7511_AUDIO
+	bool "ADV7511 audio"
+	depends on DRM_I2C_ADV7511
+	depends on SND_SOC=y || (SND_SOC && DRM_I2C_ADV7511=m)
+	default y
+	help
+	  This adds support for audio on the ADV7511(W) and ADV7513 HDMI
+	  encoders.
+
+	  By selecting this option the ADV7511 will register a codec interface
+	  into ALSA.
+
diff --git a/drivers/gpu/drm/i2c/adv7511/Makefile b/drivers/gpu/drm/i2c/adv7511/Makefile
index c13f5a1..d68773a 100644
--- a/drivers/gpu/drm/i2c/adv7511/Makefile
+++ b/drivers/gpu/drm/i2c/adv7511/Makefile
@@ -1,2 +1,3 @@
 adv7511-y := adv7511_core.o
+adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
diff --git a/drivers/gpu/drm/i2c/adv7511/adv7511.h b/drivers/gpu/drm/i2c/adv7511/adv7511.h
index fcae1ee..35828f0 100644
--- a/drivers/gpu/drm/i2c/adv7511/adv7511.h
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511.h
@@ -10,6 +10,7 @@
 #define __DRM_I2C_ADV7511_H__
 
 #include <linux/hdmi.h>
+#include <drm/drmP.h>
 
 #define ADV7511_REG_CHIP_REVISION		0x00
 #define ADV7511_REG_N0				0x01
@@ -241,6 +242,7 @@ enum adv7511_sync_polarity {
  * @sync_pulse:			Select the sync pulse
  * @vsync_polarity:		vsync input signal configuration
  * @hsync_polarity:		hsync input signal configuration
+ * @enable_audio		True if audio is enabled
  */
 struct adv7511_link_config {
 	unsigned int input_color_depth;
@@ -255,6 +257,8 @@ struct adv7511_link_config {
 	enum adv7511_input_sync_pulse sync_pulse;
 	enum adv7511_sync_polarity vsync_polarity;
 	enum adv7511_sync_polarity hsync_polarity;
+
+	bool enable_audio;
 };
 
 /**
@@ -296,6 +300,10 @@ struct adv7511 {
 	bool powered;
 
 	unsigned int f_tmds;
+	unsigned int f_audio;
+
+	unsigned int audio_source;
+	bool enable_audio;
 
 	unsigned int current_edid_segment;
 	uint8_t edid_buf[256];
@@ -317,4 +325,18 @@ struct adv7511 {
 int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
 int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
 
+#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO
+int adv7511_audio_init(struct device *dev);
+void adv7511_audio_exit(struct device *dev);
+#else
+int adv7511_audio_init(struct device *dev)
+{
+	return 0;
+}
+void adv7511_audio_exit(struct device *dev)
+{
+
+}
+#endif
+
 #endif /* __DRM_I2C_ADV7511_H__ */
diff --git a/drivers/gpu/drm/i2c/adv7511/adv7511_audio.c b/drivers/gpu/drm/i2c/adv7511/adv7511_audio.c
new file mode 100644
index 0000000..5562ed5
--- /dev/null
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511_audio.c
@@ -0,0 +1,310 @@
+/*
+ * Analog Devices ADV7511 HDMI transmitter driver
+ *
+ * Copyright 2012 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/pm.h>
+#include <linux/i2c.h>
+#include <linux/spi/spi.h>
+#include <linux/slab.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/initval.h>
+#include <sound/tlv.h>
+
+#include "adv7511.h"
+
+static const struct snd_soc_dapm_widget adv7511_dapm_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("TMDS"),
+	SND_SOC_DAPM_AIF_IN("AIFIN", "Playback", 0, SND_SOC_NOPM, 0, 0),
+};
+
+static const struct snd_soc_dapm_route adv7511_routes[] = {
+	{ "TMDS", NULL, "AIFIN" },
+};
+
+static void adv7511_calc_cts_n(unsigned int f_tmds, unsigned int fs,
+			       unsigned int *cts, unsigned int *n)
+{
+	switch (fs) {
+	case 32000:
+		*n = 4096;
+		break;
+	case 44100:
+		*n = 6272;
+		break;
+	case 48000:
+		*n = 6144;
+		break;
+	}
+
+	*cts = ((f_tmds * *n) / (128 * fs)) * 1000;
+}
+
+static int adv7511_update_cts_n(struct adv7511 *adv7511)
+{
+	unsigned int cts = 0;
+	unsigned int n = 0;
+
+	adv7511_calc_cts_n(adv7511->f_tmds, adv7511->f_audio, &cts, &n);
+
+	regmap_write(adv7511->regmap, ADV7511_REG_N0, (n >> 16) & 0xf);
+	regmap_write(adv7511->regmap, ADV7511_REG_N1, (n >> 8) & 0xff);
+	regmap_write(adv7511->regmap, ADV7511_REG_N2, n & 0xff);
+
+	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL0,
+		     (cts >> 16) & 0xf);
+	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL1,
+		     (cts >> 8) & 0xff);
+	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL2,
+		     cts & 0xff);
+
+	return 0;
+}
+
+static int adv7511_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_codec *codec = rtd->codec;
+	struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec);
+	unsigned int rate;
+	unsigned int len;
+
+	switch (params_rate(params)) {
+	case 32000:
+		rate = ADV7511_SAMPLE_FREQ_32000;
+		break;
+	case 44100:
+		rate = ADV7511_SAMPLE_FREQ_44100;
+		break;
+	case 48000:
+		rate = ADV7511_SAMPLE_FREQ_48000;
+		break;
+	case 88200:
+		rate = ADV7511_SAMPLE_FREQ_88200;
+		break;
+	case 96000:
+		rate = ADV7511_SAMPLE_FREQ_96000;
+		break;
+	case 176400:
+		rate = ADV7511_SAMPLE_FREQ_176400;
+		break;
+	case 192000:
+		rate = ADV7511_SAMPLE_FREQ_192000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		len = ADV7511_I2S_SAMPLE_LEN_16;
+		break;
+	case SNDRV_PCM_FORMAT_S18_3LE:
+		len = ADV7511_I2S_SAMPLE_LEN_18;
+		break;
+	case SNDRV_PCM_FORMAT_S20_3LE:
+		len = ADV7511_I2S_SAMPLE_LEN_20;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		len = ADV7511_I2S_SAMPLE_LEN_24;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	adv7511->f_audio = params_rate(params);
+
+	adv7511_update_cts_n(adv7511);
+
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CFG3,
+			   ADV7511_AUDIO_CFG3_LEN_MASK, len);
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG,
+			   ADV7511_I2C_FREQ_ID_CFG_RATE_MASK, rate << 4);
+
+	return 0;
+}
+
+static int adv7511_set_dai_fmt(struct snd_soc_dai *codec_dai,
+			       unsigned int fmt)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec);
+	unsigned int audio_source, i2s_format = 0;
+	unsigned int invert_clock;
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		audio_source = ADV7511_AUDIO_SOURCE_I2S;
+		i2s_format = ADV7511_I2S_FORMAT_I2S;
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		audio_source = ADV7511_AUDIO_SOURCE_I2S;
+		i2s_format = ADV7511_I2S_FORMAT_RIGHT_J;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		audio_source = ADV7511_AUDIO_SOURCE_I2S;
+		i2s_format = ADV7511_I2S_FORMAT_LEFT_J;
+		break;
+	case SND_SOC_DAIFMT_SPDIF:
+		audio_source = ADV7511_AUDIO_SOURCE_SPDIF;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFS:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		invert_clock = 0;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		invert_clock = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_SOURCE, 0x70,
+			   audio_source << 4);
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, BIT(6),
+			   invert_clock << 6);
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_I2S_CONFIG, 0x03,
+			   i2s_format);
+
+	adv7511->audio_source = audio_source;
+
+	return 0;
+}
+
+static int adv7511_set_bias_level(struct snd_soc_codec *codec,
+				  enum snd_soc_bias_level level)
+{
+	struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec);
+
+	switch (level) {
+	case SND_SOC_BIAS_ON:
+		switch (adv7511->audio_source) {
+		case ADV7511_AUDIO_SOURCE_I2S:
+			break;
+		case ADV7511_AUDIO_SOURCE_SPDIF:
+			regmap_update_bits(adv7511->regmap,
+					   ADV7511_REG_AUDIO_CONFIG, BIT(7),
+					   BIT(7));
+			break;
+		}
+		break;
+	case SND_SOC_BIAS_PREPARE:
+		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_STANDBY) {
+			adv7511_packet_enable(adv7511,
+					ADV7511_PACKET_ENABLE_AUDIO_SAMPLE);
+			adv7511_packet_enable(adv7511,
+					ADV7511_PACKET_ENABLE_AUDIO_INFOFRAME);
+			adv7511_packet_enable(adv7511,
+					ADV7511_PACKET_ENABLE_N_CTS);
+		} else {
+			adv7511_packet_disable(adv7511,
+					ADV7511_PACKET_ENABLE_AUDIO_SAMPLE);
+			adv7511_packet_disable(adv7511,
+					ADV7511_PACKET_ENABLE_AUDIO_INFOFRAME);
+			adv7511_packet_disable(adv7511,
+					ADV7511_PACKET_ENABLE_N_CTS);
+		}
+		break;
+	case SND_SOC_BIAS_STANDBY:
+		regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG,
+				   BIT(7), 0);
+		break;
+	case SND_SOC_BIAS_OFF:
+		break;
+	}
+	return 0;
+}
+
+#define ADV7511_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)
+
+#define ADV7511_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S18_3LE |\
+		SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S24_LE)
+
+static const struct snd_soc_dai_ops adv7511_dai_ops = {
+	.hw_params	= adv7511_hw_params,
+	/*.set_sysclk	= adv7511_set_dai_sysclk,*/
+	.set_fmt	= adv7511_set_dai_fmt,
+};
+
+static struct snd_soc_dai_driver adv7511_dai = {
+	.name = "adv7511",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = ADV7511_RATES,
+		.formats = ADV7511_FORMATS,
+	},
+	.ops = &adv7511_dai_ops,
+};
+
+static int adv7511_suspend(struct snd_soc_codec *codec)
+{
+	return adv7511_set_bias_level(codec, SND_SOC_BIAS_OFF);
+}
+
+static int adv7511_resume(struct snd_soc_codec *codec)
+{
+	return adv7511_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
+}
+
+static int adv7511_probe(struct snd_soc_codec *codec)
+{
+	return adv7511_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
+}
+
+static int adv7511_remove(struct snd_soc_codec *codec)
+{
+	adv7511_set_bias_level(codec, SND_SOC_BIAS_OFF);
+	return 0;
+}
+
+static struct snd_soc_codec_driver adv7511_codec_driver = {
+	.probe		    = adv7511_probe,
+	.remove		    = adv7511_remove,
+	.suspend	    = adv7511_suspend,
+	.resume		    = adv7511_resume,
+	.set_bias_level	    = adv7511_set_bias_level,
+
+	.dapm_widgets	    = adv7511_dapm_widgets,
+	.num_dapm_widgets   = ARRAY_SIZE(adv7511_dapm_widgets),
+	.dapm_routes	    = adv7511_routes,
+	.num_dapm_routes    = ARRAY_SIZE(adv7511_routes),
+};
+
+int adv7511_audio_init(struct device *dev)
+{
+	return snd_soc_register_codec(dev, &adv7511_codec_driver,
+			&adv7511_dai, 1);
+}
+
+void adv7511_audio_exit(struct device *dev)
+{
+	snd_soc_unregister_codec(dev);
+}
diff --git a/drivers/gpu/drm/i2c/adv7511/adv7511_core.c b/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
index 2b00581..140a64b 100644
--- a/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
@@ -329,6 +329,7 @@ static void adv7511_set_link_config(struct adv7511 *adv7511,
 	adv7511->hsync_polarity = config->hsync_polarity;
 	adv7511->vsync_polarity = config->vsync_polarity;
 	adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB;
+	adv7511->enable_audio = config->enable_audio;
 }
 
 static void adv7511_power_on(struct adv7511 *adv7511)
@@ -822,6 +823,7 @@ static int adv7511_parse_dt(struct device_node *np,
 		return -EINVAL;
 
 	config->embedded_sync = of_property_read_bool(np, "adi,embedded-sync");
+	config->enable_audio = of_property_read_bool(np, "adi,enable-audio");
 
 	/* Hardcode the sync pulse configurations for now. */
 	config->sync_pulse = ADV7511_INPUT_SYNC_PULSE_NONE;
@@ -916,6 +918,12 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 
 	adv7511_set_link_config(adv7511, &link_config);
 
+	if (link_config.enable_audio) {
+		ret = adv7511_audio_init(&i2c->dev);
+		if (ret)
+			goto err_i2c_unregister_device;
+	}
+
 	return 0;
 
 err_i2c_unregister_device:
@@ -930,6 +938,9 @@ static int adv7511_remove(struct i2c_client *i2c)
 
 	i2c_unregister_device(adv7511->i2c_edid);
 
+	if (adv7511->enable_audio)
+		adv7511_audio_exit(&i2c->dev);
+
 	kfree(adv7511->edid);
 
 	return 0;
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 964b7de..539c091 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -33,6 +33,7 @@ struct snd_compr_stream;
 #define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
 #define SND_SOC_DAIFMT_AC97		6 /* AC97 */
 #define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */
+#define SND_SOC_DAIFMT_SPDIF		8 /* SPDIF */
 
 /* left and right justified also known as MSB and LSB respectively */
 #define SND_SOC_DAIFMT_MSB		SND_SOC_DAIFMT_LEFT_J
-- 
1.9.1

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

* [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-07 16:53   ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc

This patch adds audio support for the ADV7511 HDMI transmitter
using ALSA SoC.

The code was ported from Analog Devices linux tree from
commit 1770c4a1e32b ("Merge remote-tracking branch
'xilinx/master' into xcomm_zynq"), which is available at:
	- https://github.com/analogdevicesinc/linux/

The audio can be disabled using menu-config and/or device tree
so it is possible to use only video mode. The audio
(when enabled) registers as a codec into ALSA.

SPDIF DAI format was also added to ASoC as it is required
by adv7511 audio.

Signed-off-by: Jose Abreu <joabreu at synopsys.com>
---

No changes v3 -> v4.

Changes v2 -> v3:
* Removed HDMI start at adv7511_core (as suggested by Archit Taneja)
* Use NOP functions for adv7511_audio (as suggested by Archit Taneja)
* Added adv7511_audio_exit() function (as suggested by Archit Taneja)
* Moved adv7511 to its own folder (as suggested by Archit Taneja)
* Compile adv7511 as module if ALSA SoC is compiled as module
* Load adv7511 audio only if declared in device tree (as suggested by Laurent Pinchart)

No changes v1 -> v2.

 .../bindings/display/bridge/adi,adv7511.txt        |   3 +
 drivers/gpu/drm/i2c/adv7511/Kconfig                |  12 +
 drivers/gpu/drm/i2c/adv7511/Makefile               |   1 +
 drivers/gpu/drm/i2c/adv7511/adv7511.h              |  22 ++
 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c        | 310 +++++++++++++++++++++
 drivers/gpu/drm/i2c/adv7511/adv7511_core.c         |  11 +
 include/sound/soc-dai.h                            |   1 +
 7 files changed, 360 insertions(+)
 create mode 100644 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 96c25ee..920e542 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -43,6 +43,9 @@ Optional properties:
   data stream (similar to BT.656). Defaults to separate H/V synchronization
   signals.
 
+- adi,enable-audio: If set the ADV7511 driver will register a codec interface
+  into ALSA SoC.
+
 Required nodes:
 
 The ADV7511 has two video ports. Their connections are modelled using the OF
diff --git a/drivers/gpu/drm/i2c/adv7511/Kconfig b/drivers/gpu/drm/i2c/adv7511/Kconfig
index 302c8e34..900f3e9 100644
--- a/drivers/gpu/drm/i2c/adv7511/Kconfig
+++ b/drivers/gpu/drm/i2c/adv7511/Kconfig
@@ -4,3 +4,15 @@ config DRM_I2C_ADV7511
 	help
 	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
 
+config DRM_I2C_ADV7511_AUDIO
+	bool "ADV7511 audio"
+	depends on DRM_I2C_ADV7511
+	depends on SND_SOC=y || (SND_SOC && DRM_I2C_ADV7511=m)
+	default y
+	help
+	  This adds support for audio on the ADV7511(W) and ADV7513 HDMI
+	  encoders.
+
+	  By selecting this option the ADV7511 will register a codec interface
+	  into ALSA.
+
diff --git a/drivers/gpu/drm/i2c/adv7511/Makefile b/drivers/gpu/drm/i2c/adv7511/Makefile
index c13f5a1..d68773a 100644
--- a/drivers/gpu/drm/i2c/adv7511/Makefile
+++ b/drivers/gpu/drm/i2c/adv7511/Makefile
@@ -1,2 +1,3 @@
 adv7511-y := adv7511_core.o
+adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
diff --git a/drivers/gpu/drm/i2c/adv7511/adv7511.h b/drivers/gpu/drm/i2c/adv7511/adv7511.h
index fcae1ee..35828f0 100644
--- a/drivers/gpu/drm/i2c/adv7511/adv7511.h
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511.h
@@ -10,6 +10,7 @@
 #define __DRM_I2C_ADV7511_H__
 
 #include <linux/hdmi.h>
+#include <drm/drmP.h>
 
 #define ADV7511_REG_CHIP_REVISION		0x00
 #define ADV7511_REG_N0				0x01
@@ -241,6 +242,7 @@ enum adv7511_sync_polarity {
  * @sync_pulse:			Select the sync pulse
  * @vsync_polarity:		vsync input signal configuration
  * @hsync_polarity:		hsync input signal configuration
+ * @enable_audio		True if audio is enabled
  */
 struct adv7511_link_config {
 	unsigned int input_color_depth;
@@ -255,6 +257,8 @@ struct adv7511_link_config {
 	enum adv7511_input_sync_pulse sync_pulse;
 	enum adv7511_sync_polarity vsync_polarity;
 	enum adv7511_sync_polarity hsync_polarity;
+
+	bool enable_audio;
 };
 
 /**
@@ -296,6 +300,10 @@ struct adv7511 {
 	bool powered;
 
 	unsigned int f_tmds;
+	unsigned int f_audio;
+
+	unsigned int audio_source;
+	bool enable_audio;
 
 	unsigned int current_edid_segment;
 	uint8_t edid_buf[256];
@@ -317,4 +325,18 @@ struct adv7511 {
 int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
 int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
 
+#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO
+int adv7511_audio_init(struct device *dev);
+void adv7511_audio_exit(struct device *dev);
+#else
+int adv7511_audio_init(struct device *dev)
+{
+	return 0;
+}
+void adv7511_audio_exit(struct device *dev)
+{
+
+}
+#endif
+
 #endif /* __DRM_I2C_ADV7511_H__ */
diff --git a/drivers/gpu/drm/i2c/adv7511/adv7511_audio.c b/drivers/gpu/drm/i2c/adv7511/adv7511_audio.c
new file mode 100644
index 0000000..5562ed5
--- /dev/null
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511_audio.c
@@ -0,0 +1,310 @@
+/*
+ * Analog Devices ADV7511 HDMI transmitter driver
+ *
+ * Copyright 2012 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/pm.h>
+#include <linux/i2c.h>
+#include <linux/spi/spi.h>
+#include <linux/slab.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/initval.h>
+#include <sound/tlv.h>
+
+#include "adv7511.h"
+
+static const struct snd_soc_dapm_widget adv7511_dapm_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("TMDS"),
+	SND_SOC_DAPM_AIF_IN("AIFIN", "Playback", 0, SND_SOC_NOPM, 0, 0),
+};
+
+static const struct snd_soc_dapm_route adv7511_routes[] = {
+	{ "TMDS", NULL, "AIFIN" },
+};
+
+static void adv7511_calc_cts_n(unsigned int f_tmds, unsigned int fs,
+			       unsigned int *cts, unsigned int *n)
+{
+	switch (fs) {
+	case 32000:
+		*n = 4096;
+		break;
+	case 44100:
+		*n = 6272;
+		break;
+	case 48000:
+		*n = 6144;
+		break;
+	}
+
+	*cts = ((f_tmds * *n) / (128 * fs)) * 1000;
+}
+
+static int adv7511_update_cts_n(struct adv7511 *adv7511)
+{
+	unsigned int cts = 0;
+	unsigned int n = 0;
+
+	adv7511_calc_cts_n(adv7511->f_tmds, adv7511->f_audio, &cts, &n);
+
+	regmap_write(adv7511->regmap, ADV7511_REG_N0, (n >> 16) & 0xf);
+	regmap_write(adv7511->regmap, ADV7511_REG_N1, (n >> 8) & 0xff);
+	regmap_write(adv7511->regmap, ADV7511_REG_N2, n & 0xff);
+
+	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL0,
+		     (cts >> 16) & 0xf);
+	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL1,
+		     (cts >> 8) & 0xff);
+	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL2,
+		     cts & 0xff);
+
+	return 0;
+}
+
+static int adv7511_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_codec *codec = rtd->codec;
+	struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec);
+	unsigned int rate;
+	unsigned int len;
+
+	switch (params_rate(params)) {
+	case 32000:
+		rate = ADV7511_SAMPLE_FREQ_32000;
+		break;
+	case 44100:
+		rate = ADV7511_SAMPLE_FREQ_44100;
+		break;
+	case 48000:
+		rate = ADV7511_SAMPLE_FREQ_48000;
+		break;
+	case 88200:
+		rate = ADV7511_SAMPLE_FREQ_88200;
+		break;
+	case 96000:
+		rate = ADV7511_SAMPLE_FREQ_96000;
+		break;
+	case 176400:
+		rate = ADV7511_SAMPLE_FREQ_176400;
+		break;
+	case 192000:
+		rate = ADV7511_SAMPLE_FREQ_192000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		len = ADV7511_I2S_SAMPLE_LEN_16;
+		break;
+	case SNDRV_PCM_FORMAT_S18_3LE:
+		len = ADV7511_I2S_SAMPLE_LEN_18;
+		break;
+	case SNDRV_PCM_FORMAT_S20_3LE:
+		len = ADV7511_I2S_SAMPLE_LEN_20;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		len = ADV7511_I2S_SAMPLE_LEN_24;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	adv7511->f_audio = params_rate(params);
+
+	adv7511_update_cts_n(adv7511);
+
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CFG3,
+			   ADV7511_AUDIO_CFG3_LEN_MASK, len);
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG,
+			   ADV7511_I2C_FREQ_ID_CFG_RATE_MASK, rate << 4);
+
+	return 0;
+}
+
+static int adv7511_set_dai_fmt(struct snd_soc_dai *codec_dai,
+			       unsigned int fmt)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec);
+	unsigned int audio_source, i2s_format = 0;
+	unsigned int invert_clock;
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		audio_source = ADV7511_AUDIO_SOURCE_I2S;
+		i2s_format = ADV7511_I2S_FORMAT_I2S;
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		audio_source = ADV7511_AUDIO_SOURCE_I2S;
+		i2s_format = ADV7511_I2S_FORMAT_RIGHT_J;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		audio_source = ADV7511_AUDIO_SOURCE_I2S;
+		i2s_format = ADV7511_I2S_FORMAT_LEFT_J;
+		break;
+	case SND_SOC_DAIFMT_SPDIF:
+		audio_source = ADV7511_AUDIO_SOURCE_SPDIF;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFS:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		invert_clock = 0;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		invert_clock = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_SOURCE, 0x70,
+			   audio_source << 4);
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, BIT(6),
+			   invert_clock << 6);
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_I2S_CONFIG, 0x03,
+			   i2s_format);
+
+	adv7511->audio_source = audio_source;
+
+	return 0;
+}
+
+static int adv7511_set_bias_level(struct snd_soc_codec *codec,
+				  enum snd_soc_bias_level level)
+{
+	struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec);
+
+	switch (level) {
+	case SND_SOC_BIAS_ON:
+		switch (adv7511->audio_source) {
+		case ADV7511_AUDIO_SOURCE_I2S:
+			break;
+		case ADV7511_AUDIO_SOURCE_SPDIF:
+			regmap_update_bits(adv7511->regmap,
+					   ADV7511_REG_AUDIO_CONFIG, BIT(7),
+					   BIT(7));
+			break;
+		}
+		break;
+	case SND_SOC_BIAS_PREPARE:
+		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_STANDBY) {
+			adv7511_packet_enable(adv7511,
+					ADV7511_PACKET_ENABLE_AUDIO_SAMPLE);
+			adv7511_packet_enable(adv7511,
+					ADV7511_PACKET_ENABLE_AUDIO_INFOFRAME);
+			adv7511_packet_enable(adv7511,
+					ADV7511_PACKET_ENABLE_N_CTS);
+		} else {
+			adv7511_packet_disable(adv7511,
+					ADV7511_PACKET_ENABLE_AUDIO_SAMPLE);
+			adv7511_packet_disable(adv7511,
+					ADV7511_PACKET_ENABLE_AUDIO_INFOFRAME);
+			adv7511_packet_disable(adv7511,
+					ADV7511_PACKET_ENABLE_N_CTS);
+		}
+		break;
+	case SND_SOC_BIAS_STANDBY:
+		regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG,
+				   BIT(7), 0);
+		break;
+	case SND_SOC_BIAS_OFF:
+		break;
+	}
+	return 0;
+}
+
+#define ADV7511_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)
+
+#define ADV7511_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S18_3LE |\
+		SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S24_LE)
+
+static const struct snd_soc_dai_ops adv7511_dai_ops = {
+	.hw_params	= adv7511_hw_params,
+	/*.set_sysclk	= adv7511_set_dai_sysclk,*/
+	.set_fmt	= adv7511_set_dai_fmt,
+};
+
+static struct snd_soc_dai_driver adv7511_dai = {
+	.name = "adv7511",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = ADV7511_RATES,
+		.formats = ADV7511_FORMATS,
+	},
+	.ops = &adv7511_dai_ops,
+};
+
+static int adv7511_suspend(struct snd_soc_codec *codec)
+{
+	return adv7511_set_bias_level(codec, SND_SOC_BIAS_OFF);
+}
+
+static int adv7511_resume(struct snd_soc_codec *codec)
+{
+	return adv7511_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
+}
+
+static int adv7511_probe(struct snd_soc_codec *codec)
+{
+	return adv7511_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
+}
+
+static int adv7511_remove(struct snd_soc_codec *codec)
+{
+	adv7511_set_bias_level(codec, SND_SOC_BIAS_OFF);
+	return 0;
+}
+
+static struct snd_soc_codec_driver adv7511_codec_driver = {
+	.probe		    = adv7511_probe,
+	.remove		    = adv7511_remove,
+	.suspend	    = adv7511_suspend,
+	.resume		    = adv7511_resume,
+	.set_bias_level	    = adv7511_set_bias_level,
+
+	.dapm_widgets	    = adv7511_dapm_widgets,
+	.num_dapm_widgets   = ARRAY_SIZE(adv7511_dapm_widgets),
+	.dapm_routes	    = adv7511_routes,
+	.num_dapm_routes    = ARRAY_SIZE(adv7511_routes),
+};
+
+int adv7511_audio_init(struct device *dev)
+{
+	return snd_soc_register_codec(dev, &adv7511_codec_driver,
+			&adv7511_dai, 1);
+}
+
+void adv7511_audio_exit(struct device *dev)
+{
+	snd_soc_unregister_codec(dev);
+}
diff --git a/drivers/gpu/drm/i2c/adv7511/adv7511_core.c b/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
index 2b00581..140a64b 100644
--- a/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
+++ b/drivers/gpu/drm/i2c/adv7511/adv7511_core.c
@@ -329,6 +329,7 @@ static void adv7511_set_link_config(struct adv7511 *adv7511,
 	adv7511->hsync_polarity = config->hsync_polarity;
 	adv7511->vsync_polarity = config->vsync_polarity;
 	adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB;
+	adv7511->enable_audio = config->enable_audio;
 }
 
 static void adv7511_power_on(struct adv7511 *adv7511)
@@ -822,6 +823,7 @@ static int adv7511_parse_dt(struct device_node *np,
 		return -EINVAL;
 
 	config->embedded_sync = of_property_read_bool(np, "adi,embedded-sync");
+	config->enable_audio = of_property_read_bool(np, "adi,enable-audio");
 
 	/* Hardcode the sync pulse configurations for now. */
 	config->sync_pulse = ADV7511_INPUT_SYNC_PULSE_NONE;
@@ -916,6 +918,12 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 
 	adv7511_set_link_config(adv7511, &link_config);
 
+	if (link_config.enable_audio) {
+		ret = adv7511_audio_init(&i2c->dev);
+		if (ret)
+			goto err_i2c_unregister_device;
+	}
+
 	return 0;
 
 err_i2c_unregister_device:
@@ -930,6 +938,9 @@ static int adv7511_remove(struct i2c_client *i2c)
 
 	i2c_unregister_device(adv7511->i2c_edid);
 
+	if (adv7511->enable_audio)
+		adv7511_audio_exit(&i2c->dev);
+
 	kfree(adv7511->edid);
 
 	return 0;
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 964b7de..539c091 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -33,6 +33,7 @@ struct snd_compr_stream;
 #define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
 #define SND_SOC_DAIFMT_AC97		6 /* AC97 */
 #define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */
+#define SND_SOC_DAIFMT_SPDIF		8 /* SPDIF */
 
 /* left and right justified also known as MSB and LSB respectively */
 #define SND_SOC_DAIFMT_MSB		SND_SOC_DAIFMT_LEFT_J
-- 
1.9.1

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

* [PATCH 3/5 v4] ASoC: dwc: Use fifo depth to program FCR
  2016-04-07 16:53 ` Jose Abreu
  (?)
@ 2016-04-07 16:53   ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: broonie, CARLOS.PALMINHA, Alexey.Brodkin, Vineet.Gupta1, architt,
	airlied, lgirdwood, perex, tiwai, Jose Abreu

This patch makes Designware I2S driver use the fifo
depth value to program the fifo configuration register
instead of using hardcoded values.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---

This patch was only introduced in v4.

 sound/soc/dwc/designware_i2s.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 3effcd1..0db69b7 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -100,6 +100,7 @@ struct dw_i2s_dev {
 	struct device *dev;
 	u32 ccr;
 	u32 xfer_resolution;
+	u32 fifo_th;
 
 	/* data related to DMA transfers b/w i2s and DMAC */
 	union dw_i2s_snd_dma_data play_dma_data;
@@ -232,14 +233,16 @@ static void dw_i2s_config(struct dw_i2s_dev *dev, int stream)
 		if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
 			i2s_write_reg(dev->i2s_base, TCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
+			i2s_write_reg(dev->i2s_base, TFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
 			i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
 		} else {
 			i2s_write_reg(dev->i2s_base, RCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
+			i2s_write_reg(dev->i2s_base, RFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
 			i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
@@ -499,6 +502,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 	 */
 	u32 comp1 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp1);
 	u32 comp2 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp2);
+	u32 fifo_depth = 1 << (1 + COMP1_FIFO_DEPTH_GLOBAL(comp1));
 	u32 idx;
 
 	if (dev->capability & DWC_I2S_RECORD &&
@@ -537,6 +541,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 		dev->capability |= DW_I2S_SLAVE;
 	}
 
+	dev->fifo_th = fifo_depth / 2;
 	return 0;
 }
 
-- 
1.9.1

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

* [PATCH 3/5 v4] ASoC: dwc: Use fifo depth to program FCR
@ 2016-04-07 16:53   ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: Jose Abreu, architt, airlied, Vineet.Gupta1, Alexey.Brodkin,
	CARLOS.PALMINHA, lgirdwood, broonie, tiwai, perex

This patch makes Designware I2S driver use the fifo
depth value to program the fifo configuration register
instead of using hardcoded values.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---

This patch was only introduced in v4.

 sound/soc/dwc/designware_i2s.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 3effcd1..0db69b7 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -100,6 +100,7 @@ struct dw_i2s_dev {
 	struct device *dev;
 	u32 ccr;
 	u32 xfer_resolution;
+	u32 fifo_th;
 
 	/* data related to DMA transfers b/w i2s and DMAC */
 	union dw_i2s_snd_dma_data play_dma_data;
@@ -232,14 +233,16 @@ static void dw_i2s_config(struct dw_i2s_dev *dev, int stream)
 		if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
 			i2s_write_reg(dev->i2s_base, TCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
+			i2s_write_reg(dev->i2s_base, TFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
 			i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
 		} else {
 			i2s_write_reg(dev->i2s_base, RCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
+			i2s_write_reg(dev->i2s_base, RFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
 			i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
@@ -499,6 +502,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 	 */
 	u32 comp1 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp1);
 	u32 comp2 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp2);
+	u32 fifo_depth = 1 << (1 + COMP1_FIFO_DEPTH_GLOBAL(comp1));
 	u32 idx;
 
 	if (dev->capability & DWC_I2S_RECORD &&
@@ -537,6 +541,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 		dev->capability |= DW_I2S_SLAVE;
 	}
 
+	dev->fifo_th = fifo_depth / 2;
 	return 0;
 }
 
-- 
1.9.1

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

* [PATCH 3/5 v4] ASoC: dwc: Use fifo depth to program FCR
@ 2016-04-07 16:53   ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc

This patch makes Designware I2S driver use the fifo
depth value to program the fifo configuration register
instead of using hardcoded values.

Signed-off-by: Jose Abreu <joabreu at synopsys.com>
---

This patch was only introduced in v4.

 sound/soc/dwc/designware_i2s.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 3effcd1..0db69b7 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -100,6 +100,7 @@ struct dw_i2s_dev {
 	struct device *dev;
 	u32 ccr;
 	u32 xfer_resolution;
+	u32 fifo_th;
 
 	/* data related to DMA transfers b/w i2s and DMAC */
 	union dw_i2s_snd_dma_data play_dma_data;
@@ -232,14 +233,16 @@ static void dw_i2s_config(struct dw_i2s_dev *dev, int stream)
 		if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
 			i2s_write_reg(dev->i2s_base, TCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
+			i2s_write_reg(dev->i2s_base, TFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
 			i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
 		} else {
 			i2s_write_reg(dev->i2s_base, RCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
+			i2s_write_reg(dev->i2s_base, RFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
 			i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
@@ -499,6 +502,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 	 */
 	u32 comp1 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp1);
 	u32 comp2 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp2);
+	u32 fifo_depth = 1 << (1 + COMP1_FIFO_DEPTH_GLOBAL(comp1));
 	u32 idx;
 
 	if (dev->capability & DWC_I2S_RECORD &&
@@ -537,6 +541,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 		dev->capability |= DW_I2S_SLAVE;
 	}
 
+	dev->fifo_th = fifo_depth / 2;
 	return 0;
 }
 
-- 
1.9.1

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

* [PATCH 4/5 v4] ASoC: dwc: Add custom PCM driver
  2016-04-07 16:53 ` Jose Abreu
@ 2016-04-07 16:53   ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: broonie, CARLOS.PALMINHA, Alexey.Brodkin, Vineet.Gupta1, architt,
	airlied, lgirdwood, perex, tiwai, Jose Abreu

HDMI audio support was added to the AXS board using an
I2S cpu driver and a custom platform driver.

The platform driver supports two channels @ 16 bits with
rates 32k, 44.1k and 48k.

Although the mainline I2S driver uses ALSA DMA engine,
this controller can be built without DMA support so it
was necessary to add this custom platform driver so that
HDMI audio works in AXS boards.

The selection between the use of DMA engine or custom
PCM can be made using a device tree boolean parameter
which was introduced in this patch ('snps,use-dmaengine').

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---

Changes v3 -> v4:
* Reintroduced custom PCM driver
* Use DT boolean to switch between ALSA DMA engine PCM or custom PCM

Changes v2 -> v3:
* Removed pll_config functions (as suggested by Alexey Brodkin)
* Dropped custom platform driver, using now ALSA DMA engine
* Dropped IRQ handler for I2S

No changes v1 -> v2.

 sound/soc/dwc/Kconfig          |   9 ++
 sound/soc/dwc/Makefile         |   1 +
 sound/soc/dwc/designware.h     |  70 +++++++++++++
 sound/soc/dwc/designware_i2s.c |  99 +++++++++++++-----
 sound/soc/dwc/designware_pcm.c | 230 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 382 insertions(+), 27 deletions(-)
 create mode 100644 sound/soc/dwc/designware.h
 create mode 100644 sound/soc/dwc/designware_pcm.c

diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
index d50e085..2a21120 100644
--- a/sound/soc/dwc/Kconfig
+++ b/sound/soc/dwc/Kconfig
@@ -7,4 +7,13 @@ config SND_DESIGNWARE_I2S
 	 Synopsys desigwnware I2S device. The device supports upto
 	 maximum of 8 channels each for play and record.
 
+config SND_DESIGNWARE_PCM
+	tristate "Synopsys I2S PCM Driver"
+	help
+	 Say Y or M if you want to add support for ALSA ASoC platform driver
+	 using I2S.
+
+	 Select this option if you want to be able to create a sound interface
+	 using the I2S device driver as CPU driver. Instead of using ALSA
+	 DMA engine by selecting this driver a custom PCM driver will be used.
 
diff --git a/sound/soc/dwc/Makefile b/sound/soc/dwc/Makefile
index 319371f..1b48bccc 100644
--- a/sound/soc/dwc/Makefile
+++ b/sound/soc/dwc/Makefile
@@ -1,3 +1,4 @@
 # SYNOPSYS Platform Support
 obj-$(CONFIG_SND_DESIGNWARE_I2S) += designware_i2s.o
+obj-$(CONFIG_SND_DESIGNWARE_PCM) += designware_pcm.o
 
diff --git a/sound/soc/dwc/designware.h b/sound/soc/dwc/designware.h
new file mode 100644
index 0000000..196296c
--- /dev/null
+++ b/sound/soc/dwc/designware.h
@@ -0,0 +1,70 @@
+/*
+ * ALSA SoC Synopsys Audio Layer
+ *
+ * sound/soc/dwc/designware.h
+ *
+ * Copyright (C) 2016 Synopsys
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef __DESIGNWARE_H
+#define __DESIGNWARE_H
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <sound/designware_i2s.h>
+#include <sound/dmaengine_pcm.h>
+
+struct dw_pcm_binfo {
+	struct snd_pcm_substream *stream;
+	unsigned char *dma_base;
+	unsigned char *dma_pointer;
+	unsigned int period_size_frames;
+	unsigned int size;
+	snd_pcm_uframes_t period_pointer;
+	unsigned int total_periods;
+	unsigned int current_period;
+};
+
+union dw_i2s_snd_dma_data {
+	struct i2s_dma_data pd;
+	struct snd_dmaengine_dai_dma_data dt;
+};
+
+struct dw_i2s_dev {
+	void __iomem *i2s_base;
+	struct clk *clk;
+	int active;
+	unsigned int capability;
+	unsigned int quirks;
+	unsigned int i2s_reg_comp1;
+	unsigned int i2s_reg_comp2;
+	struct device *dev;
+	u32 ccr;
+	u32 xfer_resolution;
+	u32 fifo_th;
+
+	/* data related to DMA transfers b/w i2s and DMAC */
+	bool use_dmaengine;
+	union dw_i2s_snd_dma_data play_dma_data;
+	union dw_i2s_snd_dma_data capture_dma_data;
+	struct i2s_clk_config_data config;
+	int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
+	struct dw_pcm_binfo binfo;
+};
+
+#ifdef CONFIG_SND_DESIGNWARE_PCM
+int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
+		struct dw_pcm_binfo *bi);
+#else
+int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
+		struct dw_pcm_binfo *bi)
+{
+	return 0;
+}
+#endif
+
+#endif
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 0db69b7..16056c1 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -24,6 +24,7 @@
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/dmaengine_pcm.h>
+#include "designware.h"
 
 /* common register for all channel */
 #define IER		0x000
@@ -84,31 +85,6 @@
 #define MAX_CHANNEL_NUM		8
 #define MIN_CHANNEL_NUM		2
 
-union dw_i2s_snd_dma_data {
-	struct i2s_dma_data pd;
-	struct snd_dmaengine_dai_dma_data dt;
-};
-
-struct dw_i2s_dev {
-	void __iomem *i2s_base;
-	struct clk *clk;
-	int active;
-	unsigned int capability;
-	unsigned int quirks;
-	unsigned int i2s_reg_comp1;
-	unsigned int i2s_reg_comp2;
-	struct device *dev;
-	u32 ccr;
-	u32 xfer_resolution;
-	u32 fifo_th;
-
-	/* data related to DMA transfers b/w i2s and DMAC */
-	union dw_i2s_snd_dma_data play_dma_data;
-	union dw_i2s_snd_dma_data capture_dma_data;
-	struct i2s_clk_config_data config;
-	int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
-};
-
 static inline void i2s_write_reg(void __iomem *io_base, int reg, u32 val)
 {
 	writel(val, io_base + reg);
@@ -145,6 +121,54 @@ static inline void i2s_clear_irqs(struct dw_i2s_dev *dev, u32 stream)
 	}
 }
 
+static irqreturn_t dw_i2s_irq_handler(int irq, void *dev_id)
+{
+	struct dw_i2s_dev *dev = dev_id;
+	u32 isr[4], sleft[dev->fifo_th], sright[dev->fifo_th];
+	int i, j, xfer_bytes = dev->config.data_width / 8;
+	int dir = dev->binfo.stream->stream;
+
+	for (i = 0; i < 4; i++)
+		isr[i] = i2s_read_reg(dev->i2s_base, ISR(i));
+
+	i2s_clear_irqs(dev, SNDRV_PCM_STREAM_PLAYBACK);
+	i2s_clear_irqs(dev, SNDRV_PCM_STREAM_CAPTURE);
+
+	if (dev->use_dmaengine)
+		return IRQ_HANDLED;
+
+	for (i = 0; i < 4; i++) {
+		/* Copy only to/from first two channels
+		 * TODO: Remaining channels
+		 */
+		if ((isr[i] & 0x10) && (i == 0) &&
+				(dir == SNDRV_PCM_STREAM_PLAYBACK)) {
+			/* TXFEM - TX FIFO is empty */
+			dw_pcm_transfer(sleft, sright, xfer_bytes, dev->fifo_th,
+					&dev->binfo);
+			for (j = 0; j < dev->fifo_th; j++) {
+				i2s_write_reg(dev->i2s_base, LRBR_LTHR(i),
+						sleft[j]);
+				i2s_write_reg(dev->i2s_base, RRBR_RTHR(i),
+						sright[j]);
+			}
+		} else if ((isr[i] & 0x01) && (i == 0) &&
+				(dir == SNDRV_PCM_STREAM_CAPTURE)) {
+			/* RXDAM - RX FIFO is full */
+			for (j = 0; j < dev->fifo_th; j++) {
+				sleft[j] = i2s_read_reg(dev->i2s_base,
+						LRBR_LTHR(i));
+				sright[j] = i2s_read_reg(dev->i2s_base,
+						RRBR_RTHR(i));
+			}
+			dw_pcm_transfer(sleft, sright, xfer_bytes, dev->fifo_th,
+					&dev->binfo);
+		}
+	}
+
+	return IRQ_HANDLED;
+}
+
 static void i2s_start(struct dw_i2s_dev *dev,
 		      struct snd_pcm_substream *substream)
 {
@@ -626,7 +650,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
 	const struct i2s_platform_data *pdata = pdev->dev.platform_data;
 	struct dw_i2s_dev *dev;
 	struct resource *res;
-	int ret;
+	int ret, irq_number;
 	struct snd_soc_dai_driver *dw_i2s_dai;
 	const char *clk_id;
 
@@ -649,6 +673,24 @@ static int dw_i2s_probe(struct platform_device *pdev)
 	if (IS_ERR(dev->i2s_base))
 		return PTR_ERR(dev->i2s_base);
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	dev->i2s_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(dev->i2s_base))
+		return PTR_ERR(dev->i2s_base);
+
+	irq_number = platform_get_irq(pdev, 0);
+	if (irq_number <= 0) {
+		dev_err(&pdev->dev, "get_irq fail\n");
+		return -EINVAL;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq_number, dw_i2s_irq_handler,
+			IRQF_SHARED, "dw_i2s_irq_handler", dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "request_irq fail\n");
+		return ret;
+	}
+
 	dev->dev = &pdev->dev;
 
 	dev->i2s_reg_comp1 = I2S_COMP_PARAM_1;
@@ -657,6 +699,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
 		dev->capability = pdata->cap;
 		clk_id = NULL;
 		dev->quirks = pdata->quirks;
+		dev->use_dmaengine = false;
 		if (dev->quirks & DW_I2S_QUIRK_COMP_REG_OFFSET) {
 			dev->i2s_reg_comp1 = pdata->i2s_reg_comp1;
 			dev->i2s_reg_comp2 = pdata->i2s_reg_comp2;
@@ -664,6 +707,8 @@ static int dw_i2s_probe(struct platform_device *pdev)
 		ret = dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata);
 	} else {
 		clk_id = "i2sclk";
+		dev->use_dmaengine = of_property_read_bool(pdev->dev.of_node,
+				"snps,use-dmaengine");
 		ret = dw_configure_dai_by_dt(dev, dw_i2s_dai, res);
 	}
 	if (ret < 0)
@@ -695,7 +740,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
 		goto err_clk_disable;
 	}
 
-	if (!pdata) {
+	if (dev->use_dmaengine) {
 		ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
 		if (ret) {
 			dev_err(&pdev->dev,
diff --git a/sound/soc/dwc/designware_pcm.c b/sound/soc/dwc/designware_pcm.c
new file mode 100644
index 0000000..84ff6b9
--- /dev/null
+++ b/sound/soc/dwc/designware_pcm.c
@@ -0,0 +1,230 @@
+/*
+ * Synopsys I2S PCM Driver
+ *
+ * Copyright (C) 2016 Synopsys
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/slab.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/dmaengine_pcm.h>
+#include "designware.h"
+
+#define BUFFER_BYTES_MAX	384000
+#define PERIOD_BYTES_MIN	2048
+#define PERIODS_MIN		8
+
+static const struct snd_pcm_hardware dw_pcm_hardware = {
+	.info = SNDRV_PCM_INFO_INTERLEAVED |
+		SNDRV_PCM_INFO_MMAP |
+		SNDRV_PCM_INFO_MMAP_VALID |
+		SNDRV_PCM_INFO_BLOCK_TRANSFER,
+	.rates = SNDRV_PCM_RATE_32000 |
+		SNDRV_PCM_RATE_44100 |
+		SNDRV_PCM_RATE_48000,
+	.rate_min = 32000,
+	.rate_max = 48000,
+	.formats = SNDRV_PCM_FMTBIT_S16_LE,
+	.channels_min = 2,
+	.channels_max = 2,
+	.buffer_bytes_max = BUFFER_BYTES_MAX,
+	.period_bytes_min = PERIOD_BYTES_MIN,
+	.period_bytes_max = BUFFER_BYTES_MAX / PERIODS_MIN,
+	.periods_min = PERIODS_MIN,
+	.periods_max = BUFFER_BYTES_MAX / PERIOD_BYTES_MIN,
+};
+
+int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
+		struct dw_pcm_binfo *bi)
+{
+	struct snd_pcm_runtime *rt = bi->stream->runtime;
+	int dir = bi->stream->stream;
+	int i;
+
+	for (i = 0; i < buf_size; i++) {
+		if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
+			memcpy(&lsample[i], bi->dma_pointer, bytes);
+			bi->dma_pointer += bytes;
+			memcpy(&rsample[i], bi->dma_pointer, bytes);
+			bi->dma_pointer += bytes;
+		} else {
+			memcpy(bi->dma_pointer, &lsample[i], bytes);
+			bi->dma_pointer += bytes;
+			memcpy(bi->dma_pointer, &rsample[i], bytes);
+			bi->dma_pointer += bytes;
+		}
+	}
+
+	bi->period_pointer += bytes_to_frames(rt, bytes * 2 * buf_size);
+
+	if (bi->period_pointer >= (bi->period_size_frames * bi->current_period)) {
+		bi->current_period++;
+		if (bi->current_period > bi->total_periods) {
+			bi->dma_pointer = bi->dma_base;
+			bi->period_pointer = 0;
+			bi->current_period = 1;
+		}
+		snd_pcm_period_elapsed(bi->stream);
+	}
+
+	return 0;
+}
+
+static int dw_pcm_open(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_pcm_runtime *rt = substream->runtime;
+	struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(rtd->cpu_dai);
+
+	snd_soc_set_runtime_hwparams(substream, &dw_pcm_hardware);
+	snd_pcm_hw_constraint_integer(rt, SNDRV_PCM_HW_PARAM_PERIODS);
+
+	dev->binfo.stream = substream;
+	rt->private_data = &dev->binfo;
+	return 0;
+}
+
+static int dw_pcm_close(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int dw_pcm_hw_params(struct snd_pcm_substream *substream,
+		struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_pcm_runtime *rt = substream->runtime;
+	struct dw_pcm_binfo *bi = rt->private_data;
+	int ret;
+
+	ret = snd_pcm_lib_alloc_vmalloc_buffer(substream,
+			params_buffer_bytes(hw_params));
+	if (ret < 0)
+		return ret;
+
+	memset(rt->dma_area, 0, params_buffer_bytes(hw_params));
+	bi->dma_base = rt->dma_area;
+	bi->dma_pointer = bi->dma_base;
+
+	return 0;
+}
+
+static int dw_pcm_hw_free(struct snd_pcm_substream *substream)
+{
+	int ret;
+
+	ret = snd_pcm_lib_free_vmalloc_buffer(substream);
+	if (ret < 0)
+		return ret;
+	return 0;
+}
+
+static int dw_pcm_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *rt = substream->runtime;
+	struct dw_pcm_binfo *bi = rt->private_data;
+	u32 buffer_size_frames = 0;
+
+	bi->period_size_frames = bytes_to_frames(rt,
+			snd_pcm_lib_period_bytes(substream));
+	bi->size = snd_pcm_lib_buffer_bytes(substream);
+	buffer_size_frames = bytes_to_frames(rt, bi->size);
+	bi->total_periods = buffer_size_frames / bi->period_size_frames;
+	bi->current_period = 1;
+
+	if ((buffer_size_frames % bi->period_size_frames) != 0)
+		return -EINVAL;
+	return 0;
+}
+
+static int dw_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static snd_pcm_uframes_t dw_pcm_pointer(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *rt = substream->runtime;
+	struct dw_pcm_binfo *bi = rt->private_data;
+
+	return bi->period_pointer;
+}
+
+static struct snd_pcm_ops dw_pcm_ops = {
+	.open = dw_pcm_open,
+	.close = dw_pcm_close,
+	.ioctl = snd_pcm_lib_ioctl,
+	.hw_params = dw_pcm_hw_params,
+	.hw_free = dw_pcm_hw_free,
+	.prepare = dw_pcm_prepare,
+	.trigger = dw_pcm_trigger,
+	.pointer = dw_pcm_pointer,
+	.page = snd_pcm_lib_get_vmalloc_page,
+	.mmap = snd_pcm_lib_mmap_vmalloc,
+};
+
+static int dw_pcm_new(struct snd_soc_pcm_runtime *runtime)
+{
+	struct snd_pcm *pcm = runtime->pcm;
+
+	return snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV,
+			snd_dma_continuous_data(GFP_KERNEL), BUFFER_BYTES_MAX,
+			BUFFER_BYTES_MAX);
+}
+
+static void dw_pcm_free(struct snd_pcm *pcm)
+{
+	snd_pcm_lib_preallocate_free_for_all(pcm);
+}
+
+static struct snd_soc_platform_driver dw_pcm_platform = {
+	.pcm_new = dw_pcm_new,
+	.pcm_free = dw_pcm_free,
+	.ops = &dw_pcm_ops,
+};
+
+static int dw_pcm_probe(struct platform_device *pdev)
+{
+	return devm_snd_soc_register_platform(&pdev->dev, &dw_pcm_platform);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id dw_pcm_of[] = {
+	{ .compatible = "snps,designware-pcm" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, dw_pcm_of);
+#endif
+
+static struct platform_driver dw_pcm_driver = {
+	.driver = {
+		.name = "designware-pcm",
+		.of_match_table = of_match_ptr(dw_pcm_of),
+	},
+	.probe = dw_pcm_probe,
+};
+
+module_platform_driver(dw_pcm_driver);
+
+MODULE_AUTHOR("Jose Abreu <joabreu@synopsys.com>, Tiago Duarte");
+MODULE_DESCRIPTION("Synopsys Designware PCM Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

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

* [PATCH 4/5 v4] ASoC: dwc: Add custom PCM driver
@ 2016-04-07 16:53   ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc

HDMI audio support was added to the AXS board using an
I2S cpu driver and a custom platform driver.

The platform driver supports two channels @ 16 bits with
rates 32k, 44.1k and 48k.

Although the mainline I2S driver uses ALSA DMA engine,
this controller can be built without DMA support so it
was necessary to add this custom platform driver so that
HDMI audio works in AXS boards.

The selection between the use of DMA engine or custom
PCM can be made using a device tree boolean parameter
which was introduced in this patch ('snps,use-dmaengine').

Signed-off-by: Jose Abreu <joabreu at synopsys.com>
---

Changes v3 -> v4:
* Reintroduced custom PCM driver
* Use DT boolean to switch between ALSA DMA engine PCM or custom PCM

Changes v2 -> v3:
* Removed pll_config functions (as suggested by Alexey Brodkin)
* Dropped custom platform driver, using now ALSA DMA engine
* Dropped IRQ handler for I2S

No changes v1 -> v2.

 sound/soc/dwc/Kconfig          |   9 ++
 sound/soc/dwc/Makefile         |   1 +
 sound/soc/dwc/designware.h     |  70 +++++++++++++
 sound/soc/dwc/designware_i2s.c |  99 +++++++++++++-----
 sound/soc/dwc/designware_pcm.c | 230 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 382 insertions(+), 27 deletions(-)
 create mode 100644 sound/soc/dwc/designware.h
 create mode 100644 sound/soc/dwc/designware_pcm.c

diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
index d50e085..2a21120 100644
--- a/sound/soc/dwc/Kconfig
+++ b/sound/soc/dwc/Kconfig
@@ -7,4 +7,13 @@ config SND_DESIGNWARE_I2S
 	 Synopsys desigwnware I2S device. The device supports upto
 	 maximum of 8 channels each for play and record.
 
+config SND_DESIGNWARE_PCM
+	tristate "Synopsys I2S PCM Driver"
+	help
+	 Say Y or M if you want to add support for ALSA ASoC platform driver
+	 using I2S.
+
+	 Select this option if you want to be able to create a sound interface
+	 using the I2S device driver as CPU driver. Instead of using ALSA
+	 DMA engine by selecting this driver a custom PCM driver will be used.
 
diff --git a/sound/soc/dwc/Makefile b/sound/soc/dwc/Makefile
index 319371f..1b48bccc 100644
--- a/sound/soc/dwc/Makefile
+++ b/sound/soc/dwc/Makefile
@@ -1,3 +1,4 @@
 # SYNOPSYS Platform Support
 obj-$(CONFIG_SND_DESIGNWARE_I2S) += designware_i2s.o
+obj-$(CONFIG_SND_DESIGNWARE_PCM) += designware_pcm.o
 
diff --git a/sound/soc/dwc/designware.h b/sound/soc/dwc/designware.h
new file mode 100644
index 0000000..196296c
--- /dev/null
+++ b/sound/soc/dwc/designware.h
@@ -0,0 +1,70 @@
+/*
+ * ALSA SoC Synopsys Audio Layer
+ *
+ * sound/soc/dwc/designware.h
+ *
+ * Copyright (C) 2016 Synopsys
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef __DESIGNWARE_H
+#define __DESIGNWARE_H
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <sound/designware_i2s.h>
+#include <sound/dmaengine_pcm.h>
+
+struct dw_pcm_binfo {
+	struct snd_pcm_substream *stream;
+	unsigned char *dma_base;
+	unsigned char *dma_pointer;
+	unsigned int period_size_frames;
+	unsigned int size;
+	snd_pcm_uframes_t period_pointer;
+	unsigned int total_periods;
+	unsigned int current_period;
+};
+
+union dw_i2s_snd_dma_data {
+	struct i2s_dma_data pd;
+	struct snd_dmaengine_dai_dma_data dt;
+};
+
+struct dw_i2s_dev {
+	void __iomem *i2s_base;
+	struct clk *clk;
+	int active;
+	unsigned int capability;
+	unsigned int quirks;
+	unsigned int i2s_reg_comp1;
+	unsigned int i2s_reg_comp2;
+	struct device *dev;
+	u32 ccr;
+	u32 xfer_resolution;
+	u32 fifo_th;
+
+	/* data related to DMA transfers b/w i2s and DMAC */
+	bool use_dmaengine;
+	union dw_i2s_snd_dma_data play_dma_data;
+	union dw_i2s_snd_dma_data capture_dma_data;
+	struct i2s_clk_config_data config;
+	int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
+	struct dw_pcm_binfo binfo;
+};
+
+#ifdef CONFIG_SND_DESIGNWARE_PCM
+int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
+		struct dw_pcm_binfo *bi);
+#else
+int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
+		struct dw_pcm_binfo *bi)
+{
+	return 0;
+}
+#endif
+
+#endif
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 0db69b7..16056c1 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -24,6 +24,7 @@
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/dmaengine_pcm.h>
+#include "designware.h"
 
 /* common register for all channel */
 #define IER		0x000
@@ -84,31 +85,6 @@
 #define MAX_CHANNEL_NUM		8
 #define MIN_CHANNEL_NUM		2
 
-union dw_i2s_snd_dma_data {
-	struct i2s_dma_data pd;
-	struct snd_dmaengine_dai_dma_data dt;
-};
-
-struct dw_i2s_dev {
-	void __iomem *i2s_base;
-	struct clk *clk;
-	int active;
-	unsigned int capability;
-	unsigned int quirks;
-	unsigned int i2s_reg_comp1;
-	unsigned int i2s_reg_comp2;
-	struct device *dev;
-	u32 ccr;
-	u32 xfer_resolution;
-	u32 fifo_th;
-
-	/* data related to DMA transfers b/w i2s and DMAC */
-	union dw_i2s_snd_dma_data play_dma_data;
-	union dw_i2s_snd_dma_data capture_dma_data;
-	struct i2s_clk_config_data config;
-	int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
-};
-
 static inline void i2s_write_reg(void __iomem *io_base, int reg, u32 val)
 {
 	writel(val, io_base + reg);
@@ -145,6 +121,54 @@ static inline void i2s_clear_irqs(struct dw_i2s_dev *dev, u32 stream)
 	}
 }
 
+static irqreturn_t dw_i2s_irq_handler(int irq, void *dev_id)
+{
+	struct dw_i2s_dev *dev = dev_id;
+	u32 isr[4], sleft[dev->fifo_th], sright[dev->fifo_th];
+	int i, j, xfer_bytes = dev->config.data_width / 8;
+	int dir = dev->binfo.stream->stream;
+
+	for (i = 0; i < 4; i++)
+		isr[i] = i2s_read_reg(dev->i2s_base, ISR(i));
+
+	i2s_clear_irqs(dev, SNDRV_PCM_STREAM_PLAYBACK);
+	i2s_clear_irqs(dev, SNDRV_PCM_STREAM_CAPTURE);
+
+	if (dev->use_dmaengine)
+		return IRQ_HANDLED;
+
+	for (i = 0; i < 4; i++) {
+		/* Copy only to/from first two channels
+		 * TODO: Remaining channels
+		 */
+		if ((isr[i] & 0x10) && (i == 0) &&
+				(dir == SNDRV_PCM_STREAM_PLAYBACK)) {
+			/* TXFEM - TX FIFO is empty */
+			dw_pcm_transfer(sleft, sright, xfer_bytes, dev->fifo_th,
+					&dev->binfo);
+			for (j = 0; j < dev->fifo_th; j++) {
+				i2s_write_reg(dev->i2s_base, LRBR_LTHR(i),
+						sleft[j]);
+				i2s_write_reg(dev->i2s_base, RRBR_RTHR(i),
+						sright[j]);
+			}
+		} else if ((isr[i] & 0x01) && (i == 0) &&
+				(dir == SNDRV_PCM_STREAM_CAPTURE)) {
+			/* RXDAM - RX FIFO is full */
+			for (j = 0; j < dev->fifo_th; j++) {
+				sleft[j] = i2s_read_reg(dev->i2s_base,
+						LRBR_LTHR(i));
+				sright[j] = i2s_read_reg(dev->i2s_base,
+						RRBR_RTHR(i));
+			}
+			dw_pcm_transfer(sleft, sright, xfer_bytes, dev->fifo_th,
+					&dev->binfo);
+		}
+	}
+
+	return IRQ_HANDLED;
+}
+
 static void i2s_start(struct dw_i2s_dev *dev,
 		      struct snd_pcm_substream *substream)
 {
@@ -626,7 +650,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
 	const struct i2s_platform_data *pdata = pdev->dev.platform_data;
 	struct dw_i2s_dev *dev;
 	struct resource *res;
-	int ret;
+	int ret, irq_number;
 	struct snd_soc_dai_driver *dw_i2s_dai;
 	const char *clk_id;
 
@@ -649,6 +673,24 @@ static int dw_i2s_probe(struct platform_device *pdev)
 	if (IS_ERR(dev->i2s_base))
 		return PTR_ERR(dev->i2s_base);
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	dev->i2s_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(dev->i2s_base))
+		return PTR_ERR(dev->i2s_base);
+
+	irq_number = platform_get_irq(pdev, 0);
+	if (irq_number <= 0) {
+		dev_err(&pdev->dev, "get_irq fail\n");
+		return -EINVAL;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq_number, dw_i2s_irq_handler,
+			IRQF_SHARED, "dw_i2s_irq_handler", dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "request_irq fail\n");
+		return ret;
+	}
+
 	dev->dev = &pdev->dev;
 
 	dev->i2s_reg_comp1 = I2S_COMP_PARAM_1;
@@ -657,6 +699,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
 		dev->capability = pdata->cap;
 		clk_id = NULL;
 		dev->quirks = pdata->quirks;
+		dev->use_dmaengine = false;
 		if (dev->quirks & DW_I2S_QUIRK_COMP_REG_OFFSET) {
 			dev->i2s_reg_comp1 = pdata->i2s_reg_comp1;
 			dev->i2s_reg_comp2 = pdata->i2s_reg_comp2;
@@ -664,6 +707,8 @@ static int dw_i2s_probe(struct platform_device *pdev)
 		ret = dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata);
 	} else {
 		clk_id = "i2sclk";
+		dev->use_dmaengine = of_property_read_bool(pdev->dev.of_node,
+				"snps,use-dmaengine");
 		ret = dw_configure_dai_by_dt(dev, dw_i2s_dai, res);
 	}
 	if (ret < 0)
@@ -695,7 +740,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
 		goto err_clk_disable;
 	}
 
-	if (!pdata) {
+	if (dev->use_dmaengine) {
 		ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
 		if (ret) {
 			dev_err(&pdev->dev,
diff --git a/sound/soc/dwc/designware_pcm.c b/sound/soc/dwc/designware_pcm.c
new file mode 100644
index 0000000..84ff6b9
--- /dev/null
+++ b/sound/soc/dwc/designware_pcm.c
@@ -0,0 +1,230 @@
+/*
+ * Synopsys I2S PCM Driver
+ *
+ * Copyright (C) 2016 Synopsys
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/slab.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/dmaengine_pcm.h>
+#include "designware.h"
+
+#define BUFFER_BYTES_MAX	384000
+#define PERIOD_BYTES_MIN	2048
+#define PERIODS_MIN		8
+
+static const struct snd_pcm_hardware dw_pcm_hardware = {
+	.info = SNDRV_PCM_INFO_INTERLEAVED |
+		SNDRV_PCM_INFO_MMAP |
+		SNDRV_PCM_INFO_MMAP_VALID |
+		SNDRV_PCM_INFO_BLOCK_TRANSFER,
+	.rates = SNDRV_PCM_RATE_32000 |
+		SNDRV_PCM_RATE_44100 |
+		SNDRV_PCM_RATE_48000,
+	.rate_min = 32000,
+	.rate_max = 48000,
+	.formats = SNDRV_PCM_FMTBIT_S16_LE,
+	.channels_min = 2,
+	.channels_max = 2,
+	.buffer_bytes_max = BUFFER_BYTES_MAX,
+	.period_bytes_min = PERIOD_BYTES_MIN,
+	.period_bytes_max = BUFFER_BYTES_MAX / PERIODS_MIN,
+	.periods_min = PERIODS_MIN,
+	.periods_max = BUFFER_BYTES_MAX / PERIOD_BYTES_MIN,
+};
+
+int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
+		struct dw_pcm_binfo *bi)
+{
+	struct snd_pcm_runtime *rt = bi->stream->runtime;
+	int dir = bi->stream->stream;
+	int i;
+
+	for (i = 0; i < buf_size; i++) {
+		if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
+			memcpy(&lsample[i], bi->dma_pointer, bytes);
+			bi->dma_pointer += bytes;
+			memcpy(&rsample[i], bi->dma_pointer, bytes);
+			bi->dma_pointer += bytes;
+		} else {
+			memcpy(bi->dma_pointer, &lsample[i], bytes);
+			bi->dma_pointer += bytes;
+			memcpy(bi->dma_pointer, &rsample[i], bytes);
+			bi->dma_pointer += bytes;
+		}
+	}
+
+	bi->period_pointer += bytes_to_frames(rt, bytes * 2 * buf_size);
+
+	if (bi->period_pointer >= (bi->period_size_frames * bi->current_period)) {
+		bi->current_period++;
+		if (bi->current_period > bi->total_periods) {
+			bi->dma_pointer = bi->dma_base;
+			bi->period_pointer = 0;
+			bi->current_period = 1;
+		}
+		snd_pcm_period_elapsed(bi->stream);
+	}
+
+	return 0;
+}
+
+static int dw_pcm_open(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_pcm_runtime *rt = substream->runtime;
+	struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(rtd->cpu_dai);
+
+	snd_soc_set_runtime_hwparams(substream, &dw_pcm_hardware);
+	snd_pcm_hw_constraint_integer(rt, SNDRV_PCM_HW_PARAM_PERIODS);
+
+	dev->binfo.stream = substream;
+	rt->private_data = &dev->binfo;
+	return 0;
+}
+
+static int dw_pcm_close(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int dw_pcm_hw_params(struct snd_pcm_substream *substream,
+		struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_pcm_runtime *rt = substream->runtime;
+	struct dw_pcm_binfo *bi = rt->private_data;
+	int ret;
+
+	ret = snd_pcm_lib_alloc_vmalloc_buffer(substream,
+			params_buffer_bytes(hw_params));
+	if (ret < 0)
+		return ret;
+
+	memset(rt->dma_area, 0, params_buffer_bytes(hw_params));
+	bi->dma_base = rt->dma_area;
+	bi->dma_pointer = bi->dma_base;
+
+	return 0;
+}
+
+static int dw_pcm_hw_free(struct snd_pcm_substream *substream)
+{
+	int ret;
+
+	ret = snd_pcm_lib_free_vmalloc_buffer(substream);
+	if (ret < 0)
+		return ret;
+	return 0;
+}
+
+static int dw_pcm_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *rt = substream->runtime;
+	struct dw_pcm_binfo *bi = rt->private_data;
+	u32 buffer_size_frames = 0;
+
+	bi->period_size_frames = bytes_to_frames(rt,
+			snd_pcm_lib_period_bytes(substream));
+	bi->size = snd_pcm_lib_buffer_bytes(substream);
+	buffer_size_frames = bytes_to_frames(rt, bi->size);
+	bi->total_periods = buffer_size_frames / bi->period_size_frames;
+	bi->current_period = 1;
+
+	if ((buffer_size_frames % bi->period_size_frames) != 0)
+		return -EINVAL;
+	return 0;
+}
+
+static int dw_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static snd_pcm_uframes_t dw_pcm_pointer(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *rt = substream->runtime;
+	struct dw_pcm_binfo *bi = rt->private_data;
+
+	return bi->period_pointer;
+}
+
+static struct snd_pcm_ops dw_pcm_ops = {
+	.open = dw_pcm_open,
+	.close = dw_pcm_close,
+	.ioctl = snd_pcm_lib_ioctl,
+	.hw_params = dw_pcm_hw_params,
+	.hw_free = dw_pcm_hw_free,
+	.prepare = dw_pcm_prepare,
+	.trigger = dw_pcm_trigger,
+	.pointer = dw_pcm_pointer,
+	.page = snd_pcm_lib_get_vmalloc_page,
+	.mmap = snd_pcm_lib_mmap_vmalloc,
+};
+
+static int dw_pcm_new(struct snd_soc_pcm_runtime *runtime)
+{
+	struct snd_pcm *pcm = runtime->pcm;
+
+	return snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV,
+			snd_dma_continuous_data(GFP_KERNEL), BUFFER_BYTES_MAX,
+			BUFFER_BYTES_MAX);
+}
+
+static void dw_pcm_free(struct snd_pcm *pcm)
+{
+	snd_pcm_lib_preallocate_free_for_all(pcm);
+}
+
+static struct snd_soc_platform_driver dw_pcm_platform = {
+	.pcm_new = dw_pcm_new,
+	.pcm_free = dw_pcm_free,
+	.ops = &dw_pcm_ops,
+};
+
+static int dw_pcm_probe(struct platform_device *pdev)
+{
+	return devm_snd_soc_register_platform(&pdev->dev, &dw_pcm_platform);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id dw_pcm_of[] = {
+	{ .compatible = "snps,designware-pcm" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, dw_pcm_of);
+#endif
+
+static struct platform_driver dw_pcm_driver = {
+	.driver = {
+		.name = "designware-pcm",
+		.of_match_table = of_match_ptr(dw_pcm_of),
+	},
+	.probe = dw_pcm_probe,
+};
+
+module_platform_driver(dw_pcm_driver);
+
+MODULE_AUTHOR("Jose Abreu <joabreu at synopsys.com>, Tiago Duarte");
+MODULE_DESCRIPTION("Synopsys Designware PCM Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

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

* [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
  2016-04-07 16:53 ` Jose Abreu
@ 2016-04-07 16:53   ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: broonie, CARLOS.PALMINHA, Alexey.Brodkin, Vineet.Gupta1, architt,
	airlied, lgirdwood, perex, tiwai, Jose Abreu

This patch updates documentation for the Designware I2S
driver.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---

This patch was only introduced in v4.

 Documentation/devicetree/bindings/sound/designware-i2s.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt b/Documentation/devicetree/bindings/sound/designware-i2s.txt
index 7bb5424..f3b5c17 100644
--- a/Documentation/devicetree/bindings/sound/designware-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
@@ -7,6 +7,10 @@ Required properties:
    clocks. The controller expects one clock: the clock used as the sampling
    rate reference clock sample.
  - clock-names : "i2sclk" for the sample rate reference clock.
+
+ Optional properties:
+ - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
+   it is required to use the properties 'dmas' and 'dma-names'.
  - dmas: Pairs of phandle and specifier for the DMA channels that are used by
    the core. The core expects one or two dma channels: one for transmit and
    one for receive.
@@ -26,6 +30,7 @@ Example:
 		clocks = <&scpi_i2sclk 0>;
 		clock-names = "i2sclk";
 		#sound-dai-cells = <0>;
+		snps,use-dmaengine;
 		dmas = <&dma0 5>;
 		dma-names = "tx";
 	};
-- 
1.9.1

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

* [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-07 16:53   ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-07 16:53 UTC (permalink / raw)
  To: linux-snps-arc

This patch updates documentation for the Designware I2S
driver.

Signed-off-by: Jose Abreu <joabreu at synopsys.com>
---

This patch was only introduced in v4.

 Documentation/devicetree/bindings/sound/designware-i2s.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt b/Documentation/devicetree/bindings/sound/designware-i2s.txt
index 7bb5424..f3b5c17 100644
--- a/Documentation/devicetree/bindings/sound/designware-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
@@ -7,6 +7,10 @@ Required properties:
    clocks. The controller expects one clock: the clock used as the sampling
    rate reference clock sample.
  - clock-names : "i2sclk" for the sample rate reference clock.
+
+ Optional properties:
+ - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
+   it is required to use the properties 'dmas' and 'dma-names'.
  - dmas: Pairs of phandle and specifier for the DMA channels that are used by
    the core. The core expects one or two dma channels: one for transmit and
    one for receive.
@@ -26,6 +30,7 @@ Example:
 		clocks = <&scpi_i2sclk 0>;
 		clock-names = "i2sclk";
 		#sound-dai-cells = <0>;
+		snps,use-dmaengine;
 		dmas = <&dma0 5>;
 		dma-names = "tx";
 	};
-- 
1.9.1

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

* Re: [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
  2016-04-07 16:53   ` Jose Abreu
  (?)
@ 2016-04-07 17:53     ` Mark Brown
  -1 siblings, 0 replies; 66+ messages in thread
From: Mark Brown @ 2016-04-07 17:53 UTC (permalink / raw)
  To: Jose Abreu
  Cc: linux-snps-arc, linux-kernel, dri-devel, alsa-devel,
	CARLOS.PALMINHA, Alexey.Brodkin, Vineet.Gupta1, architt, airlied,
	lgirdwood, perex, tiwai

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

On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:

> + Optional properties:
> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
> +   it is required to use the properties 'dmas' and 'dma-names'.

This is not a good interface, it's describing Linux internal APIs.  If
the device needs to operate in PIO mode it should just do that.

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

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

* Re: [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-07 17:53     ` Mark Brown
  0 siblings, 0 replies; 66+ messages in thread
From: Mark Brown @ 2016-04-07 17:53 UTC (permalink / raw)
  To: Jose Abreu
  Cc: alsa-devel, lgirdwood, Vineet.Gupta1, Alexey.Brodkin,
	linux-kernel, dri-devel, CARLOS.PALMINHA, perex, tiwai,
	linux-snps-arc


[-- Attachment #1.1: Type: text/plain, Size: 366 bytes --]

On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:

> + Optional properties:
> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
> +   it is required to use the properties 'dmas' and 'dma-names'.

This is not a good interface, it's describing Linux internal APIs.  If
the device needs to operate in PIO mode it should just do that.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

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

* [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-07 17:53     ` Mark Brown
  0 siblings, 0 replies; 66+ messages in thread
From: Mark Brown @ 2016-04-07 17:53 UTC (permalink / raw)
  To: linux-snps-arc

On Thu, Apr 07, 2016@05:53:59PM +0100, Jose Abreu wrote:

> + Optional properties:
> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
> +   it is required to use the properties 'dmas' and 'dma-names'.

This is not a good interface, it's describing Linux internal APIs.  If
the device needs to operate in PIO mode it should just do that.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-snps-arc/attachments/20160407/566c90a6/attachment-0001.sig>

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

* Re: [PATCH 4/5 v4] ASoC: dwc: Add custom PCM driver
  2016-04-07 16:53   ` Jose Abreu
  (?)
@ 2016-04-07 17:54     ` kbuild test robot
  -1 siblings, 0 replies; 66+ messages in thread
From: kbuild test robot @ 2016-04-07 17:54 UTC (permalink / raw)
  To: Jose Abreu
  Cc: kbuild-all, linux-snps-arc, linux-kernel, dri-devel, alsa-devel,
	broonie, CARLOS.PALMINHA, Alexey.Brodkin, Vineet.Gupta1, architt,
	airlied, lgirdwood, perex, tiwai, Jose Abreu

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

Hi Jose,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.6-rc2 next-20160407]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jose-Abreu/Add-I2S-ADV7511-audio-support-for-ARC-AXS10x-boards/20160408-005829
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: i386-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> sound/soc/dwc/designware_pcm.c:46:5: error: redefinition of 'dw_pcm_transfer'
    int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
        ^
   In file included from sound/soc/dwc/designware_pcm.c:20:0:
   sound/soc/dwc/designware.h:63:5: note: previous definition of 'dw_pcm_transfer' was here
    int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
        ^

vim +/dw_pcm_transfer +46 sound/soc/dwc/designware_pcm.c

    40		.period_bytes_min = PERIOD_BYTES_MIN,
    41		.period_bytes_max = BUFFER_BYTES_MAX / PERIODS_MIN,
    42		.periods_min = PERIODS_MIN,
    43		.periods_max = BUFFER_BYTES_MAX / PERIOD_BYTES_MIN,
    44	};
    45	
  > 46	int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
    47			struct dw_pcm_binfo *bi)
    48	{
    49		struct snd_pcm_runtime *rt = bi->stream->runtime;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 54459 bytes --]

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

* Re: [PATCH 4/5 v4] ASoC: dwc: Add custom PCM driver
@ 2016-04-07 17:54     ` kbuild test robot
  0 siblings, 0 replies; 66+ messages in thread
From: kbuild test robot @ 2016-04-07 17:54 UTC (permalink / raw)
  Cc: kbuild-all, linux-snps-arc, linux-kernel, dri-devel, alsa-devel,
	broonie, CARLOS.PALMINHA, Alexey.Brodkin, Vineet.Gupta1, architt,
	airlied, lgirdwood, perex, tiwai, Jose Abreu

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

Hi Jose,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.6-rc2 next-20160407]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jose-Abreu/Add-I2S-ADV7511-audio-support-for-ARC-AXS10x-boards/20160408-005829
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: i386-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> sound/soc/dwc/designware_pcm.c:46:5: error: redefinition of 'dw_pcm_transfer'
    int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
        ^
   In file included from sound/soc/dwc/designware_pcm.c:20:0:
   sound/soc/dwc/designware.h:63:5: note: previous definition of 'dw_pcm_transfer' was here
    int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
        ^

vim +/dw_pcm_transfer +46 sound/soc/dwc/designware_pcm.c

    40		.period_bytes_min = PERIOD_BYTES_MIN,
    41		.period_bytes_max = BUFFER_BYTES_MAX / PERIODS_MIN,
    42		.periods_min = PERIODS_MIN,
    43		.periods_max = BUFFER_BYTES_MAX / PERIOD_BYTES_MIN,
    44	};
    45	
  > 46	int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
    47			struct dw_pcm_binfo *bi)
    48	{
    49		struct snd_pcm_runtime *rt = bi->stream->runtime;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 54459 bytes --]

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

* [PATCH 4/5 v4] ASoC: dwc: Add custom PCM driver
@ 2016-04-07 17:54     ` kbuild test robot
  0 siblings, 0 replies; 66+ messages in thread
From: kbuild test robot @ 2016-04-07 17:54 UTC (permalink / raw)
  To: linux-snps-arc

Hi Jose,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.6-rc2 next-20160407]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jose-Abreu/Add-I2S-ADV7511-audio-support-for-ARC-AXS10x-boards/20160408-005829
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: i386-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> sound/soc/dwc/designware_pcm.c:46:5: error: redefinition of 'dw_pcm_transfer'
    int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
        ^
   In file included from sound/soc/dwc/designware_pcm.c:20:0:
   sound/soc/dwc/designware.h:63:5: note: previous definition of 'dw_pcm_transfer' was here
    int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
        ^

vim +/dw_pcm_transfer +46 sound/soc/dwc/designware_pcm.c

    40		.period_bytes_min = PERIOD_BYTES_MIN,
    41		.period_bytes_max = BUFFER_BYTES_MAX / PERIODS_MIN,
    42		.periods_min = PERIODS_MIN,
    43		.periods_max = BUFFER_BYTES_MAX / PERIOD_BYTES_MIN,
    44	};
    45	
  > 46	int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size,
    47			struct dw_pcm_binfo *bi)
    48	{
    49		struct snd_pcm_runtime *rt = bi->stream->runtime;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 54459 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-snps-arc/attachments/20160408/fe14ec37/attachment-0001.obj>

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

* Applied "ASoC: dwc: Use fifo depth to program FCR" to the asoc tree
  2016-04-07 16:53   ` Jose Abreu
  (?)
@ 2016-04-07 17:57     ` Mark Brown
  -1 siblings, 0 replies; 66+ messages in thread
From: Mark Brown @ 2016-04-07 17:57 UTC (permalink / raw)
  To: Jose Abreu
  Cc: Jose Abreu, Mark Brown, linux-snps-arc, linux-kernel, dri-devel,
	alsa-devel, architt, airlied, Vineet.Gupta1, Alexey.Brodkin,
	CARLOS.PALMINHA, lgirdwood, broonie, tiwai

The patch

   ASoC: dwc: Use fifo depth to program FCR

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

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

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

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

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

Thanks,
Mark

>From 3fafd14d9422c46f5c2a142298384dc15dbf88b2 Mon Sep 17 00:00:00 2001
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Thu, 7 Apr 2016 17:53:57 +0100
Subject: [PATCH] ASoC: dwc: Use fifo depth to program FCR

This patch makes Designware I2S driver use the fifo
depth value to program the fifo configuration register
instead of using hardcoded values.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/dwc/designware_i2s.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 3effcd1a7df8..0db69b7e9617 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -100,6 +100,7 @@ struct dw_i2s_dev {
 	struct device *dev;
 	u32 ccr;
 	u32 xfer_resolution;
+	u32 fifo_th;
 
 	/* data related to DMA transfers b/w i2s and DMAC */
 	union dw_i2s_snd_dma_data play_dma_data;
@@ -232,14 +233,16 @@ static void dw_i2s_config(struct dw_i2s_dev *dev, int stream)
 		if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
 			i2s_write_reg(dev->i2s_base, TCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
+			i2s_write_reg(dev->i2s_base, TFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
 			i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
 		} else {
 			i2s_write_reg(dev->i2s_base, RCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
+			i2s_write_reg(dev->i2s_base, RFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
 			i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
@@ -499,6 +502,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 	 */
 	u32 comp1 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp1);
 	u32 comp2 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp2);
+	u32 fifo_depth = 1 << (1 + COMP1_FIFO_DEPTH_GLOBAL(comp1));
 	u32 idx;
 
 	if (dev->capability & DWC_I2S_RECORD &&
@@ -537,6 +541,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 		dev->capability |= DW_I2S_SLAVE;
 	}
 
+	dev->fifo_th = fifo_depth / 2;
 	return 0;
 }
 
-- 
2.8.0.rc3

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

* Applied "ASoC: dwc: Use fifo depth to program FCR" to the asoc tree
@ 2016-04-07 17:57     ` Mark Brown
  0 siblings, 0 replies; 66+ messages in thread
From: Mark Brown @ 2016-04-07 17:57 UTC (permalink / raw)
  To: Jose Abreu
  Cc: Jose Abreu, alsa-devel, lgirdwood, Vineet.Gupta1, Alexey.Brodkin,
	linux-kernel, dri-devel, broonie, CARLOS.PALMINHA, tiwai,
	linux-snps-arc

The patch

   ASoC: dwc: Use fifo depth to program FCR

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

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

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

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

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

Thanks,
Mark

From 3fafd14d9422c46f5c2a142298384dc15dbf88b2 Mon Sep 17 00:00:00 2001
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Thu, 7 Apr 2016 17:53:57 +0100
Subject: [PATCH] ASoC: dwc: Use fifo depth to program FCR

This patch makes Designware I2S driver use the fifo
depth value to program the fifo configuration register
instead of using hardcoded values.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/dwc/designware_i2s.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 3effcd1a7df8..0db69b7e9617 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -100,6 +100,7 @@ struct dw_i2s_dev {
 	struct device *dev;
 	u32 ccr;
 	u32 xfer_resolution;
+	u32 fifo_th;
 
 	/* data related to DMA transfers b/w i2s and DMAC */
 	union dw_i2s_snd_dma_data play_dma_data;
@@ -232,14 +233,16 @@ static void dw_i2s_config(struct dw_i2s_dev *dev, int stream)
 		if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
 			i2s_write_reg(dev->i2s_base, TCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
+			i2s_write_reg(dev->i2s_base, TFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
 			i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
 		} else {
 			i2s_write_reg(dev->i2s_base, RCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
+			i2s_write_reg(dev->i2s_base, RFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
 			i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
@@ -499,6 +502,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 	 */
 	u32 comp1 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp1);
 	u32 comp2 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp2);
+	u32 fifo_depth = 1 << (1 + COMP1_FIFO_DEPTH_GLOBAL(comp1));
 	u32 idx;
 
 	if (dev->capability & DWC_I2S_RECORD &&
@@ -537,6 +541,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 		dev->capability |= DW_I2S_SLAVE;
 	}
 
+	dev->fifo_th = fifo_depth / 2;
 	return 0;
 }
 
-- 
2.8.0.rc3

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

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

* Applied "ASoC: dwc: Use fifo depth to program FCR" to the asoc tree
@ 2016-04-07 17:57     ` Mark Brown
  0 siblings, 0 replies; 66+ messages in thread
From: Mark Brown @ 2016-04-07 17:57 UTC (permalink / raw)
  To: linux-snps-arc

The patch

   ASoC: dwc: Use fifo depth to program FCR

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

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

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

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

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

Thanks,
Mark

>From 3fafd14d9422c46f5c2a142298384dc15dbf88b2 Mon Sep 17 00:00:00 2001
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Thu, 7 Apr 2016 17:53:57 +0100
Subject: [PATCH] ASoC: dwc: Use fifo depth to program FCR

This patch makes Designware I2S driver use the fifo
depth value to program the fifo configuration register
instead of using hardcoded values.

Signed-off-by: Jose Abreu <joabreu at synopsys.com>
Signed-off-by: Mark Brown <broonie at kernel.org>
---
 sound/soc/dwc/designware_i2s.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 3effcd1a7df8..0db69b7e9617 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -100,6 +100,7 @@ struct dw_i2s_dev {
 	struct device *dev;
 	u32 ccr;
 	u32 xfer_resolution;
+	u32 fifo_th;
 
 	/* data related to DMA transfers b/w i2s and DMAC */
 	union dw_i2s_snd_dma_data play_dma_data;
@@ -232,14 +233,16 @@ static void dw_i2s_config(struct dw_i2s_dev *dev, int stream)
 		if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
 			i2s_write_reg(dev->i2s_base, TCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
+			i2s_write_reg(dev->i2s_base, TFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
 			i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
 		} else {
 			i2s_write_reg(dev->i2s_base, RCR(ch_reg),
 				      dev->xfer_resolution);
-			i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
+			i2s_write_reg(dev->i2s_base, RFCR(ch_reg),
+				      dev->fifo_th - 1);
 			irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
 			i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
 			i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
@@ -499,6 +502,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 	 */
 	u32 comp1 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp1);
 	u32 comp2 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp2);
+	u32 fifo_depth = 1 << (1 + COMP1_FIFO_DEPTH_GLOBAL(comp1));
 	u32 idx;
 
 	if (dev->capability & DWC_I2S_RECORD &&
@@ -537,6 +541,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev,
 		dev->capability |= DW_I2S_SLAVE;
 	}
 
+	dev->fifo_th = fifo_depth / 2;
 	return 0;
 }
 
-- 
2.8.0.rc3

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

* Re: [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
  2016-04-07 17:53     ` Mark Brown
  (?)
@ 2016-04-08 10:06       ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-08 10:06 UTC (permalink / raw)
  To: Mark Brown, Jose Abreu
  Cc: linux-snps-arc, linux-kernel, dri-devel, alsa-devel,
	CARLOS.PALMINHA, Alexey.Brodkin, Vineet.Gupta1, architt, airlied,
	lgirdwood, perex, tiwai

Hi Mark,


On 07-04-2016 18:53, Mark Brown wrote:
> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>
>> + Optional properties:
>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>> +   it is required to use the properties 'dmas' and 'dma-names'.
> This is not a good interface, it's describing Linux internal APIs.  If
> the device needs to operate in PIO mode it should just do that.

I added this interface because there is no direct way to check if DMA is
available on the I2S controller so it is not possible to automatically change
between DMA and PIO mode. As the I2S controller can be built with or without DMA
support it is necessary to somehow check if DMA is enabled or not and according
to that use either ALSA DMA engine or the custom platform driver sent in these
patches. I did not want to remove drivers functionality so I added this property
to the DT. This way a user can select between DMA and PIO mode.

Is there a better option to do this without removing the possibility of using
ALSA DMA engine in the driver?

Best regards,
Jose Miguel Abreu

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

* Re: [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-08 10:06       ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-08 10:06 UTC (permalink / raw)
  To: Mark Brown, Jose Abreu
  Cc: alsa-devel, lgirdwood, airlied, Vineet.Gupta1, Alexey.Brodkin,
	linux-kernel, dri-devel, CARLOS.PALMINHA, tiwai, linux-snps-arc,
	architt

Hi Mark,


On 07-04-2016 18:53, Mark Brown wrote:
> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>
>> + Optional properties:
>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>> +   it is required to use the properties 'dmas' and 'dma-names'.
> This is not a good interface, it's describing Linux internal APIs.  If
> the device needs to operate in PIO mode it should just do that.

I added this interface because there is no direct way to check if DMA is
available on the I2S controller so it is not possible to automatically change
between DMA and PIO mode. As the I2S controller can be built with or without DMA
support it is necessary to somehow check if DMA is enabled or not and according
to that use either ALSA DMA engine or the custom platform driver sent in these
patches. I did not want to remove drivers functionality so I added this property
to the DT. This way a user can select between DMA and PIO mode.

Is there a better option to do this without removing the possibility of using
ALSA DMA engine in the driver?

Best regards,
Jose Miguel Abreu

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

* [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-08 10:06       ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-08 10:06 UTC (permalink / raw)
  To: linux-snps-arc

Hi Mark,


On 07-04-2016 18:53, Mark Brown wrote:
> On Thu, Apr 07, 2016@05:53:59PM +0100, Jose Abreu wrote:
>
>> + Optional properties:
>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>> +   it is required to use the properties 'dmas' and 'dma-names'.
> This is not a good interface, it's describing Linux internal APIs.  If
> the device needs to operate in PIO mode it should just do that.

I added this interface because there is no direct way to check if DMA is
available on the I2S controller so it is not possible to automatically change
between DMA and PIO mode. As the I2S controller can be built with or without DMA
support it is necessary to somehow check if DMA is enabled or not and according
to that use either ALSA DMA engine or the custom platform driver sent in these
patches. I did not want to remove drivers functionality so I added this property
to the DT. This way a user can select between DMA and PIO mode.

Is there a better option to do this without removing the possibility of using
ALSA DMA engine in the driver?

Best regards,
Jose Miguel Abreu

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-07 16:53   ` Jose Abreu
@ 2016-04-08 15:46     ` Lars-Peter Clausen
  -1 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-08 15:46 UTC (permalink / raw)
  To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, CARLOS.PALMINHA,
	lgirdwood, broonie, tiwai

On 04/07/2016 06:53 PM, Jose Abreu wrote:
> This patch adds audio support for the ADV7511 HDMI transmitter
> using ALSA SoC.
> 
> The code was ported from Analog Devices linux tree from
> commit 1770c4a1e32b ("Merge remote-tracking branch
> 'xilinx/master' into xcomm_zynq"), which is available at:
> 	- https://github.com/analogdevicesinc/linux/

The reason why this driver is still out of tree, is because there has been
no conclusion yet on how to go forward with the whole HDMI integration. So
this is not going to get merged until that issue has been addressed.

[...]
> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
> +  into ALSA SoC.

This is not a description of the hardware.

[...]
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index 964b7de..539c091 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -33,6 +33,7 @@ struct snd_compr_stream;
>  #define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
>  #define SND_SOC_DAIFMT_AC97		6 /* AC97 */
>  #define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */
> +#define SND_SOC_DAIFMT_SPDIF		8 /* SPDIF */

This needs to be a separate patch.

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

* [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-08 15:46     ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-08 15:46 UTC (permalink / raw)
  To: linux-snps-arc

On 04/07/2016 06:53 PM, Jose Abreu wrote:
> This patch adds audio support for the ADV7511 HDMI transmitter
> using ALSA SoC.
> 
> The code was ported from Analog Devices linux tree from
> commit 1770c4a1e32b ("Merge remote-tracking branch
> 'xilinx/master' into xcomm_zynq"), which is available at:
> 	- https://github.com/analogdevicesinc/linux/

The reason why this driver is still out of tree, is because there has been
no conclusion yet on how to go forward with the whole HDMI integration. So
this is not going to get merged until that issue has been addressed.

[...]
> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
> +  into ALSA SoC.

This is not a description of the hardware.

[...]
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index 964b7de..539c091 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -33,6 +33,7 @@ struct snd_compr_stream;
>  #define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
>  #define SND_SOC_DAIFMT_AC97		6 /* AC97 */
>  #define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */
> +#define SND_SOC_DAIFMT_SPDIF		8 /* SPDIF */

This needs to be a separate patch.

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

* Re: [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
  2016-04-08 10:06       ` Jose Abreu
  (?)
@ 2016-04-08 15:52         ` Lars-Peter Clausen
  -1 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-08 15:52 UTC (permalink / raw)
  To: Jose Abreu, Mark Brown
  Cc: alsa-devel, lgirdwood, airlied, Vineet.Gupta1, Alexey.Brodkin,
	linux-kernel, dri-devel, CARLOS.PALMINHA, tiwai, linux-snps-arc,
	architt

On 04/08/2016 12:06 PM, Jose Abreu wrote:
> Hi Mark,
> 
> 
> On 07-04-2016 18:53, Mark Brown wrote:
>> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>>
>>> + Optional properties:
>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>> This is not a good interface, it's describing Linux internal APIs.  If
>> the device needs to operate in PIO mode it should just do that.
> 
> I added this interface because there is no direct way to check if DMA is
> available on the I2S controller so it is not possible to automatically change
> between DMA and PIO mode. As the I2S controller can be built with or without DMA
> support it is necessary to somehow check if DMA is enabled or not and according
> to that use either ALSA DMA engine or the custom platform driver sent in these
> patches. I did not want to remove drivers functionality so I added this property
> to the DT. This way a user can select between DMA and PIO mode.

That's OK, but you need to describe the hardware, not the indented behavior
of the software driver.

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

* Re: [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-08 15:52         ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-08 15:52 UTC (permalink / raw)
  To: Jose Abreu, Mark Brown
  Cc: alsa-devel, Vineet.Gupta1, Alexey.Brodkin, lgirdwood, dri-devel,
	linux-kernel, CARLOS.PALMINHA, tiwai, linux-snps-arc

On 04/08/2016 12:06 PM, Jose Abreu wrote:
> Hi Mark,
> 
> 
> On 07-04-2016 18:53, Mark Brown wrote:
>> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>>
>>> + Optional properties:
>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>> This is not a good interface, it's describing Linux internal APIs.  If
>> the device needs to operate in PIO mode it should just do that.
> 
> I added this interface because there is no direct way to check if DMA is
> available on the I2S controller so it is not possible to automatically change
> between DMA and PIO mode. As the I2S controller can be built with or without DMA
> support it is necessary to somehow check if DMA is enabled or not and according
> to that use either ALSA DMA engine or the custom platform driver sent in these
> patches. I did not want to remove drivers functionality so I added this property
> to the DT. This way a user can select between DMA and PIO mode.

That's OK, but you need to describe the hardware, not the indented behavior
of the software driver.

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

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

* [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-08 15:52         ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-08 15:52 UTC (permalink / raw)
  To: linux-snps-arc

On 04/08/2016 12:06 PM, Jose Abreu wrote:
> Hi Mark,
> 
> 
> On 07-04-2016 18:53, Mark Brown wrote:
>> On Thu, Apr 07, 2016@05:53:59PM +0100, Jose Abreu wrote:
>>
>>> + Optional properties:
>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>> This is not a good interface, it's describing Linux internal APIs.  If
>> the device needs to operate in PIO mode it should just do that.
> 
> I added this interface because there is no direct way to check if DMA is
> available on the I2S controller so it is not possible to automatically change
> between DMA and PIO mode. As the I2S controller can be built with or without DMA
> support it is necessary to somehow check if DMA is enabled or not and according
> to that use either ALSA DMA engine or the custom platform driver sent in these
> patches. I did not want to remove drivers functionality so I added this property
> to the DT. This way a user can select between DMA and PIO mode.

That's OK, but you need to describe the hardware, not the indented behavior
of the software driver.

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

* [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-08 15:46     ` Lars-Peter Clausen
  (?)
@ 2016-04-08 16:05     ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-08 16:05 UTC (permalink / raw)
  To: linux-snps-arc

Hi Lars,


On 08-04-2016 16:46, Lars-Peter Clausen wrote:
> On 04/07/2016 06:53 PM, Jose Abreu wrote:
>> This patch adds audio support for the ADV7511 HDMI transmitter
>> using ALSA SoC.
>>
>> The code was ported from Analog Devices linux tree from
>> commit 1770c4a1e32b ("Merge remote-tracking branch
>> 'xilinx/master' into xcomm_zynq"), which is available at:
>> 	- https://github.com/analogdevicesinc/linux/
> The reason why this driver is still out of tree, is because there has been
> no conclusion yet on how to go forward with the whole HDMI integration. So
> this is not going to get merged until that issue has been addressed.

Ok, will then drop the patches related with adv7511 but will update with your
comments so that when this HDMI issue is addressed I can send again the patches.
Is this okay?

>
> [...]
>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>> +  into ALSA SoC.
> This is not a description of the hardware.

Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
transmitter routes audio signals" ?

>
> [...]
>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
>> index 964b7de..539c091 100644
>> --- a/include/sound/soc-dai.h
>> +++ b/include/sound/soc-dai.h
>> @@ -33,6 +33,7 @@ struct snd_compr_stream;
>>  #define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
>>  #define SND_SOC_DAIFMT_AC97		6 /* AC97 */
>>  #define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */
>> +#define SND_SOC_DAIFMT_SPDIF		8 /* SPDIF */
> This needs to be a separate patch.

Ok.

>
>
> _______________________________________________
> linux-snps-arc mailing list
> linux-snps-arc at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Best regards,
Jose Miguel Abreu

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

* Re: [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
  2016-04-08 15:52         ` Lars-Peter Clausen
  (?)
@ 2016-04-08 16:08           ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-08 16:08 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, Mark Brown
  Cc: alsa-devel, lgirdwood, airlied, Vineet.Gupta1, Alexey.Brodkin,
	linux-kernel, dri-devel, CARLOS.PALMINHA, tiwai, linux-snps-arc,
	architt

Hi Lars,


On 08-04-2016 16:52, Lars-Peter Clausen wrote:
> On 04/08/2016 12:06 PM, Jose Abreu wrote:
>> Hi Mark,
>>
>>
>> On 07-04-2016 18:53, Mark Brown wrote:
>>> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>>>
>>>> + Optional properties:
>>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>>> This is not a good interface, it's describing Linux internal APIs.  If
>>> the device needs to operate in PIO mode it should just do that.
>> I added this interface because there is no direct way to check if DMA is
>> available on the I2S controller so it is not possible to automatically change
>> between DMA and PIO mode. As the I2S controller can be built with or without DMA
>> support it is necessary to somehow check if DMA is enabled or not and according
>> to that use either ALSA DMA engine or the custom platform driver sent in these
>> patches. I did not want to remove drivers functionality so I added this property
>> to the DT. This way a user can select between DMA and PIO mode.
> That's OK, but you need to describe the hardware, not the indented behavior
> of the software driver.
>

Is this okay: "snps,use-dmaengine: Set this boolean paramater if I2S controller
has DMA support. If set the properties 'dmas' and 'dma-names' must be also set" ?

Best regards,
Jose Miguel Abreu

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

* Re: [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-08 16:08           ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-08 16:08 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, Mark Brown
  Cc: alsa-devel, lgirdwood, airlied, Vineet.Gupta1, Alexey.Brodkin,
	linux-kernel, dri-devel, CARLOS.PALMINHA, tiwai, linux-snps-arc,
	architt

Hi Lars,


On 08-04-2016 16:52, Lars-Peter Clausen wrote:
> On 04/08/2016 12:06 PM, Jose Abreu wrote:
>> Hi Mark,
>>
>>
>> On 07-04-2016 18:53, Mark Brown wrote:
>>> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>>>
>>>> + Optional properties:
>>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>>> This is not a good interface, it's describing Linux internal APIs.  If
>>> the device needs to operate in PIO mode it should just do that.
>> I added this interface because there is no direct way to check if DMA is
>> available on the I2S controller so it is not possible to automatically change
>> between DMA and PIO mode. As the I2S controller can be built with or without DMA
>> support it is necessary to somehow check if DMA is enabled or not and according
>> to that use either ALSA DMA engine or the custom platform driver sent in these
>> patches. I did not want to remove drivers functionality so I added this property
>> to the DT. This way a user can select between DMA and PIO mode.
> That's OK, but you need to describe the hardware, not the indented behavior
> of the software driver.
>

Is this okay: "snps,use-dmaengine: Set this boolean paramater if I2S controller
has DMA support. If set the properties 'dmas' and 'dma-names' must be also set" ?

Best regards,
Jose Miguel Abreu

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

* [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-08 16:08           ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-08 16:08 UTC (permalink / raw)
  To: linux-snps-arc

Hi Lars,


On 08-04-2016 16:52, Lars-Peter Clausen wrote:
> On 04/08/2016 12:06 PM, Jose Abreu wrote:
>> Hi Mark,
>>
>>
>> On 07-04-2016 18:53, Mark Brown wrote:
>>> On Thu, Apr 07, 2016@05:53:59PM +0100, Jose Abreu wrote:
>>>
>>>> + Optional properties:
>>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>>> This is not a good interface, it's describing Linux internal APIs.  If
>>> the device needs to operate in PIO mode it should just do that.
>> I added this interface because there is no direct way to check if DMA is
>> available on the I2S controller so it is not possible to automatically change
>> between DMA and PIO mode. As the I2S controller can be built with or without DMA
>> support it is necessary to somehow check if DMA is enabled or not and according
>> to that use either ALSA DMA engine or the custom platform driver sent in these
>> patches. I did not want to remove drivers functionality so I added this property
>> to the DT. This way a user can select between DMA and PIO mode.
> That's OK, but you need to describe the hardware, not the indented behavior
> of the software driver.
>

Is this okay: "snps,use-dmaengine: Set this boolean paramater if I2S controller
has DMA support. If set the properties 'dmas' and 'dma-names' must be also set" ?

Best regards,
Jose Miguel Abreu

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-08 15:46     ` Lars-Peter Clausen
  (?)
@ 2016-04-08 16:12       ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-08 16:12 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, linux-snps-arc, linux-kernel,
	dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, lgirdwood,
	CARLOS.PALMINHA, broonie, tiwai

Hi Lars,


On 08-04-2016 16:46, Lars-Peter Clausen wrote:
> On 04/07/2016 06:53 PM, Jose Abreu wrote:
>> This patch adds audio support for the ADV7511 HDMI transmitter
>> using ALSA SoC.
>>
>> The code was ported from Analog Devices linux tree from
>> commit 1770c4a1e32b ("Merge remote-tracking branch
>> 'xilinx/master' into xcomm_zynq"), which is available at:
>> 	- https://github.com/analogdevicesinc/linux/
> The reason why this driver is still out of tree, is because there has been
> no conclusion yet on how to go forward with the whole HDMI integration. So
> this is not going to get merged until that issue has been addressed.

Ok, will then drop the patches related with adv7511 but will update with your
comments so that when this HDMI issue is addressed I can send again the patches.
Is this okay?

>
> [...]
>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>> +  into ALSA SoC.
> This is not a description of the hardware.

Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
transmitter routes audio signals" ?

>
> [...]
>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
>> index 964b7de..539c091 100644
>> --- a/include/sound/soc-dai.h
>> +++ b/include/sound/soc-dai.h
>> @@ -33,6 +33,7 @@ struct snd_compr_stream;
>>  #define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
>>  #define SND_SOC_DAIFMT_AC97		6 /* AC97 */
>>  #define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */
>> +#define SND_SOC_DAIFMT_SPDIF		8 /* SPDIF */
> This needs to be a separate patch.

Ok.

>
>
> _______________________________________________
> linux-snps-arc mailing list
> linux-snps-arc@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Best regards,
Jose Miguel Abreu

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

* Re: [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-08 16:12       ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-08 16:12 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, linux-snps-arc, linux-kernel,
	dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, CARLOS.PALMINHA,
	lgirdwood, broonie, tiwai

Hi Lars,


On 08-04-2016 16:46, Lars-Peter Clausen wrote:
> On 04/07/2016 06:53 PM, Jose Abreu wrote:
>> This patch adds audio support for the ADV7511 HDMI transmitter
>> using ALSA SoC.
>>
>> The code was ported from Analog Devices linux tree from
>> commit 1770c4a1e32b ("Merge remote-tracking branch
>> 'xilinx/master' into xcomm_zynq"), which is available at:
>> 	- https://github.com/analogdevicesinc/linux/
> The reason why this driver is still out of tree, is because there has been
> no conclusion yet on how to go forward with the whole HDMI integration. So
> this is not going to get merged until that issue has been addressed.

Ok, will then drop the patches related with adv7511 but will update with your
comments so that when this HDMI issue is addressed I can send again the patches.
Is this okay?

>
> [...]
>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>> +  into ALSA SoC.
> This is not a description of the hardware.

Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
transmitter routes audio signals" ?

>
> [...]
>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
>> index 964b7de..539c091 100644
>> --- a/include/sound/soc-dai.h
>> +++ b/include/sound/soc-dai.h
>> @@ -33,6 +33,7 @@ struct snd_compr_stream;
>>  #define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
>>  #define SND_SOC_DAIFMT_AC97		6 /* AC97 */
>>  #define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */
>> +#define SND_SOC_DAIFMT_SPDIF		8 /* SPDIF */
> This needs to be a separate patch.

Ok.

>
>
> _______________________________________________
> linux-snps-arc mailing list
> linux-snps-arc@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Best regards,
Jose Miguel Abreu

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

* [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-08 16:12       ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-08 16:12 UTC (permalink / raw)
  To: linux-snps-arc

Hi Lars,


On 08-04-2016 16:46, Lars-Peter Clausen wrote:
> On 04/07/2016 06:53 PM, Jose Abreu wrote:
>> This patch adds audio support for the ADV7511 HDMI transmitter
>> using ALSA SoC.
>>
>> The code was ported from Analog Devices linux tree from
>> commit 1770c4a1e32b ("Merge remote-tracking branch
>> 'xilinx/master' into xcomm_zynq"), which is available at:
>> 	- https://github.com/analogdevicesinc/linux/
> The reason why this driver is still out of tree, is because there has been
> no conclusion yet on how to go forward with the whole HDMI integration. So
> this is not going to get merged until that issue has been addressed.

Ok, will then drop the patches related with adv7511 but will update with your
comments so that when this HDMI issue is addressed I can send again the patches.
Is this okay?

>
> [...]
>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>> +  into ALSA SoC.
> This is not a description of the hardware.

Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
transmitter routes audio signals" ?

>
> [...]
>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
>> index 964b7de..539c091 100644
>> --- a/include/sound/soc-dai.h
>> +++ b/include/sound/soc-dai.h
>> @@ -33,6 +33,7 @@ struct snd_compr_stream;
>>  #define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
>>  #define SND_SOC_DAIFMT_AC97		6 /* AC97 */
>>  #define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */
>> +#define SND_SOC_DAIFMT_SPDIF		8 /* SPDIF */
> This needs to be a separate patch.

Ok.

>
>
> _______________________________________________
> linux-snps-arc mailing list
> linux-snps-arc at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Best regards,
Jose Miguel Abreu

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

* Re: [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
  2016-04-08 16:08           ` Jose Abreu
  (?)
@ 2016-04-09 14:55             ` Lars-Peter Clausen
  -1 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-09 14:55 UTC (permalink / raw)
  To: Jose Abreu, Mark Brown
  Cc: alsa-devel, lgirdwood, airlied, Vineet.Gupta1, Alexey.Brodkin,
	linux-kernel, dri-devel, CARLOS.PALMINHA, tiwai, linux-snps-arc,
	architt

On 04/08/2016 06:08 PM, Jose Abreu wrote:
> Hi Lars,
> 
> 
> On 08-04-2016 16:52, Lars-Peter Clausen wrote:
>> On 04/08/2016 12:06 PM, Jose Abreu wrote:
>>> Hi Mark,
>>>
>>>
>>> On 07-04-2016 18:53, Mark Brown wrote:
>>>> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>>>>
>>>>> + Optional properties:
>>>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>>>> This is not a good interface, it's describing Linux internal APIs.  If
>>>> the device needs to operate in PIO mode it should just do that.
>>> I added this interface because there is no direct way to check if DMA is
>>> available on the I2S controller so it is not possible to automatically change
>>> between DMA and PIO mode. As the I2S controller can be built with or without DMA
>>> support it is necessary to somehow check if DMA is enabled or not and according
>>> to that use either ALSA DMA engine or the custom platform driver sent in these
>>> patches. I did not want to remove drivers functionality so I added this property
>>> to the DT. This way a user can select between DMA and PIO mode.
>> That's OK, but you need to describe the hardware, not the indented behavior
>> of the software driver.
>>
> 
> Is this okay: "snps,use-dmaengine: Set this boolean paramater if I2S controller
> has DMA support. If set the properties 'dmas' and 'dma-names' must be also set" ?

The description is better. But the name of the property is still imperative
rather then descriptive. It tells the software what should be done rather
then describing what the hardware looks like.

Since there is already the dmas property which is present if a DMA is
connected and is absent when no DMA is present it should be enough to just
check that property rather than requiring an additional one.

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

* Re: [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-09 14:55             ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-09 14:55 UTC (permalink / raw)
  To: Jose Abreu, Mark Brown
  Cc: alsa-devel, Vineet.Gupta1, Alexey.Brodkin, lgirdwood, dri-devel,
	linux-kernel, CARLOS.PALMINHA, tiwai, linux-snps-arc

On 04/08/2016 06:08 PM, Jose Abreu wrote:
> Hi Lars,
> 
> 
> On 08-04-2016 16:52, Lars-Peter Clausen wrote:
>> On 04/08/2016 12:06 PM, Jose Abreu wrote:
>>> Hi Mark,
>>>
>>>
>>> On 07-04-2016 18:53, Mark Brown wrote:
>>>> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>>>>
>>>>> + Optional properties:
>>>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>>>> This is not a good interface, it's describing Linux internal APIs.  If
>>>> the device needs to operate in PIO mode it should just do that.
>>> I added this interface because there is no direct way to check if DMA is
>>> available on the I2S controller so it is not possible to automatically change
>>> between DMA and PIO mode. As the I2S controller can be built with or without DMA
>>> support it is necessary to somehow check if DMA is enabled or not and according
>>> to that use either ALSA DMA engine or the custom platform driver sent in these
>>> patches. I did not want to remove drivers functionality so I added this property
>>> to the DT. This way a user can select between DMA and PIO mode.
>> That's OK, but you need to describe the hardware, not the indented behavior
>> of the software driver.
>>
> 
> Is this okay: "snps,use-dmaengine: Set this boolean paramater if I2S controller
> has DMA support. If set the properties 'dmas' and 'dma-names' must be also set" ?

The description is better. But the name of the property is still imperative
rather then descriptive. It tells the software what should be done rather
then describing what the hardware looks like.

Since there is already the dmas property which is present if a DMA is
connected and is absent when no DMA is present it should be enough to just
check that property rather than requiring an additional one.

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

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

* [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-09 14:55             ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-09 14:55 UTC (permalink / raw)
  To: linux-snps-arc

On 04/08/2016 06:08 PM, Jose Abreu wrote:
> Hi Lars,
> 
> 
> On 08-04-2016 16:52, Lars-Peter Clausen wrote:
>> On 04/08/2016 12:06 PM, Jose Abreu wrote:
>>> Hi Mark,
>>>
>>>
>>> On 07-04-2016 18:53, Mark Brown wrote:
>>>> On Thu, Apr 07, 2016@05:53:59PM +0100, Jose Abreu wrote:
>>>>
>>>>> + Optional properties:
>>>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>>>> This is not a good interface, it's describing Linux internal APIs.  If
>>>> the device needs to operate in PIO mode it should just do that.
>>> I added this interface because there is no direct way to check if DMA is
>>> available on the I2S controller so it is not possible to automatically change
>>> between DMA and PIO mode. As the I2S controller can be built with or without DMA
>>> support it is necessary to somehow check if DMA is enabled or not and according
>>> to that use either ALSA DMA engine or the custom platform driver sent in these
>>> patches. I did not want to remove drivers functionality so I added this property
>>> to the DT. This way a user can select between DMA and PIO mode.
>> That's OK, but you need to describe the hardware, not the indented behavior
>> of the software driver.
>>
> 
> Is this okay: "snps,use-dmaengine: Set this boolean paramater if I2S controller
> has DMA support. If set the properties 'dmas' and 'dma-names' must be also set" ?

The description is better. But the name of the property is still imperative
rather then descriptive. It tells the software what should be done rather
then describing what the hardware looks like.

Since there is already the dmas property which is present if a DMA is
connected and is absent when no DMA is present it should be enough to just
check that property rather than requiring an additional one.

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-08 16:12       ` Jose Abreu
  (?)
@ 2016-04-09 15:02         ` Lars-Peter Clausen
  -1 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-09 15:02 UTC (permalink / raw)
  To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, lgirdwood,
	CARLOS.PALMINHA, broonie, tiwai

On 04/08/2016 06:12 PM, Jose Abreu wrote:
[...]
>>
>> [...]
>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>> +  into ALSA SoC.
>> This is not a description of the hardware.
> 
> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
> transmitter routes audio signals" ?

I don't think we need this property. There is no problem with registering
the audio part unconditionally. As long as there is no connection we wont
create a sound card that is exposed to userspace.

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-09 15:02         ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-09 15:02 UTC (permalink / raw)
  To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: Vineet.Gupta1, Alexey.Brodkin, CARLOS.PALMINHA, lgirdwood,
	broonie, tiwai

On 04/08/2016 06:12 PM, Jose Abreu wrote:
[...]
>>
>> [...]
>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>> +  into ALSA SoC.
>> This is not a description of the hardware.
> 
> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
> transmitter routes audio signals" ?

I don't think we need this property. There is no problem with registering
the audio part unconditionally. As long as there is no connection we wont
create a sound card that is exposed to userspace.

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

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

* [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-09 15:02         ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-09 15:02 UTC (permalink / raw)
  To: linux-snps-arc

On 04/08/2016 06:12 PM, Jose Abreu wrote:
[...]
>>
>> [...]
>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>> +  into ALSA SoC.
>> This is not a description of the hardware.
> 
> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
> transmitter routes audio signals" ?

I don't think we need this property. There is no problem with registering
the audio part unconditionally. As long as there is no connection we wont
create a sound card that is exposed to userspace.

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

* Re: [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
  2016-04-09 14:55             ` Lars-Peter Clausen
  (?)
@ 2016-04-11  9:24               ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11  9:24 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, Mark Brown
  Cc: alsa-devel, Vineet.Gupta1, Alexey.Brodkin, lgirdwood, dri-devel,
	linux-kernel, CARLOS.PALMINHA, tiwai, linux-snps-arc

Hi Lars,


On 09-04-2016 15:55, Lars-Peter Clausen wrote:
> On 04/08/2016 06:08 PM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 08-04-2016 16:52, Lars-Peter Clausen wrote:
>>> On 04/08/2016 12:06 PM, Jose Abreu wrote:
>>>> Hi Mark,
>>>>
>>>>
>>>> On 07-04-2016 18:53, Mark Brown wrote:
>>>>> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>>>>>
>>>>>> + Optional properties:
>>>>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>>>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>>>>> This is not a good interface, it's describing Linux internal APIs.  If
>>>>> the device needs to operate in PIO mode it should just do that.
>>>> I added this interface because there is no direct way to check if DMA is
>>>> available on the I2S controller so it is not possible to automatically change
>>>> between DMA and PIO mode. As the I2S controller can be built with or without DMA
>>>> support it is necessary to somehow check if DMA is enabled or not and according
>>>> to that use either ALSA DMA engine or the custom platform driver sent in these
>>>> patches. I did not want to remove drivers functionality so I added this property
>>>> to the DT. This way a user can select between DMA and PIO mode.
>>> That's OK, but you need to describe the hardware, not the indented behavior
>>> of the software driver.
>>>
>> Is this okay: "snps,use-dmaengine: Set this boolean paramater if I2S controller
>> has DMA support. If set the properties 'dmas' and 'dma-names' must be also set" ?
> The description is better. But the name of the property is still imperative
> rather then descriptive. It tells the software what should be done rather
> then describing what the hardware looks like.
>
> Since there is already the dmas property which is present if a DMA is
> connected and is absent when no DMA is present it should be enough to just
> check that property rather than requiring an additional one.

Ok, will then use the DMA property to decide which mode to use: PIO or DMA.

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

Best regards,
Jose Miguel Abreu

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

* Re: [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-11  9:24               ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11  9:24 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, Mark Brown
  Cc: alsa-devel, Vineet.Gupta1, Alexey.Brodkin, linux-kernel,
	dri-devel, lgirdwood, CARLOS.PALMINHA, tiwai, linux-snps-arc

Hi Lars,


On 09-04-2016 15:55, Lars-Peter Clausen wrote:
> On 04/08/2016 06:08 PM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 08-04-2016 16:52, Lars-Peter Clausen wrote:
>>> On 04/08/2016 12:06 PM, Jose Abreu wrote:
>>>> Hi Mark,
>>>>
>>>>
>>>> On 07-04-2016 18:53, Mark Brown wrote:
>>>>> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>>>>>
>>>>>> + Optional properties:
>>>>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>>>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>>>>> This is not a good interface, it's describing Linux internal APIs.  If
>>>>> the device needs to operate in PIO mode it should just do that.
>>>> I added this interface because there is no direct way to check if DMA is
>>>> available on the I2S controller so it is not possible to automatically change
>>>> between DMA and PIO mode. As the I2S controller can be built with or without DMA
>>>> support it is necessary to somehow check if DMA is enabled or not and according
>>>> to that use either ALSA DMA engine or the custom platform driver sent in these
>>>> patches. I did not want to remove drivers functionality so I added this property
>>>> to the DT. This way a user can select between DMA and PIO mode.
>>> That's OK, but you need to describe the hardware, not the indented behavior
>>> of the software driver.
>>>
>> Is this okay: "snps,use-dmaengine: Set this boolean paramater if I2S controller
>> has DMA support. If set the properties 'dmas' and 'dma-names' must be also set" ?
> The description is better. But the name of the property is still imperative
> rather then descriptive. It tells the software what should be done rather
> then describing what the hardware looks like.
>
> Since there is already the dmas property which is present if a DMA is
> connected and is absent when no DMA is present it should be enough to just
> check that property rather than requiring an additional one.

Ok, will then use the DMA property to decide which mode to use: PIO or DMA.

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

Best regards,
Jose Miguel Abreu

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

* [alsa-devel] [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver
@ 2016-04-11  9:24               ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11  9:24 UTC (permalink / raw)
  To: linux-snps-arc

Hi Lars,


On 09-04-2016 15:55, Lars-Peter Clausen wrote:
> On 04/08/2016 06:08 PM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 08-04-2016 16:52, Lars-Peter Clausen wrote:
>>> On 04/08/2016 12:06 PM, Jose Abreu wrote:
>>>> Hi Mark,
>>>>
>>>>
>>>> On 07-04-2016 18:53, Mark Brown wrote:
>>>>> On Thu, Apr 07, 2016@05:53:59PM +0100, Jose Abreu wrote:
>>>>>
>>>>>> + Optional properties:
>>>>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>>>>> +   it is required to use the properties 'dmas' and 'dma-names'.
>>>>> This is not a good interface, it's describing Linux internal APIs.  If
>>>>> the device needs to operate in PIO mode it should just do that.
>>>> I added this interface because there is no direct way to check if DMA is
>>>> available on the I2S controller so it is not possible to automatically change
>>>> between DMA and PIO mode. As the I2S controller can be built with or without DMA
>>>> support it is necessary to somehow check if DMA is enabled or not and according
>>>> to that use either ALSA DMA engine or the custom platform driver sent in these
>>>> patches. I did not want to remove drivers functionality so I added this property
>>>> to the DT. This way a user can select between DMA and PIO mode.
>>> That's OK, but you need to describe the hardware, not the indented behavior
>>> of the software driver.
>>>
>> Is this okay: "snps,use-dmaengine: Set this boolean paramater if I2S controller
>> has DMA support. If set the properties 'dmas' and 'dma-names' must be also set" ?
> The description is better. But the name of the property is still imperative
> rather then descriptive. It tells the software what should be done rather
> then describing what the hardware looks like.
>
> Since there is already the dmas property which is present if a DMA is
> connected and is absent when no DMA is present it should be enough to just
> check that property rather than requiring an additional one.

Ok, will then use the DMA property to decide which mode to use: PIO or DMA.

>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Best regards,
Jose Miguel Abreu

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-09 15:02         ` Lars-Peter Clausen
  (?)
@ 2016-04-11  9:27           ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11  9:27 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, linux-snps-arc, linux-kernel,
	dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, lgirdwood,
	CARLOS.PALMINHA, broonie, tiwai

Hi Lars,


On 09-04-2016 16:02, Lars-Peter Clausen wrote:
> On 04/08/2016 06:12 PM, Jose Abreu wrote:
> [...]
>>> [...]
>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>> +  into ALSA SoC.
>>> This is not a description of the hardware.
>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>> transmitter routes audio signals" ?
> I don't think we need this property. There is no problem with registering
> the audio part unconditionally. As long as there is no connection we wont
> create a sound card that is exposed to userspace.
>

This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
Laurent:
"The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
with whether the system includes audio support. It would be confusing, and would
also waste resources, to create a Linux sound device when no sound channel is
routed on the board."

Should I revert this?

Best regards,
Jose Miguel Abreu

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11  9:27           ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11  9:27 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, linux-snps-arc, linux-kernel,
	dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, lgirdwood,
	CARLOS.PALMINHA, broonie, tiwai

Hi Lars,


On 09-04-2016 16:02, Lars-Peter Clausen wrote:
> On 04/08/2016 06:12 PM, Jose Abreu wrote:
> [...]
>>> [...]
>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>> +  into ALSA SoC.
>>> This is not a description of the hardware.
>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>> transmitter routes audio signals" ?
> I don't think we need this property. There is no problem with registering
> the audio part unconditionally. As long as there is no connection we wont
> create a sound card that is exposed to userspace.
>

This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
Laurent:
"The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
with whether the system includes audio support. It would be confusing, and would
also waste resources, to create a Linux sound device when no sound channel is
routed on the board."

Should I revert this?

Best regards,
Jose Miguel Abreu

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

* [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11  9:27           ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11  9:27 UTC (permalink / raw)
  To: linux-snps-arc

Hi Lars,


On 09-04-2016 16:02, Lars-Peter Clausen wrote:
> On 04/08/2016 06:12 PM, Jose Abreu wrote:
> [...]
>>> [...]
>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>> +  into ALSA SoC.
>>> This is not a description of the hardware.
>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>> transmitter routes audio signals" ?
> I don't think we need this property. There is no problem with registering
> the audio part unconditionally. As long as there is no connection we wont
> create a sound card that is exposed to userspace.
>

This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
Laurent:
"The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
with whether the system includes audio support. It would be confusing, and would
also waste resources, to create a Linux sound device when no sound channel is
routed on the board."

Should I revert this?

Best regards,
Jose Miguel Abreu

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-11  9:27           ` Jose Abreu
  (?)
@ 2016-04-11  9:33             ` Lars-Peter Clausen
  -1 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-11  9:33 UTC (permalink / raw)
  To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, lgirdwood,
	CARLOS.PALMINHA, broonie, tiwai

On 04/11/2016 11:27 AM, Jose Abreu wrote:
> Hi Lars,
> 
> 
> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>> [...]
>>>> [...]
>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>> +  into ALSA SoC.
>>>> This is not a description of the hardware.
>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>> transmitter routes audio signals" ?
>> I don't think we need this property. There is no problem with registering
>> the audio part unconditionally. As long as there is no connection we wont
>> create a sound card that is exposed to userspace.
>>
> 
> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
> Laurent:
> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
> with whether the system includes audio support. It would be confusing, and would
> also waste resources, to create a Linux sound device when no sound channel is
> routed on the board."

I wouldn't care too much about this at this point, the extra amount of
resources required for registering the CODEC (but not the sound card) is
just a few bytes (sizeof(struct snd_soc_codec)).

Nevertheless what we should do is describe the hardware and from this
information infer whether there is a audio connection or not and if there is
none we might skip registering the CODEC. In my opinion this hardware
description should be modeled using of-graph, having a connection between
the SoC side and the adv7511 SPDIF or I2S port.

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11  9:33             ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-11  9:33 UTC (permalink / raw)
  To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: Vineet.Gupta1, Alexey.Brodkin, CARLOS.PALMINHA, lgirdwood,
	broonie, tiwai

On 04/11/2016 11:27 AM, Jose Abreu wrote:
> Hi Lars,
> 
> 
> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>> [...]
>>>> [...]
>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>> +  into ALSA SoC.
>>>> This is not a description of the hardware.
>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>> transmitter routes audio signals" ?
>> I don't think we need this property. There is no problem with registering
>> the audio part unconditionally. As long as there is no connection we wont
>> create a sound card that is exposed to userspace.
>>
> 
> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
> Laurent:
> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
> with whether the system includes audio support. It would be confusing, and would
> also waste resources, to create a Linux sound device when no sound channel is
> routed on the board."

I wouldn't care too much about this at this point, the extra amount of
resources required for registering the CODEC (but not the sound card) is
just a few bytes (sizeof(struct snd_soc_codec)).

Nevertheless what we should do is describe the hardware and from this
information infer whether there is a audio connection or not and if there is
none we might skip registering the CODEC. In my opinion this hardware
description should be modeled using of-graph, having a connection between
the SoC side and the adv7511 SPDIF or I2S port.

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

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

* [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11  9:33             ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-11  9:33 UTC (permalink / raw)
  To: linux-snps-arc

On 04/11/2016 11:27 AM, Jose Abreu wrote:
> Hi Lars,
> 
> 
> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>> [...]
>>>> [...]
>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>> +  into ALSA SoC.
>>>> This is not a description of the hardware.
>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>> transmitter routes audio signals" ?
>> I don't think we need this property. There is no problem with registering
>> the audio part unconditionally. As long as there is no connection we wont
>> create a sound card that is exposed to userspace.
>>
> 
> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
> Laurent:
> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
> with whether the system includes audio support. It would be confusing, and would
> also waste resources, to create a Linux sound device when no sound channel is
> routed on the board."

I wouldn't care too much about this at this point, the extra amount of
resources required for registering the CODEC (but not the sound card) is
just a few bytes (sizeof(struct snd_soc_codec)).

Nevertheless what we should do is describe the hardware and from this
information infer whether there is a audio connection or not and if there is
none we might skip registering the CODEC. In my opinion this hardware
description should be modeled using of-graph, having a connection between
the SoC side and the adv7511 SPDIF or I2S port.

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-11  9:33             ` Lars-Peter Clausen
  (?)
@ 2016-04-11 11:32               ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11 11:32 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, linux-snps-arc, linux-kernel,
	dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, lgirdwood,
	CARLOS.PALMINHA, broonie, tiwai

Hi Lars,


On 11-04-2016 10:33, Lars-Peter Clausen wrote:
> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>>> [...]
>>>>> [...]
>>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>>> +  into ALSA SoC.
>>>>> This is not a description of the hardware.
>>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>>> transmitter routes audio signals" ?
>>> I don't think we need this property. There is no problem with registering
>>> the audio part unconditionally. As long as there is no connection we wont
>>> create a sound card that is exposed to userspace.
>>>
>> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
>> Laurent:
>> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
>> with whether the system includes audio support. It would be confusing, and would
>> also waste resources, to create a Linux sound device when no sound channel is
>> routed on the board."
> I wouldn't care too much about this at this point, the extra amount of
> resources required for registering the CODEC (but not the sound card) is
> just a few bytes (sizeof(struct snd_soc_codec)).
>
> Nevertheless what we should do is describe the hardware and from this
> information infer whether there is a audio connection or not and if there is
> none we might skip registering the CODEC. In my opinion this hardware
> description should be modeled using of-graph, having a connection between
> the SoC side and the adv7511 SPDIF or I2S port.
>

You mean something like this:

sound_playback: sound_playback {
    compatible = "simple-audio-card";
    [...]
    simple-audio-card,format = "i2s";
    [...]
}

adv7511@xx {
    compatible = "adi,adv7511";
    [...]

    ports {
        [...]
        /* Audio Output */
        port@x {
            reg = <x>;
            endpoint {
                remote-endpoint = <&sound_playback>;
            }
        }
    }
}

?

Best regards,
Jose Miguel Abreu

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

* Re: [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11 11:32               ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11 11:32 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, linux-snps-arc, linux-kernel,
	dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, CARLOS.PALMINHA,
	lgirdwood, broonie, tiwai

Hi Lars,


On 11-04-2016 10:33, Lars-Peter Clausen wrote:
> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>>> [...]
>>>>> [...]
>>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>>> +  into ALSA SoC.
>>>>> This is not a description of the hardware.
>>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>>> transmitter routes audio signals" ?
>>> I don't think we need this property. There is no problem with registering
>>> the audio part unconditionally. As long as there is no connection we wont
>>> create a sound card that is exposed to userspace.
>>>
>> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
>> Laurent:
>> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
>> with whether the system includes audio support. It would be confusing, and would
>> also waste resources, to create a Linux sound device when no sound channel is
>> routed on the board."
> I wouldn't care too much about this at this point, the extra amount of
> resources required for registering the CODEC (but not the sound card) is
> just a few bytes (sizeof(struct snd_soc_codec)).
>
> Nevertheless what we should do is describe the hardware and from this
> information infer whether there is a audio connection or not and if there is
> none we might skip registering the CODEC. In my opinion this hardware
> description should be modeled using of-graph, having a connection between
> the SoC side and the adv7511 SPDIF or I2S port.
>

You mean something like this:

sound_playback: sound_playback {
    compatible = "simple-audio-card";
    [...]
    simple-audio-card,format = "i2s";
    [...]
}

adv7511@xx {
    compatible = "adi,adv7511";
    [...]

    ports {
        [...]
        /* Audio Output */
        port@x {
            reg = <x>;
            endpoint {
                remote-endpoint = <&sound_playback>;
            }
        }
    }
}

?

Best regards,
Jose Miguel Abreu

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

* [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11 11:32               ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11 11:32 UTC (permalink / raw)
  To: linux-snps-arc

Hi Lars,


On 11-04-2016 10:33, Lars-Peter Clausen wrote:
> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>>> [...]
>>>>> [...]
>>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>>> +  into ALSA SoC.
>>>>> This is not a description of the hardware.
>>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>>> transmitter routes audio signals" ?
>>> I don't think we need this property. There is no problem with registering
>>> the audio part unconditionally. As long as there is no connection we wont
>>> create a sound card that is exposed to userspace.
>>>
>> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
>> Laurent:
>> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
>> with whether the system includes audio support. It would be confusing, and would
>> also waste resources, to create a Linux sound device when no sound channel is
>> routed on the board."
> I wouldn't care too much about this at this point, the extra amount of
> resources required for registering the CODEC (but not the sound card) is
> just a few bytes (sizeof(struct snd_soc_codec)).
>
> Nevertheless what we should do is describe the hardware and from this
> information infer whether there is a audio connection or not and if there is
> none we might skip registering the CODEC. In my opinion this hardware
> description should be modeled using of-graph, having a connection between
> the SoC side and the adv7511 SPDIF or I2S port.
>

You mean something like this:

sound_playback: sound_playback {
    compatible = "simple-audio-card";
    [...]
    simple-audio-card,format = "i2s";
    [...]
}

adv7511 at xx {
    compatible = "adi,adv7511";
    [...]

    ports {
        [...]
        /* Audio Output */
        port at x {
            reg = <x>;
            endpoint {
                remote-endpoint = <&sound_playback>;
            }
        }
    }
}

?

Best regards,
Jose Miguel Abreu

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-11 11:32               ` Jose Abreu
  (?)
@ 2016-04-11 12:23                 ` Lars-Peter Clausen
  -1 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-11 12:23 UTC (permalink / raw)
  To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, lgirdwood,
	CARLOS.PALMINHA, broonie, tiwai

On 04/11/2016 01:32 PM, Jose Abreu wrote:
> Hi Lars,
> 
> 
> On 11-04-2016 10:33, Lars-Peter Clausen wrote:
>> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>>> Hi Lars,
>>>
>>>
>>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>>>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>>>> [...]
>>>>>> [...]
>>>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>>>> +  into ALSA SoC.
>>>>>> This is not a description of the hardware.
>>>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>>>> transmitter routes audio signals" ?
>>>> I don't think we need this property. There is no problem with registering
>>>> the audio part unconditionally. As long as there is no connection we wont
>>>> create a sound card that is exposed to userspace.
>>>>
>>> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
>>> Laurent:
>>> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
>>> with whether the system includes audio support. It would be confusing, and would
>>> also waste resources, to create a Linux sound device when no sound channel is
>>> routed on the board."
>> I wouldn't care too much about this at this point, the extra amount of
>> resources required for registering the CODEC (but not the sound card) is
>> just a few bytes (sizeof(struct snd_soc_codec)).
>>
>> Nevertheless what we should do is describe the hardware and from this
>> information infer whether there is a audio connection or not and if there is
>> none we might skip registering the CODEC. In my opinion this hardware
>> description should be modeled using of-graph, having a connection between
>> the SoC side and the adv7511 SPDIF or I2S port.
>>
> 
> You mean something like this:
> 
> sound_playback: sound_playback {
>     compatible = "simple-audio-card";
>     [...]
>     simple-audio-card,format = "i2s";
>     [...]
> }
> 
> adv7511@xx {
>     compatible = "adi,adv7511";
>     [...]
> 
>     ports {
>         [...]
>         /* Audio Output */
>         port@x {
>             reg = <x>;
>             endpoint {
>                 remote-endpoint = <&sound_playback>;
>             }
>         }
>     }
> }

Yes, something like that. Not exactly like that, but similar. One of the
core concepts of of-graph is that there is always a description of the
connection from both sides, this way each side can independently figure out
where it is connected.

Currently there is also zero support of of-graph in ASoC, so a bit of work
is required to get this integrated properly.

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11 12:23                 ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-11 12:23 UTC (permalink / raw)
  To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: Vineet.Gupta1, Alexey.Brodkin, CARLOS.PALMINHA, lgirdwood,
	broonie, tiwai

On 04/11/2016 01:32 PM, Jose Abreu wrote:
> Hi Lars,
> 
> 
> On 11-04-2016 10:33, Lars-Peter Clausen wrote:
>> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>>> Hi Lars,
>>>
>>>
>>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>>>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>>>> [...]
>>>>>> [...]
>>>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>>>> +  into ALSA SoC.
>>>>>> This is not a description of the hardware.
>>>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>>>> transmitter routes audio signals" ?
>>>> I don't think we need this property. There is no problem with registering
>>>> the audio part unconditionally. As long as there is no connection we wont
>>>> create a sound card that is exposed to userspace.
>>>>
>>> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
>>> Laurent:
>>> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
>>> with whether the system includes audio support. It would be confusing, and would
>>> also waste resources, to create a Linux sound device when no sound channel is
>>> routed on the board."
>> I wouldn't care too much about this at this point, the extra amount of
>> resources required for registering the CODEC (but not the sound card) is
>> just a few bytes (sizeof(struct snd_soc_codec)).
>>
>> Nevertheless what we should do is describe the hardware and from this
>> information infer whether there is a audio connection or not and if there is
>> none we might skip registering the CODEC. In my opinion this hardware
>> description should be modeled using of-graph, having a connection between
>> the SoC side and the adv7511 SPDIF or I2S port.
>>
> 
> You mean something like this:
> 
> sound_playback: sound_playback {
>     compatible = "simple-audio-card";
>     [...]
>     simple-audio-card,format = "i2s";
>     [...]
> }
> 
> adv7511@xx {
>     compatible = "adi,adv7511";
>     [...]
> 
>     ports {
>         [...]
>         /* Audio Output */
>         port@x {
>             reg = <x>;
>             endpoint {
>                 remote-endpoint = <&sound_playback>;
>             }
>         }
>     }
> }

Yes, something like that. Not exactly like that, but similar. One of the
core concepts of of-graph is that there is always a description of the
connection from both sides, this way each side can independently figure out
where it is connected.

Currently there is also zero support of of-graph in ASoC, so a bit of work
is required to get this integrated properly.

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

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

* [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11 12:23                 ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-11 12:23 UTC (permalink / raw)
  To: linux-snps-arc

On 04/11/2016 01:32 PM, Jose Abreu wrote:
> Hi Lars,
> 
> 
> On 11-04-2016 10:33, Lars-Peter Clausen wrote:
>> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>>> Hi Lars,
>>>
>>>
>>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>>>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>>>> [...]
>>>>>> [...]
>>>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>>>> +  into ALSA SoC.
>>>>>> This is not a description of the hardware.
>>>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>>>> transmitter routes audio signals" ?
>>>> I don't think we need this property. There is no problem with registering
>>>> the audio part unconditionally. As long as there is no connection we wont
>>>> create a sound card that is exposed to userspace.
>>>>
>>> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
>>> Laurent:
>>> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
>>> with whether the system includes audio support. It would be confusing, and would
>>> also waste resources, to create a Linux sound device when no sound channel is
>>> routed on the board."
>> I wouldn't care too much about this at this point, the extra amount of
>> resources required for registering the CODEC (but not the sound card) is
>> just a few bytes (sizeof(struct snd_soc_codec)).
>>
>> Nevertheless what we should do is describe the hardware and from this
>> information infer whether there is a audio connection or not and if there is
>> none we might skip registering the CODEC. In my opinion this hardware
>> description should be modeled using of-graph, having a connection between
>> the SoC side and the adv7511 SPDIF or I2S port.
>>
> 
> You mean something like this:
> 
> sound_playback: sound_playback {
>     compatible = "simple-audio-card";
>     [...]
>     simple-audio-card,format = "i2s";
>     [...]
> }
> 
> adv7511 at xx {
>     compatible = "adi,adv7511";
>     [...]
> 
>     ports {
>         [...]
>         /* Audio Output */
>         port at x {
>             reg = <x>;
>             endpoint {
>                 remote-endpoint = <&sound_playback>;
>             }
>         }
>     }
> }

Yes, something like that. Not exactly like that, but similar. One of the
core concepts of of-graph is that there is always a description of the
connection from both sides, this way each side can independently figure out
where it is connected.

Currently there is also zero support of of-graph in ASoC, so a bit of work
is required to get this integrated properly.

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-11 12:23                 ` Lars-Peter Clausen
  (?)
@ 2016-04-11 14:08                   ` Jose Abreu
  -1 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11 14:08 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, linux-snps-arc, linux-kernel,
	dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, lgirdwood,
	CARLOS.PALMINHA, broonie, tiwai

Hi Lars,


On 11-04-2016 13:23, Lars-Peter Clausen wrote:
> On 04/11/2016 01:32 PM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 11-04-2016 10:33, Lars-Peter Clausen wrote:
>>> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>>>> Hi Lars,
>>>>
>>>>
>>>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>>>>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>>>>> [...]
>>>>>>> [...]
>>>>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>>>>> +  into ALSA SoC.
>>>>>>> This is not a description of the hardware.
>>>>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>>>>> transmitter routes audio signals" ?
>>>>> I don't think we need this property. There is no problem with registering
>>>>> the audio part unconditionally. As long as there is no connection we wont
>>>>> create a sound card that is exposed to userspace.
>>>>>
>>>> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
>>>> Laurent:
>>>> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
>>>> with whether the system includes audio support. It would be confusing, and would
>>>> also waste resources, to create a Linux sound device when no sound channel is
>>>> routed on the board."
>>> I wouldn't care too much about this at this point, the extra amount of
>>> resources required for registering the CODEC (but not the sound card) is
>>> just a few bytes (sizeof(struct snd_soc_codec)).
>>>
>>> Nevertheless what we should do is describe the hardware and from this
>>> information infer whether there is a audio connection or not and if there is
>>> none we might skip registering the CODEC. In my opinion this hardware
>>> description should be modeled using of-graph, having a connection between
>>> the SoC side and the adv7511 SPDIF or I2S port.
>>>
>> You mean something like this:
>>
>> sound_playback: sound_playback {
>>     compatible = "simple-audio-card";
>>     [...]
>>     simple-audio-card,format = "i2s";
>>     [...]
>> }
>>
>> adv7511@xx {
>>     compatible = "adi,adv7511";
>>     [...]
>>
>>     ports {
>>         [...]
>>         /* Audio Output */
>>         port@x {
>>             reg = <x>;
>>             endpoint {
>>                 remote-endpoint = <&sound_playback>;
>>             }
>>         }
>>     }
>> }
> Yes, something like that. Not exactly like that, but similar. One of the
> core concepts of of-graph is that there is always a description of the
> connection from both sides, this way each side can independently figure out
> where it is connected.
>
> Currently there is also zero support of of-graph in ASoC, so a bit of work
> is required to get this integrated properly.
>

I also believe this would be the better option but in the meantime can't I
integrate the audio like it is being done in the dw-hdmi driver[1]? In this
driver the audio is registered as a sound card and is conditionally built using
Kconfig, just like I was doing in the previous versions. I know you said the
HDMI audio is still an open issue but it seems that for this case it was accepted.

[1] http://lxr.free-electrons.com/source/drivers/gpu/drm/bridge/dw-hdmi-ahb-audio.c

Best regards,
Jose Miguel Abreu

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

* Re: [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11 14:08                   ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11 14:08 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jose Abreu, linux-snps-arc, linux-kernel,
	dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, CARLOS.PALMINHA,
	lgirdwood, broonie, tiwai

Hi Lars,


On 11-04-2016 13:23, Lars-Peter Clausen wrote:
> On 04/11/2016 01:32 PM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 11-04-2016 10:33, Lars-Peter Clausen wrote:
>>> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>>>> Hi Lars,
>>>>
>>>>
>>>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>>>>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>>>>> [...]
>>>>>>> [...]
>>>>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>>>>> +  into ALSA SoC.
>>>>>>> This is not a description of the hardware.
>>>>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>>>>> transmitter routes audio signals" ?
>>>>> I don't think we need this property. There is no problem with registering
>>>>> the audio part unconditionally. As long as there is no connection we wont
>>>>> create a sound card that is exposed to userspace.
>>>>>
>>>> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
>>>> Laurent:
>>>> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
>>>> with whether the system includes audio support. It would be confusing, and would
>>>> also waste resources, to create a Linux sound device when no sound channel is
>>>> routed on the board."
>>> I wouldn't care too much about this at this point, the extra amount of
>>> resources required for registering the CODEC (but not the sound card) is
>>> just a few bytes (sizeof(struct snd_soc_codec)).
>>>
>>> Nevertheless what we should do is describe the hardware and from this
>>> information infer whether there is a audio connection or not and if there is
>>> none we might skip registering the CODEC. In my opinion this hardware
>>> description should be modeled using of-graph, having a connection between
>>> the SoC side and the adv7511 SPDIF or I2S port.
>>>
>> You mean something like this:
>>
>> sound_playback: sound_playback {
>>     compatible = "simple-audio-card";
>>     [...]
>>     simple-audio-card,format = "i2s";
>>     [...]
>> }
>>
>> adv7511@xx {
>>     compatible = "adi,adv7511";
>>     [...]
>>
>>     ports {
>>         [...]
>>         /* Audio Output */
>>         port@x {
>>             reg = <x>;
>>             endpoint {
>>                 remote-endpoint = <&sound_playback>;
>>             }
>>         }
>>     }
>> }
> Yes, something like that. Not exactly like that, but similar. One of the
> core concepts of of-graph is that there is always a description of the
> connection from both sides, this way each side can independently figure out
> where it is connected.
>
> Currently there is also zero support of of-graph in ASoC, so a bit of work
> is required to get this integrated properly.
>

I also believe this would be the better option but in the meantime can't I
integrate the audio like it is being done in the dw-hdmi driver[1]? In this
driver the audio is registered as a sound card and is conditionally built using
Kconfig, just like I was doing in the previous versions. I know you said the
HDMI audio is still an open issue but it seems that for this case it was accepted.

[1] http://lxr.free-electrons.com/source/drivers/gpu/drm/bridge/dw-hdmi-ahb-audio.c

Best regards,
Jose Miguel Abreu

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

* [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11 14:08                   ` Jose Abreu
  0 siblings, 0 replies; 66+ messages in thread
From: Jose Abreu @ 2016-04-11 14:08 UTC (permalink / raw)
  To: linux-snps-arc

Hi Lars,


On 11-04-2016 13:23, Lars-Peter Clausen wrote:
> On 04/11/2016 01:32 PM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 11-04-2016 10:33, Lars-Peter Clausen wrote:
>>> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>>>> Hi Lars,
>>>>
>>>>
>>>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>>>>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>>>>> [...]
>>>>>>> [...]
>>>>>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>>>>>>> +  into ALSA SoC.
>>>>>>> This is not a description of the hardware.
>>>>>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>>>>>> transmitter routes audio signals" ?
>>>>> I don't think we need this property. There is no problem with registering
>>>>> the audio part unconditionally. As long as there is no connection we wont
>>>>> create a sound card that is exposed to userspace.
>>>>>
>>>> This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
>>>> Laurent:
>>>> "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
>>>> with whether the system includes audio support. It would be confusing, and would
>>>> also waste resources, to create a Linux sound device when no sound channel is
>>>> routed on the board."
>>> I wouldn't care too much about this at this point, the extra amount of
>>> resources required for registering the CODEC (but not the sound card) is
>>> just a few bytes (sizeof(struct snd_soc_codec)).
>>>
>>> Nevertheless what we should do is describe the hardware and from this
>>> information infer whether there is a audio connection or not and if there is
>>> none we might skip registering the CODEC. In my opinion this hardware
>>> description should be modeled using of-graph, having a connection between
>>> the SoC side and the adv7511 SPDIF or I2S port.
>>>
>> You mean something like this:
>>
>> sound_playback: sound_playback {
>>     compatible = "simple-audio-card";
>>     [...]
>>     simple-audio-card,format = "i2s";
>>     [...]
>> }
>>
>> adv7511 at xx {
>>     compatible = "adi,adv7511";
>>     [...]
>>
>>     ports {
>>         [...]
>>         /* Audio Output */
>>         port at x {
>>             reg = <x>;
>>             endpoint {
>>                 remote-endpoint = <&sound_playback>;
>>             }
>>         }
>>     }
>> }
> Yes, something like that. Not exactly like that, but similar. One of the
> core concepts of of-graph is that there is always a description of the
> connection from both sides, this way each side can independently figure out
> where it is connected.
>
> Currently there is also zero support of of-graph in ASoC, so a bit of work
> is required to get this integrated properly.
>

I also believe this would be the better option but in the meantime can't I
integrate the audio like it is being done in the dw-hdmi driver[1]? In this
driver the audio is registered as a sound card and is conditionally built using
Kconfig, just like I was doing in the previous versions. I know you said the
HDMI audio is still an open issue but it seems that for this case it was accepted.

[1] http://lxr.free-electrons.com/source/drivers/gpu/drm/bridge/dw-hdmi-ahb-audio.c

Best regards,
Jose Miguel Abreu

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
  2016-04-11 14:08                   ` Jose Abreu
  (?)
@ 2016-04-11 19:34                     ` Lars-Peter Clausen
  -1 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-11 19:34 UTC (permalink / raw)
  To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: architt, airlied, Vineet.Gupta1, Alexey.Brodkin, lgirdwood,
	CARLOS.PALMINHA, broonie, tiwai

On 04/11/2016 04:08 PM, Jose Abreu wrote:
[...]
>> Currently there is also zero support of of-graph in ASoC, so a bit of work
>> is required to get this integrated properly.
>>
> 
> I also believe this would be the better option but in the meantime can't I
> integrate the audio like it is being done in the dw-hdmi driver[1]? In this
> driver the audio is registered as a sound card and is conditionally built using
> Kconfig, just like I was doing in the previous versions. I know you said the
> HDMI audio is still an open issue but it seems that for this case it was accepted.
> 
> [1] http://lxr.free-electrons.com/source/drivers/gpu/drm/bridge/dw-hdmi-ahb-audio.c

Maybe, but I'm not sure it would work in this case. Resources are probably
better invested in working towards a proper solution.

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

* Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11 19:34                     ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-11 19:34 UTC (permalink / raw)
  To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel
  Cc: Vineet.Gupta1, Alexey.Brodkin, CARLOS.PALMINHA, lgirdwood,
	broonie, tiwai

On 04/11/2016 04:08 PM, Jose Abreu wrote:
[...]
>> Currently there is also zero support of of-graph in ASoC, so a bit of work
>> is required to get this integrated properly.
>>
> 
> I also believe this would be the better option but in the meantime can't I
> integrate the audio like it is being done in the dw-hdmi driver[1]? In this
> driver the audio is registered as a sound card and is conditionally built using
> Kconfig, just like I was doing in the previous versions. I know you said the
> HDMI audio is still an open issue but it seems that for this case it was accepted.
> 
> [1] http://lxr.free-electrons.com/source/drivers/gpu/drm/bridge/dw-hdmi-ahb-audio.c

Maybe, but I'm not sure it would work in this case. Resources are probably
better invested in working towards a proper solution.

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

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

* [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
@ 2016-04-11 19:34                     ` Lars-Peter Clausen
  0 siblings, 0 replies; 66+ messages in thread
From: Lars-Peter Clausen @ 2016-04-11 19:34 UTC (permalink / raw)
  To: linux-snps-arc

On 04/11/2016 04:08 PM, Jose Abreu wrote:
[...]
>> Currently there is also zero support of of-graph in ASoC, so a bit of work
>> is required to get this integrated properly.
>>
> 
> I also believe this would be the better option but in the meantime can't I
> integrate the audio like it is being done in the dw-hdmi driver[1]? In this
> driver the audio is registered as a sound card and is conditionally built using
> Kconfig, just like I was doing in the previous versions. I know you said the
> HDMI audio is still an open issue but it seems that for this case it was accepted.
> 
> [1] http://lxr.free-electrons.com/source/drivers/gpu/drm/bridge/dw-hdmi-ahb-audio.c

Maybe, but I'm not sure it would work in this case. Resources are probably
better invested in working towards a proper solution.

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

end of thread, other threads:[~2016-04-11 19:34 UTC | newest]

Thread overview: 66+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-07 16:53 [PATCH 0/5 v4] Add I2S/ADV7511 audio support for ARC AXS10x boards Jose Abreu
2016-04-07 16:53 ` Jose Abreu
2016-04-07 16:53 ` Jose Abreu
2016-04-07 16:53 ` [PATCH 1/5 v4] drm/i2c/adv7511: Rename and move to separate folder Jose Abreu
2016-04-07 16:53   ` Jose Abreu
2016-04-07 16:53   ` Jose Abreu
2016-04-07 16:53 ` [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support Jose Abreu
2016-04-07 16:53   ` Jose Abreu
2016-04-08 15:46   ` [alsa-devel] " Lars-Peter Clausen
2016-04-08 15:46     ` Lars-Peter Clausen
2016-04-08 16:05     ` Jose Abreu
2016-04-08 16:12     ` Jose Abreu
2016-04-08 16:12       ` Jose Abreu
2016-04-08 16:12       ` Jose Abreu
2016-04-09 15:02       ` [alsa-devel] " Lars-Peter Clausen
2016-04-09 15:02         ` Lars-Peter Clausen
2016-04-09 15:02         ` Lars-Peter Clausen
2016-04-11  9:27         ` Jose Abreu
2016-04-11  9:27           ` Jose Abreu
2016-04-11  9:27           ` Jose Abreu
2016-04-11  9:33           ` Lars-Peter Clausen
2016-04-11  9:33             ` Lars-Peter Clausen
2016-04-11  9:33             ` Lars-Peter Clausen
2016-04-11 11:32             ` Jose Abreu
2016-04-11 11:32               ` Jose Abreu
2016-04-11 11:32               ` Jose Abreu
2016-04-11 12:23               ` [alsa-devel] " Lars-Peter Clausen
2016-04-11 12:23                 ` Lars-Peter Clausen
2016-04-11 12:23                 ` Lars-Peter Clausen
2016-04-11 14:08                 ` Jose Abreu
2016-04-11 14:08                   ` Jose Abreu
2016-04-11 14:08                   ` Jose Abreu
2016-04-11 19:34                   ` [alsa-devel] " Lars-Peter Clausen
2016-04-11 19:34                     ` Lars-Peter Clausen
2016-04-11 19:34                     ` Lars-Peter Clausen
2016-04-07 16:53 ` [PATCH 3/5 v4] ASoC: dwc: Use fifo depth to program FCR Jose Abreu
2016-04-07 16:53   ` Jose Abreu
2016-04-07 16:53   ` Jose Abreu
2016-04-07 17:57   ` Applied "ASoC: dwc: Use fifo depth to program FCR" to the asoc tree Mark Brown
2016-04-07 17:57     ` Mark Brown
2016-04-07 17:57     ` Mark Brown
2016-04-07 16:53 ` [PATCH 4/5 v4] ASoC: dwc: Add custom PCM driver Jose Abreu
2016-04-07 16:53   ` Jose Abreu
2016-04-07 17:54   ` kbuild test robot
2016-04-07 17:54     ` kbuild test robot
2016-04-07 17:54     ` kbuild test robot
2016-04-07 16:53 ` [PATCH 5/5 v4] ASoC: dwc: Update DOCUMENTATION for I2S Driver Jose Abreu
2016-04-07 16:53   ` Jose Abreu
2016-04-07 17:53   ` Mark Brown
2016-04-07 17:53     ` Mark Brown
2016-04-07 17:53     ` Mark Brown
2016-04-08 10:06     ` Jose Abreu
2016-04-08 10:06       ` Jose Abreu
2016-04-08 10:06       ` Jose Abreu
2016-04-08 15:52       ` [alsa-devel] " Lars-Peter Clausen
2016-04-08 15:52         ` Lars-Peter Clausen
2016-04-08 15:52         ` Lars-Peter Clausen
2016-04-08 16:08         ` Jose Abreu
2016-04-08 16:08           ` Jose Abreu
2016-04-08 16:08           ` Jose Abreu
2016-04-09 14:55           ` Lars-Peter Clausen
2016-04-09 14:55             ` Lars-Peter Clausen
2016-04-09 14:55             ` Lars-Peter Clausen
2016-04-11  9:24             ` Jose Abreu
2016-04-11  9:24               ` Jose Abreu
2016-04-11  9:24               ` Jose Abreu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.