Re: [PATCH v2 6/7] drm/i2c: tda998x: add CEC support

Hi Russell,

Thanks for this patch series!

On 12/06/17 13:35, Russell King wrote:

The TDA998x is a HDMI transmitter with a TDA9950 CEC engine integrated onto the same die. Add support for the TDA9950 CEC engine to the TDA998x driver.

Signed-off-by: Russell King rmk+kernel@armlinux.org.uk

drivers/gpu/drm/i2c/Kconfig | 1 + drivers/gpu/drm/i2c/tda998x_drv.c | 209 +++++++++++++++++++++++++++++++++++--- 2 files changed, 196 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig index 3a232f5ff0a1..096d2139e733 100644 --- a/drivers/gpu/drm/i2c/Kconfig +++ b/drivers/gpu/drm/i2c/Kconfig @@ -22,6 +22,7 @@ config DRM_I2C_SIL164 config DRM_I2C_NXP_TDA998X tristate "NXP Semiconductors TDA998X HDMI encoder" default m if DRM_TILCDC

• select CEC_NOTIFIER

I believe this should be 'select CEC_CORE if CEC_NOTIFIER', conform the other drivers that do something similar.

Otherwise if tda9950 is configured as a module, and this as built-in, then cec is built as a module as well and this can't find the cec functions from the module.

Regards,

Hans

select SND_SOC_HDMI_CODEC if SND_SOC help Support for NXP Semiconductors TDA998X HDMI encoders. diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index e294f5b50236..3ad39d018ab6 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -16,8 +16,10 @@ */

#include <linux/component.h> +#include <linux/gpio/consumer.h> #include <linux/hdmi.h> #include <linux/module.h> +#include <linux/platform_data/tda9950.h> #include <linux/irq.h> #include <sound/asoundef.h> #include <sound/hdmi-codec.h> @@ -29,6 +31,8 @@ #include <drm/drm_of.h> #include <drm/i2c/tda998x.h>

+#include <media/cec-notifier.h>

#define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)

struct tda998x_audio_port { @@ -55,6 +59,7 @@ struct tda998x_priv { struct platform_device *audio_pdev; struct mutex audio_mutex;

• struct mutex edid_mutex; wait_queue_head_t wq_edid; volatile int wq_edid_wait;

@@ -67,6 +72,9 @@ struct tda998x_priv { struct drm_connector connector;

struct tda998x_audio_port audio_port[2];

• struct tda9950_glue cec_glue;
• struct gpio_desc *calib;
• struct cec_notifier *cec_notify;

};

#define conn_to_tda998x_priv(x) \ @@ -345,6 +353,12 @@ struct tda998x_priv { #define REG_CEC_INTSTATUS 0xee /* read */ # define CEC_INTSTATUS_CEC (1 << 0) # define CEC_INTSTATUS_HDMI (1 << 1) +#define REG_CEC_CAL_XOSC_CTRL1 0xf2 +# define CEC_CAL_XOSC_CTRL1_ENA_CAL BIT(0) +#define REG_CEC_DES_FREQ2 0xf5 +# define CEC_DES_FREQ2_DIS_AUTOCAL BIT(7) +#define REG_CEC_CLK 0xf6 +# define CEC_CLK_FRO 0x11 #define REG_CEC_FRO_IM_CLK_CTRL 0xfb /* read/write */ # define CEC_FRO_IM_CLK_CTRL_GHOST_DIS (1 << 7) # define CEC_FRO_IM_CLK_CTRL_ENA_OTP (1 << 6) @@ -359,6 +373,7 @@ struct tda998x_priv { # define CEC_RXSHPDLEV_HPD (1 << 1)

#define REG_CEC_ENAMODS 0xff /* read/write */ +# define CEC_ENAMODS_EN_CEC_CLK (1 << 7) # define CEC_ENAMODS_DIS_FRO (1 << 6) # define CEC_ENAMODS_DIS_CCLK (1 << 5) # define CEC_ENAMODS_EN_RXSENS (1 << 2) @@ -417,6 +432,114 @@ cec_read(struct tda998x_priv *priv, u8 addr) return val; }

+static void cec_enamods(struct tda998x_priv *priv, u8 mods, bool enable) +{

• int val = cec_read(priv, REG_CEC_ENAMODS);
• if (val < 0)

• return;
  

• if (enable)

• val |= mods;
  

• else

• val &= ~mods;
  

• cec_write(priv, REG_CEC_ENAMODS, val);

+}

+static void tda998x_cec_set_calibration(struct tda998x_priv *priv, bool enable) +{

• if (enable) {

• u8 val;
  

• cec_write(priv, 0xf3, 0xc0);
  

• cec_write(priv, 0xf4, 0xd4);
  

• /* Enable automatic calibration mode */
  

• val = cec_read(priv, REG_CEC_DES_FREQ2);
  

• val &= ~CEC_DES_FREQ2_DIS_AUTOCAL;
  

• cec_write(priv, REG_CEC_DES_FREQ2, val);
  

• /* Enable free running oscillator */
  

• cec_write(priv, REG_CEC_CLK, CEC_CLK_FRO);
  

• cec_enamods(priv, CEC_ENAMODS_DIS_FRO, false);
  

• cec_write(priv, REG_CEC_CAL_XOSC_CTRL1,
  

• 	  CEC_CAL_XOSC_CTRL1_ENA_CAL);
  

• } else {

• cec_write(priv, REG_CEC_CAL_XOSC_CTRL1, 0);
  

• }

+}

+/*

• • Calibration for the internal oscillator: we need to set calibration mode,
• • and then pulse the IRQ line low for a 10ms ± 1% period.
• */

+static void tda998x_cec_calibration(struct tda998x_priv *priv) +{

• struct gpio_desc *calib = priv->calib;
• mutex_lock(&priv->edid_mutex);
• if (priv->hdmi->irq > 0)

• disable_irq(priv->hdmi->irq);
  

• gpiod_direction_output(calib, 1);
• tda998x_cec_set_calibration(priv, true);
• local_irq_disable();
• gpiod_set_value(calib, 0);
• mdelay(10);
• gpiod_set_value(calib, 1);
• local_irq_enable();
• tda998x_cec_set_calibration(priv, false);
• gpiod_direction_input(calib);
• if (priv->hdmi->irq > 0)

• enable_irq(priv->hdmi->irq);
  

• mutex_unlock(&priv->edid_mutex);

+}

+static int tda998x_cec_hook_init(void *data) +{

• struct tda998x_priv *priv = data;
• struct gpio_desc *calib;
• calib = gpiod_get(&priv->hdmi->dev, "nxp,calib", GPIOD_ASIS);
• if (IS_ERR(calib)) {

• dev_warn(&priv->hdmi->dev, "failed to get calibration gpio: %ld\n",
  

• 	 PTR_ERR(calib));
  

• return PTR_ERR(calib);
  

• }
• priv->calib = calib;
• return 0;

+}

+static void tda998x_cec_hook_exit(void *data) +{

• struct tda998x_priv *priv = data;
• gpiod_put(priv->calib);
• priv->calib = NULL;

+}

+static int tda998x_cec_hook_open(void *data) +{

• struct tda998x_priv *priv = data;
• cec_enamods(priv, CEC_ENAMODS_EN_CEC_CLK | CEC_ENAMODS_EN_CEC, true);
• tda998x_cec_calibration(priv);
• return 0;

+}

+static void tda998x_cec_hook_release(void *data) +{

• struct tda998x_priv *priv = data;
• cec_enamods(priv, CEC_ENAMODS_EN_CEC_CLK | CEC_ENAMODS_EN_CEC, false);

+}

static int set_page(struct tda998x_priv *priv, u16 reg) { @@ -657,10 +780,13 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) sta, cec, lvl, flag0, flag1, flag2);

if (cec & CEC_RXSHPDINT_HPD) {

• 	if (lvl & CEC_RXSHPDLEV_HPD)
  

• 	if (lvl & CEC_RXSHPDLEV_HPD) {
  	tda998x_edid_delay_start(priv);
  

• 	else
  

• 	} else {
  	schedule_work(&priv->detect_work);
  

• 		cec_notifier_set_phys_addr(priv->cec_notify,
  

• 				   CEC_PHYS_ADDR_INVALID);
  

• 	}
  
  handled = true;
  

  }

@@ -981,6 +1107,8 @@ static int tda998x_connector_fill_modes(struct drm_connector *connector, if (connector->edid_blob_ptr) { struct edid *edid = (void *)connector->edid_blob_ptr->data;

• cec_notifier_set_phys_addr_from_edid(priv->cec_notify, edid);
  

• priv->sink_has_audio = drm_detect_monitor_audio(edid); } else { priv->sink_has_audio = false;

@@ -1024,6 +1152,8 @@ static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length) offset = (blk & 1) ? 128 : 0; segptr = blk / 2;

• mutex_lock(&priv->edid_mutex);
• reg_write(priv, REG_DDC_ADDR, 0xa0); reg_write(priv, REG_DDC_OFFS, offset); reg_write(priv, REG_DDC_SEGM_ADDR, 0x60);

@@ -1043,14 +1173,15 @@ static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length) msecs_to_jiffies(100)); if (i < 0) { dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i);

• 	return i;
  

• 	ret = i;
  

• 	goto failed;
  

  } } else { for (i = 100; i > 0; i--) { msleep(1); ret = reg_read(priv, REG_INT_FLAGS_2); if (ret < 0)

• 		return ret;
  

• 		goto failed;
  if (ret & INT_FLAGS_2_EDID_BLK_RD)
  	break;
  

  }

@@ -1058,17 +1189,22 @@ static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)

if (i == 0) { dev_err(&priv->hdmi->dev, "read edid timeout\n");

• return -ETIMEDOUT;
  

• ret = -ETIMEDOUT;
  

• goto failed;
  

  }

  ret = reg_read_range(priv, REG_EDID_DATA_0, buf, length); if (ret != length) { dev_err(&priv->hdmi->dev, "failed to read edid block %d: %d\n", blk, ret);

• return ret;
  

• goto failed;
  

  }

• return 0;

• ret = 0;
• failed:
• mutex_unlock(&priv->edid_mutex);
• return ret;

}

static int tda998x_connector_get_modes(struct drm_connector *connector) @@ -1424,6 +1560,9 @@ static void tda998x_destroy(struct tda998x_priv *priv) cancel_work_sync(&priv->detect_work);

i2c_unregister_device(priv->cec);

• if (priv->cec_notify)

• cec_notifier_put(priv->cec_notify);
  

}

/* I2C driver functions */ @@ -1473,11 +1612,13 @@ static int tda998x_get_audio_ports(struct tda998x_priv *priv, static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) { struct device_node *np = client->dev.of_node;

• struct i2c_board_info cec_info; u32 video; int rev_lo, rev_hi, ret;

  mutex_init(&priv->mutex); /* protect the page access */ mutex_init(&priv->audio_mutex); /* protect access from audio thread */

• mutex_init(&priv->edid_mutex); init_waitqueue_head(&priv->edid_delay_waitq); timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0); INIT_WORK(&priv->detect_work, tda998x_detect_work);

@@ -1556,11 +1697,8 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) reg_read(priv, REG_INT_FLAGS_1); reg_read(priv, REG_INT_FLAGS_2);

• /* initialize the optional IRQ */
• priv->cec = i2c_new_dummy(client->adapter, priv->cec_addr);
• if (!priv->cec)

• return -ENODEV;
  
  

• /* initialize the optional IRQ */ if (client->irq) { unsigned long irq_flags;

@@ -1569,6 +1707,9 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)

irq_flags =
	irqd_get_trigger_type(irq_get_irq_data(client->irq));

• priv->cec_glue.irq_flags = irq_flags;
  

• irq_flags |= IRQF_SHARED | IRQF_ONESHOT; ret = request_threaded_irq(client->irq, NULL, tda998x_irq_thread, irq_flags,

@@ -1577,13 +1718,46 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) dev_err(&client->dev, "failed to request IRQ#%u: %d\n", client->irq, ret);

• 	goto err_irq;
  

• 	return ret;
  

  }

  /* enable HPD irq */ cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD); }

• priv->cec_notify = cec_notifier_get(&client->dev);

• if (!priv->cec_notify) {

• ret = -ENOMEM;
  

• goto fail;
  

• }

• priv->cec_glue.parent = &client->dev;

• priv->cec_glue.data = priv;

• priv->cec_glue.init = tda998x_cec_hook_init;

• priv->cec_glue.exit = tda998x_cec_hook_exit;

• priv->cec_glue.open = tda998x_cec_hook_open;

• priv->cec_glue.release = tda998x_cec_hook_release;

• /*

• * Some TDA998x are actually two I2C devices merged onto one piece
  

• * of silicon: TDA9989 and TDA19989 combine the HDMI transmitter
  

• * with a slightly modified TDA9950 CEC device.  The CEC device
  

• * is at the TDA9950 address, with the address pins strapped across
  

• * to the TDA998x address pins.  Hence, it always has the same
  

• * offset.
  

• */
  

• memset(&cec_info, 0, sizeof(cec_info));

• strlcpy(cec_info.type, "tda9950", sizeof(cec_info.type));

• cec_info.addr = priv->cec_addr;

• cec_info.platform_data = &priv->cec_glue;

• cec_info.irq = client->irq;

• priv->cec = i2c_new_device(client->adapter, &cec_info);

• if (!priv->cec) {

• free_irq(priv->hdmi->irq, priv);
  

• return -ENODEV;
  

• }

• /* enable EDID read irq: */ reg_set(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);

@@ -1610,8 +1784,15 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) err_audio: if (client->irq) free_irq(client->irq, priv); -err_irq:

• i2c_unregister_device(priv->cec);

+fail:

• /* if encoder_init fails, the encoder slave is never registered,

• * so cleanup here:
  

• */
  

• if (priv->cec)

• i2c_unregister_device(priv->cec);
  

• if (priv->cec_notify)

• cec_notifier_put(priv->cec_notify);
  

• free_irq(priv->hdmi->irq, priv); return ret;

}