[PATCH 1/2 v3] drm/bridge: adv7511: Add Audio support.

[Lars-Peter Clausen]

On 09/07/2016 01:22 AM, John Stultz wrote:
> This patch adds support to Audio for both adv7511 and adv7533
> bridge chips.
> 
> This patch was originally from [1] by Lars-Peter Clausen <lars at metafoo.de>
> and was adapted by Archit Taneja <architt at codeaurora.org> and
> Srinivas Kandagatla <srinivas.kandagatla at linaro.org>.
> 
> Then I heavily reworked it to use the hdmi-codec driver. And also
> folded in some audio packet initialization done by Andy Green
> <andy.green at linaro.org>. So credit to them, but blame to me.
> 
> [1] https://github.com/analogdevicesinc/linux/blob/xcomm_zynq/drivers/gpu/drm/i2c/adv7511_audio.c
> 
> Cc: David Airlie <airlied at linux.ie>
> Cc: Archit Taneja <architt at codeaurora.org>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Wolfram Sang <wsa+renesas at sang-engineering.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla at linaro.org>
> Cc: "Ville Syrjälä" <ville.syrjala at linux.intel.com>
> Cc: Boris Brezillon <boris.brezillon at free-electrons.com>
> Cc: Andy Green <andy at warmcat.com>
> Cc: Dave Long <dave.long at linaro.org>
> Cc: Guodong Xu <guodong.xu at linaro.org>
> Cc: Zhangfei Gao <zhangfei.gao at linaro.org>
> Cc: Mark Brown <broonie at kernel.org>
> Cc: Lars-Peter Clausen <lars at metafoo.de>
> Cc: Jose Abreu <joabreu at synopsys.com>
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz at linaro.org>

I'm not to fond of the hdmi-codec stuff, but within its context this looks
ok. Thanks for getting this upstream ready. Just one tiny bit regarding the
Kconfig entry.

If that is fixed feel free to add on the next revision:

Acked-by: Lars-Peter Clausen <lars at metafoo.de>

> ---
> v3:
> * Allowed audio support to be configured in or out, as suggested by Laurent
> * Minor cleanups suggested by Laurent
> * Folded in Andy's audio packet initialization patch, as suggested by Archit
> 
>  drivers/gpu/drm/bridge/adv7511/Kconfig         |   9 ++
>  drivers/gpu/drm/bridge/adv7511/Makefile        |   1 +
>  drivers/gpu/drm/bridge/adv7511/adv7511.h       |  16 ++
>  drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 213 +++++++++++++++++++++++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   4 +
>  5 files changed, 243 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig
> index d2b0499..b303ad1 100644
> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig
> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
> @@ -3,9 +3,18 @@ config DRM_I2C_ADV7511
>  	depends on OF
>  	select DRM_KMS_HELPER
>  	select REGMAP_I2C
> +	select SND_SOC_HDMI_CODEC if SND_SOC

This seems to be redundant considering the select done by DRM_I2C_ADV7511_AUDIO.

>  	help
>  	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
>  
> +config DRM_I2C_ADV7511_AUDIO
> +	tristate "ADV7511 HDMI Audio driver"

This should be bool. It either gets built into the ADV7511 driver (which
itself could either be a module or built-in) or not. But setting this option
itself to module wont work.

> +	depends on DRM_I2C_ADV7511 && SND_SOC
> +	select SND_SOC_HDMI_CODEC
> +	help
> +	  Support the ADV7511 HDMI Audio interface. This is used in
> +	  conjunction with the AV7511  HDMI driver.
> +

[...]
> +static void audio_shutdown(struct device *dev, void *data)
> +{
> +}

Unrelated to this patch, but it looks like the shutdown callback should
maybe be made optional.

> +static const struct hdmi_codec_ops adv7511_codec_ops = {
> +	.hw_params	= adv7511_hdmi_hw_params,
> +	.audio_shutdown = audio_shutdown,
> +	.audio_startup	= audio_startup,
> +};