From ba300c1787f793d9d6c84bb30d12b684c957f1b5 Mon Sep 17 00:00:00 2001 From: Russell King Date: Thu, 17 Nov 2016 23:55:00 +0000 Subject: drm/i2c: tda998x: move audio mutex initialisation We will need the audio mutex initialised in all cases, so lets move this to be early, rather than only being initialised for the DT case. Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 088900d78ceb..168c0b4b8f47 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1405,6 +1405,8 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) int rev_lo, rev_hi, ret; unsigned short cec_addr; + mutex_init(&priv->audio_mutex); /* Protect access from audio thread */ + priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3); priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1); priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5); @@ -1519,8 +1521,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) priv->vip_cntrl_2 = video; } - mutex_init(&priv->audio_mutex); /* Protect access from audio thread */ - ret = tda998x_get_audio_ports(priv, np); if (ret) goto fail; -- cgit v1.2.3 From 6d30c0f728cac32ce8bf3d48ef6df485e0c641f1 Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 23 Oct 2016 11:31:44 +0100 Subject: drm/i2c: tda998x: avoid race in tda998x_encoder_mode_set() As priv->audio_params can now be changed at run time, we need to be more careful about how we deal with a mode set. We must take the audio lock while checking if there's a valid audio configuration. However, it's slightly worse than that - during mode set, we mute the audio, and it must not be unmuted until we have finished the mode set. It is possible that the audio side may start while a mode set is in progress, so take the audio_mutex lock around the whole mode setting procedure. Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 168c0b4b8f47..9184487b10ad 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1074,13 +1074,12 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, tda998x_write_avi(priv, adjusted_mode); - if (priv->audio_params.format != AFMT_UNUSED) { - mutex_lock(&priv->audio_mutex); + mutex_lock(&priv->audio_mutex); + if (priv->audio_params.format != AFMT_UNUSED) tda998x_configure_audio(priv, &priv->audio_params, adjusted_mode->clock); - mutex_unlock(&priv->audio_mutex); - } + mutex_unlock(&priv->audio_mutex); } } -- cgit v1.2.3 From 319e658c78befa51557f4d18a4092a04984290d4 Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 23 Oct 2016 11:32:43 +0100 Subject: drm/i2c: tda998x: avoid racy access to mode clock Avoid a racy access to the mode clock by storing the current mode clock during a mode set under the audio mutex. This allows us to access it from the audio path in a safe way. Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 9184487b10ad..422d63d5fb75 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -47,6 +47,7 @@ struct tda998x_priv { u8 vip_cntrl_0; u8 vip_cntrl_1; u8 vip_cntrl_2; + unsigned long tmds_clock; struct tda998x_audio_params audio_params; struct platform_device *audio_pdev; @@ -713,8 +714,7 @@ static void tda998x_audio_mute(struct tda998x_priv *priv, bool on) static int tda998x_configure_audio(struct tda998x_priv *priv, - struct tda998x_audio_params *params, - unsigned mode_clock) + struct tda998x_audio_params *params) { u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv; u32 n; @@ -771,7 +771,7 @@ tda998x_configure_audio(struct tda998x_priv *priv, * assume 100MHz requires larger divider. */ adiv = AUDIO_DIV_SERCLK_8; - if (mode_clock > 100000) + if (priv->tmds_clock > 100000) adiv++; /* AUDIO_DIV_SERCLK_16 */ /* S/PDIF asks for a larger divider */ @@ -1064,6 +1064,10 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, /* must be last register set: */ reg_write(priv, REG_TBG_CNTRL_0, 0); + mutex_lock(&priv->audio_mutex); + + priv->tmds_clock = adjusted_mode->clock; + /* Only setup the info frames if the sink is HDMI */ if (priv->is_hdmi_sink) { /* We need to turn HDMI HDCP stuff on to get audio through */ @@ -1074,13 +1078,11 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, tda998x_write_avi(priv, adjusted_mode); - mutex_lock(&priv->audio_mutex); if (priv->audio_params.format != AFMT_UNUSED) - tda998x_configure_audio(priv, - &priv->audio_params, - adjusted_mode->clock); - mutex_unlock(&priv->audio_mutex); + tda998x_configure_audio(priv, &priv->audio_params); } + + mutex_unlock(&priv->audio_mutex); } static enum drm_connector_status @@ -1226,9 +1228,6 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, .cea = params->cea, }; - if (!priv->encoder.crtc) - return -ENODEV; - memcpy(audio.status, params->iec.status, min(sizeof(audio.status), sizeof(params->iec.status))); @@ -1264,9 +1263,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, } mutex_lock(&priv->audio_mutex); - ret = tda998x_configure_audio(priv, - &audio, - priv->encoder.crtc->hwmode.clock); + ret = tda998x_configure_audio(priv, &audio); if (ret == 0) priv->audio_params = audio; -- cgit v1.2.3 From 2cae8e028ecb44003f607584e014c15a2f415e8a Mon Sep 17 00:00:00 2001 From: Russell King Date: Wed, 2 Nov 2016 21:38:34 +0000 Subject: drm/i2c: tda998x: avoid race when programming audio Avoid a race between programming audio and an in-progress mode set. A mode set is complex, and disables the ability to send infoframes to the sink, and is disruptive to audio - we have to mute the audio FIFO while doing a mode set. If an attempt is made to start up the audio side, we will undo the audio FIFO mute before the mode set has completed. Move the lock so that we prevent audio interfering with an in-progress mode set. Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 422d63d5fb75..dd183cee98b0 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -971,6 +971,8 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, div = 3; } + mutex_lock(&priv->audio_mutex); + /* mute the audio FIFO: */ reg_set(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_FIFO); @@ -1064,8 +1066,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, /* must be last register set: */ reg_write(priv, REG_TBG_CNTRL_0, 0); - mutex_lock(&priv->audio_mutex); - priv->tmds_clock = adjusted_mode->clock; /* Only setup the info frames if the sink is HDMI */ -- cgit v1.2.3 From 896a4130b8e60cbdc92cfdc5b56da1fc2a03f4ad Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 23 Oct 2016 11:32:42 +0100 Subject: drm/i2c: tda998x: only configure infoframes and audio if supported The CEA 861B specification indicates the situations when we are able to send each infoframe based on the version of the EDID's CEA extension. Update the tda998x driver to follow the CEA specification wrt sending of infoframes. Since we only support the generation of AVI version 2, this limits us to CEA extension version 3, so we treat CEA extension version 2 as CEA 861 (no infoframes, no audio.) Tested-by: Robin Murphy Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index dd183cee98b0..c78c966fabfa 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -43,7 +43,7 @@ struct tda998x_priv { u16 rev; u8 current_page; int dpms; - bool is_hdmi_sink; + bool supports_infoframes; u8 vip_cntrl_0; u8 vip_cntrl_1; u8 vip_cntrl_2; @@ -1068,8 +1068,20 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, priv->tmds_clock = adjusted_mode->clock; - /* Only setup the info frames if the sink is HDMI */ - if (priv->is_hdmi_sink) { + /* CEA-861B section 6 says that: + * CEA version 1 (CEA-861) has no support for infoframes. + * CEA version 2 (CEA-861A) supports version 1 AVI infoframes, + * and optional basic audio. + * CEA version 3 (CEA-861B) supports version 1 and 2 AVI infoframes, + * and optional digital audio, with audio infoframes. + * + * Since we only support generation of version 2 AVI infoframes, + * ignore CEA version 2 and below (iow, behave as if we're a + * CEA-861 source.) + */ + priv->supports_infoframes = priv->connector.display_info.cea_rev >= 3; + + if (priv->supports_infoframes) { /* We need to turn HDMI HDCP stuff on to get audio through */ reg &= ~TBG_CNTRL_1_DWIN_DIS; reg_write(priv, REG_TBG_CNTRL_1, reg); @@ -1180,7 +1192,6 @@ static int tda998x_connector_get_modes(struct drm_connector *connector) drm_mode_connector_update_edid_property(connector, edid); n = drm_add_edid_modes(connector, edid); - priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid); drm_edid_to_eld(connector, edid); kfree(edid); @@ -1263,7 +1274,10 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, } mutex_lock(&priv->audio_mutex); - ret = tda998x_configure_audio(priv, &audio); + if (priv->supports_infoframes) + ret = tda998x_configure_audio(priv, &audio); + else + ret = 0; if (ret == 0) priv->audio_params = audio; -- cgit v1.2.3 From 8f3f21f63c3625165a1a1a6d2978f65df9652d26 Mon Sep 17 00:00:00 2001 From: Russell King Date: Wed, 2 Nov 2016 21:15:04 +0000 Subject: drm/i2c: tda998x: only enable audio if supported by sink Check for audio support by the attached sink by consulting the EDID prior to enabling audio over the TMDS link. We must consult the EDID after calling drm_helper_probe_single_connector_modes(), as this can use an override EDID, or load a replacement EDID. Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index c78c966fabfa..f23c1a8d0e33 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -44,6 +44,7 @@ struct tda998x_priv { u8 current_page; int dpms; bool supports_infoframes; + bool sink_has_audio; u8 vip_cntrl_0; u8 vip_cntrl_1; u8 vip_cntrl_2; @@ -1090,13 +1091,33 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, tda998x_write_avi(priv, adjusted_mode); - if (priv->audio_params.format != AFMT_UNUSED) + if (priv->audio_params.format != AFMT_UNUSED && + priv->sink_has_audio) tda998x_configure_audio(priv, &priv->audio_params); } mutex_unlock(&priv->audio_mutex); } +static int tda998x_connector_fill_modes(struct drm_connector *connector, + uint32_t maxX, uint32_t maxY) +{ + struct tda998x_priv *priv = conn_to_tda998x_priv(connector); + int ret; + + ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY); + + if (connector->edid_blob_ptr) { + struct edid *edid = (void *)connector->edid_blob_ptr->data; + + priv->sink_has_audio = drm_detect_monitor_audio(edid); + } else { + priv->sink_has_audio = false; + } + + return ret; +} + static enum drm_connector_status tda998x_connector_detect(struct drm_connector *connector, bool force) { @@ -1274,7 +1295,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, } mutex_lock(&priv->audio_mutex); - if (priv->supports_infoframes) + if (priv->supports_infoframes && priv->sink_has_audio) ret = tda998x_configure_audio(priv, &audio); else ret = 0; @@ -1608,7 +1629,7 @@ static int tda998x_connector_dpms(struct drm_connector *connector, int mode) static const struct drm_connector_funcs tda998x_connector_funcs = { .dpms = tda998x_connector_dpms, .reset = drm_atomic_helper_connector_reset, - .fill_modes = drm_helper_probe_single_connector_modes, + .fill_modes = tda998x_connector_fill_modes, .detect = tda998x_connector_detect, .destroy = tda998x_connector_destroy, .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, -- cgit v1.2.3 From 95a9b686d679059adf491ff116f936de63fcec29 Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 23 Oct 2016 11:24:22 +0100 Subject: drm/i2c: tda998x: correct function name in comments Correct two references to tda998x_connector_get_modes() which were incorrectly referring to tda998x_encoder_get_modes(). Tested-by: Robin Murphy Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index f23c1a8d0e33..662ac93ccc19 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -581,9 +581,9 @@ tda998x_reset(struct tda998x_priv *priv) * 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 + * However, tda998x_connector_get_modes() may be called at any moment * after tda998x_connector_detect() indicates that we are connected, so - * we need to delay probing modes in tda998x_encoder_get_modes() after + * we need to delay probing modes in tda998x_connector_get_modes() after * we have seen a HPD inactive->active transition. This code implements * that delay. */ -- cgit v1.2.3 From 94579273b893f67c5806d8a61a0ca072b0f7a141 Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 23 Oct 2016 11:25:02 +0100 Subject: drm/i2c: tda998x: move and rename tda998x_encoder_set_config() The naming of tda998x_encoder_set_config() is a left-over from when TDA998x was a slave encoder. Since this is part of the initialisation, drop the _encoder from the name, and move it near tda998x_bind(). Tested-by: Robin Murphy Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 40 +++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 662ac93ccc19..dc23ae5374d1 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -822,25 +822,6 @@ tda998x_configure_audio(struct tda998x_priv *priv, /* DRM encoder functions */ -static void tda998x_encoder_set_config(struct tda998x_priv *priv, - const struct tda998x_encoder_params *p) -{ - priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(p->swap_a) | - (p->mirr_a ? VIP_CNTRL_0_MIRR_A : 0) | - VIP_CNTRL_0_SWAP_B(p->swap_b) | - (p->mirr_b ? VIP_CNTRL_0_MIRR_B : 0); - priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(p->swap_c) | - (p->mirr_c ? VIP_CNTRL_1_MIRR_C : 0) | - VIP_CNTRL_1_SWAP_D(p->swap_d) | - (p->mirr_d ? VIP_CNTRL_1_MIRR_D : 0); - priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(p->swap_e) | - (p->mirr_e ? VIP_CNTRL_2_MIRR_E : 0) | - VIP_CNTRL_2_SWAP_F(p->swap_f) | - (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0); - - priv->audio_params = p->audio_params; -} - static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) { struct tda998x_priv *priv = enc_to_tda998x_priv(encoder); @@ -1636,6 +1617,25 @@ static const struct drm_connector_funcs tda998x_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; +static void tda998x_set_config(struct tda998x_priv *priv, + const struct tda998x_encoder_params *p) +{ + priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(p->swap_a) | + (p->mirr_a ? VIP_CNTRL_0_MIRR_A : 0) | + VIP_CNTRL_0_SWAP_B(p->swap_b) | + (p->mirr_b ? VIP_CNTRL_0_MIRR_B : 0); + priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(p->swap_c) | + (p->mirr_c ? VIP_CNTRL_1_MIRR_C : 0) | + VIP_CNTRL_1_SWAP_D(p->swap_d) | + (p->mirr_d ? VIP_CNTRL_1_MIRR_D : 0); + priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(p->swap_e) | + (p->mirr_e ? VIP_CNTRL_2_MIRR_E : 0) | + VIP_CNTRL_2_SWAP_F(p->swap_f) | + (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0); + + priv->audio_params = p->audio_params; +} + static int tda998x_bind(struct device *dev, struct device *master, void *data) { struct tda998x_encoder_params *params = dev->platform_data; @@ -1668,7 +1668,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) return ret; if (!dev->of_node && params) - tda998x_encoder_set_config(priv, params); + tda998x_set_config(priv, params); tda998x_encoder_set_polling(priv, &priv->connector); -- cgit v1.2.3 From 25576733ec6e051e541b13ba77a8e1c63619336f Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 23 Oct 2016 11:29:59 +0100 Subject: drm/i2c: tda998x: group connector functions and funcs together Group the TDA998x connector functions and funcs structures together before the encoder support, rather than scattered amongst the rest of the file. This keeps like code together. Tested-by: Robin Murphy Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 350 +++++++++++++++++++------------------- 1 file changed, 176 insertions(+), 174 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index dc23ae5374d1..cca2f397eea7 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -820,6 +820,182 @@ tda998x_configure_audio(struct tda998x_priv *priv, return tda998x_write_aif(priv, ¶ms->cea); } +/* DRM connector functions */ + +static int tda998x_connector_dpms(struct drm_connector *connector, int mode) +{ + if (drm_core_check_feature(connector->dev, DRIVER_ATOMIC)) + return drm_atomic_helper_connector_dpms(connector, mode); + else + return drm_helper_connector_dpms(connector, mode); +} + +static int tda998x_connector_fill_modes(struct drm_connector *connector, + uint32_t maxX, uint32_t maxY) +{ + struct tda998x_priv *priv = conn_to_tda998x_priv(connector); + int ret; + + ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY); + + if (connector->edid_blob_ptr) { + struct edid *edid = (void *)connector->edid_blob_ptr->data; + + priv->sink_has_audio = drm_detect_monitor_audio(edid); + } else { + priv->sink_has_audio = false; + } + + return ret; +} + +static enum drm_connector_status +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 : + connector_status_disconnected; +} + +static void tda998x_connector_destroy(struct drm_connector *connector) +{ + drm_connector_cleanup(connector); +} + +static const struct drm_connector_funcs tda998x_connector_funcs = { + .dpms = tda998x_connector_dpms, + .reset = drm_atomic_helper_connector_reset, + .fill_modes = tda998x_connector_fill_modes, + .detect = tda998x_connector_detect, + .destroy = tda998x_connector_destroy, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length) +{ + struct tda998x_priv *priv = data; + u8 offset, segptr; + int ret, i; + + offset = (blk & 1) ? 128 : 0; + segptr = blk / 2; + + reg_write(priv, REG_DDC_ADDR, 0xa0); + reg_write(priv, REG_DDC_OFFS, offset); + reg_write(priv, REG_DDC_SEGM_ADDR, 0x60); + reg_write(priv, REG_DDC_SEGM, segptr); + + /* enable reading EDID: */ + priv->wq_edid_wait = 1; + reg_write(priv, REG_EDID_CTRL, 0x1); + + /* flag must be cleared by sw: */ + reg_write(priv, REG_EDID_CTRL, 0x0); + + /* wait for block read to complete: */ + if (priv->hdmi->irq) { + i = wait_event_timeout(priv->wq_edid, + !priv->wq_edid_wait, + msecs_to_jiffies(100)); + if (i < 0) { + dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i); + return i; + } + } else { + for (i = 100; i > 0; i--) { + msleep(1); + ret = reg_read(priv, REG_INT_FLAGS_2); + if (ret < 0) + return ret; + if (ret & INT_FLAGS_2_EDID_BLK_RD) + break; + } + } + + if (i == 0) { + dev_err(&priv->hdmi->dev, "read edid timeout\n"); + return -ETIMEDOUT; + } + + 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; + } + + return 0; +} + +static int tda998x_connector_get_modes(struct drm_connector *connector) +{ + struct tda998x_priv *priv = conn_to_tda998x_priv(connector); + 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); + + edid = drm_do_get_edid(connector, read_edid_block, priv); + + if (priv->rev == TDA19988) + reg_set(priv, REG_TX4, TX4_PD_RAM); + + if (!edid) { + dev_warn(&priv->hdmi->dev, "failed to read EDID\n"); + return 0; + } + + drm_mode_connector_update_edid_property(connector, edid); + n = drm_add_edid_modes(connector, edid); + drm_edid_to_eld(connector, edid); + + kfree(edid); + + return n; +} + +static int tda998x_connector_mode_valid(struct drm_connector *connector, + struct drm_display_mode *mode) +{ + /* TDA19988 dotclock can go up to 165MHz */ + struct tda998x_priv *priv = conn_to_tda998x_priv(connector); + + if (mode->clock > ((priv->rev == TDA19988) ? 165000 : 150000)) + return MODE_CLOCK_HIGH; + if (mode->htotal >= BIT(13)) + return MODE_BAD_HVALUE; + if (mode->vtotal >= BIT(11)) + return MODE_BAD_VVALUE; + return MODE_OK; +} + +static struct drm_encoder * +tda998x_connector_best_encoder(struct drm_connector *connector) +{ + struct tda998x_priv *priv = conn_to_tda998x_priv(connector); + + return &priv->encoder; +} + +static +const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = { + .get_modes = tda998x_connector_get_modes, + .mode_valid = tda998x_connector_mode_valid, + .best_encoder = tda998x_connector_best_encoder, +}; + /* DRM encoder functions */ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) @@ -855,21 +1031,6 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) priv->dpms = mode; } -static int tda998x_connector_mode_valid(struct drm_connector *connector, - struct drm_display_mode *mode) -{ - /* TDA19988 dotclock can go up to 165MHz */ - struct tda998x_priv *priv = conn_to_tda998x_priv(connector); - - if (mode->clock > ((priv->rev == TDA19988) ? 165000 : 150000)) - return MODE_CLOCK_HIGH; - if (mode->htotal >= BIT(13)) - return MODE_BAD_HVALUE; - if (mode->vtotal >= BIT(11)) - return MODE_BAD_VVALUE; - return MODE_OK; -} - static void tda998x_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, @@ -1080,127 +1241,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, mutex_unlock(&priv->audio_mutex); } -static int tda998x_connector_fill_modes(struct drm_connector *connector, - uint32_t maxX, uint32_t maxY) -{ - struct tda998x_priv *priv = conn_to_tda998x_priv(connector); - int ret; - - ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY); - - if (connector->edid_blob_ptr) { - struct edid *edid = (void *)connector->edid_blob_ptr->data; - - priv->sink_has_audio = drm_detect_monitor_audio(edid); - } else { - priv->sink_has_audio = false; - } - - return ret; -} - -static enum drm_connector_status -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 : - connector_status_disconnected; -} - -static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length) -{ - struct tda998x_priv *priv = data; - u8 offset, segptr; - int ret, i; - - offset = (blk & 1) ? 128 : 0; - segptr = blk / 2; - - reg_write(priv, REG_DDC_ADDR, 0xa0); - reg_write(priv, REG_DDC_OFFS, offset); - reg_write(priv, REG_DDC_SEGM_ADDR, 0x60); - reg_write(priv, REG_DDC_SEGM, segptr); - - /* enable reading EDID: */ - priv->wq_edid_wait = 1; - reg_write(priv, REG_EDID_CTRL, 0x1); - - /* flag must be cleared by sw: */ - reg_write(priv, REG_EDID_CTRL, 0x0); - - /* wait for block read to complete: */ - if (priv->hdmi->irq) { - i = wait_event_timeout(priv->wq_edid, - !priv->wq_edid_wait, - msecs_to_jiffies(100)); - if (i < 0) { - dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i); - return i; - } - } else { - for (i = 100; i > 0; i--) { - msleep(1); - ret = reg_read(priv, REG_INT_FLAGS_2); - if (ret < 0) - return ret; - if (ret & INT_FLAGS_2_EDID_BLK_RD) - break; - } - } - - if (i == 0) { - dev_err(&priv->hdmi->dev, "read edid timeout\n"); - return -ETIMEDOUT; - } - - 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; - } - - return 0; -} - -static int tda998x_connector_get_modes(struct drm_connector *connector) -{ - struct tda998x_priv *priv = conn_to_tda998x_priv(connector); - 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); - - edid = drm_do_get_edid(connector, read_edid_block, priv); - - if (priv->rev == TDA19988) - reg_set(priv, REG_TX4, TX4_PD_RAM); - - if (!edid) { - dev_warn(&priv->hdmi->dev, "failed to read EDID\n"); - return 0; - } - - drm_mode_connector_update_edid_property(connector, edid); - n = drm_add_edid_modes(connector, edid); - drm_edid_to_eld(connector, edid); - - kfree(edid); - - return n; -} - static void tda998x_encoder_set_polling(struct tda998x_priv *priv, struct drm_connector *connector) { @@ -1579,44 +1619,6 @@ static const struct drm_encoder_funcs tda998x_encoder_funcs = { .destroy = tda998x_encoder_destroy, }; -static struct drm_encoder * -tda998x_connector_best_encoder(struct drm_connector *connector) -{ - struct tda998x_priv *priv = conn_to_tda998x_priv(connector); - - return &priv->encoder; -} - -static -const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = { - .get_modes = tda998x_connector_get_modes, - .mode_valid = tda998x_connector_mode_valid, - .best_encoder = tda998x_connector_best_encoder, -}; - -static void tda998x_connector_destroy(struct drm_connector *connector) -{ - drm_connector_cleanup(connector); -} - -static int tda998x_connector_dpms(struct drm_connector *connector, int mode) -{ - if (drm_core_check_feature(connector->dev, DRIVER_ATOMIC)) - return drm_atomic_helper_connector_dpms(connector, mode); - else - return drm_helper_connector_dpms(connector, mode); -} - -static const struct drm_connector_funcs tda998x_connector_funcs = { - .dpms = tda998x_connector_dpms, - .reset = drm_atomic_helper_connector_reset, - .fill_modes = tda998x_connector_fill_modes, - .detect = tda998x_connector_detect, - .destroy = tda998x_connector_destroy, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - static void tda998x_set_config(struct tda998x_priv *priv, const struct tda998x_encoder_params *p) { -- cgit v1.2.3 From a2f75662b7c3db2ca2e18aaaa5fa86d5991b0d70 Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 23 Oct 2016 11:30:56 +0100 Subject: drm/i2c: tda998x: separate connector initialisation Separate out the connector initialisation from the rest of the drivers initialisation. Tested-by: Robin Murphy Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 46 ++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 20 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index cca2f397eea7..44f3a4e9b902 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -996,6 +996,31 @@ const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = { .best_encoder = tda998x_connector_best_encoder, }; +static int tda998x_connector_init(struct tda998x_priv *priv, + struct drm_device *drm) +{ + struct drm_connector *connector = &priv->connector; + int ret; + + connector->interlace_allowed = 1; + + if (priv->hdmi->irq) + connector->polled = DRM_CONNECTOR_POLL_HPD; + else + connector->polled = DRM_CONNECTOR_POLL_CONNECT | + DRM_CONNECTOR_POLL_DISCONNECT; + + drm_connector_helper_add(connector, &tda998x_connector_helper_funcs); + ret = drm_connector_init(drm, connector, &tda998x_connector_funcs, + DRM_MODE_CONNECTOR_HDMIA); + if (ret) + return ret; + + drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder); + + return 0; +} + /* DRM encoder functions */ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) @@ -1241,16 +1266,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, mutex_unlock(&priv->audio_mutex); } -static void tda998x_encoder_set_polling(struct tda998x_priv *priv, - struct drm_connector *connector) -{ - if (priv->hdmi->irq) - connector->polled = DRM_CONNECTOR_POLL_HPD; - else - connector->polled = DRM_CONNECTOR_POLL_CONNECT | - DRM_CONNECTOR_POLL_DISCONNECT; -} - static void tda998x_destroy(struct tda998x_priv *priv) { /* disable all IRQs and free the IRQ handler */ @@ -1662,7 +1677,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) crtcs = 1 << 0; } - priv->connector.interlace_allowed = 1; priv->encoder.possible_crtcs = crtcs; ret = tda998x_create(client, priv); @@ -1672,24 +1686,16 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) if (!dev->of_node && params) tda998x_set_config(priv, params); - tda998x_encoder_set_polling(priv, &priv->connector); - drm_encoder_helper_add(&priv->encoder, &tda998x_encoder_helper_funcs); ret = drm_encoder_init(drm, &priv->encoder, &tda998x_encoder_funcs, DRM_MODE_ENCODER_TMDS, NULL); if (ret) goto err_encoder; - drm_connector_helper_add(&priv->connector, - &tda998x_connector_helper_funcs); - ret = drm_connector_init(drm, &priv->connector, - &tda998x_connector_funcs, - DRM_MODE_CONNECTOR_HDMIA); + ret = tda998x_connector_init(priv, drm); if (ret) goto err_connector; - drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder); - return 0; err_connector: -- cgit v1.2.3 From ad975f9364a3e1beb0909a009671c122b47763cd Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 23 Oct 2016 11:30:56 +0100 Subject: drm/i2c: tda998x: group audio functions together Group the TDA998x audio functions together rather than split between two different locations in the file, keeping like code together. Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 276 +++++++++++++++++++------------------- 1 file changed, 139 insertions(+), 137 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 44f3a4e9b902..0f604d2dd1c7 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -702,6 +702,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode) tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame); } +/* Audio support */ + static void tda998x_audio_mute(struct tda998x_priv *priv, bool on) { if (on) { @@ -820,6 +822,143 @@ tda998x_configure_audio(struct tda998x_priv *priv, return tda998x_write_aif(priv, ¶ms->cea); } +static int tda998x_audio_hw_params(struct device *dev, void *data, + struct hdmi_codec_daifmt *daifmt, + struct hdmi_codec_params *params) +{ + struct tda998x_priv *priv = dev_get_drvdata(dev); + int i, ret; + struct tda998x_audio_params audio = { + .sample_width = params->sample_width, + .sample_rate = params->sample_rate, + .cea = params->cea, + }; + + memcpy(audio.status, params->iec.status, + min(sizeof(audio.status), sizeof(params->iec.status))); + + switch (daifmt->fmt) { + case HDMI_I2S: + if (daifmt->bit_clk_inv || daifmt->frame_clk_inv || + daifmt->bit_clk_master || daifmt->frame_clk_master) { + dev_err(dev, "%s: Bad flags %d %d %d %d\n", __func__, + daifmt->bit_clk_inv, daifmt->frame_clk_inv, + daifmt->bit_clk_master, + daifmt->frame_clk_master); + return -EINVAL; + } + for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) + if (priv->audio_port[i].format == AFMT_I2S) + audio.config = priv->audio_port[i].config; + audio.format = AFMT_I2S; + break; + case HDMI_SPDIF: + for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) + if (priv->audio_port[i].format == AFMT_SPDIF) + audio.config = priv->audio_port[i].config; + audio.format = AFMT_SPDIF; + break; + default: + dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt); + return -EINVAL; + } + + if (audio.config == 0) { + dev_err(dev, "%s: No audio configutation found\n", __func__); + return -EINVAL; + } + + mutex_lock(&priv->audio_mutex); + if (priv->supports_infoframes && priv->sink_has_audio) + ret = tda998x_configure_audio(priv, &audio); + else + ret = 0; + + if (ret == 0) + priv->audio_params = audio; + mutex_unlock(&priv->audio_mutex); + + return ret; +} + +static void tda998x_audio_shutdown(struct device *dev, void *data) +{ + struct tda998x_priv *priv = dev_get_drvdata(dev); + + mutex_lock(&priv->audio_mutex); + + reg_write(priv, REG_ENA_AP, 0); + + priv->audio_params.format = AFMT_UNUSED; + + mutex_unlock(&priv->audio_mutex); +} + +int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable) +{ + struct tda998x_priv *priv = dev_get_drvdata(dev); + + mutex_lock(&priv->audio_mutex); + + tda998x_audio_mute(priv, enable); + + mutex_unlock(&priv->audio_mutex); + return 0; +} + +static int tda998x_audio_get_eld(struct device *dev, void *data, + uint8_t *buf, size_t len) +{ + struct tda998x_priv *priv = dev_get_drvdata(dev); + struct drm_mode_config *config = &priv->encoder.dev->mode_config; + struct drm_connector *connector; + int ret = -ENODEV; + + mutex_lock(&config->mutex); + list_for_each_entry(connector, &config->connector_list, head) { + if (&priv->encoder == connector->encoder) { + memcpy(buf, connector->eld, + min(sizeof(connector->eld), len)); + ret = 0; + } + } + mutex_unlock(&config->mutex); + + return ret; +} + +static const struct hdmi_codec_ops audio_codec_ops = { + .hw_params = tda998x_audio_hw_params, + .audio_shutdown = tda998x_audio_shutdown, + .digital_mute = tda998x_audio_digital_mute, + .get_eld = tda998x_audio_get_eld, +}; + +static int tda998x_audio_codec_init(struct tda998x_priv *priv, + struct device *dev) +{ + struct hdmi_codec_pdata codec_data = { + .ops = &audio_codec_ops, + .max_i2s_channels = 2, + }; + int i; + + for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) { + if (priv->audio_port[i].format == AFMT_I2S && + priv->audio_port[i].config != 0) + codec_data.i2s = 1; + if (priv->audio_port[i].format == AFMT_SPDIF && + priv->audio_port[i].config != 0) + codec_data.spdif = 1; + } + + priv->audio_pdev = platform_device_register_data( + dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO, + &codec_data, sizeof(codec_data)); + + return PTR_ERR_OR_ZERO(priv->audio_pdev); +} + /* DRM connector functions */ static int tda998x_connector_dpms(struct drm_connector *connector, int mode) @@ -1284,143 +1423,6 @@ static void tda998x_destroy(struct tda998x_priv *priv) i2c_unregister_device(priv->cec); } -static int tda998x_audio_hw_params(struct device *dev, void *data, - struct hdmi_codec_daifmt *daifmt, - struct hdmi_codec_params *params) -{ - struct tda998x_priv *priv = dev_get_drvdata(dev); - int i, ret; - struct tda998x_audio_params audio = { - .sample_width = params->sample_width, - .sample_rate = params->sample_rate, - .cea = params->cea, - }; - - memcpy(audio.status, params->iec.status, - min(sizeof(audio.status), sizeof(params->iec.status))); - - switch (daifmt->fmt) { - case HDMI_I2S: - if (daifmt->bit_clk_inv || daifmt->frame_clk_inv || - daifmt->bit_clk_master || daifmt->frame_clk_master) { - dev_err(dev, "%s: Bad flags %d %d %d %d\n", __func__, - daifmt->bit_clk_inv, daifmt->frame_clk_inv, - daifmt->bit_clk_master, - daifmt->frame_clk_master); - return -EINVAL; - } - for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) - if (priv->audio_port[i].format == AFMT_I2S) - audio.config = priv->audio_port[i].config; - audio.format = AFMT_I2S; - break; - case HDMI_SPDIF: - for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) - if (priv->audio_port[i].format == AFMT_SPDIF) - audio.config = priv->audio_port[i].config; - audio.format = AFMT_SPDIF; - break; - default: - dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt); - return -EINVAL; - } - - if (audio.config == 0) { - dev_err(dev, "%s: No audio configutation found\n", __func__); - return -EINVAL; - } - - mutex_lock(&priv->audio_mutex); - if (priv->supports_infoframes && priv->sink_has_audio) - ret = tda998x_configure_audio(priv, &audio); - else - ret = 0; - - if (ret == 0) - priv->audio_params = audio; - mutex_unlock(&priv->audio_mutex); - - return ret; -} - -static void tda998x_audio_shutdown(struct device *dev, void *data) -{ - struct tda998x_priv *priv = dev_get_drvdata(dev); - - mutex_lock(&priv->audio_mutex); - - reg_write(priv, REG_ENA_AP, 0); - - priv->audio_params.format = AFMT_UNUSED; - - mutex_unlock(&priv->audio_mutex); -} - -int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable) -{ - struct tda998x_priv *priv = dev_get_drvdata(dev); - - mutex_lock(&priv->audio_mutex); - - tda998x_audio_mute(priv, enable); - - mutex_unlock(&priv->audio_mutex); - return 0; -} - -static int tda998x_audio_get_eld(struct device *dev, void *data, - uint8_t *buf, size_t len) -{ - struct tda998x_priv *priv = dev_get_drvdata(dev); - struct drm_mode_config *config = &priv->encoder.dev->mode_config; - struct drm_connector *connector; - int ret = -ENODEV; - - mutex_lock(&config->mutex); - list_for_each_entry(connector, &config->connector_list, head) { - if (&priv->encoder == connector->encoder) { - memcpy(buf, connector->eld, - min(sizeof(connector->eld), len)); - ret = 0; - } - } - mutex_unlock(&config->mutex); - - return ret; -} - -static const struct hdmi_codec_ops audio_codec_ops = { - .hw_params = tda998x_audio_hw_params, - .audio_shutdown = tda998x_audio_shutdown, - .digital_mute = tda998x_audio_digital_mute, - .get_eld = tda998x_audio_get_eld, -}; - -static int tda998x_audio_codec_init(struct tda998x_priv *priv, - struct device *dev) -{ - struct hdmi_codec_pdata codec_data = { - .ops = &audio_codec_ops, - .max_i2s_channels = 2, - }; - int i; - - for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) { - if (priv->audio_port[i].format == AFMT_I2S && - priv->audio_port[i].config != 0) - codec_data.i2s = 1; - if (priv->audio_port[i].format == AFMT_SPDIF && - priv->audio_port[i].config != 0) - codec_data.spdif = 1; - } - - priv->audio_pdev = platform_device_register_data( - dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO, - &codec_data, sizeof(codec_data)); - - return PTR_ERR_OR_ZERO(priv->audio_pdev); -} - /* I2C driver functions */ static int tda998x_get_audio_ports(struct tda998x_priv *priv, -- cgit v1.2.3 From 02efac0fbfd59a9b9cf57cd09c8bdf5d8471c533 Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 23 Oct 2016 11:31:44 +0100 Subject: drm/i2c: tda998x: remove complexity from tda998x_audio_get_eld() tda998x_audio_get_eld() is needlessly complex - the connector associated with the encoder is always our own priv->connector. Remove this complexity, but ensure that there are no races when copying out the ELD. Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 0f604d2dd1c7..78df92b3e055 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -910,21 +910,13 @@ static int tda998x_audio_get_eld(struct device *dev, void *data, uint8_t *buf, size_t len) { struct tda998x_priv *priv = dev_get_drvdata(dev); - struct drm_mode_config *config = &priv->encoder.dev->mode_config; - struct drm_connector *connector; - int ret = -ENODEV; - - mutex_lock(&config->mutex); - list_for_each_entry(connector, &config->connector_list, head) { - if (&priv->encoder == connector->encoder) { - memcpy(buf, connector->eld, - min(sizeof(connector->eld), len)); - ret = 0; - } - } - mutex_unlock(&config->mutex); - return ret; + mutex_lock(&priv->audio_mutex); + memcpy(buf, priv->connector.eld, + min(sizeof(priv->connector.eld), len)); + mutex_unlock(&priv->audio_mutex); + + return 0; } static const struct hdmi_codec_ops audio_codec_ops = { @@ -975,6 +967,7 @@ static int tda998x_connector_fill_modes(struct drm_connector *connector, struct tda998x_priv *priv = conn_to_tda998x_priv(connector); int ret; + mutex_lock(&priv->audio_mutex); ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY); if (connector->edid_blob_ptr) { @@ -984,6 +977,7 @@ static int tda998x_connector_fill_modes(struct drm_connector *connector, } else { priv->sink_has_audio = false; } + mutex_unlock(&priv->audio_mutex); return ret; } -- cgit v1.2.3 From 3cb43378d83e373c19c811776a5096fa40e95448 Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 23 Oct 2016 11:39:04 +0100 Subject: drm/i2c: tda998x: switch to boolean is_on Rather than storing the DPMS mode (which will always be on or off) use a boolean to store this instead. Tested-by: Robin Murphy Tested-by: Jon Medhurst Acked-by: Jon Medhurst Tested-by: Jyri Sarha Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 78df92b3e055..ffd87edde840 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -42,7 +42,7 @@ struct tda998x_priv { struct mutex mutex; u16 rev; u8 current_page; - int dpms; + bool is_on; bool supports_infoframes; bool sink_has_audio; u8 vip_cntrl_0; @@ -1159,16 +1159,15 @@ static int tda998x_connector_init(struct tda998x_priv *priv, static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) { struct tda998x_priv *priv = enc_to_tda998x_priv(encoder); + bool on; /* we only care about on or off: */ - if (mode != DRM_MODE_DPMS_ON) - mode = DRM_MODE_DPMS_OFF; + on = mode == DRM_MODE_DPMS_ON; - if (mode == priv->dpms) + if (on == priv->is_on) return; - switch (mode) { - case DRM_MODE_DPMS_ON: + if (on) { /* enable video ports, audio will be enabled later */ reg_write(priv, REG_ENA_VP_0, 0xff); reg_write(priv, REG_ENA_VP_1, 0xff); @@ -1177,16 +1176,16 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) reg_write(priv, REG_VIP_CNTRL_0, priv->vip_cntrl_0); reg_write(priv, REG_VIP_CNTRL_1, priv->vip_cntrl_1); reg_write(priv, REG_VIP_CNTRL_2, priv->vip_cntrl_2); - break; - case DRM_MODE_DPMS_OFF: + + priv->is_on = true; + } else { /* disable video ports */ reg_write(priv, REG_ENA_VP_0, 0x00); reg_write(priv, REG_ENA_VP_1, 0x00); reg_write(priv, REG_ENA_VP_2, 0x00); - break; - } - priv->dpms = mode; + priv->is_on = false; + } } static void @@ -1482,8 +1481,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) if (!priv->cec) return -ENODEV; - 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, -- cgit v1.2.3 From 9476ed2e3883b11da1c8065e6d5a1785cae35588 Mon Sep 17 00:00:00 2001 From: Russell King Date: Thu, 3 Nov 2016 15:19:06 +0000 Subject: drm/i2c: tda998x: power down pre-filter and color conversion Disabling the pre-filter block of the TDA998x saves 40mW and the colour conversion block saves 15mW. As we always disable these two blocks, we can power these sections of the chip down to save 55mW of unnecessary power consumption. Tested-by: Brian Starkey Reviewed-by: Brian Starkey Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index ffd87edde840..5a97444798d3 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -107,6 +107,8 @@ struct tda998x_priv { # define I2C_MASTER_DIS_FILT (1 << 1) # define I2C_MASTER_APP_STRT_LAT (1 << 2) #define REG_FEAT_POWERDOWN REG(0x00, 0x0e) /* read/write */ +# define FEAT_POWERDOWN_PREFILT BIT(0) +# define FEAT_POWERDOWN_CSC BIT(1) # define FEAT_POWERDOWN_SPDIF (1 << 3) #define REG_INT_FLAGS_0 REG(0x00, 0x0f) /* read/write */ #define REG_INT_FLAGS_1 REG(0x00, 0x10) /* read/write */ @@ -1284,6 +1286,7 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, /* no pre-filter or interpolator: */ reg_write(priv, REG_HVF_CNTRL_0, HVF_CNTRL_0_PREFIL(0) | HVF_CNTRL_0_INTPOL(0)); + reg_set(priv, REG_FEAT_POWERDOWN, FEAT_POWERDOWN_PREFILT); reg_write(priv, REG_VIP_CNTRL_5, VIP_CNTRL_5_SP_CNT(0)); reg_write(priv, REG_VIP_CNTRL_4, VIP_CNTRL_4_BLANKIT(0) | VIP_CNTRL_4_BLC(0)); @@ -1306,6 +1309,7 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, /* set color matrix bypass flag: */ reg_write(priv, REG_MAT_CONTRL, MAT_CONTRL_MAT_BP | MAT_CONTRL_MAT_SC(1)); + reg_set(priv, REG_FEAT_POWERDOWN, FEAT_POWERDOWN_CSC); /* set BIAS tmds value: */ reg_write(priv, REG_ANA_GENERAL, 0x09); -- cgit v1.2.3 From ae81553c30ef86305048fcac379b8b16b71d3099 Mon Sep 17 00:00:00 2001 From: Russell King Date: Thu, 3 Nov 2016 08:58:04 +0000 Subject: drm/i2c: tda998x: allow interrupt to be shared Some TDA998x contain several different I2C devices - there is the HDMI encoder, and there is a TDA9950 CEC engine. These two share the same interrupt signal. In order to allow a driver for the CEC engine to work, we need to be able to share the interrupt with the CEC driver, so convert the handler and registration to allow this to happen. Tested-by: Brian Starkey Reviewed-by: Brian Starkey Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 52 ++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 25 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 5a97444798d3..5063371e1e08 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -634,28 +634,30 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) bool handled = false; sta = cec_read(priv, REG_CEC_INTSTATUS); - cec = cec_read(priv, REG_CEC_RXSHPDINT); - lvl = cec_read(priv, REG_CEC_RXSHPDLEV); - flag0 = reg_read(priv, REG_INT_FLAGS_0); - flag1 = reg_read(priv, REG_INT_FLAGS_1); - flag2 = reg_read(priv, REG_INT_FLAGS_2); - 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 (cec & CEC_RXSHPDINT_HPD) { - if (lvl & CEC_RXSHPDLEV_HPD) - tda998x_edid_delay_start(priv); - else - schedule_work(&priv->detect_work); - - handled = true; - } + if (sta & CEC_INTSTATUS_HDMI) { + cec = cec_read(priv, REG_CEC_RXSHPDINT); + lvl = cec_read(priv, REG_CEC_RXSHPDLEV); + flag0 = reg_read(priv, REG_INT_FLAGS_0); + flag1 = reg_read(priv, REG_INT_FLAGS_1); + flag2 = reg_read(priv, REG_INT_FLAGS_2); + 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 (cec & CEC_RXSHPDINT_HPD) { + if (lvl & CEC_RXSHPDLEV_HPD) + tda998x_edid_delay_start(priv); + else + schedule_work(&priv->detect_work); + + 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; + 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); @@ -1544,7 +1546,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) /* initialize the optional IRQ */ if (client->irq) { - int irqf_trigger; + unsigned long irq_flags; /* init read EDID waitqueue and HDP work */ init_waitqueue_head(&priv->wq_edid); @@ -1554,11 +1556,11 @@ 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); - irqf_trigger = + irq_flags = irqd_get_trigger_type(irq_get_irq_data(client->irq)); + irq_flags |= IRQF_SHARED | IRQF_ONESHOT; ret = request_threaded_irq(client->irq, NULL, - tda998x_irq_thread, - irqf_trigger | IRQF_ONESHOT, + tda998x_irq_thread, irq_flags, "tda998x", priv); if (ret) { dev_err(&client->dev, -- cgit v1.2.3 From 14e5b5889d75894b633a393aff49d6583bce3b0d Mon Sep 17 00:00:00 2001 From: Russell King Date: Thu, 3 Nov 2016 10:16:17 +0000 Subject: drm/i2c: tda998x: allow sharing of the CEC device accesses cec_read() is non-atomic in the presence of other I2C bus transactions to the same device. This presents a problem when we add support for the TDA9950 CEC engine part - both drivers can be trying to access the device. Avoid the inherent problems by switching to i2c_transfer() instead, which allows us to perform more than one bus transaction atomically. As this means we will be using I2C transactions rather than SMBUS, we have to check that the host supports I2C functionality. Tested-by: Brian Starkey Reviewed-by: Brian Starkey Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 53 +++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 19 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 5063371e1e08..da8430bfb96c 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -41,6 +41,7 @@ struct tda998x_priv { struct i2c_client *hdmi; struct mutex mutex; u16 rev; + u8 cec_addr; u8 current_page; bool is_on; bool supports_infoframes; @@ -374,35 +375,46 @@ struct tda998x_priv { static void cec_write(struct tda998x_priv *priv, u16 addr, u8 val) { - struct i2c_client *client = priv->cec; u8 buf[] = {addr, val}; + struct i2c_msg msg = { + .addr = priv->cec_addr, + .len = 2, + .buf = buf, + }; int ret; - ret = i2c_master_send(client, buf, sizeof(buf)); + ret = i2c_transfer(priv->hdmi->adapter, &msg, 1); if (ret < 0) - dev_err(&client->dev, "Error %d writing to cec:0x%x\n", ret, addr); + dev_err(&priv->hdmi->dev, "Error %d writing to cec:0x%x\n", + ret, addr); } static u8 cec_read(struct tda998x_priv *priv, u8 addr) { - struct i2c_client *client = priv->cec; u8 val; + struct i2c_msg msg[2] = { + { + .addr = priv->cec_addr, + .len = 1, + .buf = &addr, + }, { + .addr = priv->cec_addr, + .flags = I2C_M_RD, + .len = 1, + .buf = &val, + }, + }; int ret; - ret = i2c_master_send(client, &addr, sizeof(addr)); - if (ret < 0) - goto fail; - - ret = i2c_master_recv(client, &val, sizeof(val)); - if (ret < 0) - goto fail; + ret = i2c_transfer(priv->hdmi->adapter, msg, ARRAY_SIZE(msg)); + if (ret < 0) { + dev_err(&priv->hdmi->dev, "Error %d reading from cec:0x%x\n", + ret, addr); + val = 0; + } return val; - -fail: - dev_err(&client->dev, "Error %d reading from cec:0x%x\n", ret, addr); - return 0; } static int @@ -1471,7 +1483,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) struct device_node *np = client->dev.of_node; u32 video; int rev_lo, rev_hi, ret; - unsigned short cec_addr; mutex_init(&priv->audio_mutex); /* Protect access from audio thread */ @@ -1479,11 +1490,11 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1); priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5); + /* CEC I2C address bound to TDA998x I2C addr by configuration pins */ + priv->cec_addr = 0x34 + (client->addr & 0x03); priv->current_page = 0xff; priv->hdmi = client; - /* CEC I2C address bound to TDA998x I2C addr by configuration pins */ - cec_addr = 0x34 + (client->addr & 0x03); - priv->cec = i2c_new_dummy(client->adapter, cec_addr); + priv->cec = i2c_new_dummy(client->adapter, priv->cec_addr); if (!priv->cec) return -ENODEV; @@ -1722,6 +1733,10 @@ static const struct component_ops tda998x_ops = { static int tda998x_probe(struct i2c_client *client, const struct i2c_device_id *id) { + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { + dev_warn(&client->dev, "adapter does not support I2C\n"); + return -EIO; + } return component_add(&client->dev, &tda998x_ops); } -- cgit v1.2.3 From 9b2502b6ebc632ff49743b3639ea12d4f08808a5 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Mon, 14 Nov 2016 22:46:43 +0000 Subject: drm/i2c: tda998x: fix spelling mistake Trivial fix to spelling mistake "configutation" to "configuration" in dev_err message Signed-off-by: Colin Ian King Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index da8430bfb96c..86f47e190309 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -880,7 +880,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, } if (audio.config == 0) { - dev_err(dev, "%s: No audio configutation found\n", __func__); + dev_err(dev, "%s: No audio configuration found\n", __func__); return -EINVAL; } -- cgit v1.2.3