This patch set picks up several patches sent during the past months related with NXP TDA998x HDMI transmitter driver. The patches have been tested on Marvell Dove (Armada DRM) on several HDMI and DVI modes with audio playing on S/PDIF.
I bumped all patches to v2, although only patches 2, 5, and 6 have changed. I also fixed a typo in commit line of patch 8. As Darren Etheridge already gave his Tested-by and tilcdc related patches have not changed, I added it to all patches for v2, too.
I have not added Russell King's Tested-by, as I hope he will have a close look at what I changed after his review comments.
Darren Etheridge (1): drm/tilcdc fixup mode to workaround sync for tda998x
Russell King (5): drm/i2c: tda998x: fix EDID reading on TDA19988 devices drm/i2c: tda998x: ensure VIP output mux is properly set drm/i2c: tda998x: fix npix/nline programming drm/i2c: tda998x: prepare for video input configuration drm/i2c: tda998x: add video and audio input configuration
Sebastian Hesselbarth (2): drm/i2c: tda998x: fix sync generation and calculation drm/i2c: tda998x: prepare for broken sync workaround
drivers/gpu/drm/i2c/tda998x_drv.c | 481 +++++++++++++++++++++++++++------ drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 +- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +- include/drm/i2c/tda998x.h | 30 ++ 4 files changed, 467 insertions(+), 78 deletions(-) create mode 100644 include/drm/i2c/tda998x.h
--- Cc: David Airlie airlied@linux.ie Cc: Darren Etheridge detheridge@ti.com Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org
From: Russell King rmk+kernel@arm.linux.org.uk
TDA19988 devices need their RAM enabled in order to read EDID information. Add support for this.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk Signed-off-by: Rob Clark robdclark@gmail.com Tested-by: Darren Etheridge detheridge@ti.com Tested-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com --- Cc: David Airlie airlied@linux.ie Cc: Darren Etheridge detheridge@ti.com Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org --- drivers/gpu/drm/i2c/tda998x_drv.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index e68b58a..d71c408 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -229,6 +229,8 @@ struct tda998x_priv {
/* Page 12h: HDCP and OTP */ #define REG_TX3 REG(0x12, 0x9a) /* read/write */ +#define REG_TX4 REG(0x12, 0x9b) /* read/write */ +# define TX4_PD_RAM (1 << 1) #define REG_TX33 REG(0x12, 0xb8) /* read/write */ # define TX33_HDMI (1 << 1)
@@ -673,6 +675,7 @@ read_edid_block(struct drm_encoder *encoder, uint8_t *buf, int blk) static uint8_t * do_get_edid(struct drm_encoder *encoder) { + struct tda998x_priv *priv = to_tda998x_priv(encoder); int j = 0, valid_extensions = 0; uint8_t *block, *new; bool print_bad_edid = drm_debug & DRM_UT_KMS; @@ -680,6 +683,9 @@ do_get_edid(struct drm_encoder *encoder) if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL) return NULL;
+ if (priv->rev == TDA19988) + reg_clear(encoder, REG_TX4, TX4_PD_RAM); + /* base block fetch */ if (read_edid_block(encoder, block, 0)) goto fail; @@ -689,7 +695,7 @@ do_get_edid(struct drm_encoder *encoder)
/* if there's no extensions, we're done */ if (block[0x7e] == 0) - return block; + goto done;
new = krealloc(block, (block[0x7e] + 1) * EDID_LENGTH, GFP_KERNEL); if (!new) @@ -716,9 +722,15 @@ do_get_edid(struct drm_encoder *encoder) block = new; }
+done: + if (priv->rev == TDA19988) + reg_set(encoder, REG_TX4, TX4_PD_RAM); + return block;
fail: + if (priv->rev == TDA19988) + reg_set(encoder, REG_TX4, TX4_PD_RAM); dev_warn(encoder->dev->dev, "failed to read EDID\n"); kfree(block); return NULL;
From: Russell King rmk+kernel@arm.linux.org.uk
When switching between various drivers for this device, it's possible that some critical registers are left containing values which affect the device operation. One such case encountered is the VIP output mux register. This defaults to 0x24 on powerup, but other drivers may set this to 0x12. This results in incorrect colours.
Fix this by ensuring that the register is always set to the power on default setting.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk Tested-by: Darren Etheridge detheridge@ti.com Tested-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com --- Changelog: v1->v2: - move reg_write to tda998x_reset as last patch was based on an old version (Reported by Russell King)
Cc: David Airlie airlied@linux.ie Cc: Darren Etheridge detheridge@ti.com Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org --- drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index d71c408..cb9b13a 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -110,6 +110,7 @@ struct tda998x_priv { #define REG_VIP_CNTRL_5 REG(0x00, 0x25) /* write */ # define VIP_CNTRL_5_CKCASE (1 << 0) # define VIP_CNTRL_5_SP_CNT(x) (((x) & 3) << 1) +#define REG_MUX_VP_VIP_OUT REG(0x00, 0x27) /* read/write */ #define REG_MAT_CONTRL REG(0x00, 0x80) /* write */ # define MAT_CONTRL_MAT_SC(x) (((x) & 3) << 0) # define MAT_CONTRL_MAT_BP (1 << 2) @@ -415,6 +416,9 @@ tda998x_reset(struct drm_encoder *encoder) reg_write(encoder, REG_PLL_SCGR1, 0x5b); reg_write(encoder, REG_PLL_SCGR2, 0x00); reg_write(encoder, REG_PLL_SCG2, 0x10); + + /* Write the default value MUX register */ + reg_write(encoder, REG_MUX_VP_VIP_OUT, 0x24); }
/* DRM encoder functions */
From: Russell King rmk+kernel@arm.linux.org.uk
The npix/nline registers are supposed to be programmed with the total number of pixels/lines, not the displayed pixels/lines, and not minus one either.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk Tested-by: Darren Etheridge detheridge@ti.com Tested-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com --- Cc: David Airlie airlied@linux.ie Cc: Darren Etheridge detheridge@ti.com Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org --- drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index cb9b13a..a701411 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -587,8 +587,8 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, reg_set(encoder, REG_VIP_CNTRL_3, VIP_CNTRL_3_H_TGL);
reg_write(encoder, REG_VIDFORMAT, 0x00); - reg_write16(encoder, REG_NPIX_MSB, mode->hdisplay - 1); - reg_write16(encoder, REG_NLINE_MSB, mode->vdisplay - 1); + reg_write16(encoder, REG_NPIX_MSB, mode->htotal); + reg_write16(encoder, REG_NLINE_MSB, mode->vtotal); reg_write16(encoder, REG_VS_LINE_STRT_1_MSB, line_start); reg_write16(encoder, REG_VS_LINE_END_1_MSB, line_end); reg_write16(encoder, REG_VS_PIX_STRT_1_MSB, hs_start);
From: Russell King rmk+kernel@arm.linux.org.uk
The video-input-port (VIP) is highly configurable. This prepares current driver to allow to configure VIP configuration, as some boards connect lcd controller and TDA998x "pin-swapped" and depend on VIP to swap the pins by register configuration.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk Tested-by: Darren Etheridge detheridge@ti.com Tested-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com --- Cc: David Airlie airlied@linux.ie Cc: Darren Etheridge detheridge@ti.com Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org --- drivers/gpu/drm/i2c/tda998x_drv.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index a701411..527d11b 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -32,6 +32,9 @@ struct tda998x_priv { uint16_t rev; uint8_t current_page; int dpms; + u8 vip_cntrl_0; + u8 vip_cntrl_1; + u8 vip_cntrl_2; };
#define to_tda998x_priv(x) ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv) @@ -448,12 +451,9 @@ tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) reg_write(encoder, REG_ENA_VP_1, 0xff); reg_write(encoder, REG_ENA_VP_2, 0xff); /* set muxing after enabling ports: */ - reg_write(encoder, REG_VIP_CNTRL_0, - VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3)); - reg_write(encoder, REG_VIP_CNTRL_1, - VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1)); - reg_write(encoder, REG_VIP_CNTRL_2, - VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5)); + reg_write(encoder, REG_VIP_CNTRL_0, priv->vip_cntrl_0); + reg_write(encoder, REG_VIP_CNTRL_1, priv->vip_cntrl_1); + reg_write(encoder, REG_VIP_CNTRL_2, priv->vip_cntrl_2); break; case DRM_MODE_DPMS_OFF: /* disable audio and video ports */ @@ -823,6 +823,10 @@ tda998x_encoder_init(struct i2c_client *client, if (!priv) return -ENOMEM;
+ priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3); + priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1); + priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5); + priv->current_page = 0; priv->cec = i2c_new_dummy(client->adapter, 0x34); priv->dpms = DRM_MODE_DPMS_OFF;
From: Russell King rmk+kernel@arm.linux.org.uk
This patch adds tda998x specific parameters to allow it to be configured for different boards using it. Also, this implements rudimentary audio support for S/PDIF attached controllers.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk Signed-off-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com Tested-by: Darren Etheridge detheridge@ti.com --- Changelog: for v1: - set AUDIO_DIV to SERCLK/16 for modes with pixclk >100MHz - also calculate CTS v1->v2: - Remove CTS calculation as it isn't used in current TDA998x setup (Reported by Russell King) - Remove debug left-over (Reported by Russell King) - Use correct tda998x include (Reported by Russell King)
Cc: David Airlie airlied@linux.ie Cc: Darren Etheridge detheridge@ti.com Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org --- drivers/gpu/drm/i2c/tda998x_drv.c | 268 +++++++++++++++++++++++++++++++++++-- include/drm/i2c/tda998x.h | 30 +++++ 2 files changed, 290 insertions(+), 8 deletions(-) create mode 100644 include/drm/i2c/tda998x.h
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 527d11b..2b64dfa 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -23,7 +23,7 @@ #include <drm/drm_crtc_helper.h> #include <drm/drm_encoder_slave.h> #include <drm/drm_edid.h> - +#include <drm/i2c/tda998x.h>
#define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
@@ -32,9 +32,11 @@ struct tda998x_priv { uint16_t rev; uint8_t current_page; int dpms; + bool is_hdmi_sink; u8 vip_cntrl_0; u8 vip_cntrl_1; u8 vip_cntrl_2; + struct tda998x_encoder_params params; };
#define to_tda998x_priv(x) ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv) @@ -71,10 +73,13 @@ struct tda998x_priv { # define I2C_MASTER_DIS_MM (1 << 0) # define I2C_MASTER_DIS_FILT (1 << 1) # define I2C_MASTER_APP_STRT_LAT (1 << 2) +#define REG_FEAT_POWERDOWN REG(0x00, 0x0e) /* read/write */ +# define FEAT_POWERDOWN_SPDIF (1 << 3) #define REG_INT_FLAGS_0 REG(0x00, 0x0f) /* read/write */ #define REG_INT_FLAGS_1 REG(0x00, 0x10) /* read/write */ #define REG_INT_FLAGS_2 REG(0x00, 0x11) /* read/write */ # define INT_FLAGS_2_EDID_BLK_RD (1 << 1) +#define REG_ENA_ACLK REG(0x00, 0x16) /* read/write */ #define REG_ENA_VP_0 REG(0x00, 0x18) /* read/write */ #define REG_ENA_VP_1 REG(0x00, 0x19) /* read/write */ #define REG_ENA_VP_2 REG(0x00, 0x1a) /* read/write */ @@ -113,6 +118,7 @@ struct tda998x_priv { #define REG_VIP_CNTRL_5 REG(0x00, 0x25) /* write */ # define VIP_CNTRL_5_CKCASE (1 << 0) # define VIP_CNTRL_5_SP_CNT(x) (((x) & 3) << 1) +#define REG_MUX_AP REG(0x00, 0x26) /* read/write */ #define REG_MUX_VP_VIP_OUT REG(0x00, 0x27) /* read/write */ #define REG_MAT_CONTRL REG(0x00, 0x80) /* write */ # define MAT_CONTRL_MAT_SC(x) (((x) & 3) << 0) @@ -175,6 +181,12 @@ struct tda998x_priv { # define HVF_CNTRL_1_PAD(x) (((x) & 3) << 4) # 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 REG_AIP_CLKSEL REG(0x00, 0xfd) /* write */ +# define AIP_CLKSEL_FS(x) (((x) & 3) << 0) +# define AIP_CLKSEL_CLK_POL(x) (((x) & 1) << 2) +# define AIP_CLKSEL_AIP(x) (((x) & 7) << 3)
/* Page 02h: PLL settings */ @@ -198,6 +210,12 @@ struct tda998x_priv { #define REG_PLL_SCGR1 REG(0x02, 0x09) /* read/write */ #define REG_PLL_SCGR2 REG(0x02, 0x0a) /* read/write */ #define REG_AUDIO_DIV REG(0x02, 0x0e) /* read/write */ +# define AUDIO_DIV_SERCLK_1 0 +# define AUDIO_DIV_SERCLK_2 1 +# define AUDIO_DIV_SERCLK_4 2 +# define AUDIO_DIV_SERCLK_8 3 +# define AUDIO_DIV_SERCLK_16 4 +# define AUDIO_DIV_SERCLK_32 5 #define REG_SEL_CLK REG(0x02, 0x11) /* read/write */ # define SEL_CLK_SEL_CLK1 (1 << 0) # define SEL_CLK_SEL_VRF_CLK(x) (((x) & 3) << 1) @@ -216,6 +234,11 @@ struct tda998x_priv {
/* Page 10h: information frames and packets */ +#define REG_IF1_HB0 REG(0x10, 0x20) /* read/write */ +#define REG_IF2_HB0 REG(0x10, 0x40) /* read/write */ +#define REG_IF3_HB0 REG(0x10, 0x60) /* read/write */ +#define REG_IF4_HB0 REG(0x10, 0x80) /* read/write */ +#define REG_IF5_HB0 REG(0x10, 0xa0) /* read/write */
/* Page 11h: audio settings and content info packets */ @@ -225,10 +248,33 @@ struct tda998x_priv { # define AIP_CNTRL_0_LAYOUT (1 << 2) # define AIP_CNTRL_0_ACR_MAN (1 << 5) # define AIP_CNTRL_0_RST_CTS (1 << 6) +#define REG_CA_I2S REG(0x11, 0x01) /* read/write */ +# define CA_I2S_CA_I2S(x) (((x) & 31) << 0) +# define CA_I2S_HBR_CHSTAT (1 << 6) +#define REG_LATENCY_RD REG(0x11, 0x04) /* read/write */ +#define REG_ACR_CTS_0 REG(0x11, 0x05) /* read/write */ +#define REG_ACR_CTS_1 REG(0x11, 0x06) /* read/write */ +#define REG_ACR_CTS_2 REG(0x11, 0x07) /* read/write */ +#define REG_ACR_N_0 REG(0x11, 0x08) /* read/write */ +#define REG_ACR_N_1 REG(0x11, 0x09) /* read/write */ +#define REG_ACR_N_2 REG(0x11, 0x0a) /* read/write */ +#define REG_CTS_N REG(0x11, 0x0c) /* read/write */ +# define CTS_N_K(x) (((x) & 7) << 0) +# define CTS_N_M(x) (((x) & 3) << 4) #define REG_ENC_CNTRL REG(0x11, 0x0d) /* read/write */ # define ENC_CNTRL_RST_ENC (1 << 0) # define ENC_CNTRL_RST_SEL (1 << 1) # define ENC_CNTRL_CTL_CODE(x) (((x) & 3) << 2) +#define REG_DIP_FLAGS REG(0x11, 0x0e) /* read/write */ +# define DIP_FLAGS_ACR (1 << 0) +# define DIP_FLAGS_GC (1 << 1) +#define REG_DIP_IF_FLAGS REG(0x11, 0x0f) /* read/write */ +# define DIP_IF_FLAGS_IF1 (1 << 1) +# define DIP_IF_FLAGS_IF2 (1 << 2) +# define DIP_IF_FLAGS_IF3 (1 << 3) +# define DIP_IF_FLAGS_IF4 (1 << 4) +# define DIP_IF_FLAGS_IF5 (1 << 5) +#define REG_CH_STAT_B(x) REG(0x11, 0x14 + (x)) /* read/write */
/* Page 12h: HDCP and OTP */ @@ -344,6 +390,23 @@ fail: return ret; }
+static void +reg_write_range(struct drm_encoder *encoder, uint16_t reg, uint8_t *p, int cnt) +{ + struct i2c_client *client = drm_i2c_encoder_get_client(encoder); + uint8_t buf[cnt+1]; + int ret; + + buf[0] = REG2ADDR(reg); + memcpy(&buf[1], p, cnt); + + set_page(encoder, reg); + + ret = i2c_master_send(client, buf, cnt + 1); + if (ret < 0) + dev_err(&client->dev, "Error %d writing to 0x%x\n", ret, reg); +} + static uint8_t reg_read(struct drm_encoder *encoder, uint16_t reg) { @@ -412,7 +475,7 @@ tda998x_reset(struct drm_encoder *encoder) reg_write(encoder, REG_SERIALIZER, 0x00); reg_write(encoder, REG_BUFFER_OUT, 0x00); reg_write(encoder, REG_PLL_SCG1, 0x00); - reg_write(encoder, REG_AUDIO_DIV, 0x03); + reg_write(encoder, REG_AUDIO_DIV, AUDIO_DIV_SERCLK_8); reg_write(encoder, REG_SEL_CLK, SEL_CLK_SEL_CLK1 | SEL_CLK_ENA_SC_CLK); reg_write(encoder, REG_PLL_SCGN1, 0xfa); reg_write(encoder, REG_PLL_SCGN2, 0x00); @@ -424,11 +487,184 @@ tda998x_reset(struct drm_encoder *encoder) reg_write(encoder, REG_MUX_VP_VIP_OUT, 0x24); }
+static uint8_t tda998x_cksum(uint8_t *buf, size_t bytes) +{ + uint8_t sum = 0; + + while (bytes--) + sum += *buf++; + return (255 - sum) + 1; +} + +#define HB(x) (x) +#define PB(x) (HB(2) + 1 + (x)) + +static void +tda998x_write_if(struct drm_encoder *encoder, uint8_t bit, uint16_t addr, + uint8_t *buf, size_t size) +{ + buf[PB(0)] = tda998x_cksum(buf, size); + + reg_clear(encoder, REG_DIP_IF_FLAGS, bit); + reg_write_range(encoder, addr, buf, size); + reg_set(encoder, REG_DIP_IF_FLAGS, bit); +} + +static void +tda998x_write_aif(struct drm_encoder *encoder, struct tda998x_encoder_params *p) +{ + uint8_t buf[PB(5) + 1]; + + buf[HB(0)] = 0x84; + buf[HB(1)] = 0x01; + buf[HB(2)] = 10; + buf[PB(0)] = 0; + buf[PB(1)] = p->audio_frame[1] & 0x07; /* CC */ + buf[PB(2)] = p->audio_frame[2] & 0x1c; /* SF */ + buf[PB(4)] = p->audio_frame[4]; + buf[PB(5)] = p->audio_frame[5] & 0xf8; /* DM_INH + LSV */ + + tda998x_write_if(encoder, DIP_IF_FLAGS_IF4, REG_IF4_HB0, buf, + sizeof(buf)); +} + +static void +tda998x_write_avi(struct drm_encoder *encoder, struct drm_display_mode *mode) +{ + uint8_t buf[PB(13) + 1]; + + memset(buf, 0, sizeof(buf)); + buf[HB(0)] = 0x82; + buf[HB(1)] = 0x02; + buf[HB(2)] = 13; + buf[PB(4)] = drm_match_cea_mode(mode); + + tda998x_write_if(encoder, DIP_IF_FLAGS_IF2, REG_IF2_HB0, buf, + sizeof(buf)); +} + +static void tda998x_audio_mute(struct drm_encoder *encoder, bool on) +{ + if (on) { + reg_set(encoder, REG_SOFTRESET, SOFTRESET_AUDIO); + reg_clear(encoder, REG_SOFTRESET, SOFTRESET_AUDIO); + reg_set(encoder, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_FIFO); + } else { + reg_clear(encoder, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_FIFO); + } +} + +static void +tda998x_configure_audio(struct drm_encoder *encoder, + struct drm_display_mode *mode, struct tda998x_encoder_params *p) +{ + uint8_t buf[6], clksel_aip, clksel_fs, ca_i2s, cts_n, adiv; + uint32_t n; + + /* Enable audio ports */ + reg_write(encoder, REG_ENA_AP, p->audio_cfg); + reg_write(encoder, REG_ENA_ACLK, p->audio_clk_cfg); + + /* Set audio input source */ + switch (p->audio_format) { + case AFMT_SPDIF: + reg_write(encoder, REG_MUX_AP, 0x40); + clksel_aip = AIP_CLKSEL_AIP(0); + /* FS64SPDIF */ + clksel_fs = AIP_CLKSEL_FS(2); + cts_n = CTS_N_M(3) | CTS_N_K(3); + ca_i2s = 0; + break; + + case AFMT_I2S: + reg_write(encoder, REG_MUX_AP, 0x64); + clksel_aip = AIP_CLKSEL_AIP(1); + /* ACLK */ + clksel_fs = AIP_CLKSEL_FS(0); + cts_n = CTS_N_M(3) | CTS_N_K(3); + ca_i2s = CA_I2S_CA_I2S(0); + break; + } + + reg_write(encoder, REG_AIP_CLKSEL, clksel_aip); + reg_clear(encoder, REG_AIP_CNTRL_0, AIP_CNTRL_0_LAYOUT); + + /* Enable automatic CTS generation */ + reg_clear(encoder, REG_AIP_CNTRL_0, AIP_CNTRL_0_ACR_MAN); + reg_write(encoder, REG_CTS_N, cts_n); + + /* + * Audio input somehow depends on HDMI line rate which is + * related to pixclk. Testing showed that modes with pixclk + * >100MHz need a larger divider while <40MHz need the default. + * There is no detailed info in the datasheet, so we just + * assume 100MHz requires larger divider. + */ + if (mode->clock > 100000) + adiv = AUDIO_DIV_SERCLK_16; + else + adiv = AUDIO_DIV_SERCLK_8; + reg_write(encoder, REG_AUDIO_DIV, adiv); + + /* + * This is the approximate value of N, which happens to be + * the recommended values for non-coherent clocks. + */ + n = 128 * p->audio_sample_rate / 1000; + + /* Write the CTS and N values */ + buf[0] = 0x44; + buf[1] = 0x42; + buf[2] = 0x01; + buf[3] = n; + buf[4] = n >> 8; + buf[5] = n >> 16; + reg_write_range(encoder, REG_ACR_CTS_0, buf, 6); + + /* Set CTS clock reference */ + reg_write(encoder, REG_AIP_CLKSEL, clksel_aip | clksel_fs); + + /* Reset CTS generator */ + reg_set(encoder, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_CTS); + reg_clear(encoder, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_CTS); + + /* Write the channel status */ + buf[0] = 0x04; + buf[1] = 0x00; + buf[2] = 0x00; + buf[3] = 0xf1; + reg_write_range(encoder, REG_CH_STAT_B(0), buf, 4); + + tda998x_audio_mute(encoder, true); + mdelay(20); + tda998x_audio_mute(encoder, false); + + /* Write the audio information packet */ + tda998x_write_aif(encoder, p); +} + /* DRM encoder functions */
static void tda998x_encoder_set_config(struct drm_encoder *encoder, void *params) { + struct tda998x_priv *priv = to_tda998x_priv(encoder); + struct tda998x_encoder_params *p = params; + + priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(p->swap_a) | + (p->mirr_a ? VIP_CNTRL_0_MIRR_A : 0) | + VIP_CNTRL_0_SWAP_B(p->swap_b) | + (p->mirr_b ? VIP_CNTRL_0_MIRR_B : 0); + priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(p->swap_c) | + (p->mirr_c ? VIP_CNTRL_1_MIRR_C : 0) | + VIP_CNTRL_1_SWAP_D(p->swap_d) | + (p->mirr_d ? VIP_CNTRL_1_MIRR_D : 0); + priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(p->swap_e) | + (p->mirr_e ? VIP_CNTRL_2_MIRR_E : 0) | + VIP_CNTRL_2_SWAP_F(p->swap_f) | + (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0); + + priv->params = *p; }
static void @@ -445,8 +681,7 @@ tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
switch (mode) { case DRM_MODE_DPMS_ON: - /* enable audio and video ports */ - reg_write(encoder, REG_ENA_AP, 0xff); + /* enable video ports, audio will be enabled later */ reg_write(encoder, REG_ENA_VP_0, 0xff); reg_write(encoder, REG_ENA_VP_1, 0xff); reg_write(encoder, REG_ENA_VP_2, 0xff); @@ -608,17 +843,32 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, reg_write16(encoder, REG_REFPIX_MSB, ref_pix); reg_write16(encoder, REG_REFLINE_MSB, ref_line);
- reg = TBG_CNTRL_1_VHX_EXT_DE | - TBG_CNTRL_1_VHX_EXT_HS | - TBG_CNTRL_1_VHX_EXT_VS | - TBG_CNTRL_1_DWIN_DIS | /* HDCP off */ + reg = TBG_CNTRL_1_DWIN_DIS | /* HDCP off */ TBG_CNTRL_1_VH_TGL_2; + /* + * It is questionable whether this is correct - the nxp driver + * does not set VH_TGL_2 and the below for all display modes. + */ if (mode->flags & (DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC)) reg |= TBG_CNTRL_1_VH_TGL_0; reg_set(encoder, REG_TBG_CNTRL_1, reg);
/* must be last register set: */ reg_clear(encoder, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE); + + /* Only setup the info frames if the sink is HDMI */ + if (priv->is_hdmi_sink) { + /* We need to turn HDMI HDCP stuff on to get audio through */ + reg_clear(encoder, REG_TBG_CNTRL_1, TBG_CNTRL_1_DWIN_DIS); + reg_write(encoder, REG_ENC_CNTRL, ENC_CNTRL_CTL_CODE(1)); + reg_set(encoder, REG_TX33, TX33_HDMI); + + tda998x_write_avi(encoder, adjusted_mode); + + if (priv->params.audio_cfg) + tda998x_configure_audio(encoder, adjusted_mode, + &priv->params); + } }
static enum drm_connector_status @@ -744,12 +994,14 @@ static int tda998x_encoder_get_modes(struct drm_encoder *encoder, struct drm_connector *connector) { + struct tda998x_priv *priv = to_tda998x_priv(encoder); struct edid *edid = (struct edid *)do_get_edid(encoder); int n = 0;
if (edid) { drm_mode_connector_update_edid_property(connector, edid); n = drm_add_edid_modes(connector, edid); + priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid); kfree(edid); }
diff --git a/include/drm/i2c/tda998x.h b/include/drm/i2c/tda998x.h new file mode 100644 index 0000000..3e419d9 --- /dev/null +++ b/include/drm/i2c/tda998x.h @@ -0,0 +1,30 @@ +#ifndef __DRM_I2C_TDA998X_H__ +#define __DRM_I2C_TDA998X_H__ + +struct tda998x_encoder_params { + u8 swap_b:3; + u8 mirr_b:1; + u8 swap_a:3; + u8 mirr_a:1; + u8 swap_d:3; + u8 mirr_d:1; + u8 swap_c:3; + u8 mirr_c:1; + u8 swap_f:3; + u8 mirr_f:1; + u8 swap_e:3; + u8 mirr_e:1; + + u8 audio_cfg; + u8 audio_clk_cfg; + u8 audio_frame[6]; + + enum { + AFMT_SPDIF, + AFMT_I2S + } audio_format; + + unsigned audio_sample_rate; +}; + +#endif
On Thu, Aug 15, 2013 at 5:43 AM, Sebastian Hesselbarth sebastian.hesselbarth@gmail.com wrote:
From: Russell King rmk+kernel@arm.linux.org.uk
This patch adds tda998x specific parameters to allow it to be configured for different boards using it. Also, this implements rudimentary audio support for S/PDIF attached controllers.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk Signed-off-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com Tested-by: Darren Etheridge detheridge@ti.com
I've merged the series,
this one generates a warning though: CC [M] drivers/gpu/drm/i2c/tda998x_drv.o /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i2c/tda998x_drv.c: In function ‘tda998x_encoder_mode_set’: /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i2c/tda998x_drv.c:637:11: warning: ‘clksel_fs’ may be used uninitialized in this function [-Wmaybe-uninitialized] /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i2c/tda998x_drv.c:573:30: note: ‘clksel_fs’ was declared here /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i2c/tda998x_drv.c:637:11: warning: ‘clksel_aip’ may be used uninitialized in this function [-Wmaybe-uninitialized] /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i2c/tda998x_drv.c:573:18: note: ‘clksel_aip’ was declared here
It doesn't seem like a real problem, since the function is unlikely to be called any way to make that case happen.
Dave.
On Mon, Aug 19, 2013 at 09:23:17AM +1000, Dave Airlie wrote:
On Thu, Aug 15, 2013 at 5:43 AM, Sebastian Hesselbarth sebastian.hesselbarth@gmail.com wrote:
From: Russell King rmk+kernel@arm.linux.org.uk
This patch adds tda998x specific parameters to allow it to be configured for different boards using it. Also, this implements rudimentary audio support for S/PDIF attached controllers.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk Signed-off-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com Tested-by: Darren Etheridge detheridge@ti.com
I've merged the series,
Thanks.
this one generates a warning though: CC [M] drivers/gpu/drm/i2c/tda998x_drv.o /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i2c/tda998x_drv.c: In function ‘tda998x_encoder_mode_set’: /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i2c/tda998x_drv.c:637:11: warning: ‘clksel_fs’ may be used uninitialized in this function [-Wmaybe-uninitialized] /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i2c/tda998x_drv.c:573:30: note: ‘clksel_fs’ was declared here /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i2c/tda998x_drv.c:637:11: warning: ‘clksel_aip’ may be used uninitialized in this function [-Wmaybe-uninitialized] /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i2c/tda998x_drv.c:573:18: note: ‘clksel_aip’ was declared here
It doesn't seem like a real problem, since the function is unlikely to be called any way to make that case happen.
Ok, I'll squash those warnings by a slight rearrangement of the code.
On Wed, Aug 14, 2013 at 09:43:30PM +0200, Sebastian Hesselbarth wrote:
From: Russell King rmk+kernel@arm.linux.org.uk
This patch adds tda998x specific parameters to allow it to be configured for different boards using it. Also, this implements rudimentary audio support for S/PDIF attached controllers.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk Signed-off-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com Tested-by: Darren Etheridge detheridge@ti.com
It looks like there's been a bug introduced in this patch (which wasn't in my original).
@@ -445,8 +681,7 @@ tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
switch (mode) { case DRM_MODE_DPMS_ON:
/* enable audio and video ports */reg_write(encoder, REG_ENA_AP, 0xff);
/* enable video ports, audio will be enabled later */
I also disabled the writing to REG_ENA_AP in the DPMS off path as well, which clears this register.
That seems to be missing from this patch, and it means that when the display is placed into DPMS-off mode, the audio inputs are disabled, never to be re-enabled. There is no need to disable the audio input in DPMS-off mode.
8<============= From: Russell King rmk+kernel@arm.linux.org.uk Subject: [PATCH] drm/i2c: Fix broken TDA998x audio
In patch "drm/i2c: tda998x: add video and audio input configuration" in its original version, disabling the audio input port was removed. The version which was submitted for merging had this change deleted, which results in audio being non-functional. Fix this by removing the write. While here, update the comment too.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 5742cfc..59878af 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -705,8 +705,7 @@ tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) reg_write(encoder, REG_VIP_CNTRL_2, priv->vip_cntrl_2); break; case DRM_MODE_DPMS_OFF: - /* disable audio and video ports */ - reg_write(encoder, REG_ENA_AP, 0x00); + /* disable video ports */ reg_write(encoder, REG_ENA_VP_0, 0x00); reg_write(encoder, REG_ENA_VP_1, 0x00); reg_write(encoder, REG_ENA_VP_2, 0x00);
Ping?
On Mon, Sep 02, 2013 at 03:50:57PM +0100, Russell King - ARM Linux wrote:
On Wed, Aug 14, 2013 at 09:43:30PM +0200, Sebastian Hesselbarth wrote:
From: Russell King rmk+kernel@arm.linux.org.uk
This patch adds tda998x specific parameters to allow it to be configured for different boards using it. Also, this implements rudimentary audio support for S/PDIF attached controllers.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk Signed-off-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com Tested-by: Darren Etheridge detheridge@ti.com
It looks like there's been a bug introduced in this patch (which wasn't in my original).
@@ -445,8 +681,7 @@ tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
switch (mode) { case DRM_MODE_DPMS_ON:
/* enable audio and video ports */reg_write(encoder, REG_ENA_AP, 0xff);
/* enable video ports, audio will be enabled later */I also disabled the writing to REG_ENA_AP in the DPMS off path as well, which clears this register.
That seems to be missing from this patch, and it means that when the display is placed into DPMS-off mode, the audio inputs are disabled, never to be re-enabled. There is no need to disable the audio input in DPMS-off mode.
8<============= From: Russell King rmk+kernel@arm.linux.org.uk Subject: [PATCH] drm/i2c: Fix broken TDA998x audio
In patch "drm/i2c: tda998x: add video and audio input configuration" in its original version, disabling the audio input port was removed. The version which was submitted for merging had this change deleted, which results in audio being non-functional. Fix this by removing the write. While here, update the comment too.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
drivers/gpu/drm/i2c/tda998x_drv.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 5742cfc..59878af 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -705,8 +705,7 @@ tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) reg_write(encoder, REG_VIP_CNTRL_2, priv->vip_cntrl_2); break; case DRM_MODE_DPMS_OFF:
/* disable audio and video ports */reg_write(encoder, REG_ENA_AP, 0x00);
/* disable video ports */
This fixes the wrong sync generation and sync calculation of TDA998x for HS/VS-based sync detection.
Signed-off-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com Tested-by: Darren Etheridge detheridge@ti.com --- Changelog: v1->v2: - revert calculation of hs/de_pix_s/e (Reported by Russell King)
Cc: David Airlie airlied@linux.ie Cc: Darren Etheridge detheridge@ti.com Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org --- drivers/gpu/drm/i2c/tda998x_drv.c | 181 +++++++++++++++++++++++-------------- 1 file changed, 115 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 2b64dfa..92fcb3d 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -140,8 +140,12 @@ struct tda998x_priv { #define REG_VS_LINE_END_1_LSB REG(0x00, 0xae) /* write */ #define REG_VS_PIX_END_1_MSB REG(0x00, 0xaf) /* write */ #define REG_VS_PIX_END_1_LSB REG(0x00, 0xb0) /* write */ +#define REG_VS_LINE_STRT_2_MSB REG(0x00, 0xb1) /* write */ +#define REG_VS_LINE_STRT_2_LSB REG(0x00, 0xb2) /* write */ #define REG_VS_PIX_STRT_2_MSB REG(0x00, 0xb3) /* write */ #define REG_VS_PIX_STRT_2_LSB REG(0x00, 0xb4) /* write */ +#define REG_VS_LINE_END_2_MSB REG(0x00, 0xb5) /* write */ +#define REG_VS_LINE_END_2_LSB REG(0x00, 0xb6) /* write */ #define REG_VS_PIX_END_2_MSB REG(0x00, 0xb7) /* write */ #define REG_VS_PIX_END_2_LSB REG(0x00, 0xb8) /* write */ #define REG_HS_PIX_START_MSB REG(0x00, 0xb9) /* write */ @@ -152,21 +156,29 @@ struct tda998x_priv { #define REG_VWIN_START_1_LSB REG(0x00, 0xbe) /* write */ #define REG_VWIN_END_1_MSB REG(0x00, 0xbf) /* write */ #define REG_VWIN_END_1_LSB REG(0x00, 0xc0) /* write */ +#define REG_VWIN_START_2_MSB REG(0x00, 0xc1) /* write */ +#define REG_VWIN_START_2_LSB REG(0x00, 0xc2) /* write */ +#define REG_VWIN_END_2_MSB REG(0x00, 0xc3) /* write */ +#define REG_VWIN_END_2_LSB REG(0x00, 0xc4) /* write */ #define REG_DE_START_MSB REG(0x00, 0xc5) /* write */ #define REG_DE_START_LSB REG(0x00, 0xc6) /* write */ #define REG_DE_STOP_MSB REG(0x00, 0xc7) /* write */ #define REG_DE_STOP_LSB REG(0x00, 0xc8) /* write */ #define REG_TBG_CNTRL_0 REG(0x00, 0xca) /* write */ +# define TBG_CNTRL_0_TOP_TGL (1 << 0) +# define TBG_CNTRL_0_TOP_SEL (1 << 1) +# define TBG_CNTRL_0_DE_EXT (1 << 2) +# define TBG_CNTRL_0_TOP_EXT (1 << 3) # define TBG_CNTRL_0_FRAME_DIS (1 << 5) # define TBG_CNTRL_0_SYNC_MTHD (1 << 6) # define TBG_CNTRL_0_SYNC_ONCE (1 << 7) #define REG_TBG_CNTRL_1 REG(0x00, 0xcb) /* write */ -# define TBG_CNTRL_1_VH_TGL_0 (1 << 0) -# define TBG_CNTRL_1_VH_TGL_1 (1 << 1) -# define TBG_CNTRL_1_VH_TGL_2 (1 << 2) -# define TBG_CNTRL_1_VHX_EXT_DE (1 << 3) -# define TBG_CNTRL_1_VHX_EXT_HS (1 << 4) -# define TBG_CNTRL_1_VHX_EXT_VS (1 << 5) +# define TBG_CNTRL_1_H_TGL (1 << 0) +# define TBG_CNTRL_1_V_TGL (1 << 1) +# define TBG_CNTRL_1_TGL_EN (1 << 2) +# define TBG_CNTRL_1_X_EXT (1 << 3) +# define TBG_CNTRL_1_H_EXT (1 << 4) +# define TBG_CNTRL_1_V_EXT (1 << 5) # define TBG_CNTRL_1_DWIN_DIS (1 << 6) #define REG_ENABLE_SPACE REG(0x00, 0xd6) /* write */ #define REG_HVF_CNTRL_0 REG(0x00, 0xe4) /* write */ @@ -735,43 +747,70 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *adjusted_mode) { struct tda998x_priv *priv = to_tda998x_priv(encoder); - uint16_t hs_start, hs_end, line_start, line_end; - uint16_t vwin_start, vwin_end, de_start, de_end; - uint16_t ref_pix, ref_line, pix_start2; + uint16_t ref_pix, ref_line, n_pix, n_line; + uint16_t hs_pix_s, hs_pix_e; + uint16_t vs1_pix_s, vs1_pix_e, vs1_line_s, vs1_line_e; + uint16_t vs2_pix_s, vs2_pix_e, vs2_line_s, vs2_line_e; + uint16_t vwin1_line_s, vwin1_line_e; + uint16_t vwin2_line_s, vwin2_line_e; + uint16_t de_pix_s, de_pix_e; uint8_t reg, div, rep;
- hs_start = mode->hsync_start - mode->hdisplay; - hs_end = mode->hsync_end - mode->hdisplay; - line_start = 1; - line_end = 1 + mode->vsync_end - mode->vsync_start; - vwin_start = mode->vtotal - mode->vsync_start; - vwin_end = vwin_start + mode->vdisplay; - de_start = mode->htotal - mode->hdisplay; - de_end = mode->htotal; - - pix_start2 = 0; - if (mode->flags & DRM_MODE_FLAG_INTERLACE) - pix_start2 = (mode->htotal / 2) + hs_start; - - /* TODO how is this value calculated? It is 2 for all common - * formats in the tables in out of tree nxp driver (assuming - * I've properly deciphered their byzantine table system) + /* + * Internally TDA998x is using ITU-R BT.656 style sync but + * we get VESA style sync. TDA998x is using a reference pixel + * relative to ITU to sync to the input frame and for output + * sync generation. Currently, we are using reference detection + * from HS/VS, i.e. REFPIX/REFLINE denote frame start sync point + * which is position of rising VS with coincident rising HS. + * + * Now there is some issues to take care of: + * - HDMI data islands require sync-before-active + * - TDA998x register values must be > 0 to be enabled + * - REFLINE needs an additional offset of +1 + * - REFPIX needs an addtional offset of +1 for UYUV and +3 for RGB + * + * So we add +1 to all horizontal and vertical register values, + * plus an additional +3 for REFPIX as we are using RGB input only. */ - ref_line = 2; - - /* this might changes for other color formats from the CRTC: */ - ref_pix = 3 + hs_start; + n_pix = mode->htotal; + n_line = mode->vtotal; + + hs_pix_e = mode->hsync_end - mode->hdisplay; + hs_pix_s = mode->hsync_start - mode->hdisplay; + de_pix_e = mode->htotal; + de_pix_s = mode->htotal - mode->hdisplay; + ref_pix = 3 + hs_pix_s; + + if ((mode->flags & DRM_MODE_FLAG_INTERLACE) == 0) { + ref_line = 1 + mode->vsync_start - mode->vdisplay; + vwin1_line_s = mode->vtotal - mode->vdisplay - 1; + vwin1_line_e = vwin1_line_s + mode->vdisplay; + vs1_pix_s = vs1_pix_e = hs_pix_s; + vs1_line_s = mode->vsync_start - mode->vdisplay; + vs1_line_e = vs1_line_s + + mode->vsync_end - mode->vsync_start; + vwin2_line_s = vwin2_line_e = 0; + vs2_pix_s = vs2_pix_e = 0; + vs2_line_s = vs2_line_e = 0; + } else { + ref_line = 1 + (mode->vsync_start - mode->vdisplay)/2; + vwin1_line_s = (mode->vtotal - mode->vdisplay)/2; + vwin1_line_e = vwin1_line_s + mode->vdisplay/2; + vs1_pix_s = vs1_pix_e = hs_pix_s; + vs1_line_s = (mode->vsync_start - mode->vdisplay)/2; + vs1_line_e = vs1_line_s + + (mode->vsync_end - mode->vsync_start)/2; + vwin2_line_s = vwin1_line_s + mode->vtotal/2; + vwin2_line_e = vwin2_line_s + mode->vdisplay/2; + vs2_pix_s = vs2_pix_e = hs_pix_s + mode->htotal/2; + vs2_line_s = vs1_line_s + mode->vtotal/2 ; + vs2_line_e = vs2_line_s + + (mode->vsync_end - mode->vsync_start)/2; + }
div = 148500 / mode->clock;
- DBG("clock=%d, div=%u", mode->clock, div); - DBG("hs_start=%u, hs_end=%u, line_start=%u, line_end=%u", - hs_start, hs_end, line_start, line_end); - DBG("vwin_start=%u, vwin_end=%u, de_start=%u, de_end=%u", - vwin_start, vwin_end, de_start, de_end); - DBG("ref_line=%u, ref_pix=%u, pix_start2=%u", - ref_line, ref_pix, pix_start2); - /* mute the audio FIFO: */ reg_set(encoder, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_FIFO);
@@ -802,9 +841,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, reg_write(encoder, REG_PLL_SERIAL_2, PLL_SERIAL_2_SRL_NOSC(div) | PLL_SERIAL_2_SRL_PR(rep));
- reg_write16(encoder, REG_VS_PIX_STRT_2_MSB, pix_start2); - reg_write16(encoder, REG_VS_PIX_END_2_MSB, pix_start2); - /* set color matrix bypass flag: */ reg_set(encoder, REG_MAT_CONTRL, MAT_CONTRL_MAT_BP);
@@ -813,46 +849,59 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
reg_clear(encoder, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_MTHD);
+ /* + * Sync on rising HSYNC/VSYNC + */ reg_write(encoder, REG_VIP_CNTRL_3, 0); reg_set(encoder, REG_VIP_CNTRL_3, VIP_CNTRL_3_SYNC_HS); + + /* + * TDA19988 requires high-active sync at input stage, + * so invert low-active sync provided by master encoder here + */ + if (mode->flags & DRM_MODE_FLAG_NHSYNC) + reg_set(encoder, REG_VIP_CNTRL_3, VIP_CNTRL_3_H_TGL); if (mode->flags & DRM_MODE_FLAG_NVSYNC) reg_set(encoder, REG_VIP_CNTRL_3, VIP_CNTRL_3_V_TGL);
+ /* + * Always generate sync polarity relative to input sync and + * revert input stage toggled sync at output stage + */ + reg = TBG_CNTRL_1_TGL_EN; if (mode->flags & DRM_MODE_FLAG_NHSYNC) - reg_set(encoder, REG_VIP_CNTRL_3, VIP_CNTRL_3_H_TGL); + reg |= TBG_CNTRL_1_H_TGL; + if (mode->flags & DRM_MODE_FLAG_NVSYNC) + reg |= TBG_CNTRL_1_V_TGL; + reg_write(encoder, REG_TBG_CNTRL_1, reg);
reg_write(encoder, REG_VIDFORMAT, 0x00); - reg_write16(encoder, REG_NPIX_MSB, mode->htotal); - reg_write16(encoder, REG_NLINE_MSB, mode->vtotal); - reg_write16(encoder, REG_VS_LINE_STRT_1_MSB, line_start); - reg_write16(encoder, REG_VS_LINE_END_1_MSB, line_end); - reg_write16(encoder, REG_VS_PIX_STRT_1_MSB, hs_start); - reg_write16(encoder, REG_VS_PIX_END_1_MSB, hs_start); - reg_write16(encoder, REG_HS_PIX_START_MSB, hs_start); - reg_write16(encoder, REG_HS_PIX_STOP_MSB, hs_end); - reg_write16(encoder, REG_VWIN_START_1_MSB, vwin_start); - reg_write16(encoder, REG_VWIN_END_1_MSB, vwin_end); - reg_write16(encoder, REG_DE_START_MSB, de_start); - reg_write16(encoder, REG_DE_STOP_MSB, de_end); + reg_write16(encoder, REG_REFPIX_MSB, ref_pix); + reg_write16(encoder, REG_REFLINE_MSB, ref_line); + reg_write16(encoder, REG_NPIX_MSB, n_pix); + reg_write16(encoder, REG_NLINE_MSB, n_line); + reg_write16(encoder, REG_VS_LINE_STRT_1_MSB, vs1_line_s); + reg_write16(encoder, REG_VS_PIX_STRT_1_MSB, vs1_pix_s); + reg_write16(encoder, REG_VS_LINE_END_1_MSB, vs1_line_e); + reg_write16(encoder, REG_VS_PIX_END_1_MSB, vs1_pix_e); + reg_write16(encoder, REG_VS_LINE_STRT_2_MSB, vs2_line_s); + reg_write16(encoder, REG_VS_PIX_STRT_2_MSB, vs2_pix_s); + reg_write16(encoder, REG_VS_LINE_END_2_MSB, vs2_line_e); + reg_write16(encoder, REG_VS_PIX_END_2_MSB, vs2_pix_e); + reg_write16(encoder, REG_HS_PIX_START_MSB, hs_pix_s); + reg_write16(encoder, REG_HS_PIX_STOP_MSB, hs_pix_e); + reg_write16(encoder, REG_VWIN_START_1_MSB, vwin1_line_s); + reg_write16(encoder, REG_VWIN_END_1_MSB, vwin1_line_e); + reg_write16(encoder, REG_VWIN_START_2_MSB, vwin2_line_s); + reg_write16(encoder, REG_VWIN_END_2_MSB, vwin2_line_e); + reg_write16(encoder, REG_DE_START_MSB, de_pix_s); + reg_write16(encoder, REG_DE_STOP_MSB, de_pix_e);
if (priv->rev == TDA19988) { /* let incoming pixels fill the active space (if any) */ reg_write(encoder, REG_ENABLE_SPACE, 0x01); }
- reg_write16(encoder, REG_REFPIX_MSB, ref_pix); - reg_write16(encoder, REG_REFLINE_MSB, ref_line); - - reg = TBG_CNTRL_1_DWIN_DIS | /* HDCP off */ - TBG_CNTRL_1_VH_TGL_2; - /* - * It is questionable whether this is correct - the nxp driver - * does not set VH_TGL_2 and the below for all display modes. - */ - if (mode->flags & (DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC)) - reg |= TBG_CNTRL_1_VH_TGL_0; - reg_set(encoder, REG_TBG_CNTRL_1, reg); - /* must be last register set: */ reg_clear(encoder, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE);
Some LCD controller cannot provide valid VESA style sync, i.e. coincident HS/VS edges. First, this patch adds hskew passed from the adjusted_mode to reference pixel calculation to allow those controllers to add an offset relative to the expected reference pixel.
Signed-off-by: Darren Etheridge detheridge@ti.com Signed-off-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com --- Changelog: for v1: - reword comment
Cc: David Airlie airlied@linux.ie Cc: Darren Etheridge detheridge@ti.com Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org --- drivers/gpu/drm/i2c/tda998x_drv.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 92fcb3d..c2bd711 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -782,6 +782,14 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, de_pix_s = mode->htotal - mode->hdisplay; ref_pix = 3 + hs_pix_s;
+ /* + * Attached LCD controllers may generate broken sync. Allow + * those to adjust the position of the rising VS edge by adding + * HSKEW to ref_pix. + */ + if (adjusted_mode->flags & DRM_MODE_FLAG_HSKEW) + ref_pix += adjusted_mode->hskew; + if ((mode->flags & DRM_MODE_FLAG_INTERLACE) == 0) { ref_line = 1 + mode->vsync_start - mode->vdisplay; vwin1_line_s = mode->vtotal - mode->vdisplay - 1;
From: Darren Etheridge detheridge@ti.com
Add a fixup function that will flip the hsync priority and add a hskew value that is used to shift the tda998x to the right by a variable number of pixels depending on the mode. This works around an issue with the sync timings that tilcdc is outputing.
Signed-off-by: Darren Etheridge detheridge@ti.com Tested-by: Sebastian Hesselbarth sebastian.hesselbarth@gmail.com --- Changelog: v1->v2: - fix typo in commit line (s/workaound/workaround)
Cc: David Airlie airlied@linux.ie Cc: Darren Etheridge detheridge@ti.com Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 ++++++- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 ++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 7418dcd..6d05240 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -379,7 +379,12 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc, else tilcdc_clear(dev, LCDC_RASTER_TIMING_2_REG, LCDC_SYNC_EDGE);
- if (mode->flags & DRM_MODE_FLAG_NHSYNC) + /* + * use value from adjusted_mode here as this might have been + * changed as part of the fixup for slave encoders to solve the + * issue where tilcdc timings are not VESA compliant + */ + if (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC) tilcdc_set(dev, LCDC_RASTER_TIMING_2_REG, LCDC_INVERT_HSYNC); else tilcdc_clear(dev, LCDC_RASTER_TIMING_2_REG, LCDC_INVERT_HSYNC); diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c index 0bf5999..f6e9c26 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c @@ -73,13 +73,38 @@ static void slave_encoder_prepare(struct drm_encoder *encoder) tilcdc_crtc_set_panel_info(encoder->crtc, &slave_info); }
+static bool slave_encoder_fixup(struct drm_encoder *encoder, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + /* + * tilcdc does not generate VESA-complient sync but aligns + * VS on the second edge of HS instead of first edge. + * We use adjusted_mode, to fixup sync by aligning both rising + * edges and add HSKEW offset to let the slave encoder fix it up. + */ + adjusted_mode->hskew = mode->hsync_end - mode->hsync_start; + adjusted_mode->flags |= DRM_MODE_FLAG_HSKEW; + + if (mode->flags & DRM_MODE_FLAG_NHSYNC) { + adjusted_mode->flags |= DRM_MODE_FLAG_PHSYNC; + adjusted_mode->flags &= ~DRM_MODE_FLAG_NHSYNC; + } else { + adjusted_mode->flags |= DRM_MODE_FLAG_NHSYNC; + adjusted_mode->flags &= ~DRM_MODE_FLAG_PHSYNC; + } + + return drm_i2c_encoder_mode_fixup(encoder, mode, adjusted_mode); +} + + static const struct drm_encoder_funcs slave_encoder_funcs = { .destroy = slave_encoder_destroy, };
static const struct drm_encoder_helper_funcs slave_encoder_helper_funcs = { .dpms = drm_i2c_encoder_dpms, - .mode_fixup = drm_i2c_encoder_mode_fixup, + .mode_fixup = slave_encoder_fixup, .prepare = slave_encoder_prepare, .commit = drm_i2c_encoder_commit, .mode_set = drm_i2c_encoder_mode_set,
On Wed, Aug 14, 2013 at 09:43:25PM +0200, Sebastian Hesselbarth wrote:
This patch set picks up several patches sent during the past months related with NXP TDA998x HDMI transmitter driver. The patches have been tested on Marvell Dove (Armada DRM) on several HDMI and DVI modes with audio playing on S/PDIF.
I bumped all patches to v2, although only patches 2, 5, and 6 have changed. I also fixed a typo in commit line of patch 8. As Darren Etheridge already gave his Tested-by and tilcdc related patches have not changed, I added it to all patches for v2, too.
I have not added Russell King's Tested-by, as I hope he will have a close look at what I changed after his review comments.
Darren Etheridge (1): drm/tilcdc fixup mode to workaround sync for tda998x
Russell King (5): drm/i2c: tda998x: fix EDID reading on TDA19988 devices drm/i2c: tda998x: ensure VIP output mux is properly set drm/i2c: tda998x: fix npix/nline programming drm/i2c: tda998x: prepare for video input configuration drm/i2c: tda998x: add video and audio input configuration
Sebastian Hesselbarth (2): drm/i2c: tda998x: fix sync generation and calculation drm/i2c: tda998x: prepare for broken sync workaround
drivers/gpu/drm/i2c/tda998x_drv.c | 481 +++++++++++++++++++++++++++------ drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 +- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +- include/drm/i2c/tda998x.h | 30 ++ 4 files changed, 467 insertions(+), 78 deletions(-) create mode 100644 include/drm/i2c/tda998x.h
Cc: David Airlie airlied@linux.ie Cc: Darren Etheridge detheridge@ti.com Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org
Okay, I'm happy with this set. Patches 1-7 can get a:
Tested-by: Russell King rmk+kernel@arm.linux.org.uk
Thanks.
dri-devel@lists.freedesktop.org
-
Dave Airlie -
Russell King - ARM Linux -
Sebastian Hesselbarth