Re: [PATCH 2/2 v2][resend] drm: bridge: add DesignWare HDMI I2S audio support

2016-09-16 Thread Mark Brown
On Fri, Sep 16, 2016 at 08:01:01AM +, Kuninori Morimoto wrote:
> 
> Hi Mark
> 
> Can I have feedback about this patch ?

These are DRM patches, I'd expect them to go via the DRM subsystem.

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, though there are some other maintainers who like them - if in
doubt look at how patches for the subsystem are normally handled.


signature.asc
Description: PGP signature


Re: [PATCH 2/2 v2][resend] drm: bridge: add DesignWare HDMI I2S audio support

2016-09-16 Thread Kuninori Morimoto

Hi Mark

Can I have feedback about this patch ?

> From: Kuninori Morimoto 
> 
> 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 
> Signed-off-by: Kuninori Morimoto 
> ---
> 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 000..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 
> + *
> + * 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 
> +
> +#include 
> +
> +#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(