[PATCH RFC 1/3] drm/i2c: tda998x: implement different I2S flavours
Peter Ujfalusi
peter.ujfalusi at ti.com
Mon Feb 25 13:28:51 UTC 2019
hi Russell,
On 22/02/2019 23.27, Russell King wrote:
> Add support for the left and right justified I2S formats as well as the
> more tranditional "Philips" I2S format.
First of all, thank you for the patch, it works.
Tested-by: Peter Ujfalusi <peter.ujfalusi at ti.com>
There is however one thing I'm not sure about.
the 3.8 kernel configured the page0:0xfc register [1]:
/* select I2S format, and datasize */
reg_write(encoder, REG_I2S_FORMAT, 0x0a);
In theory this should select left_j and set bit3 which does something.
It looks like that the McASP is configured to I2S mode in 3.8 kernel
which would result channel swap at least there (I2S vs left_j).
Do you know what the bit3 is configuring and to what?
[1]
https://github.com/beagleboard/linux/blob/1f2f3402a6e4b8c90148c0ca2fd3acba91738eb3/drivers/gpu/drm/i2c/tda998x_drv.c#L851
>
> Signed-off-by: Russell King <rmk+kernel at armlinux.org.uk>
> ---
> drivers/gpu/drm/i2c/tda998x_drv.c | 57 ++++++++++++++++++++++++++-------------
> include/drm/i2c/tda998x.h | 11 +++++---
> 2 files changed, 47 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index a7c39f39793f..645d884fb9e8 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -242,7 +242,9 @@ struct tda998x_priv {
> # define HVF_CNTRL_1_SEMI_PLANAR (1 << 6)
> #define REG_RPT_CNTRL REG(0x00, 0xf0) /* write */
> #define REG_I2S_FORMAT REG(0x00, 0xfc) /* read/write */
> -# define I2S_FORMAT(x) (((x) & 3) << 0)
> +# define I2S_FORMAT_PHILIPS (0 << 0)
> +# define I2S_FORMAT_LEFT_J (2 << 0)
> +# define I2S_FORMAT_RIGHT_J (3 << 0)
> #define REG_AIP_CLKSEL REG(0x00, 0xfd) /* write */
> # define AIP_CLKSEL_AIP_SPDIF (0 << 3)
> # define AIP_CLKSEL_AIP_I2S (1 << 3)
> @@ -872,14 +874,14 @@ static int
> tda998x_configure_audio(struct tda998x_priv *priv,
> struct tda998x_audio_params *params)
> {
> - u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv;
> + u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv, i2s_fmt;
> u32 n;
>
> /* Enable audio ports */
> reg_write(priv, REG_ENA_AP, params->config);
>
> /* Set audio input source */
> - switch (params->format) {
> + switch (params->format & AFMT_MASK) {
> case AFMT_SPDIF:
> reg_write(priv, REG_ENA_ACLK, 0);
> reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_SPDIF);
> @@ -907,6 +909,19 @@ tda998x_configure_audio(struct tda998x_priv *priv,
> cts_n = CTS_N_M(3) | CTS_N_K(3);
> break;
> }
> +
> + switch (params->format & AFMT_I2S_MASK) {
> + case AFMT_I2S_LEFT_J:
> + i2s_fmt = I2S_FORMAT_LEFT_J;
> + break;
> + case AFMT_I2S_RIGHT_J:
> + i2s_fmt = I2S_FORMAT_RIGHT_J;
> + break;
> + default:
> + i2s_fmt = I2S_FORMAT_PHILIPS;
> + break;
> + }
> + reg_write(priv, REG_I2S_FORMAT, i2s_fmt);
> break;
>
> default:
> @@ -992,23 +1007,15 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
>
> switch (daifmt->fmt) {
> case HDMI_I2S:
> - if (daifmt->bit_clk_inv || daifmt->frame_clk_inv ||
> - daifmt->bit_clk_master || daifmt->frame_clk_master) {
> - dev_err(dev, "%s: Bad flags %d %d %d %d\n", __func__,
> - daifmt->bit_clk_inv, daifmt->frame_clk_inv,
> - daifmt->bit_clk_master,
> - daifmt->frame_clk_master);
> - return -EINVAL;
> - }
> - for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++)
> - if (priv->audio_port[i].format == AFMT_I2S)
> - audio.config = priv->audio_port[i].config;
> - audio.format = AFMT_I2S;
> + audio.format = AFMT_I2S | AFMT_I2S_PHILIPS;
> + break;
> + case HDMI_LEFT_J:
> + audio.format = AFMT_I2S | AFMT_I2S_LEFT_J;
> + break;
> + case HDMI_RIGHT_J:
> + audio.format = AFMT_I2S | AFMT_I2S_RIGHT_J;
> break;
> case HDMI_SPDIF:
> - for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++)
> - if (priv->audio_port[i].format == AFMT_SPDIF)
> - audio.config = priv->audio_port[i].config;
> audio.format = AFMT_SPDIF;
> break;
> default:
> @@ -1016,11 +1023,25 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
> return -EINVAL;
> }
>
> + for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++)
> + if (priv->audio_port[i].format == (audio.format & AFMT_MASK))
> + audio.config = priv->audio_port[i].config;
> +
> if (audio.config == 0) {
> dev_err(dev, "%s: No audio configuration found\n", __func__);
> return -EINVAL;
> }
>
> + if ((audio.format & AFMT_MASK) == HDMI_I2S &&
> + (daifmt->bit_clk_inv || daifmt->frame_clk_inv ||
> + daifmt->bit_clk_master || daifmt->frame_clk_master)) {
> + dev_err(dev, "%s: Bad flags %d %d %d %d\n", __func__,
> + daifmt->bit_clk_inv, daifmt->frame_clk_inv,
> + daifmt->bit_clk_master,
> + daifmt->frame_clk_master);
> + return -EINVAL;
> + }
> +
> mutex_lock(&priv->audio_mutex);
> if (priv->supports_infoframes && priv->sink_has_audio)
> ret = tda998x_configure_audio(priv, &audio);
> diff --git a/include/drm/i2c/tda998x.h b/include/drm/i2c/tda998x.h
> index 3cb25ccbe5e6..b0864f0be017 100644
> --- a/include/drm/i2c/tda998x.h
> +++ b/include/drm/i2c/tda998x.h
> @@ -6,9 +6,14 @@
> #include <dt-bindings/display/tda998x.h>
>
> enum {
> - AFMT_UNUSED = 0,
> - AFMT_SPDIF = TDA998x_SPDIF,
> - AFMT_I2S = TDA998x_I2S,
> + AFMT_UNUSED = 0,
> + AFMT_SPDIF = TDA998x_SPDIF,
> + AFMT_I2S = TDA998x_I2S,
> + AFMT_MASK = AFMT_SPDIF | AFMT_I2S,
> + AFMT_I2S_PHILIPS = 0 << 4,
> + AFMT_I2S_LEFT_J = 1 << 4,
> + AFMT_I2S_RIGHT_J = 2 << 4,
> + AFMT_I2S_MASK = 3 << 4,
> };
>
> struct tda998x_audio_params {
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
More information about the dri-devel
mailing list