Hi Mark

Can I have feedback about this patch ?

> From: Kuninori Morimoto <kuninori.morimoto...@renesas.com>
> 
> Current dw-hdmi is supporting sound via AHB bus, but it has
> I2S audio feature too. This patch adds I2S audio support to dw-hdmi.
> This HDMI I2S is supported by using ALSA SoC common HDMI encoder
> driver.
> 
> Tested-by: Jose Abreu <joab...@synopsys.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto...@renesas.com>
> ---
> v1 -> v2
> 
>  - tidyup return value of snd_dw_hdmi_probe()
> 
>  drivers/gpu/drm/bridge/Kconfig             |   8 ++
>  drivers/gpu/drm/bridge/Makefile            |   1 +
>  drivers/gpu/drm/bridge/dw-hdmi-audio.h     |   7 ++
>  drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c | 130 
> +++++++++++++++++++++++++++++
>  drivers/gpu/drm/bridge/dw-hdmi.c           |  22 ++++-
>  drivers/gpu/drm/bridge/dw-hdmi.h           |  21 +++++
>  6 files changed, 187 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 8f7423f..8e2a22d 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -32,6 +32,14 @@ config DRM_DW_HDMI_AHB_AUDIO
>         Designware HDMI block.  This is used in conjunction with
>         the i.MX6 HDMI driver.
>  
> +config DRM_DW_HDMI_I2S_AUDIO
> +     tristate "Synopsis Designware I2S Audio interface"
> +     depends on DRM_DW_HDMI
> +     select SND_SOC_HDMI_CODEC
> +     help
> +       Support the I2S Audio interface which is part of the Synopsis
> +       Designware HDMI block.
> +
>  config DRM_NXP_PTN3460
>       tristate "NXP PTN3460 DP/LVDS bridge"
>       depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 96b13b3..1af92ad 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -3,6 +3,7 @@ ccflags-y := -Iinclude/drm
>  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
>  obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
> +obj-$(CONFIG_DRM_DW_HDMI_I2S_AUDIO) += dw-hdmi-i2s-audio.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>  obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi-audio.h 
> b/drivers/gpu/drm/bridge/dw-hdmi-audio.h
> index 91f631b..fd1f745 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi-audio.h
> +++ b/drivers/gpu/drm/bridge/dw-hdmi-audio.h
> @@ -11,4 +11,11 @@ struct dw_hdmi_audio_data {
>       u8 *eld;
>  };
>  
> +struct dw_hdmi_i2s_audio_data {
> +     struct dw_hdmi *hdmi;
> +
> +     void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
> +     u8 (*read)(struct dw_hdmi *hdmi, int offset);
> +};
> +
>  #endif
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c 
> b/drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c
> new file mode 100644
> index 0000000..7dd2091
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c
> @@ -0,0 +1,130 @@
> +/*
> + * dw-hdmi-i2s-audio.c
> + *
> + * Copyright (c) 2016 Kuninori Morimoto <kuninori.morimoto...@renesas.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.
> + */
> +#include <drm/bridge/dw_hdmi.h>
> +
> +#include <sound/hdmi-codec.h>
> +
> +#include "dw-hdmi.h"
> +#include "dw-hdmi-audio.h"
> +
> +#define DRIVER_NAME "dw-hdmi-i2s-audio"
> +
> +static inline void hdmi_write(struct dw_hdmi_i2s_audio_data *audio,
> +                           u8 val, int offset)
> +{
> +     struct dw_hdmi *hdmi = audio->hdmi;
> +
> +     audio->write(hdmi, val, offset);
> +}
> +
> +static inline u8 hdmi_read(struct dw_hdmi_i2s_audio_data *audio, int offset)
> +{
> +     struct dw_hdmi *hdmi = audio->hdmi;
> +
> +     return audio->read(hdmi, offset);
> +}
> +
> +static int dw_hdmi_i2s_hw_params(struct device *dev, void *data,
> +                              struct hdmi_codec_daifmt *fmt,
> +                              struct hdmi_codec_params *hparms)
> +{
> +     struct dw_hdmi_i2s_audio_data *audio = data;
> +     struct dw_hdmi *hdmi = audio->hdmi;
> +     u8 conf0 = 0;
> +     u8 conf1 = 0;
> +     u8 inputclkfs = 0;
> +
> +     /* it cares I2S only */
> +     if ((fmt->fmt != HDMI_I2S) ||
> +         (fmt->bit_clk_master | fmt->frame_clk_master)) {
> +             dev_err(dev, "unsupported format/settings\n");
> +             return -EINVAL;
> +     }
> +
> +     inputclkfs      = HDMI_AUD_INPUTCLKFS_64FS;
> +     conf0           = HDMI_AUD_CONF0_I2S_ALL_ENABLE;
> +
> +     switch (hparms->sample_width) {
> +     case 16:
> +             conf1 = HDMI_AUD_CONF1_WIDTH_16;
> +             break;
> +     case 24:
> +     case 32:
> +             conf1 = HDMI_AUD_CONF1_WIDTH_24;
> +             break;
> +     }
> +
> +     dw_hdmi_set_sample_rate(hdmi, hparms->sample_rate);
> +
> +     hdmi_write(audio, inputclkfs, HDMI_AUD_INPUTCLKFS);
> +     hdmi_write(audio, conf0, HDMI_AUD_CONF0);
> +     hdmi_write(audio, conf1, HDMI_AUD_CONF1);
> +
> +     dw_hdmi_audio_enable(hdmi);
> +
> +     return 0;
> +}
> +
> +static void dw_hdmi_i2s_audio_shutdown(struct device *dev, void *data)
> +{
> +     struct dw_hdmi_i2s_audio_data *audio = data;
> +     struct dw_hdmi *hdmi = audio->hdmi;
> +
> +     dw_hdmi_audio_disable(hdmi);
> +
> +     hdmi_write(audio, HDMI_AUD_CONF0_SW_RESET, HDMI_AUD_CONF0);
> +}
> +
> +static struct hdmi_codec_ops dw_hdmi_i2s_ops = {
> +     .hw_params      = dw_hdmi_i2s_hw_params,
> +     .audio_shutdown = dw_hdmi_i2s_audio_shutdown,
> +};
> +
> +static int snd_dw_hdmi_probe(struct platform_device *pdev)
> +{
> +     struct dw_hdmi_i2s_audio_data *audio = pdev->dev.platform_data;
> +     struct platform_device_info pdevinfo;
> +     struct hdmi_codec_pdata pdata;
> +     struct platform_device *platform;
> +
> +
> +     pdata.ops               = &dw_hdmi_i2s_ops;
> +     pdata.i2s               = 1;
> +     pdata.max_i2s_channels  = 6;
> +     pdata.data              = audio;
> +
> +     memset(&pdevinfo, 0, sizeof(pdevinfo));
> +     pdevinfo.parent         = pdev->dev.parent;
> +     pdevinfo.id             = PLATFORM_DEVID_AUTO;
> +     pdevinfo.name           = HDMI_CODEC_DRV_NAME;
> +     pdevinfo.data           = &pdata;
> +     pdevinfo.size_data      = sizeof(pdata);
> +     pdevinfo.dma_mask       = DMA_BIT_MASK(32);
> +
> +     platform = platform_device_register_full(&pdevinfo);
> +     if (IS_ERR_OR_NULL(platform))
> +             return PTR_ERR(platform);
> +
> +     return 0;
> +}
> +
> +static struct platform_driver snd_dw_hdmi_driver = {
> +     .probe  = snd_dw_hdmi_probe,
> +     .driver = {
> +             .name = DRIVER_NAME,
> +             .owner = THIS_MODULE,
> +     },
> +};
> +module_platform_driver(snd_dw_hdmi_driver);
> +
> +MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto...@renesas.com>");
> +MODULE_DESCRIPTION("Synopsis Designware HDMI I2S ALSA SoC interface");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 55e73e8..e9ba59e 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -2013,10 +2013,11 @@ int dw_hdmi_bind(struct device *dev, struct device 
> *master,
>       struct device_node *np = dev->of_node;
>       struct platform_device_info pdevinfo;
>       struct device_node *ddc_node;
> -     struct dw_hdmi_audio_data audio;
>       struct dw_hdmi *hdmi;
>       int ret;
>       u32 val = 1;
> +     u8 config0;
> +     u8 config1;
>  
>       hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
>       if (!hdmi)
> @@ -2185,7 +2186,12 @@ int dw_hdmi_bind(struct device *dev, struct device 
> *master,
>       pdevinfo.parent = dev;
>       pdevinfo.id = PLATFORM_DEVID_AUTO;
>  
> -     if (hdmi_readb(hdmi, HDMI_CONFIG1_ID) & HDMI_CONFIG1_AHB) {
> +     config0 = hdmi_readb(hdmi, HDMI_CONFIG0_ID);
> +     config1 = hdmi_readb(hdmi, HDMI_CONFIG1_ID);
> +
> +     if (config1 & HDMI_CONFIG1_AHB) {
> +             struct dw_hdmi_audio_data audio;
> +
>               audio.phys = iores->start;
>               audio.base = hdmi->regs;
>               audio.irq = irq;
> @@ -2197,6 +2203,18 @@ int dw_hdmi_bind(struct device *dev, struct device 
> *master,
>               pdevinfo.size_data = sizeof(audio);
>               pdevinfo.dma_mask = DMA_BIT_MASK(32);
>               hdmi->audio = platform_device_register_full(&pdevinfo);
> +     } else if (config0 & HDMI_CONFIG0_I2S) {
> +             struct dw_hdmi_i2s_audio_data audio;
> +
> +             audio.hdmi      = hdmi;
> +             audio.write     = hdmi_writeb;
> +             audio.read      = hdmi_readb;
> +
> +             pdevinfo.name = "dw-hdmi-i2s-audio";
> +             pdevinfo.data = &audio;
> +             pdevinfo.size_data = sizeof(audio);
> +             pdevinfo.dma_mask = DMA_BIT_MASK(32);
> +             hdmi->audio = platform_device_register_full(&pdevinfo);
>       }
>  
>       /* Unmute I2CM interrupts and reset HDMI DDC I2C master controller */
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.h 
> b/drivers/gpu/drm/bridge/dw-hdmi.h
> index fc9a560..c8bdbf2 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.h
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.h
> @@ -545,6 +545,10 @@
>  #define HDMI_I2CM_FS_SCL_LCNT_0_ADDR            0x7E12
>  
>  enum {
> +
> +/* CONFIG0_ID field values */
> +     HDMI_CONFIG0_I2S = 0x10,
> +
>  /* CONFIG1_ID field values */
>       HDMI_CONFIG1_AHB = 0x01,
>  
> @@ -887,6 +891,17 @@ enum {
>       HDMI_PHY_I2CM_CTLINT_ADDR_ARBITRATION_POL = 0x08,
>       HDMI_PHY_I2CM_CTLINT_ADDR_ARBITRATION_MASK = 0x04,
>  
> +/* AUD_CONF0 field values */
> +     HDMI_AUD_CONF0_SW_RESET = 0x80,
> +     HDMI_AUD_CONF0_I2S_ALL_ENABLE = 0x2F,
> +
> +/* AUD_CONF1 field values */
> +     HDMI_AUD_CONF1_MODE_I2S = 0x00,
> +     HDMI_AUD_CONF1_MODE_RIGHT_J = 0x02,
> +     HDMI_AUD_CONF1_MODE_LEFT_J = 0x04,
> +     HDMI_AUD_CONF1_WIDTH_16 = 0x10,
> +     HDMI_AUD_CONF1_WIDTH_24 = 0x18,
> +
>  /* AUD_CTS3 field values */
>       HDMI_AUD_CTS3_N_SHIFT_OFFSET = 5,
>       HDMI_AUD_CTS3_N_SHIFT_MASK = 0xe0,
> @@ -901,6 +916,12 @@ enum {
>       HDMI_AUD_CTS3_CTS_MANUAL = 0x10,
>       HDMI_AUD_CTS3_AUDCTS19_16_MASK = 0x0f,
>  
> +/* HDMI_AUD_INPUTCLKFS field values */
> +     HDMI_AUD_INPUTCLKFS_128FS = 0,
> +     HDMI_AUD_INPUTCLKFS_256FS = 1,
> +     HDMI_AUD_INPUTCLKFS_512FS = 2,
> +     HDMI_AUD_INPUTCLKFS_64FS = 4,
> +
>  /* AHB_DMA_CONF0 field values */
>       HDMI_AHB_DMA_CONF0_SW_FIFO_RST_OFFSET = 7,
>       HDMI_AHB_DMA_CONF0_SW_FIFO_RST_MASK = 0x80,
> -- 
> 1.9.1
> 


Best regards
---
Kuninori Morimoto

Reply via email to