Hi,
Here's my currently queued TDA998x development work for 4.4.
* Remove some useless NULL checks here variables can't be NULL. * Return IRQ_HANDLED only if we handled the IRQ, otherwise return IRQ_NONE. This avoids locking the system up if the IRQ gets stuck. * Re-implement a previous patch "Fix EDID read timeout on HDMI connect" to be much more reliable: an attempt to read the EDID may come in while we're delaying the HPD detect event, violating the critical pause. * Use Linux types rather than C99 types. * Handle all outstanding interrupts, rather than only the first interrupt that we discover pending. * Use more helpers from drivers/video/hdmi.c - this removes our own infoframe checksumming code. * Remove DRM slave encoder support (which I think no one is using anymore.) This allows other tidy-ups, removing the abstractions to allow slave encoder support to co-exist with component support.
drivers/gpu/drm/i2c/tda998x_drv.c | 487 +++++++++++++++----------------------- 1 file changed, 185 insertions(+), 302 deletions(-)
There is no way 'priv' can be NULL in tda998x_irq_thread() - this can only happen if request_threaded_irq() was passed a NULL priv pointer, and we would have crashed long before then if that was the case.
We also always ensure that priv->encoder is correctly setup, which must have been initialised prior to the interrupt being claimed, so we can remove this check as well.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 424228be79ae..d8e97085f866 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -558,7 +558,7 @@ static void tda998x_hpd(struct work_struct *work) struct tda998x_priv *priv = container_of(dwork, struct tda998x_priv, dwork);
- if (priv->encoder && priv->encoder->dev) + if (priv->encoder->dev) drm_kms_helper_hotplug_event(priv->encoder->dev); }
@@ -570,8 +570,6 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) struct tda998x_priv *priv = data; u8 sta, cec, lvl, flag0, flag1, flag2;
- if (!priv) - return IRQ_HANDLED; sta = cec_read(priv, REG_CEC_INTSTATUS); cec = cec_read(priv, REG_CEC_RXSHPDINT); lvl = cec_read(priv, REG_CEC_RXSHPDLEV);
Rather than always reporting that the interrupt was handled, we should report whether we did handle the interrupt. Arrange to report IRQ_NONE for cases where we found nothing to do.
This allows us to (eventually) recover from stuck-IRQ problems, rather than causing the kernel to solidly lock up.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index d8e97085f866..ad3ce3479edf 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -569,6 +569,7 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) { struct tda998x_priv *priv = data; u8 sta, cec, lvl, flag0, flag1, flag2; + bool handled = false;
sta = cec_read(priv, REG_CEC_INTSTATUS); cec = cec_read(priv, REG_CEC_RXSHPDINT); @@ -582,10 +583,12 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) if ((flag2 & INT_FLAGS_2_EDID_BLK_RD) && priv->wq_edid_wait) { priv->wq_edid_wait = 0; wake_up(&priv->wq_edid); + handled = true; } else if (cec != 0) { /* HPD change */ schedule_delayed_work(&priv->dwork, HZ/10); + handled = true; } - return IRQ_HANDLED; + return IRQ_RETVAL(handled); }
static uint8_t tda998x_cksum(uint8_t *buf, size_t bytes)
Commit 6833d26ef823 ("drm: tda998x: Fix EDID read timeout on HDMI connect") used a weak scheme to try and delay reading EDID on a HDMI connect event. It is weak because delaying the notification of a hotplug event does not stop userspace from trying to read the EDID within the 100ms delay.
The solution provided here solves this issue: * When a HDMI connection event is detected, mark a blocking flag for EDID reads, and start a timer for the delay. * If an EDID read is attempted, and the blocking flag is set, wait for the blocking flag to clear. * When the timer expires, clear the blocking flag and wake any thread waiting for the EDID read.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 80 +++++++++++++++++++++++++++++++++------ 1 file changed, 68 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index ad3ce3479edf..a53696fd8938 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -34,7 +34,6 @@ struct tda998x_priv { struct i2c_client *cec; struct i2c_client *hdmi; struct mutex mutex; - struct delayed_work dwork; uint16_t rev; uint8_t current_page; int dpms; @@ -47,6 +46,11 @@ struct tda998x_priv { wait_queue_head_t wq_edid; volatile int wq_edid_wait; struct drm_encoder *encoder; + + struct work_struct detect_work; + struct timer_list edid_delay_timer; + wait_queue_head_t edid_delay_waitq; + bool edid_delay_active; };
#define to_tda998x_priv(x) ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv) @@ -551,15 +555,50 @@ tda998x_reset(struct tda998x_priv *priv) reg_write(priv, REG_MUX_VP_VIP_OUT, 0x24); }
-/* handle HDMI connect/disconnect */ -static void tda998x_hpd(struct work_struct *work) +/* + * The TDA998x has a problem when trying to read the EDID close to a + * HPD assertion: it needs a delay of 100ms to avoid timing out while + * trying to read EDID data. + * + * However, tda998x_encoder_get_modes() may be called at any moment + * after tda998x_encoder_detect() indicates that we are connected, so + * we need to delay probing modes in tda998x_encoder_get_modes() after + * we have seen a HPD inactive->active transition. This code implements + * that delay. + */ +static void tda998x_edid_delay_done(unsigned long data) +{ + struct tda998x_priv *priv = (struct tda998x_priv *)data; + + priv->edid_delay_active = false; + wake_up(&priv->edid_delay_waitq); + schedule_work(&priv->detect_work); +} + +static void tda998x_edid_delay_start(struct tda998x_priv *priv) +{ + priv->edid_delay_active = true; + mod_timer(&priv->edid_delay_timer, jiffies + HZ/10); +} + +static int tda998x_edid_delay_wait(struct tda998x_priv *priv) +{ + return wait_event_killable(priv->edid_delay_waitq, !priv->edid_delay_active); +} + +/* + * We need to run the KMS hotplug event helper outside of our threaded + * interrupt routine as this can call back into our get_modes method, + * which will want to make use of interrupts. + */ +static void tda998x_detect_work(struct work_struct *work) { - struct delayed_work *dwork = to_delayed_work(work); struct tda998x_priv *priv = - container_of(dwork, struct tda998x_priv, dwork); + container_of(work, struct tda998x_priv, detect_work); + struct drm_device *dev = priv->encoder->dev;
- if (priv->encoder->dev) - drm_kms_helper_hotplug_event(priv->encoder->dev); + if (dev) + drm_kms_helper_hotplug_event(dev); }
/* @@ -585,7 +624,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) wake_up(&priv->wq_edid); handled = true; } else if (cec != 0) { /* HPD change */ - schedule_delayed_work(&priv->dwork, HZ/10); + if (lvl & CEC_RXSHPDLEV_HPD) + tda998x_edid_delay_start(priv); + else + schedule_work(&priv->detect_work); + handled = true; } return IRQ_RETVAL(handled); @@ -1103,6 +1146,14 @@ tda998x_encoder_get_modes(struct tda998x_priv *priv, struct edid *edid; int n;
+ /* + * If we get killed while waiting for the HPD timeout, return + * no modes found: we are not in a restartable path, so we + * can't handle signals gracefully. + */ + if (tda998x_edid_delay_wait(priv)) + return 0; + if (priv->rev == TDA19988) reg_clear(priv, REG_TX4, TX4_PD_RAM);
@@ -1149,10 +1200,12 @@ static void tda998x_destroy(struct tda998x_priv *priv) /* disable all IRQs and free the IRQ handler */ cec_write(priv, REG_CEC_RXSHPDINTENA, 0); reg_clear(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD); - if (priv->hdmi->irq) { + + if (priv->hdmi->irq) free_irq(priv->hdmi->irq, priv); - cancel_delayed_work_sync(&priv->dwork); - } + + del_timer_sync(&priv->edid_delay_timer); + cancel_work_sync(&priv->detect_work);
i2c_unregister_device(priv->cec); } @@ -1253,6 +1306,10 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) priv->dpms = DRM_MODE_DPMS_OFF;
mutex_init(&priv->mutex); /* protect the page access */ + init_waitqueue_head(&priv->edid_delay_waitq); + setup_timer(&priv->edid_delay_timer, tda998x_edid_delay_done, + (unsigned long)priv); + INIT_WORK(&priv->detect_work, tda998x_detect_work);
/* wake up the device: */ cec_write(priv, REG_CEC_ENAMODS, @@ -1311,7 +1368,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
/* init read EDID waitqueue and HDP work */ init_waitqueue_head(&priv->wq_edid); - INIT_DELAYED_WORK(&priv->dwork, tda998x_hpd);
/* clear pending interrupts */ reg_read(priv, REG_INT_FLAGS_0);
C99 types are against the style of the Linux kernel. Convert to using Linus-friendly types. See https://lwn.net/Articles/113367/ for more information.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 74 +++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index a53696fd8938..6d6aaadc0d2f 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -34,8 +34,8 @@ struct tda998x_priv { struct i2c_client *cec; struct i2c_client *hdmi; struct mutex mutex; - uint16_t rev; - uint8_t current_page; + u16 rev; + u8 current_page; int dpms; bool is_hdmi_sink; u8 vip_cntrl_0; @@ -349,10 +349,10 @@ struct tda998x_priv { #define TDA19988 0x0301
static void -cec_write(struct tda998x_priv *priv, uint16_t addr, uint8_t val) +cec_write(struct tda998x_priv *priv, u16 addr, u8 val) { struct i2c_client *client = priv->cec; - uint8_t buf[] = {addr, val}; + u8 buf[] = {addr, val}; int ret;
ret = i2c_master_send(client, buf, sizeof(buf)); @@ -360,11 +360,11 @@ cec_write(struct tda998x_priv *priv, uint16_t addr, uint8_t val) dev_err(&client->dev, "Error %d writing to cec:0x%x\n", ret, addr); }
-static uint8_t -cec_read(struct tda998x_priv *priv, uint8_t addr) +static u8 +cec_read(struct tda998x_priv *priv, u8 addr) { struct i2c_client *client = priv->cec; - uint8_t val; + u8 val; int ret;
ret = i2c_master_send(client, &addr, sizeof(addr)); @@ -383,11 +383,11 @@ cec_read(struct tda998x_priv *priv, uint8_t addr) }
static int -set_page(struct tda998x_priv *priv, uint16_t reg) +set_page(struct tda998x_priv *priv, u16 reg) { if (REG2PAGE(reg) != priv->current_page) { struct i2c_client *client = priv->hdmi; - uint8_t buf[] = { + u8 buf[] = { REG_CURPAGE, REG2PAGE(reg) }; int ret = i2c_master_send(client, buf, sizeof(buf)); @@ -403,10 +403,10 @@ set_page(struct tda998x_priv *priv, uint16_t reg) }
static int -reg_read_range(struct tda998x_priv *priv, uint16_t reg, char *buf, int cnt) +reg_read_range(struct tda998x_priv *priv, u16 reg, char *buf, int cnt) { struct i2c_client *client = priv->hdmi; - uint8_t addr = REG2ADDR(reg); + u8 addr = REG2ADDR(reg); int ret;
mutex_lock(&priv->mutex); @@ -432,10 +432,10 @@ reg_read_range(struct tda998x_priv *priv, uint16_t reg, char *buf, int cnt) }
static void -reg_write_range(struct tda998x_priv *priv, uint16_t reg, uint8_t *p, int cnt) +reg_write_range(struct tda998x_priv *priv, u16 reg, u8 *p, int cnt) { struct i2c_client *client = priv->hdmi; - uint8_t buf[cnt+1]; + u8 buf[cnt+1]; int ret;
buf[0] = REG2ADDR(reg); @@ -454,9 +454,9 @@ reg_write_range(struct tda998x_priv *priv, uint16_t reg, uint8_t *p, int cnt) }
static int -reg_read(struct tda998x_priv *priv, uint16_t reg) +reg_read(struct tda998x_priv *priv, u16 reg) { - uint8_t val = 0; + u8 val = 0; int ret;
ret = reg_read_range(priv, reg, &val, sizeof(val)); @@ -466,10 +466,10 @@ reg_read(struct tda998x_priv *priv, uint16_t reg) }
static void -reg_write(struct tda998x_priv *priv, uint16_t reg, uint8_t val) +reg_write(struct tda998x_priv *priv, u16 reg, u8 val) { struct i2c_client *client = priv->hdmi; - uint8_t buf[] = {REG2ADDR(reg), val}; + u8 buf[] = {REG2ADDR(reg), val}; int ret;
mutex_lock(&priv->mutex); @@ -485,10 +485,10 @@ reg_write(struct tda998x_priv *priv, uint16_t reg, uint8_t val) }
static void -reg_write16(struct tda998x_priv *priv, uint16_t reg, uint16_t val) +reg_write16(struct tda998x_priv *priv, u16 reg, u16 val) { struct i2c_client *client = priv->hdmi; - uint8_t buf[] = {REG2ADDR(reg), val >> 8, val}; + u8 buf[] = {REG2ADDR(reg), val >> 8, val}; int ret;
mutex_lock(&priv->mutex); @@ -504,7 +504,7 @@ reg_write16(struct tda998x_priv *priv, uint16_t reg, uint16_t val) }
static void -reg_set(struct tda998x_priv *priv, uint16_t reg, uint8_t val) +reg_set(struct tda998x_priv *priv, u16 reg, u8 val) { int old_val;
@@ -514,7 +514,7 @@ reg_set(struct tda998x_priv *priv, uint16_t reg, uint8_t val) }
static void -reg_clear(struct tda998x_priv *priv, uint16_t reg, uint8_t val) +reg_clear(struct tda998x_priv *priv, u16 reg, u8 val) { int old_val;
@@ -634,7 +634,7 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) return IRQ_RETVAL(handled); }
-static uint8_t tda998x_cksum(uint8_t *buf, size_t bytes) +static u8 tda998x_cksum(u8 *buf, size_t bytes) { int sum = 0;
@@ -647,8 +647,8 @@ static uint8_t tda998x_cksum(uint8_t *buf, size_t bytes) #define PB(x) (HB(2) + 1 + (x))
static void -tda998x_write_if(struct tda998x_priv *priv, uint8_t bit, uint16_t addr, - uint8_t *buf, size_t size) +tda998x_write_if(struct tda998x_priv *priv, u8 bit, u16 addr, + u8 *buf, size_t size) { reg_clear(priv, REG_DIP_IF_FLAGS, bit); reg_write_range(priv, addr, buf, size); @@ -711,8 +711,8 @@ static void tda998x_configure_audio(struct tda998x_priv *priv, struct drm_display_mode *mode, struct tda998x_encoder_params *p) { - uint8_t buf[6], clksel_aip, clksel_fs, cts_n, adiv; - uint32_t n; + u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv; + u32 n;
/* Enable audio ports */ reg_write(priv, REG_ENA_AP, p->audio_cfg); @@ -888,14 +888,14 @@ tda998x_encoder_mode_set(struct tda998x_priv *priv, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) { - 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; + u16 ref_pix, ref_line, n_pix, n_line; + u16 hs_pix_s, hs_pix_e; + u16 vs1_pix_s, vs1_pix_e, vs1_line_s, vs1_line_e; + u16 vs2_pix_s, vs2_pix_e, vs2_line_s, vs2_line_e; + u16 vwin1_line_s, vwin1_line_e; + u16 vwin2_line_s, vwin2_line_e; + u16 de_pix_s, de_pix_e; + u8 reg, div, rep;
/* * Internally TDA998x is using ITU-R BT.656 style sync but @@ -1077,7 +1077,7 @@ tda998x_encoder_mode_set(struct tda998x_priv *priv, static enum drm_connector_status tda998x_encoder_detect(struct tda998x_priv *priv) { - uint8_t val = cec_read(priv, REG_CEC_RXSHPDLEV); + u8 val = cec_read(priv, REG_CEC_RXSHPDLEV);
return (val & CEC_RXSHPDLEV_HPD) ? connector_status_connected : connector_status_disconnected; @@ -1086,7 +1086,7 @@ tda998x_encoder_detect(struct tda998x_priv *priv) static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length) { struct tda998x_priv *priv = data; - uint8_t offset, segptr; + u8 offset, segptr; int ret, i;
offset = (blk & 1) ? 128 : 0; @@ -1558,7 +1558,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) struct i2c_client *client = to_i2c_client(dev); struct drm_device *drm = data; struct tda998x_priv2 *priv; - uint32_t crtcs = 0; + u32 crtcs = 0; int ret;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
As reading the interrupt registers clears the outstanding interrupts, we must process all received interrupts to avoid dropping any. Rearrange the code to achieve this, and properly check for a HPD interrupt from the CEC_RXSHPDINT register.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 6d6aaadc0d2f..1285fb354813 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -330,6 +330,8 @@ struct tda998x_priv { # define CEC_FRO_IM_CLK_CTRL_FRO_DIV (1 << 0) #define REG_CEC_RXSHPDINTENA 0xfc /* read/write */ #define REG_CEC_RXSHPDINT 0xfd /* read */ +# define CEC_RXSHPDINT_RXSENS BIT(0) +# define CEC_RXSHPDINT_HPD BIT(1) #define REG_CEC_RXSHPDLEV 0xfe /* read */ # define CEC_RXSHPDLEV_RXSENS (1 << 0) # define CEC_RXSHPDLEV_HPD (1 << 1) @@ -619,11 +621,8 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) DRM_DEBUG_DRIVER( "tda irq sta %02x cec %02x lvl %02x f0 %02x f1 %02x f2 %02x\n", sta, cec, lvl, flag0, flag1, flag2); - if ((flag2 & INT_FLAGS_2_EDID_BLK_RD) && priv->wq_edid_wait) { - priv->wq_edid_wait = 0; - wake_up(&priv->wq_edid); - handled = true; - } else if (cec != 0) { /* HPD change */ + + if (cec & CEC_RXSHPDINT_HPD) { if (lvl & CEC_RXSHPDLEV_HPD) tda998x_edid_delay_start(priv); else @@ -631,6 +630,13 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
handled = true; } + + if ((flag2 & INT_FLAGS_2_EDID_BLK_RD) && priv->wq_edid_wait) { + priv->wq_edid_wait = 0; + wake_up(&priv->wq_edid); + handled = true; + } + return IRQ_RETVAL(handled); }
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 71 +++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 1285fb354813..1c6fc245514f 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -640,66 +640,57 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) return IRQ_RETVAL(handled); }
-static u8 tda998x_cksum(u8 *buf, size_t bytes) -{ - int sum = 0; - - while (bytes--) - sum -= *buf++; - return sum; -} - -#define HB(x) (x) -#define PB(x) (HB(2) + 1 + (x)) - static void tda998x_write_if(struct tda998x_priv *priv, u8 bit, u16 addr, - u8 *buf, size_t size) + union hdmi_infoframe *frame) { + u8 buf[32]; + ssize_t len; + + len = hdmi_infoframe_pack(frame, buf, sizeof(buf)); + if (len < 0) { + dev_err(&priv->hdmi->dev, + "hdmi_infoframe_pack() type=0x%02x failed: %zd\n", + frame->any.type, len); + return; + } + reg_clear(priv, REG_DIP_IF_FLAGS, bit); - reg_write_range(priv, addr, buf, size); + reg_write_range(priv, addr, buf, len); reg_set(priv, REG_DIP_IF_FLAGS, bit); }
static void tda998x_write_aif(struct tda998x_priv *priv, struct tda998x_encoder_params *p) { - u8 buf[PB(HDMI_AUDIO_INFOFRAME_SIZE) + 1]; + union hdmi_infoframe frame; + + hdmi_audio_infoframe_init(&frame.audio);
- memset(buf, 0, sizeof(buf)); - buf[HB(0)] = HDMI_INFOFRAME_TYPE_AUDIO; - buf[HB(1)] = 0x01; - buf[HB(2)] = HDMI_AUDIO_INFOFRAME_SIZE; - 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 */ + frame.audio.channels = p->audio_frame[1] & 0x07; + frame.audio.channel_allocation = p->audio_frame[4]; + frame.audio.level_shift_value = (p->audio_frame[5] & 0x78) >> 3; + frame.audio.downmix_inhibit = (p->audio_frame[5] & 0x80) >> 7;
- buf[PB(0)] = tda998x_cksum(buf, sizeof(buf)); + /* + * L-PCM and IEC61937 compressed audio shall always set sample + * frequency to "refer to stream". For others, see the HDMI + * specification. + */ + frame.audio.sample_frequency = (p->audio_frame[2] & 0x1c) >> 2;
- tda998x_write_if(priv, DIP_IF_FLAGS_IF4, REG_IF4_HB0, buf, - sizeof(buf)); + tda998x_write_if(priv, DIP_IF_FLAGS_IF4, REG_IF4_HB0, &frame); }
static void tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode) { - struct hdmi_avi_infoframe frame; - u8 buf[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE]; - ssize_t len; - - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + union hdmi_infoframe frame;
- frame.quantization_range = HDMI_QUANTIZATION_RANGE_FULL; - - len = hdmi_avi_infoframe_pack(&frame, buf, sizeof(buf)); - if (len < 0) { - dev_err(&priv->hdmi->dev, - "hdmi_avi_infoframe_pack() failed: %zd\n", len); - return; - } + drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode); + frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
- tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, buf, len); + tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame); }
static void tda998x_audio_mute(struct tda998x_priv *priv, bool on)
Remove the DRM slave encoder compatibility from the TDA998x driver. We now use the component helpers to manage the binding of DRM sub-drivers.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 146 +++----------------------------------- 1 file changed, 8 insertions(+), 138 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 1c6fc245514f..883025d35f87 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -23,7 +23,6 @@
#include <drm/drmP.h> #include <drm/drm_crtc_helper.h> -#include <drm/drm_encoder_slave.h> #include <drm/drm_edid.h> #include <drm/drm_of.h> #include <drm/i2c/tda998x.h> @@ -53,8 +52,6 @@ struct tda998x_priv { bool edid_delay_active; };
-#define to_tda998x_priv(x) ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv) - /* The TDA9988 series of devices use a paged register scheme.. to simplify * things we encode the page # in upper bits of the register #. To read/ * write a given register, we need to make sure CURPAGE register is set @@ -1182,16 +1179,6 @@ static void tda998x_encoder_set_polling(struct tda998x_priv *priv, DRM_CONNECTOR_POLL_DISCONNECT; }
-static int -tda998x_encoder_set_property(struct drm_encoder *encoder, - struct drm_connector *connector, - struct drm_property *property, - uint64_t val) -{ - DBG(""); - return 0; -} - static void tda998x_destroy(struct tda998x_priv *priv) { /* disable all IRQs and free the IRQ handler */ @@ -1207,78 +1194,6 @@ static void tda998x_destroy(struct tda998x_priv *priv) i2c_unregister_device(priv->cec); }
-/* Slave encoder support */ - -static void -tda998x_encoder_slave_set_config(struct drm_encoder *encoder, void *params) -{ - tda998x_encoder_set_config(to_tda998x_priv(encoder), params); -} - -static void tda998x_encoder_slave_destroy(struct drm_encoder *encoder) -{ - struct tda998x_priv *priv = to_tda998x_priv(encoder); - - tda998x_destroy(priv); - drm_i2c_encoder_destroy(encoder); - kfree(priv); -} - -static void tda998x_encoder_slave_dpms(struct drm_encoder *encoder, int mode) -{ - tda998x_encoder_dpms(to_tda998x_priv(encoder), mode); -} - -static int tda998x_encoder_slave_mode_valid(struct drm_encoder *encoder, - struct drm_display_mode *mode) -{ - return tda998x_encoder_mode_valid(to_tda998x_priv(encoder), mode); -} - -static void -tda998x_encoder_slave_mode_set(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) -{ - tda998x_encoder_mode_set(to_tda998x_priv(encoder), mode, adjusted_mode); -} - -static enum drm_connector_status -tda998x_encoder_slave_detect(struct drm_encoder *encoder, - struct drm_connector *connector) -{ - return tda998x_encoder_detect(to_tda998x_priv(encoder)); -} - -static int tda998x_encoder_slave_get_modes(struct drm_encoder *encoder, - struct drm_connector *connector) -{ - return tda998x_encoder_get_modes(to_tda998x_priv(encoder), connector); -} - -static int -tda998x_encoder_slave_create_resources(struct drm_encoder *encoder, - struct drm_connector *connector) -{ - tda998x_encoder_set_polling(to_tda998x_priv(encoder), connector); - return 0; -} - -static struct drm_encoder_slave_funcs tda998x_encoder_slave_funcs = { - .set_config = tda998x_encoder_slave_set_config, - .destroy = tda998x_encoder_slave_destroy, - .dpms = tda998x_encoder_slave_dpms, - .save = tda998x_encoder_save, - .restore = tda998x_encoder_restore, - .mode_fixup = tda998x_encoder_mode_fixup, - .mode_valid = tda998x_encoder_slave_mode_valid, - .mode_set = tda998x_encoder_slave_mode_set, - .detect = tda998x_encoder_slave_detect, - .get_modes = tda998x_encoder_slave_get_modes, - .create_resources = tda998x_encoder_slave_create_resources, - .set_property = tda998x_encoder_set_property, -}; - /* I2C driver functions */
static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) @@ -1413,31 +1328,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) return -ENXIO; }
-static int tda998x_encoder_init(struct i2c_client *client, - struct drm_device *dev, - struct drm_encoder_slave *encoder_slave) -{ - struct tda998x_priv *priv; - int ret; - - priv = kzalloc(sizeof(*priv), GFP_KERNEL); - if (!priv) - return -ENOMEM; - - priv->encoder = &encoder_slave->base; - - ret = tda998x_create(client, priv); - if (ret) { - kfree(priv); - return ret; - } - - encoder_slave->slave_priv = priv; - encoder_slave->slave_funcs = &tda998x_encoder_slave_funcs; - - return 0; -} - struct tda998x_priv2 { struct tda998x_priv base; struct drm_encoder encoder; @@ -1659,38 +1549,18 @@ static struct i2c_device_id tda998x_ids[] = { }; MODULE_DEVICE_TABLE(i2c, tda998x_ids);
-static struct drm_i2c_encoder_driver tda998x_driver = { - .i2c_driver = { - .probe = tda998x_probe, - .remove = tda998x_remove, - .driver = { - .name = "tda998x", - .of_match_table = of_match_ptr(tda998x_dt_ids), - }, - .id_table = tda998x_ids, +static struct i2c_driver tda998x_driver = { + .probe = tda998x_probe, + .remove = tda998x_remove, + .driver = { + .name = "tda998x", + .of_match_table = of_match_ptr(tda998x_dt_ids), }, - .encoder_init = tda998x_encoder_init, + .id_table = tda998x_ids, };
-/* Module initialization */ - -static int __init -tda998x_init(void) -{ - DBG(""); - return drm_i2c_encoder_register(THIS_MODULE, &tda998x_driver); -} - -static void __exit -tda998x_exit(void) -{ - DBG(""); - drm_i2c_encoder_unregister(&tda998x_driver); -} +module_i2c_driver(tda998x_driver);
MODULE_AUTHOR("Rob Clark <robdclark@gmail.com"); MODULE_DESCRIPTION("NXP Semiconductors TDA998X HDMI Encoder"); MODULE_LICENSE("GPL"); - -module_init(tda998x_init); -module_exit(tda998x_exit);
Remove the encoder pointer from struct tda998x_priv, moving the encoder itself from struct tda998x_priv2 here.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 883025d35f87..e30a2a8c2a3c 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -44,12 +44,13 @@ struct tda998x_priv {
wait_queue_head_t wq_edid; volatile int wq_edid_wait; - struct drm_encoder *encoder;
struct work_struct detect_work; struct timer_list edid_delay_timer; wait_queue_head_t edid_delay_waitq; bool edid_delay_active; + + struct drm_encoder encoder; };
/* The TDA9988 series of devices use a paged register scheme.. to simplify @@ -594,7 +595,7 @@ static void tda998x_detect_work(struct work_struct *work) { struct tda998x_priv *priv = container_of(work, struct tda998x_priv, detect_work); - struct drm_device *dev = priv->encoder->dev; + struct drm_device *dev = priv->encoder.dev;
if (dev) drm_kms_helper_hotplug_event(dev); @@ -1330,7 +1331,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
struct tda998x_priv2 { struct tda998x_priv base; - struct drm_encoder encoder; struct drm_connector connector; };
@@ -1338,7 +1338,7 @@ struct tda998x_priv2 { container_of(x, struct tda998x_priv2, connector);
#define enc_to_tda998x_priv2(x) \ - container_of(x, struct tda998x_priv2, encoder); + container_of(x, struct tda998x_priv2, base.encoder);
static void tda998x_encoder2_dpms(struct drm_encoder *encoder, int mode) { @@ -1408,7 +1408,7 @@ tda998x_connector_best_encoder(struct drm_connector *connector) { struct tda998x_priv2 *priv = conn_to_tda998x_priv2(connector);
- return &priv->encoder; + return &priv->base.encoder; }
static @@ -1463,9 +1463,8 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) crtcs = 1 << 0; }
- priv->base.encoder = &priv->encoder; priv->connector.interlace_allowed = 1; - priv->encoder.possible_crtcs = crtcs; + priv->base.encoder.possible_crtcs = crtcs;
ret = tda998x_create(client, &priv->base); if (ret) @@ -1476,8 +1475,8 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
tda998x_encoder_set_polling(&priv->base, &priv->connector);
- drm_encoder_helper_add(&priv->encoder, &tda998x_encoder_helper_funcs); - ret = drm_encoder_init(drm, &priv->encoder, &tda998x_encoder_funcs, + drm_encoder_helper_add(&priv->base.encoder, &tda998x_encoder_helper_funcs); + ret = drm_encoder_init(drm, &priv->base.encoder, &tda998x_encoder_funcs, DRM_MODE_ENCODER_TMDS); if (ret) goto err_encoder; @@ -1494,15 +1493,15 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) if (ret) goto err_sysfs;
- priv->connector.encoder = &priv->encoder; - drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder); + priv->connector.encoder = &priv->base.encoder; + drm_mode_connector_attach_encoder(&priv->connector, &priv->base.encoder);
return 0;
err_sysfs: drm_connector_cleanup(&priv->connector); err_connector: - drm_encoder_cleanup(&priv->encoder); + drm_encoder_cleanup(&priv->base.encoder); err_encoder: tda998x_destroy(&priv->base); return ret; @@ -1514,7 +1513,7 @@ static void tda998x_unbind(struct device *dev, struct device *master, struct tda998x_priv2 *priv = dev_get_drvdata(dev);
drm_connector_cleanup(&priv->connector); - drm_encoder_cleanup(&priv->encoder); + drm_encoder_cleanup(&priv->base.encoder); tda998x_destroy(&priv->base); }
Move the DRM connector structure into struct tda998x_priv from the old struct tda998x_priv2.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index e30a2a8c2a3c..a2a463cec244 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -51,6 +51,7 @@ struct tda998x_priv { bool edid_delay_active;
struct drm_encoder encoder; + struct drm_connector connector; };
/* The TDA9988 series of devices use a paged register scheme.. to simplify @@ -1331,11 +1332,10 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
struct tda998x_priv2 { struct tda998x_priv base; - struct drm_connector connector; };
#define conn_to_tda998x_priv2(x) \ - container_of(x, struct tda998x_priv2, connector); + container_of(x, struct tda998x_priv2, base.connector);
#define enc_to_tda998x_priv2(x) \ container_of(x, struct tda998x_priv2, base.encoder); @@ -1463,7 +1463,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) crtcs = 1 << 0; }
- priv->connector.interlace_allowed = 1; + priv->base.connector.interlace_allowed = 1; priv->base.encoder.possible_crtcs = crtcs;
ret = tda998x_create(client, &priv->base); @@ -1473,7 +1473,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) if (!dev->of_node && params) tda998x_encoder_set_config(&priv->base, params);
- tda998x_encoder_set_polling(&priv->base, &priv->connector); + tda998x_encoder_set_polling(&priv->base, &priv->base.connector);
drm_encoder_helper_add(&priv->base.encoder, &tda998x_encoder_helper_funcs); ret = drm_encoder_init(drm, &priv->base.encoder, &tda998x_encoder_funcs, @@ -1481,25 +1481,25 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) if (ret) goto err_encoder;
- drm_connector_helper_add(&priv->connector, + drm_connector_helper_add(&priv->base.connector, &tda998x_connector_helper_funcs); - ret = drm_connector_init(drm, &priv->connector, + ret = drm_connector_init(drm, &priv->base.connector, &tda998x_connector_funcs, DRM_MODE_CONNECTOR_HDMIA); if (ret) goto err_connector;
- ret = drm_connector_register(&priv->connector); + ret = drm_connector_register(&priv->base.connector); if (ret) goto err_sysfs;
- priv->connector.encoder = &priv->base.encoder; - drm_mode_connector_attach_encoder(&priv->connector, &priv->base.encoder); + priv->base.connector.encoder = &priv->base.encoder; + drm_mode_connector_attach_encoder(&priv->base.connector, &priv->base.encoder);
return 0;
err_sysfs: - drm_connector_cleanup(&priv->connector); + drm_connector_cleanup(&priv->base.connector); err_connector: drm_encoder_cleanup(&priv->base.encoder); err_encoder: @@ -1512,7 +1512,7 @@ static void tda998x_unbind(struct device *dev, struct device *master, { struct tda998x_priv2 *priv = dev_get_drvdata(dev);
- drm_connector_cleanup(&priv->connector); + drm_connector_cleanup(&priv->base.connector); drm_encoder_cleanup(&priv->base.encoder); tda998x_destroy(&priv->base); }
Kill the redundant tda998x_priv2 structure now that its only member is the struct tda998x_priv.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 80 +++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index a2a463cec244..8bca9155ee18 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1330,21 +1330,17 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) return -ENXIO; }
-struct tda998x_priv2 { - struct tda998x_priv base; -}; - -#define conn_to_tda998x_priv2(x) \ - container_of(x, struct tda998x_priv2, base.connector); +#define conn_to_tda998x_priv(x) \ + container_of(x, struct tda998x_priv, connector);
-#define enc_to_tda998x_priv2(x) \ - container_of(x, struct tda998x_priv2, base.encoder); +#define enc_to_tda998x_priv(x) \ + container_of(x, struct tda998x_priv, encoder);
static void tda998x_encoder2_dpms(struct drm_encoder *encoder, int mode) { - struct tda998x_priv2 *priv = enc_to_tda998x_priv2(encoder); + struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
- tda998x_encoder_dpms(&priv->base, mode); + tda998x_encoder_dpms(priv, mode); }
static void tda998x_encoder_prepare(struct drm_encoder *encoder) @@ -1361,9 +1357,9 @@ static void tda998x_encoder2_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) { - struct tda998x_priv2 *priv = enc_to_tda998x_priv2(encoder); + struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
- tda998x_encoder_mode_set(&priv->base, mode, adjusted_mode); + tda998x_encoder_mode_set(priv, mode, adjusted_mode); }
static const struct drm_encoder_helper_funcs tda998x_encoder_helper_funcs = { @@ -1378,9 +1374,9 @@ static const struct drm_encoder_helper_funcs tda998x_encoder_helper_funcs = {
static void tda998x_encoder_destroy(struct drm_encoder *encoder) { - struct tda998x_priv2 *priv = enc_to_tda998x_priv2(encoder); + struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
- tda998x_destroy(&priv->base); + tda998x_destroy(priv); drm_encoder_cleanup(encoder); }
@@ -1390,25 +1386,25 @@ static const struct drm_encoder_funcs tda998x_encoder_funcs = {
static int tda998x_connector_get_modes(struct drm_connector *connector) { - struct tda998x_priv2 *priv = conn_to_tda998x_priv2(connector); + struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
- return tda998x_encoder_get_modes(&priv->base, connector); + return tda998x_encoder_get_modes(priv, connector); }
static int tda998x_connector_mode_valid(struct drm_connector *connector, struct drm_display_mode *mode) { - struct tda998x_priv2 *priv = conn_to_tda998x_priv2(connector); + struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
- return tda998x_encoder_mode_valid(&priv->base, mode); + return tda998x_encoder_mode_valid(priv, mode); }
static struct drm_encoder * tda998x_connector_best_encoder(struct drm_connector *connector) { - struct tda998x_priv2 *priv = conn_to_tda998x_priv2(connector); + struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
- return &priv->base.encoder; + return &priv->encoder; }
static @@ -1421,9 +1417,9 @@ const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = { static enum drm_connector_status tda998x_connector_detect(struct drm_connector *connector, bool force) { - struct tda998x_priv2 *priv = conn_to_tda998x_priv2(connector); + struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
- return tda998x_encoder_detect(&priv->base); + return tda998x_encoder_detect(priv); }
static void tda998x_connector_destroy(struct drm_connector *connector) @@ -1444,7 +1440,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) struct tda998x_encoder_params *params = dev->platform_data; struct i2c_client *client = to_i2c_client(dev); struct drm_device *drm = data; - struct tda998x_priv2 *priv; + struct tda998x_priv *priv; u32 crtcs = 0; int ret;
@@ -1463,58 +1459,58 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) crtcs = 1 << 0; }
- priv->base.connector.interlace_allowed = 1; - priv->base.encoder.possible_crtcs = crtcs; + priv->connector.interlace_allowed = 1; + priv->encoder.possible_crtcs = crtcs;
- ret = tda998x_create(client, &priv->base); + ret = tda998x_create(client, priv); if (ret) return ret;
if (!dev->of_node && params) - tda998x_encoder_set_config(&priv->base, params); + tda998x_encoder_set_config(priv, params);
- tda998x_encoder_set_polling(&priv->base, &priv->base.connector); + tda998x_encoder_set_polling(priv, &priv->connector);
- drm_encoder_helper_add(&priv->base.encoder, &tda998x_encoder_helper_funcs); - ret = drm_encoder_init(drm, &priv->base.encoder, &tda998x_encoder_funcs, + drm_encoder_helper_add(&priv->encoder, &tda998x_encoder_helper_funcs); + ret = drm_encoder_init(drm, &priv->encoder, &tda998x_encoder_funcs, DRM_MODE_ENCODER_TMDS); if (ret) goto err_encoder;
- drm_connector_helper_add(&priv->base.connector, + drm_connector_helper_add(&priv->connector, &tda998x_connector_helper_funcs); - ret = drm_connector_init(drm, &priv->base.connector, + ret = drm_connector_init(drm, &priv->connector, &tda998x_connector_funcs, DRM_MODE_CONNECTOR_HDMIA); if (ret) goto err_connector;
- ret = drm_connector_register(&priv->base.connector); + ret = drm_connector_register(&priv->connector); if (ret) goto err_sysfs;
- priv->base.connector.encoder = &priv->base.encoder; - drm_mode_connector_attach_encoder(&priv->base.connector, &priv->base.encoder); + priv->connector.encoder = &priv->encoder; + drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
return 0;
err_sysfs: - drm_connector_cleanup(&priv->base.connector); + drm_connector_cleanup(&priv->connector); err_connector: - drm_encoder_cleanup(&priv->base.encoder); + drm_encoder_cleanup(&priv->encoder); err_encoder: - tda998x_destroy(&priv->base); + tda998x_destroy(priv); return ret; }
static void tda998x_unbind(struct device *dev, struct device *master, void *data) { - struct tda998x_priv2 *priv = dev_get_drvdata(dev); + struct tda998x_priv *priv = dev_get_drvdata(dev);
- drm_connector_cleanup(&priv->base.connector); - drm_encoder_cleanup(&priv->base.encoder); - tda998x_destroy(&priv->base); + drm_connector_cleanup(&priv->connector); + drm_encoder_cleanup(&priv->encoder); + tda998x_destroy(priv); }
static const struct component_ops tda998x_ops = {
We can now kill a number of glue functions which were sitting between the common tda998x code and the drm encoder/connector methods. This results in slightly cleaner code.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/i2c/tda998x_drv.c | 80 +++++++++++---------------------------- 1 file changed, 22 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 8bca9155ee18..896b6aaf8c4d 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -54,6 +54,12 @@ struct tda998x_priv { struct drm_connector connector; };
+#define conn_to_tda998x_priv(x) \ + container_of(x, struct tda998x_priv, connector) + +#define enc_to_tda998x_priv(x) \ + container_of(x, struct tda998x_priv, encoder) + /* The TDA9988 series of devices use a paged register scheme.. to simplify * things we encode the page # in upper bits of the register #. To read/ * write a given register, we need to make sure CURPAGE register is set @@ -562,7 +568,7 @@ tda998x_reset(struct tda998x_priv *priv) * trying to read EDID data. * * However, tda998x_encoder_get_modes() may be called at any moment - * after tda998x_encoder_detect() indicates that we are connected, so + * after tda998x_connector_detect() indicates that we are connected, so * we need to delay probing modes in tda998x_encoder_get_modes() after * we have seen a HPD inactive->active transition. This code implements * that delay. @@ -816,8 +822,10 @@ static void tda998x_encoder_set_config(struct tda998x_priv *priv, priv->params = *p; }
-static void tda998x_encoder_dpms(struct tda998x_priv *priv, int mode) +static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) { + struct tda998x_priv *priv = enc_to_tda998x_priv(encoder); + /* we only care about on or off: */ if (mode != DRM_MODE_DPMS_ON) mode = DRM_MODE_DPMS_OFF; @@ -867,8 +875,8 @@ tda998x_encoder_mode_fixup(struct drm_encoder *encoder, return true; }
-static int tda998x_encoder_mode_valid(struct tda998x_priv *priv, - struct drm_display_mode *mode) +static int tda998x_connector_mode_valid(struct drm_connector *connector, + struct drm_display_mode *mode) { if (mode->clock > 150000) return MODE_CLOCK_HIGH; @@ -880,10 +888,11 @@ static int tda998x_encoder_mode_valid(struct tda998x_priv *priv, }
static void -tda998x_encoder_mode_set(struct tda998x_priv *priv, +tda998x_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) { + struct tda998x_priv *priv = enc_to_tda998x_priv(encoder); u16 ref_pix, ref_line, n_pix, n_line; u16 hs_pix_s, hs_pix_e; u16 vs1_pix_s, vs1_pix_e, vs1_line_s, vs1_line_e; @@ -1071,8 +1080,9 @@ tda998x_encoder_mode_set(struct tda998x_priv *priv, }
static enum drm_connector_status -tda998x_encoder_detect(struct tda998x_priv *priv) +tda998x_connector_detect(struct drm_connector *connector, bool force) { + struct tda998x_priv *priv = conn_to_tda998x_priv(connector); u8 val = cec_read(priv, REG_CEC_RXSHPDLEV);
return (val & CEC_RXSHPDLEV_HPD) ? connector_status_connected : @@ -1135,10 +1145,9 @@ static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length) return 0; }
-static int -tda998x_encoder_get_modes(struct tda998x_priv *priv, - struct drm_connector *connector) +static int tda998x_connector_get_modes(struct drm_connector *connector) { + struct tda998x_priv *priv = conn_to_tda998x_priv(connector); struct edid *edid; int n;
@@ -1330,46 +1339,24 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) return -ENXIO; }
-#define conn_to_tda998x_priv(x) \ - container_of(x, struct tda998x_priv, connector); - -#define enc_to_tda998x_priv(x) \ - container_of(x, struct tda998x_priv, encoder); - -static void tda998x_encoder2_dpms(struct drm_encoder *encoder, int mode) -{ - struct tda998x_priv *priv = enc_to_tda998x_priv(encoder); - - tda998x_encoder_dpms(priv, mode); -} - static void tda998x_encoder_prepare(struct drm_encoder *encoder) { - tda998x_encoder2_dpms(encoder, DRM_MODE_DPMS_OFF); + tda998x_encoder_dpms(encoder, DRM_MODE_DPMS_OFF); }
static void tda998x_encoder_commit(struct drm_encoder *encoder) { - tda998x_encoder2_dpms(encoder, DRM_MODE_DPMS_ON); -} - -static void tda998x_encoder2_mode_set(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) -{ - struct tda998x_priv *priv = enc_to_tda998x_priv(encoder); - - tda998x_encoder_mode_set(priv, mode, adjusted_mode); + tda998x_encoder_dpms(encoder, DRM_MODE_DPMS_ON); }
static const struct drm_encoder_helper_funcs tda998x_encoder_helper_funcs = { - .dpms = tda998x_encoder2_dpms, + .dpms = tda998x_encoder_dpms, .save = tda998x_encoder_save, .restore = tda998x_encoder_restore, .mode_fixup = tda998x_encoder_mode_fixup, .prepare = tda998x_encoder_prepare, .commit = tda998x_encoder_commit, - .mode_set = tda998x_encoder2_mode_set, + .mode_set = tda998x_encoder_mode_set, };
static void tda998x_encoder_destroy(struct drm_encoder *encoder) @@ -1384,21 +1371,6 @@ static const struct drm_encoder_funcs tda998x_encoder_funcs = { .destroy = tda998x_encoder_destroy, };
-static int tda998x_connector_get_modes(struct drm_connector *connector) -{ - struct tda998x_priv *priv = conn_to_tda998x_priv(connector); - - return tda998x_encoder_get_modes(priv, connector); -} - -static int tda998x_connector_mode_valid(struct drm_connector *connector, - struct drm_display_mode *mode) -{ - struct tda998x_priv *priv = conn_to_tda998x_priv(connector); - - return tda998x_encoder_mode_valid(priv, mode); -} - static struct drm_encoder * tda998x_connector_best_encoder(struct drm_connector *connector) { @@ -1414,14 +1386,6 @@ const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = { .best_encoder = tda998x_connector_best_encoder, };
-static enum drm_connector_status -tda998x_connector_detect(struct drm_connector *connector, bool force) -{ - struct tda998x_priv *priv = conn_to_tda998x_priv(connector); - - return tda998x_encoder_detect(priv); -} - static void tda998x_connector_destroy(struct drm_connector *connector) { drm_connector_unregister(connector);
On Tue, Sep 29, 2015 at 07:32:16PM +0100, Russell King - ARM Linux wrote:
Hi,
Here's my currently queued TDA998x development work for 4.4.
As there have been no comments against this series, I'll send David a pull request later today.
- Remove some useless NULL checks here variables can't be NULL.
- Return IRQ_HANDLED only if we handled the IRQ, otherwise return IRQ_NONE. This avoids locking the system up if the IRQ gets stuck.
- Re-implement a previous patch "Fix EDID read timeout on HDMI connect" to be much more reliable: an attempt to read the EDID may come in while we're delaying the HPD detect event, violating the critical pause.
- Use Linux types rather than C99 types.
- Handle all outstanding interrupts, rather than only the first interrupt that we discover pending.
- Use more helpers from drivers/video/hdmi.c - this removes our own infoframe checksumming code.
- Remove DRM slave encoder support (which I think no one is using anymore.) This allows other tidy-ups, removing the abstractions to allow slave encoder support to co-exist with component support.
drivers/gpu/drm/i2c/tda998x_drv.c | 487 +++++++++++++++----------------------- 1 file changed, 185 insertions(+), 302 deletions(-)
-- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
On Fri, 9 Oct 2015 14:25:26 +0100 Russell King - ARM Linux linux@arm.linux.org.uk wrote:
Here's my currently queued TDA998x development work for 4.4.
As there have been no comments against this series, I'll send David a pull request later today.
It works fine for me. Thanks.
Tested-by: Jean-Francois Moine moinejf@free.fr
dri-devel@lists.freedesktop.org