[PATCH 00/11] tda998x updates

[PATCH 00/11] tda998x updates

[Russell King - ARM Linux]

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(-)

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 01/11] drm/i2c: tda998x: remove useless NULL checks

[Russell King]

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);
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/11] drm/i2c: tda998x: report whether we actually handled the IRQ

[Russell King]

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)
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/11] drm/i2c: tda998x: re-implement "Fix EDID read timeout on HDMI connect"

[Russell King]

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);
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/11] drm/i2c: tda998x: convert to u8/u16/u32 types

[Russell King]

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);
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/11] drm/i2c: tda998x: handle all outstanding interrupts

[Russell King]

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);
 }
 
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/11] drm/i2c: tda998x: use more HDMI helpers

[Russell King]

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)
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/11] drm/i2c: tda998x: remove DRM slave encoder support

[Russell King]

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);
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/11] drm/i2c: tda998x: remove encoder pointer

[Russell King]

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);
 }
 
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/11] drm/i2c: tda998x: move connector into struct tda998x_priv

[Russell King]

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);
 }
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/11] drm/i2c: tda998x: kill struct tda998x_priv2

[Russell King]

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 = {
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/11] drm/i2c: tda998x: clean up after struct tda998x_priv2 removal

[Russell King]

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);
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/11] tda998x updates

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/11] tda998x updates

[Jean-Francois Moine]

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>

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel