Re: [PATCH RFC 4/7] drm: exynos: Add driver for HDMI audio interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Fri, Apr 21, 2017 at 07:19:48PM +0200, Sylwester Nawrocki wrote:
> The hdmi-codec interface added in this patch is required to properly
> support HDMI audio. Currently the audio part of the SoC internal
> HDMI transmitter is configured with fixed values, which makes HDMI
> audio working by chance, only on boards equipped with external audio
> codec connected in parallel with the HDMI audio transmitter I2S input
> interface.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> ---
>  drivers/gpu/drm/exynos/Kconfig       |   1 +
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 220 +++++++++++++++++++++++++++++------
>  2 files changed, 188 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
> index 1d18534..a6edbb6 100644
> --- a/drivers/gpu/drm/exynos/Kconfig
> +++ b/drivers/gpu/drm/exynos/Kconfig
> @@ -3,6 +3,7 @@ config DRM_EXYNOS
>  	depends on OF && DRM && (ARCH_S3C64XX || ARCH_EXYNOS || ARCH_MULTIPLATFORM)
>  	select DRM_KMS_HELPER
>  	select VIDEOMODE_HELPERS
> +	select SND_SOC_HDMI_CODEC if SND_SOC
>  	help
>  	  Choose this option if you have a Samsung SoC EXYNOS chipset.
>  	  If M is selected the module will be called exynosdrm.
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 88ccc04..be18023 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -40,7 +40,7 @@
>  #include <linux/component.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
> -
> +#include <sound/hdmi-codec.h>
>  #include <drm/exynos_drm.h>
>  
>  #include "exynos_drm_drv.h"
> @@ -110,13 +110,23 @@ struct hdmi_driver_data {
>  	struct string_array_spec clk_muxes;
>  };
>  
> +struct hdmi_audio {
> +	struct platform_device *pdev;
> +	struct hdmi_audio_infoframe infoframe;
> +	unsigned int sample_rate;
> +	unsigned int sample_width;
> +	u8 enable;
> +};
> +
>  struct hdmi_context {
>  	struct drm_encoder		encoder;
>  	struct device			*dev;
>  	struct drm_device		*drm_dev;
>  	struct drm_connector		connector;
> +	struct hdmi_audio		audio;
>  	bool				powered;
>  	bool				dvi_mode;
> +	struct mutex			mutex;

I find short documentation what is protected by mutex usually quite
useful. Can you add such?

>  	struct delayed_work		hotplug_work;
>  	struct drm_display_mode		current_mode;
>  	const struct hdmi_driver_data	*drv_data;
> @@ -766,6 +776,22 @@ static int hdmi_clk_set_parents(struct hdmi_context *hdata, bool to_phy)
>  	return ret;
>  }
>  
> +static int hdmi_audio_infoframe_apply(struct hdmi_context *hdata)
> +{
> +	struct hdmi_audio_infoframe *infoframe = &hdata->audio.infoframe;
> +	u8 buf[HDMI_INFOFRAME_SIZE(AUDIO)];
> +	int len;
> +
> +	len = hdmi_audio_infoframe_pack(infoframe, buf, sizeof(buf));
> +	if (len < 0)
> +		return len;
> +
> +	hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_EVERY_VSYNC);
> +	hdmi_reg_write_buf(hdata, HDMI_AUI_HEADER0, buf, len);
> +
> +	return 0;
> +}
> +
>  static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>  {
>  	union hdmi_infoframe frm;
> @@ -803,15 +829,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>  		hdmi_reg_write_buf(hdata, HDMI_VSI_DATA(0), buf + 3, ret - 3);
>  	}
>  
> -	ret = hdmi_audio_infoframe_init(&frm.audio);
> -	if (!ret) {
> -		frm.audio.channels = 2;
> -		ret = hdmi_audio_infoframe_pack(&frm.audio, buf, sizeof(buf));
> -	}
> -	if (ret > 0) {
> -		hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_EVERY_VSYNC);
> -		hdmi_reg_write_buf(hdata, HDMI_AUI_HEADER0, buf, ret);
> -	}
> +	hdmi_audio_infoframe_apply(hdata);
>  }
>  
>  static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
> @@ -993,23 +1011,18 @@ static void hdmi_reg_acr(struct hdmi_context *hdata, u32 freq)
>  	hdmi_reg_writeb(hdata, HDMI_ACR_CON, 4);
>  }
>  
> -static void hdmi_audio_init(struct hdmi_context *hdata)
> +static void hdmi_audio_config(struct hdmi_context *hdata)
>  {
> -	u32 sample_rate, bits_per_sample;
> -	u32 data_num, bit_ch, sample_frq;
> -	u32 val;
> +	u32 data_num, sample_freq, val;
> +	u32 bit_ch = 1;
>  
> -	sample_rate = 44100;
> -	bits_per_sample = 16;
>  
> -	switch (bits_per_sample) {
> +	switch (hdata->audio.sample_width) {
>  	case 20:
>  		data_num = 2;
> -		bit_ch = 1;
>  		break;
>  	case 24:
>  		data_num = 3;
> -		bit_ch = 1;
>  		break;
>  	default:
>  		data_num = 1;
> @@ -1017,7 +1030,7 @@ static void hdmi_audio_init(struct hdmi_context *hdata)
>  		break;
>  	}
>  
> -	hdmi_reg_acr(hdata, sample_rate);
> +	hdmi_reg_acr(hdata, hdata->audio.sample_rate);
>  
>  	hdmi_reg_writeb(hdata, HDMI_I2S_MUX_CON, HDMI_I2S_IN_DISABLE
>  				| HDMI_I2S_AUD_I2S | HDMI_I2S_CUV_I2S_ENABLE
> @@ -1028,10 +1041,21 @@ static void hdmi_audio_init(struct hdmi_context *hdata)
>  
>  	hdmi_reg_writeb(hdata, HDMI_I2S_MUX_CUV, HDMI_I2S_CUV_RL_EN);
>  
> -	sample_frq = (sample_rate == 44100) ? 0 :
> -			(sample_rate == 48000) ? 2 :
> -			(sample_rate == 32000) ? 3 :
> -			(sample_rate == 96000) ? 0xa : 0x0;
> +	switch(hdata->audio.sample_rate) {
> +	case 32000:
> +		sample_freq = 0x3;
> +		break;
> +	case 48000:
> +		sample_freq = 0x2;
> +		break;
> +	case 96000:
> +		sample_freq = 0xa;
> +		break;
> +	case 44100:
> +	default:
> +		sample_freq = 0;
> +		break;
> +	}
>  
>  	hdmi_reg_writeb(hdata, HDMI_I2S_CLK_CON, HDMI_I2S_CLK_DIS);
>  	hdmi_reg_writeb(hdata, HDMI_I2S_CLK_CON, HDMI_I2S_CLK_EN);
> @@ -1065,7 +1089,7 @@ static void hdmi_audio_init(struct hdmi_context *hdata)
>  	hdmi_reg_writeb(hdata, HDMI_I2S_CH_ST_1, HDMI_I2S_CD_PLAYER);
>  	hdmi_reg_writeb(hdata, HDMI_I2S_CH_ST_2, HDMI_I2S_SET_SOURCE_NUM(0));
>  	hdmi_reg_writeb(hdata, HDMI_I2S_CH_ST_3, HDMI_I2S_CLK_ACCUR_LEVEL_2
> -			| HDMI_I2S_SET_SMP_FREQ(sample_frq));
> +			| HDMI_I2S_SET_SMP_FREQ(sample_freq));
>  	hdmi_reg_writeb(hdata, HDMI_I2S_CH_ST_4,
>  			HDMI_I2S_ORG_SMP_FREQ_44_1
>  			| HDMI_I2S_WORD_LEN_MAX24_24BITS
> @@ -1074,13 +1098,15 @@ static void hdmi_audio_init(struct hdmi_context *hdata)
>  	hdmi_reg_writeb(hdata, HDMI_I2S_CH_ST_CON, HDMI_I2S_CH_STATUS_RELOAD);
>  }
>  
> -static void hdmi_audio_control(struct hdmi_context *hdata, bool onoff)
> +static void hdmi_audio_control(struct hdmi_context *hdata)
>  {
> +	bool enable = hdata->audio.enable;
> +
>  	if (hdata->dvi_mode)
>  		return;
>  
> -	hdmi_reg_writeb(hdata, HDMI_AUI_CON, onoff ? 2 : 0);
> -	hdmi_reg_writemask(hdata, HDMI_CON_0, onoff ?
> +	hdmi_reg_writeb(hdata, HDMI_AUI_CON, enable ? 2 : 0);
> +	hdmi_reg_writemask(hdata, HDMI_CON_0, enable ?
>  			HDMI_ASP_EN : HDMI_ASP_DIS, HDMI_ASP_MASK);
>  }
>  
> @@ -1400,9 +1426,9 @@ static void hdmi_conf_apply(struct hdmi_context *hdata)
>  {
>  	hdmi_start(hdata, false);
>  	hdmi_conf_init(hdata);
> -	hdmi_audio_init(hdata);
> +	hdmi_audio_config(hdata);
>  	hdmi_mode_apply(hdata);
> -	hdmi_audio_control(hdata, true);
> +	hdmi_audio_control(hdata);
>  }
>  
>  static void hdmi_mode_set(struct drm_encoder *encoder,
> @@ -1476,8 +1502,12 @@ static void hdmi_enable(struct drm_encoder *encoder)
>  {
>  	struct hdmi_context *hdata = encoder_to_hdmi(encoder);
>  
> +	mutex_lock(&hdata->mutex);
> +
>  	hdmiphy_enable(hdata);
>  	hdmi_conf_apply(hdata);
> +
> +	mutex_unlock(&hdata->mutex);
>  }
>  
>  static void hdmi_disable(struct drm_encoder *encoder)
> @@ -1486,6 +1516,8 @@ static void hdmi_disable(struct drm_encoder *encoder)
>  	struct drm_crtc *crtc = encoder->crtc;
>  	const struct drm_crtc_helper_funcs *funcs = NULL;
>  
> +	mutex_lock(&hdata->mutex);
> +
>  	if (!hdata->powered)

Need to unlock mutex (here and maybe in other exit paths?).


Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux