Re: [RFC PATCH 4/4] alsa/soc: add hdmi audio codec based on cdf

2013-01-13 Thread Rahul Sharma
Thanks Sachin,

On Mon, Jan 14, 2013 at 11:13 AM, Sachin Kamat sachin.ka...@linaro.org wrote:
 +CC: ALSA mailing list, Mark Brown

 On 13 January 2013 18:22, Rahul Sharma rahul.sha...@samsung.com wrote:
 This patch registers hdmi-audio codec to the ALSA framework. This is the 
 second
 client to the hdmi panel. Once notified by the CDF Core it proceeds towards
 audio setting and audio control. It also subscribes for hpd notification to
 implement hpd related audio requirements.

 Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
 ---
  sound/soc/codecs/Kconfig |   4 +
  sound/soc/codecs/Makefile|   2 +
  sound/soc/codecs/exynos_hdmi_audio.c | 307 
 +++
  3 files changed, 313 insertions(+)
  create mode 100644 sound/soc/codecs/exynos_hdmi_audio.c

 diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
 index b92759a..93f3f6b 100644
 --- a/sound/soc/codecs/Kconfig
 +++ b/sound/soc/codecs/Kconfig
 @@ -496,3 +496,7 @@ config SND_SOC_ML26124

  config SND_SOC_TPA6130A2
 tristate
 +
 +config SND_SOC_EXYNOS_HDMI_AUDIO
 +   tristate
 +   default y

 Do you want to enable this by default? Shouldn't this be depending on
 HDMI support?


Yes, it should depend on Exynos HDMI support. I will add it in v2.

 diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
 index 9bd4d95..bfe93e6 100644
 --- a/sound/soc/codecs/Makefile
 +++ b/sound/soc/codecs/Makefile
 @@ -112,6 +112,7 @@ snd-soc-wm9705-objs := wm9705.o
  snd-soc-wm9712-objs := wm9712.o
  snd-soc-wm9713-objs := wm9713.o
  snd-soc-wm-hubs-objs := wm_hubs.o
 +snd-soc-exynos-hdmi-audio-objs := exynos_hdmi_audio.o

  # Amp
  snd-soc-max9877-objs := max9877.o
 @@ -230,6 +231,7 @@ obj-$(CONFIG_SND_SOC_WM9705)+= snd-soc-wm9705.o
  obj-$(CONFIG_SND_SOC_WM9712)   += snd-soc-wm9712.o
  obj-$(CONFIG_SND_SOC_WM9713)   += snd-soc-wm9713.o
  obj-$(CONFIG_SND_SOC_WM_HUBS)  += snd-soc-wm-hubs.o
 +obj-$(CONFIG_SND_SOC_EXYNOS_HDMI_AUDIO)+= 
 snd-soc-exynos-hdmi-audio.o

  # Amp
  obj-$(CONFIG_SND_SOC_MAX9877)  += snd-soc-max9877.o
 diff --git a/sound/soc/codecs/exynos_hdmi_audio.c 
 b/sound/soc/codecs/exynos_hdmi_audio.c
 new file mode 100644
 index 000..50e8564
 --- /dev/null
 +++ b/sound/soc/codecs/exynos_hdmi_audio.c
 @@ -0,0 +1,307 @@
 +/*
 + * ALSA SoC codec driver for HDMI audio on Samsung Exynos processors.
 + * Copyright (C) 2012 Samsung corp.

 Copyright (c) 2012 (-13?) Samsung Electronics Co., Ltd.


I will correct this.


 + * Author: Rahul Sharma rahul.sha...@samsung.com
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License
 + * version 2 as published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful, but
 + * WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * General Public License for more details.
 + *
 + */
 +#include linux/module.h
 +#include linux/delay.h
 +#include sound/soc.h
 +#include video/display.h
 +#include video/exynos_hdmi.h
 +#include sound/pcm.h
 +#include sound/pcm_params.h
 +#include linux/of_platform.h
 +#include linux/platform_device.h
 +
 +#undef dev_info
 +
 +#define dev_info(dev, format, arg...)  \
 +   dev_printk(KERN_CRIT, dev, format, ##arg)

 You may directly use dev_crit instead of dev_printk.


I will clean this in v2.

 +
 +static struct snd_soc_codec_driver hdmi_codec;
 +
 +/* platform device pointer for eynos hdmi audio device. */
 +static struct platform_device *exynos_hdmi_audio_pdev;
 +
 +struct hdmi_audio_context {
 +   struct platform_device  *pdev;
 +   atomic_tplugged;
 +   struct display_entity_audio_params  audio_params;
 +   struct display_entity   *entity;
 +   struct display_entity_notifier  notf;
 +   struct display_event_subscriber subscriber;
 +};
 +
 +static int exynos_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
 +   struct snd_pcm_hw_params *params,
 +   struct snd_soc_dai *dai)
 +{
 +   struct snd_soc_codec *codec = dai-codec;
 +   struct hdmi_audio_context *ctx = snd_soc_codec_get_drvdata(codec);
 +   int ret;
 +
 +   dev_info(codec-dev, [%d] %s\n, __LINE__, __func__);

 How about making this a debug message as it does not convey anything useful?
ok.

 +
 +   ctx-audio_params.type = DISPLAY_ENTITY_AUDIO_I2S;
 +
 +   switch (params_channels(params)) {
 +   case 6:
 +   case 4:
 +   case 2:
 +   case 1:
 +   ctx-audio_params.channels = params_channels(params);
 +   break;
 +   default:
 +   dev_err(codec-dev, %d channels not supported\n,
 +   params_channels(params));
 +   return -EINVAL;
 +   }
 +
 +   switch (params_format(params)) {

Re: [RFC PATCH 4/4] alsa/soc: add hdmi audio codec based on cdf

2013-01-13 Thread Sachin Kamat
+CC: ALSA mailing list, Mark Brown

On 13 January 2013 18:22, Rahul Sharma rahul.sha...@samsung.com wrote:
 This patch registers hdmi-audio codec to the ALSA framework. This is the 
 second
 client to the hdmi panel. Once notified by the CDF Core it proceeds towards
 audio setting and audio control. It also subscribes for hpd notification to
 implement hpd related audio requirements.

 Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
 ---
  sound/soc/codecs/Kconfig |   4 +
  sound/soc/codecs/Makefile|   2 +
  sound/soc/codecs/exynos_hdmi_audio.c | 307 
 +++
  3 files changed, 313 insertions(+)
  create mode 100644 sound/soc/codecs/exynos_hdmi_audio.c

 diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
 index b92759a..93f3f6b 100644
 --- a/sound/soc/codecs/Kconfig
 +++ b/sound/soc/codecs/Kconfig
 @@ -496,3 +496,7 @@ config SND_SOC_ML26124

  config SND_SOC_TPA6130A2
 tristate
 +
 +config SND_SOC_EXYNOS_HDMI_AUDIO
 +   tristate
 +   default y

Do you want to enable this by default? Shouldn't this be depending on
HDMI support?

 diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
 index 9bd4d95..bfe93e6 100644
 --- a/sound/soc/codecs/Makefile
 +++ b/sound/soc/codecs/Makefile
 @@ -112,6 +112,7 @@ snd-soc-wm9705-objs := wm9705.o
  snd-soc-wm9712-objs := wm9712.o
  snd-soc-wm9713-objs := wm9713.o
  snd-soc-wm-hubs-objs := wm_hubs.o
 +snd-soc-exynos-hdmi-audio-objs := exynos_hdmi_audio.o

  # Amp
  snd-soc-max9877-objs := max9877.o
 @@ -230,6 +231,7 @@ obj-$(CONFIG_SND_SOC_WM9705)+= snd-soc-wm9705.o
  obj-$(CONFIG_SND_SOC_WM9712)   += snd-soc-wm9712.o
  obj-$(CONFIG_SND_SOC_WM9713)   += snd-soc-wm9713.o
  obj-$(CONFIG_SND_SOC_WM_HUBS)  += snd-soc-wm-hubs.o
 +obj-$(CONFIG_SND_SOC_EXYNOS_HDMI_AUDIO)+= snd-soc-exynos-hdmi-audio.o

  # Amp
  obj-$(CONFIG_SND_SOC_MAX9877)  += snd-soc-max9877.o
 diff --git a/sound/soc/codecs/exynos_hdmi_audio.c 
 b/sound/soc/codecs/exynos_hdmi_audio.c
 new file mode 100644
 index 000..50e8564
 --- /dev/null
 +++ b/sound/soc/codecs/exynos_hdmi_audio.c
 @@ -0,0 +1,307 @@
 +/*
 + * ALSA SoC codec driver for HDMI audio on Samsung Exynos processors.
 + * Copyright (C) 2012 Samsung corp.

Copyright (c) 2012 (-13?) Samsung Electronics Co., Ltd.


 + * Author: Rahul Sharma rahul.sha...@samsung.com
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License
 + * version 2 as published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful, but
 + * WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * General Public License for more details.
 + *
 + */
 +#include linux/module.h
 +#include linux/delay.h
 +#include sound/soc.h
 +#include video/display.h
 +#include video/exynos_hdmi.h
 +#include sound/pcm.h
 +#include sound/pcm_params.h
 +#include linux/of_platform.h
 +#include linux/platform_device.h
 +
 +#undef dev_info
 +
 +#define dev_info(dev, format, arg...)  \
 +   dev_printk(KERN_CRIT, dev, format, ##arg)

You may directly use dev_crit instead of dev_printk.

 +
 +static struct snd_soc_codec_driver hdmi_codec;
 +
 +/* platform device pointer for eynos hdmi audio device. */
 +static struct platform_device *exynos_hdmi_audio_pdev;
 +
 +struct hdmi_audio_context {
 +   struct platform_device  *pdev;
 +   atomic_tplugged;
 +   struct display_entity_audio_params  audio_params;
 +   struct display_entity   *entity;
 +   struct display_entity_notifier  notf;
 +   struct display_event_subscriber subscriber;
 +};
 +
 +static int exynos_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
 +   struct snd_pcm_hw_params *params,
 +   struct snd_soc_dai *dai)
 +{
 +   struct snd_soc_codec *codec = dai-codec;
 +   struct hdmi_audio_context *ctx = snd_soc_codec_get_drvdata(codec);
 +   int ret;
 +
 +   dev_info(codec-dev, [%d] %s\n, __LINE__, __func__);

How about making this a debug message as it does not convey anything useful?

 +
 +   ctx-audio_params.type = DISPLAY_ENTITY_AUDIO_I2S;
 +
 +   switch (params_channels(params)) {
 +   case 6:
 +   case 4:
 +   case 2:
 +   case 1:
 +   ctx-audio_params.channels = params_channels(params);
 +   break;
 +   default:
 +   dev_err(codec-dev, %d channels not supported\n,
 +   params_channels(params));
 +   return -EINVAL;
 +   }
 +
 +   switch (params_format(params)) {
 +   case SNDRV_PCM_FORMAT_S8:
 +   ctx-audio_params.bits_per_sample = 8;
 +   break;
 +   case SNDRV_PCM_FORMAT_S16_LE:
 +   ctx-audio_params.bits_per_sample = 12;
 +