[alsa-devel] [PATCH 0/4 v2] Add FSI - HDMI support V2
Dear Mark, Paul
These are FSI - HDMI prototype sound support V2
Kuninori Morimoto (4): fbdev: sh-mobile: Add HDMI sound type selection ASoC: Add sh_mobile_hdmi sound support ASoC: fsi-codec: Add FSI - HDMI support ARM: mach-shmobile: ap4evb: Add HDMI sound support
Paul
1st patch is depend on [PATCH] fbdev: sh-mobile_hdmi: remove un-necessity settings which I sent to LinuxSH ML in 2010/08/27
Mark
2nd, 3rd ASoC patches are based on Mark's "for-2.6.37"
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2 no change
drivers/video/sh_mobile_hdmi.c | 21 ++++++++++++++++++++- include/video/sh_mobile_hdmi.h | 16 ++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c index afebe80..d25e348 100644 --- a/drivers/video/sh_mobile_hdmi.c +++ b/drivers/video/sh_mobile_hdmi.c @@ -318,6 +318,9 @@ static void sh_hdmi_video_config(struct sh_hdmi *hdmi) */ static void sh_hdmi_audio_config(struct sh_hdmi *hdmi) { + u8 data; + struct sh_mobile_hdmi_info *pdata = hdmi->dev->platform_data; + /* * [7:4] L/R data swap control * [3:0] appropriate N[19:16] @@ -335,7 +338,23 @@ static void sh_hdmi_audio_config(struct sh_hdmi *hdmi) * [6:5] set required down sampling rate if required * [4:3] set required audio source */ - hdmi_write(hdmi, 0x00, HDMI_AUDIO_SETTING_1); + switch (pdata->flags & HDMI_SRC_MASK) { + default: + /* FALL THROUGH */ + case HDMI_SRC_I2S: + data = (0x0 << 3); + break; + case HDMI_SRC_SPDIF: + data = (0x1 << 3); + break; + case HDMI_SRC_DSD: + data = (0x2 << 3); + break; + case HDMI_SRC_HBR: + data = (0x3 << 3); + break; + } + hdmi_write(hdmi, data, HDMI_AUDIO_SETTING_1);
/* [3:0] set sending channel number for channel status */ hdmi_write(hdmi, 0x40, HDMI_AUDIO_SETTING_2); diff --git a/include/video/sh_mobile_hdmi.h b/include/video/sh_mobile_hdmi.h index 577cf18..929c2d3 100644 --- a/include/video/sh_mobile_hdmi.h +++ b/include/video/sh_mobile_hdmi.h @@ -14,9 +14,25 @@ struct sh_mobile_lcdc_chan_cfg; struct device;
+/* + * flags format + * + * 0x0000000A + * + * A: Audio source select + */ + +/* Audio source select */ +#define HDMI_SRC_MASK (0xF << 0) +#define HDMI_SRC_I2S (0 << 0) /* default */ +#define HDMI_SRC_SPDIF (1 << 0) +#define HDMI_SRC_DSD (2 << 0) +#define HDMI_SRC_HBR (3 << 0) + struct sh_mobile_hdmi_info { struct sh_mobile_lcdc_chan_cfg *lcd_chan; struct device *lcd_dev; + unsigned int flags; };
#endif
On Tue, Aug 31, 2010 at 02:46:41PM +0900, Kuninori Morimoto wrote:
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Applied, thanks.
Hi Morimoto-san
On Tue, 31 Aug 2010, Kuninori Morimoto wrote:
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
v1 -> v2 no change
drivers/video/sh_mobile_hdmi.c | 21 ++++++++++++++++++++- include/video/sh_mobile_hdmi.h | 16 ++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c index afebe80..d25e348 100644 --- a/drivers/video/sh_mobile_hdmi.c +++ b/drivers/video/sh_mobile_hdmi.c @@ -318,6 +318,9 @@ static void sh_hdmi_video_config(struct sh_hdmi *hdmi) */ static void sh_hdmi_audio_config(struct sh_hdmi *hdmi) {
- u8 data;
- struct sh_mobile_hdmi_info *pdata = hdmi->dev->platform_data;
- /*
- [7:4] L/R data swap control
- [3:0] appropriate N[19:16]
@@ -335,7 +338,23 @@ static void sh_hdmi_audio_config(struct sh_hdmi *hdmi) * [6:5] set required down sampling rate if required * [4:3] set required audio source */
- hdmi_write(hdmi, 0x00, HDMI_AUDIO_SETTING_1);
- switch (pdata->flags & HDMI_SRC_MASK) {
- default:
/* FALL THROUGH */
I'm not sure I like the capitalisation here - no reason to shout;)
- case HDMI_SRC_I2S:
data = (0x0 << 3);break;- case HDMI_SRC_SPDIF:
data = (0x1 << 3);break;- case HDMI_SRC_DSD:
data = (0x2 << 3);break;- case HDMI_SRC_HBR:
data = (0x3 << 3);
In all above cases parenthesis are superfluous.
break;}
hdmi_write(hdmi, data, HDMI_AUDIO_SETTING_1);
/* [3:0] set sending channel number for channel status */ hdmi_write(hdmi, 0x40, HDMI_AUDIO_SETTING_2);
diff --git a/include/video/sh_mobile_hdmi.h b/include/video/sh_mobile_hdmi.h index 577cf18..929c2d3 100644 --- a/include/video/sh_mobile_hdmi.h +++ b/include/video/sh_mobile_hdmi.h @@ -14,9 +14,25 @@ struct sh_mobile_lcdc_chan_cfg; struct device;
+/*
- flags format
- 0x0000000A
- A: Audio source select
- */
+/* Audio source select */ +#define HDMI_SRC_MASK (0xF << 0) +#define HDMI_SRC_I2S (0 << 0) /* default */ +#define HDMI_SRC_SPDIF (1 << 0) +#define HDMI_SRC_DSD (2 << 0) +#define HDMI_SRC_HBR (3 << 0)
I would be more specific with these macro names, i.e., include "AUDIO" or "SND" or something similar in them, e.g., HDMI_AUDIO_SRC_I2S.
struct sh_mobile_hdmi_info { struct sh_mobile_lcdc_chan_cfg *lcd_chan; struct device *lcd_dev;
- unsigned int flags;
};
#endif
1.7.0.4
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
Dear Guennadi
Thank you for checking patch !!
But ALSA side patches are already applied to Mark's tree. So, I will send bug fix patch to ALSA ML (add new label for snd_soc_unregister_codec etc...)
and send v2 patches to SH ML
EC No. W # public
Best regards -- Kuninori Morimoto
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
o based on latest Mark's "for-2.6.37"
sound/soc/sh/Kconfig | 7 +++++ sound/soc/sh/Makefile | 2 + sound/soc/sh/fsi-hdmi.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 0 deletions(-) create mode 100644 sound/soc/sh/fsi-hdmi.c
diff --git a/sound/soc/sh/Kconfig b/sound/soc/sh/Kconfig index 52d7e8e..6b224d2 100644 --- a/sound/soc/sh/Kconfig +++ b/sound/soc/sh/Kconfig @@ -62,6 +62,13 @@ config SND_FSI_DA7210 This option enables generic sound support for the FSI - DA7210 unit
+config SND_FSI_HDMI + bool "FSI-HDMI sound support" + depends on SND_SOC_SH4_FSI && FB_SH_MOBILE_HDMI + help + This option enables generic sound support for the + FSI - HDMI unit + config SND_SIU_MIGOR tristate "SIU sound support on Migo-R" depends on SH_MIGOR diff --git a/sound/soc/sh/Makefile b/sound/soc/sh/Makefile index 8a5a192..94476d4 100644 --- a/sound/soc/sh/Makefile +++ b/sound/soc/sh/Makefile @@ -16,9 +16,11 @@ obj-$(CONFIG_SND_SOC_SH4_SIU) += snd-soc-siu.o snd-soc-sh7760-ac97-objs := sh7760-ac97.o snd-soc-fsi-ak4642-objs := fsi-ak4642.o snd-soc-fsi-da7210-objs := fsi-da7210.o +snd-soc-fsi-hdmi-objs := fsi-hdmi.o snd-soc-migor-objs := migor.o
obj-$(CONFIG_SND_SH7760_AC97) += snd-soc-sh7760-ac97.o obj-$(CONFIG_SND_FSI_AK4642) += snd-soc-fsi-ak4642.o obj-$(CONFIG_SND_FSI_DA7210) += snd-soc-fsi-da7210.o +obj-$(CONFIG_SND_FSI_HDMI) += snd-soc-fsi-hdmi.o obj-$(CONFIG_SND_SIU_MIGOR) += snd-soc-migor.o diff --git a/sound/soc/sh/fsi-hdmi.c b/sound/soc/sh/fsi-hdmi.c new file mode 100644 index 0000000..950e3e0 --- /dev/null +++ b/sound/soc/sh/fsi-hdmi.c @@ -0,0 +1,61 @@ +/* + * FSI - HDMI sound support + * + * Copyright (C) 2010 Renesas Solutions Corp. + * Kuninori Morimoto kuninori.morimoto.gx@renesas.com + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + */ + +#include <linux/platform_device.h> +#include <sound/sh_fsi.h> +#include <video/sh_mobile_hdmi.h> + +static struct snd_soc_dai_link fsi_dai_link = { + .name = "HDMI", + .stream_name = "HDMI", + .cpu_dai_name = "fsib-dai", /* fsi B */ + .codec_dai_name = "sh_mobile_hdmi-hifi", + .platform_name = "sh_fsi2", + .codec_name = "sh-mobile-hdmi", +}; + +static struct snd_soc_card fsi_soc_card = { + .name = "FSI", + .dai_link = &fsi_dai_link, + .num_links = 1, +}; + +static struct platform_device *fsi_snd_device; + +static int __init fsi_hdmi_init(void) +{ + int ret = -ENOMEM; + + fsi_snd_device = platform_device_alloc("soc-audio", FSI_PORT_B); + if (!fsi_snd_device) + goto out; + + platform_set_drvdata(fsi_snd_device, &fsi_soc_card); + ret = platform_device_add(fsi_snd_device); + + if (ret) + platform_device_put(fsi_snd_device); + +out: + return ret; +} + +static void __exit fsi_hdmi_exit(void) +{ + platform_device_unregister(fsi_snd_device); +} + +module_init(fsi_hdmi_init); +module_exit(fsi_hdmi_exit); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("Generic SH4 FSI-HDMI sound card"); +MODULE_AUTHOR("Kuninori Morimoto kuninori.morimoto.gx@renesas.com");
On Tue, Aug 31, 2010 at 02:46:53PM +0900, Kuninori Morimoto wrote:
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Applied, thanks.
On Tue, 31 Aug 2010, Kuninori Morimoto wrote:
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
v1 -> v2
o based on latest Mark's "for-2.6.37"
sound/soc/sh/Kconfig | 7 +++++ sound/soc/sh/Makefile | 2 + sound/soc/sh/fsi-hdmi.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 0 deletions(-) create mode 100644 sound/soc/sh/fsi-hdmi.c
diff --git a/sound/soc/sh/Kconfig b/sound/soc/sh/Kconfig index 52d7e8e..6b224d2 100644 --- a/sound/soc/sh/Kconfig +++ b/sound/soc/sh/Kconfig @@ -62,6 +62,13 @@ config SND_FSI_DA7210 This option enables generic sound support for the FSI - DA7210 unit
+config SND_FSI_HDMI
- bool "FSI-HDMI sound support"
- depends on SND_SOC_SH4_FSI && FB_SH_MOBILE_HDMI
- help
This option enables generic sound support for theFSI - HDMI unit
"bool" means, if someone is linking the whole ASoC into the kernel, they will not be able to build this as a module. Not a big deal, but you're stealing some freedom from the user.
With this config option you will have 3 SND_SOC_SH4_FSI implementations in the Kconfig, all selectable independently. Do you really think it makes sense and would work, if someone were to select more than one of those options at the same time?
config SND_SIU_MIGOR tristate "SIU sound support on Migo-R" depends on SH_MIGOR diff --git a/sound/soc/sh/Makefile b/sound/soc/sh/Makefile index 8a5a192..94476d4 100644 --- a/sound/soc/sh/Makefile +++ b/sound/soc/sh/Makefile @@ -16,9 +16,11 @@ obj-$(CONFIG_SND_SOC_SH4_SIU) += snd-soc-siu.o snd-soc-sh7760-ac97-objs := sh7760-ac97.o snd-soc-fsi-ak4642-objs := fsi-ak4642.o snd-soc-fsi-da7210-objs := fsi-da7210.o +snd-soc-fsi-hdmi-objs := fsi-hdmi.o snd-soc-migor-objs := migor.o
obj-$(CONFIG_SND_SH7760_AC97) += snd-soc-sh7760-ac97.o obj-$(CONFIG_SND_FSI_AK4642) += snd-soc-fsi-ak4642.o obj-$(CONFIG_SND_FSI_DA7210) += snd-soc-fsi-da7210.o +obj-$(CONFIG_SND_FSI_HDMI) += snd-soc-fsi-hdmi.o obj-$(CONFIG_SND_SIU_MIGOR) += snd-soc-migor.o diff --git a/sound/soc/sh/fsi-hdmi.c b/sound/soc/sh/fsi-hdmi.c new file mode 100644 index 0000000..950e3e0 --- /dev/null +++ b/sound/soc/sh/fsi-hdmi.c @@ -0,0 +1,61 @@ +/*
- FSI - HDMI sound support
- Copyright (C) 2010 Renesas Solutions Corp.
- Kuninori Morimoto kuninori.morimoto.gx@renesas.com
- This file is subject to the terms and conditions of the GNU General Public
- License. See the file "COPYING" in the main directory of this archive
- for more details.
- */
+#include <linux/platform_device.h> +#include <sound/sh_fsi.h> +#include <video/sh_mobile_hdmi.h>
Now that everything is done with strings - do you still need these headers?
+static struct snd_soc_dai_link fsi_dai_link = {
- .name = "HDMI",
- .stream_name = "HDMI",
- .cpu_dai_name = "fsib-dai", /* fsi B */
- .codec_dai_name = "sh_mobile_hdmi-hifi",
- .platform_name = "sh_fsi2",
- .codec_name = "sh-mobile-hdmi",
+};
+static struct snd_soc_card fsi_soc_card = {
- .name = "FSI",
- .dai_link = &fsi_dai_link,
- .num_links = 1,
+};
+static struct platform_device *fsi_snd_device;
+static int __init fsi_hdmi_init(void) +{
- int ret = -ENOMEM;
- fsi_snd_device = platform_device_alloc("soc-audio", FSI_PORT_B);
- if (!fsi_snd_device)
goto out;- platform_set_drvdata(fsi_snd_device, &fsi_soc_card);
- ret = platform_device_add(fsi_snd_device);
- if (ret)
platform_device_put(fsi_snd_device);+out:
- return ret;
+}
+static void __exit fsi_hdmi_exit(void) +{
- platform_device_unregister(fsi_snd_device);
+}
+module_init(fsi_hdmi_init); +module_exit(fsi_hdmi_exit);
+MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("Generic SH4 FSI-HDMI sound card");
+MODULE_AUTHOR("Kuninori Morimoto kuninori.morimoto.gx@renesas.com");
1.7.0.4
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
It support only 48kHz frame rate
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2 no change
arch/arm/mach-shmobile/board-ap4evb.c | 56 ++++++++++++++++++++++++++++++++- 1 files changed, 55 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c index 23d472f..1545277 100644 --- a/arch/arm/mach-shmobile/board-ap4evb.c +++ b/arch/arm/mach-shmobile/board-ap4evb.c @@ -515,6 +515,8 @@ static struct platform_device *qhd_devices[] __initdata = { /* FSI */ #define IRQ_FSI evt2irq(0x1840) #define FSIACKCR 0xE6150018 +#define FSIDIV 0xFE1F8000 +#define FSIDIV_SIZE 32 static void fsiackcr_init(struct clk *clk) { u32 status = __raw_readl(clk->enable_reg); @@ -535,12 +537,58 @@ static struct clk fsiackcr_clk = { .rate = 0, /* unknown */ };
+static int fsi_set_rate(int is_porta, int rate) +{ + struct clk *clk; + void __iomem *base; + int ret = 0; + + /* set_rate is not needed if port A */ + if (is_porta) + return 0; + + clk = clk_get(NULL, "fsib_clk"); + if (IS_ERR(clk)) + return -EINVAL; + + base = ioremap_nocache(FSIDIV, FSIDIV_SIZE); + if (!base) { + ret = -ENXIO; + goto fsi_set_rate_err; + } + + switch (rate) { + case 48000: + clk_set_rate(clk, clk_round_rate(clk, 85428000)); + __raw_writel(0x00070003, base + 0x8); + ret = SH_FSI_ACKMD_256 | SH_FSI_BPFMD_64; + break; + default: + pr_err("unsupported rate in FSI2 port B\n"); + ret = -EINVAL; + break; + } + + iounmap(base); + +fsi_set_rate_err: + clk_put(clk); + + return ret; +} + static struct sh_fsi_platform_info fsi_info = { .porta_flags = SH_FSI_BRS_INV | SH_FSI_OUT_SLAVE_MODE | SH_FSI_IN_SLAVE_MODE | SH_FSI_OFMT(PCM) | SH_FSI_IFMT(PCM), + + .portb_flags = SH_FSI_BRS_INV | + SH_FSI_BRM_INV | + SH_FSI_LRS_INV | + SH_FSI_OFMT(SPDIF), + .set_rate = fsi_set_rate, };
static struct resource fsi_resources[] = { @@ -624,6 +672,7 @@ static struct platform_device lcdc1_device = { static struct sh_mobile_hdmi_info hdmi_info = { .lcd_chan = &sh_mobile_lcdc1_info.ch[0], .lcd_dev = &lcdc1_device.dev, + .flags = HDMI_SRC_SPDIF, };
static struct resource hdmi_resources[] = { @@ -825,6 +874,7 @@ static void __init ap4evb_map_io(void)
#define GPIO_PORT9CR 0xE6051009 #define GPIO_PORT10CR 0xE605100A +#define USCCR1 0xE6058144 static void __init ap4evb_init(void) { u32 srcr4; @@ -909,7 +959,7 @@ static void __init ap4evb_init(void) /* setup USB phy */ __raw_writew(0x8a0a, 0xE6058130); /* USBCR2 */
- /* enable FSI2 */ + /* enable FSI2 port A (ak4643) */ gpio_request(GPIO_FN_FSIAIBT, NULL); gpio_request(GPIO_FN_FSIAILR, NULL); gpio_request(GPIO_FN_FSIAISLD, NULL); @@ -922,6 +972,10 @@ static void __init ap4evb_init(void) gpio_no_direction(GPIO_PORT9CR); /* FSIAOBT needs no direction */ gpio_no_direction(GPIO_PORT10CR); /* FSIAOLR needs no direction */
+ /* setup FSI2 port B (HDMI) */ + gpio_request(GPIO_FN_FSIBCK, NULL); + __raw_writew(__raw_readw(USCCR1) & ~(1 << 6), USCCR1); /* use SPDIF */ + /* set SPU2 clock to 119.6 MHz */ clk = clk_get(NULL, "spu_clk"); if (!IS_ERR(clk)) {
On Tue, Aug 31, 2010 at 02:46:58PM +0900, Kuninori Morimoto wrote:
It support only 48kHz frame rate
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Acked-by: Mark Brown broonie@opensource.wolfsonmicro.com
I can apply this if required but it seems to make sense for it to go via the SH tree.
On Tue, 31 Aug 2010, Kuninori Morimoto wrote:
It support only 48kHz frame rate
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
v1 -> v2 no change
arch/arm/mach-shmobile/board-ap4evb.c | 56 ++++++++++++++++++++++++++++++++- 1 files changed, 55 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c index 23d472f..1545277 100644 --- a/arch/arm/mach-shmobile/board-ap4evb.c +++ b/arch/arm/mach-shmobile/board-ap4evb.c @@ -515,6 +515,8 @@ static struct platform_device *qhd_devices[] __initdata = { /* FSI */ #define IRQ_FSI evt2irq(0x1840) #define FSIACKCR 0xE6150018 +#define FSIDIV 0xFE1F8000 +#define FSIDIV_SIZE 32
32 bytes? There are only 2 registers 4 bytes each in FSIDIV?
static void fsiackcr_init(struct clk *clk) { u32 status = __raw_readl(clk->enable_reg); @@ -535,12 +537,58 @@ static struct clk fsiackcr_clk = { .rate = 0, /* unknown */ };
+static int fsi_set_rate(int is_porta, int rate) +{
- struct clk *clk;
- void __iomem *base;
- int ret = 0;
No need to initialise ret.
- /* set_rate is not needed if port A */
- if (is_porta)
return 0;- clk = clk_get(NULL, "fsib_clk");
- if (IS_ERR(clk))
return -EINVAL;- base = ioremap_nocache(FSIDIV, FSIDIV_SIZE);
- if (!base) {
ret = -ENXIO;goto fsi_set_rate_err;- }
- switch (rate) {
- case 48000:
clk_set_rate(clk, clk_round_rate(clk, 85428000));__raw_writel(0x00070003, base + 0x8);
Hm, these two registers seem to be a perfect candidate for the clock API?
ret = SH_FSI_ACKMD_256 | SH_FSI_BPFMD_64;break;- default:
pr_err("unsupported rate in FSI2 port B\n");ret = -EINVAL;break;- }
- iounmap(base);
+fsi_set_rate_err:
- clk_put(clk);
- return ret;
+}
static struct sh_fsi_platform_info fsi_info = { .porta_flags = SH_FSI_BRS_INV | SH_FSI_OUT_SLAVE_MODE | SH_FSI_IN_SLAVE_MODE | SH_FSI_OFMT(PCM) | SH_FSI_IFMT(PCM),
- .portb_flags = SH_FSI_BRS_INV |
SH_FSI_BRM_INV |SH_FSI_LRS_INV |SH_FSI_OFMT(SPDIF),- .set_rate = fsi_set_rate,
};
static struct resource fsi_resources[] = { @@ -624,6 +672,7 @@ static struct platform_device lcdc1_device = { static struct sh_mobile_hdmi_info hdmi_info = { .lcd_chan = &sh_mobile_lcdc1_info.ch[0], .lcd_dev = &lcdc1_device.dev,
- .flags = HDMI_SRC_SPDIF,
};
static struct resource hdmi_resources[] = { @@ -825,6 +874,7 @@ static void __init ap4evb_map_io(void)
#define GPIO_PORT9CR 0xE6051009 #define GPIO_PORT10CR 0xE605100A +#define USCCR1 0xE6058144 static void __init ap4evb_init(void) { u32 srcr4; @@ -909,7 +959,7 @@ static void __init ap4evb_init(void) /* setup USB phy */ __raw_writew(0x8a0a, 0xE6058130); /* USBCR2 */
- /* enable FSI2 */
- /* enable FSI2 port A (ak4643) */ gpio_request(GPIO_FN_FSIAIBT, NULL); gpio_request(GPIO_FN_FSIAILR, NULL); gpio_request(GPIO_FN_FSIAISLD, NULL);
@@ -922,6 +972,10 @@ static void __init ap4evb_init(void) gpio_no_direction(GPIO_PORT9CR); /* FSIAOBT needs no direction */ gpio_no_direction(GPIO_PORT10CR); /* FSIAOLR needs no direction */
- /* setup FSI2 port B (HDMI) */
- gpio_request(GPIO_FN_FSIBCK, NULL);
- __raw_writew(__raw_readw(USCCR1) & ~(1 << 6), USCCR1); /* use SPDIF */
- /* set SPU2 clock to 119.6 MHz */ clk = clk_get(NULL, "spu_clk"); if (!IS_ERR(clk)) {
-- 1.7.0.4
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
o based on Mark's latest "for-2.6.37"
drivers/video/sh_mobile_hdmi.c | 65 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c index d25e348..f14c58a 100644 --- a/drivers/video/sh_mobile_hdmi.c +++ b/drivers/video/sh_mobile_hdmi.c @@ -22,6 +22,8 @@ #include <linux/slab.h> #include <linux/types.h> #include <linux/workqueue.h> +#include <sound/soc-dapm.h> +#include <sound/initval.h>
#include <video/sh_mobile_hdmi.h> #include <video/sh_mobile_lcdc.h> @@ -222,6 +224,62 @@ static u8 hdmi_read(struct sh_hdmi *hdmi, u8 reg) return ioread8(hdmi->base + reg); }
+/************************************************************************ + + + HDMI sound + + +************************************************************************/ +static unsigned int sh_hdmi_snd_read(struct snd_soc_codec *codec, + unsigned int reg) +{ + struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec); + + return hdmi_read(hdmi, reg); +} + +static int sh_hdmi_snd_write(struct snd_soc_codec *codec, + unsigned int reg, + unsigned int value) +{ + struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec); + + hdmi_write(hdmi, value, reg); + return 0; +} + +static struct snd_soc_dai_driver sh_hdmi_dai = { + .name = "sh_mobile_hdmi-hifi", + .playback = { + .stream_name = "Playback", + .channels_min = 1, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_48000, + .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE, + }, +}; + +static int sh_hdmi_snd_probe(struct snd_soc_codec *codec) +{ + dev_info(codec->dev, "SH Mobile HDMI Audio Codec"); + + return 0; +} + +static struct snd_soc_codec_driver soc_codec_dev_sh_hdmi = { + .probe = sh_hdmi_snd_probe, + .read = sh_hdmi_snd_read, + .write = sh_hdmi_snd_write, +}; + +/************************************************************************ + + + HDMI video + + +************************************************************************/ /* External video parameter settings */ static void hdmi_external_video_param(struct sh_hdmi *hdmi) { @@ -804,6 +862,11 @@ static int __init sh_hdmi_probe(struct platform_device *pdev) return -ENOMEM; }
+ ret = snd_soc_register_codec(&pdev->dev, + &soc_codec_dev_sh_hdmi, &sh_hdmi_dai, 1); + if (ret < 0) + goto egetclk; + hdmi->dev = &pdev->dev;
hdmi->hdmi_clk = clk_get(&pdev->dev, "ick"); @@ -901,6 +964,8 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev) struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); int irq = platform_get_irq(pdev, 0);
+ snd_soc_unregister_codec(&pdev->dev); + pdata->lcd_chan->board_cfg.display_on = NULL; pdata->lcd_chan->board_cfg.display_off = NULL; pdata->lcd_chan->board_cfg.board_data = NULL;
On Tue, Aug 31, 2010 at 02:47:07PM +0900, Kuninori Morimoto wrote:
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Applied, thanks.
On Tue, 31 Aug 2010, Kuninori Morimoto wrote:
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
v1 -> v2
o based on Mark's latest "for-2.6.37"
drivers/video/sh_mobile_hdmi.c | 65 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c index d25e348..f14c58a 100644 --- a/drivers/video/sh_mobile_hdmi.c +++ b/drivers/video/sh_mobile_hdmi.c @@ -22,6 +22,8 @@ #include <linux/slab.h> #include <linux/types.h> #include <linux/workqueue.h> +#include <sound/soc-dapm.h> +#include <sound/initval.h>
#include <video/sh_mobile_hdmi.h> #include <video/sh_mobile_lcdc.h> @@ -222,6 +224,62 @@ static u8 hdmi_read(struct sh_hdmi *hdmi, u8 reg) return ioread8(hdmi->base + reg); }
+/************************************************************************
HDMI sound+************************************************************************/
I don't think this comment deserves 7 lines of text, besides breaking the multiline comment style. If you think, one line like
/* HDMI sound */
is not enough how about just
/* * HDMI sound */
?
+static unsigned int sh_hdmi_snd_read(struct snd_soc_codec *codec,
unsigned int reg)+{
- struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
- return hdmi_read(hdmi, reg);
+}
+static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
unsigned int reg,unsigned int value)+{
- struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
- hdmi_write(hdmi, value, reg);
- return 0;
+}
Are these two actually needed? As long as you don't have a register cache - no need for these?
+static struct snd_soc_dai_driver sh_hdmi_dai = {
- .name = "sh_mobile_hdmi-hifi",
- .playback = {
.stream_name = "Playback",.channels_min = 1,
Can it actually do mono? Maybe at probe time you could look at audio flags from your previous patch and, e.g., for SPDIF set channels_min to 2?
.channels_max = 2,
That's the "smallest max," yes. With some other interfaces (I2S, DSD) it can support up to 8 channels...
.rates = SNDRV_PCM_RATE_8000_48000,
Hm, in the datasheet I see supported frequencies 32kHz to 192kHz. And if you promise support for multiple frequencies, don't you want to implement .hw_params? Besides, not all of these frequencies will be available, depending on your supplied clock and your willingness to implement downsampling.
.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
Again, this I am not sure about. The datasheet says 16 to 32 bit are possible, but then I only see configuration for 16 to 24 bits, but in any case, I think, you'd want to implement .hw_params to support non-default formats.
- },
+};
+static int sh_hdmi_snd_probe(struct snd_soc_codec *codec) +{
- dev_info(codec->dev, "SH Mobile HDMI Audio Codec");
- return 0;
+}
+static struct snd_soc_codec_driver soc_codec_dev_sh_hdmi = {
- .probe = sh_hdmi_snd_probe,
- .read = sh_hdmi_snd_read,
- .write = sh_hdmi_snd_write,
+};
+/************************************************************************
HDMI video+************************************************************************/
See above - 7 lines seem to be an overkill to me.
/* External video parameter settings */ static void hdmi_external_video_param(struct sh_hdmi *hdmi) { @@ -804,6 +862,11 @@ static int __init sh_hdmi_probe(struct platform_device *pdev) return -ENOMEM; }
ret = snd_soc_register_codec(&pdev->dev,
&soc_codec_dev_sh_hdmi, &sh_hdmi_dai, 1);if (ret < 0)
goto egetclk;hdmi->dev = &pdev->dev;
hdmi->hdmi_clk = clk_get(&pdev->dev, "ick");
NAK. This breaks the error path and has to be fixed. Firstly, please, use a new label like "esndreg," secondly, you have to add
clk_disable(hdmi->hdmi_clk); erate: clk_put(hdmi->hdmi_clk); egetclk: + snd_soc_unregister_codec(&pdev->dev); +esndreg: mutex_destroy(&hdmi->mutex); kfree(hdmi);
to the error path.
Besides, I think, this will not link without CONFIG_SND_SOC.
@@ -901,6 +964,8 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev) struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); int irq = platform_get_irq(pdev, 0);
- snd_soc_unregister_codec(&pdev->dev);
- pdata->lcd_chan->board_cfg.display_on = NULL; pdata->lcd_chan->board_cfg.display_off = NULL; pdata->lcd_chan->board_cfg.board_data = NULL;
-- 1.7.0.4
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
On Mon, Sep 06, 2010 at 10:27:56AM +0200, Guennadi Liakhovetski wrote:
On Tue, 31 Aug 2010, Kuninori Morimoto wrote:
+static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
unsigned int reg,unsigned int value)+{
- struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
- hdmi_write(hdmi, value, reg);
- return 0;
+}
Are these two actually needed? As long as you don't have a register cache
- no need for these?
Something needs to translate the ASoC register I/O functions into what the HDMI layer code is expecting.
On Mon, 6 Sep 2010, Mark Brown wrote:
On Mon, Sep 06, 2010 at 10:27:56AM +0200, Guennadi Liakhovetski wrote:
On Tue, 31 Aug 2010, Kuninori Morimoto wrote:
+static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
unsigned int reg,unsigned int value)+{
- struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
- hdmi_write(hdmi, value, reg);
- return 0;
+}
Are these two actually needed? As long as you don't have a register cache
- no need for these?
Something needs to translate the ASoC register I/O functions into what the HDMI layer code is expecting.
AFAICS, with ->reg_cache_size = 0 the ASoC core will not attempt to call them.
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
On Tue, Sep 07, 2010 at 09:11:28AM +0200, Guennadi Liakhovetski wrote:
On Mon, 6 Sep 2010, Mark Brown wrote:
+static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
Are these two actually needed? As long as you don't have a register cache
- no need for these?
Something needs to translate the ASoC register I/O functions into what the HDMI layer code is expecting.
AFAICS, with ->reg_cache_size = 0 the ASoC core will not attempt to call them.
Could you please be more explicit here? Register I/O needs to happen somehow...
On Tue, 7 Sep 2010, Mark Brown wrote:
On Tue, Sep 07, 2010 at 09:11:28AM +0200, Guennadi Liakhovetski wrote:
On Mon, 6 Sep 2010, Mark Brown wrote:
+static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
Are these two actually needed? As long as you don't have a register cache
- no need for these?
Something needs to translate the ASoC register I/O functions into what the HDMI layer code is expecting.
AFAICS, with ->reg_cache_size = 0 the ASoC core will not attempt to call them.
Could you please be more explicit here? Register I/O needs to happen somehow...
Sorry, maybe I am missing something, but my understanding is, that the ASoC core knows nothing about codec's specific register layout, so, the core itself cannot initiate any register IO. So, I presume, there can be only two instances, that can do that - the codec driver itself and some user-space (debugging) programs. The driver doesn't use cached register accesses, so, it can access the registers directly, and it doesn't have to provide an ability to the user-space to access registers - if it chooses so. So, I don't see, who should be trying to use generic ASoC register access routines here.
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
On Tue, Sep 07, 2010 at 11:56:34AM +0200, Guennadi Liakhovetski wrote:
On Tue, 7 Sep 2010, Mark Brown wrote:
Could you please be more explicit here? Register I/O needs to happen somehow...
Sorry, maybe I am missing something, but my understanding is, that the ASoC core knows nothing about codec's specific register layout, so, the core itself cannot initiate any register IO. So, I presume, there can be only two instances, that can do that - the codec driver itself and some user-space (debugging) programs. The driver doesn't use cached register accesses, so, it can access the registers directly, and it doesn't have to provide an ability to the user-space to access registers - if it chooses so. So, I don't see, who should be trying to use generic ASoC register access routines here.
All register I/O, cached or not, is supposed to go through the register I/O operations. Any cache is abstraced away within those operations. Since the driver presumably needs do do some I/O (even if only in the hw_params() you have identified as missing) I would expect it to provide register I/O functionality.
Dear Guennadi
.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,Again, this I am not sure about. The datasheet says 16 to 32 bit are possible, but then I only see configuration for 16 to 24 bits, but in any case, I think, you'd want to implement .hw_params to support non-default formats.
Yes. .hw_params implementation is needed for advanced support. I will send its patch in future.
+config SND_FSI_HDMI
- bool "FSI-HDMI sound support"
- depends on SND_SOC_SH4_FSI && FB_SH_MOBILE_HDMI
- help
This option enables generic sound support for theFSI - HDMI unit
(snip)
With this config option you will have 3 SND_SOC_SH4_FSI implementations in the Kconfig, all selectable independently. Do you really think it makes sense and would work, if someone were to select more than one of those options at the same time?
Yes. I created it for independently. for example, you can select FSI-AK4642 and FSI-HDMI in same time. If you select FSI-DA7210 and FSI-AK4642, small patch which change FSIA <-> FSIB is needed for now.
- switch (rate) {
- case 48000:
clk_set_rate(clk, clk_round_rate(clk, 85428000));__raw_writel(0x00070003, base + 0x8);Hm, these two registers seem to be a perfect candidate for the clock API?
? I think so. why ?
Best regards -- Kuninori Morimoto
On Tue, 2010-08-31 at 14:45 +0900, Kuninori Morimoto wrote:
Dear Mark, Paul
These are FSI - HDMI prototype sound support V2
Kuninori Morimoto (4): fbdev: sh-mobile: Add HDMI sound type selection ASoC: Add sh_mobile_hdmi sound support ASoC: fsi-codec: Add FSI - HDMI support ARM: mach-shmobile: ap4evb: Add HDMI sound support
Paul
1st patch is depend on [PATCH] fbdev: sh-mobile_hdmi: remove un-necessity settings which I sent to LinuxSH ML in 2010/08/27
Mark
2nd, 3rd ASoC patches are based on Mark's "for-2.6.37"
ASoC parts:-
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
participants (4)
-
Guennadi Liakhovetski -
Kuninori Morimoto -
Liam Girdwood -
Mark Brown