[PATCH v7] drm/vc4: hdmi: switch to generic CEC helpers
Dmitry Baryshkov
dmitry.baryshkov at oss.qualcomm.com
Tue May 20 22:44:55 UTC 2025
Switch VC4 driver to using CEC helpers code, simplifying hotplug and
registration / cleanup. The existing vc4_hdmi_cec_release() is kept for
now.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
Reviewed-by: Maxime Ripard <mripard at kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at oss.qualcomm.com>
---
This is a part of the HDMI CEC rework, posting separately to let it be
tested by the maintainers.
---
Changes in v7:
- Dropped all applied patches, keeping just VC4
- Link to v6: https://lore.kernel.org/r/20250517-drm-hdmi-connector-cec-v6-0-35651db6f19b@oss.qualcomm.com
Changes in v6:
- Fixed vc4 to build with changed API (Maxime)
- Reworked helpers to use drmm to unregister CEC implementations
(Maxime)
- Expanded commit message to explain void *data design (Maxime)
- Dropped extra include of drm_connector.h from drm_hdmi_cec_helper.h
(Jani)
- Link to v5: https://lore.kernel.org/r/20250407-drm-hdmi-connector-cec-v5-0-04809b10d206@oss.qualcomm.com
Changes in v5:
- Fixed the check in adv7511_bridge_hdmi_tmds_char_rate_valid().
- Major rework of HDMI CEC to follow 'helpers' design. Implemented
generic data structures that can be used for both CEC notifiers and
CEC adapters (Maxime).
- Link to v4: https://lore.kernel.org/r/20250202-drm-hdmi-connector-cec-v4-0-a71620a54d50@linaro.org
Changes in v4:
- Rebased on top of drm-misc-next + commit 78a5acf5433d ("drm/display:
hdmi: Do not read EDID on disconnected connectors")
- Moved 'select DRM_DISPLAY_HDMI_CEC_HELPER' under the
DRM_I2C_ADV7511_CEC symbol
- Fixed documentation for @hdmi_audio_i2s_formats to describe default
behaviour.
- Split drm_connector_cleanup() patch from the patch adding CEC-related
data structures (Maxime).
- Documented that CEC mutex protects all data fields in struct
drm_connector_cec (Maxime).
- Improved drm_connector_cec_funcs.unregister() documentation.
- Documented struct drm_connector_hdmi_cec_adapter_ops fields. Added a
note to the commit message on the difference between it and struct
drm_connector_cec_funcs (Maxime).
- Fixed Kconfig dependencies.
- Link to v3: https://lore.kernel.org/r/20250126-drm-hdmi-connector-cec-v3-0-5b5b2d4956da@linaro.org
Changes in v3:
- Moved default available_las setting to
drm_connector_hdmi_cec_register(), allowing drivers to pass 0 instead
of CEC_MAX_LOG_ADDRS.
- Reworked drm_bridge interface to store opaque pointer and interpret it
as drm_connector in CEC helpers code.
- Fixed EINVAL checks in drm_connector_hdmi_cec_register().
- Added the adv7511 patch, demonstrating CEC helpers for the DRM
bridges.
- Link to v2: https://lore.kernel.org/r/20250110-drm-hdmi-connector-cec-v2-0-9067c8f34663@linaro.org
Changes in v2:
- Refactored CEC funcs, adding more wrappers to provide more consistent
interface (Maxime)
- Removed export of drm_connector_cec_unregister() (Maxime)
- Restored and rephrased comment in vc4_hdmi (Maxime)
- Squashed vc4 patches
- Link to v1: https://lore.kernel.org/r/20241225-drm-hdmi-connector-cec-v1-0-b80380c67221@linaro.org
---
drivers/gpu/drm/vc4/Kconfig | 1 +
drivers/gpu/drm/vc4/vc4_hdmi.c | 137 ++++++++++++++++-------------------------
drivers/gpu/drm/vc4/vc4_hdmi.h | 1 -
3 files changed, 55 insertions(+), 84 deletions(-)
diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index 123ab0ce178157c3b39466f87c7ac39c8470f329..bb8c40be325033632d3e94db87a16b03554ad3af 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -35,6 +35,7 @@ config DRM_VC4_HDMI_CEC
bool "Broadcom VC4 HDMI CEC Support"
depends on DRM_VC4
select CEC_CORE
+ select DRM_DISPLAY_HDMI_CEC_HELPER
help
Choose this option if you have a Broadcom VC4 GPU
and want to use CEC.
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 4797ed1c21f47992fe4d497d904ee31c824cd449..194a73fb821ae5082f308c81293c22fed0dbda80 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -32,6 +32,7 @@
*/
#include <drm/display/drm_hdmi_audio_helper.h>
+#include <drm/display/drm_hdmi_cec_helper.h>
#include <drm/display/drm_hdmi_helper.h>
#include <drm/display/drm_hdmi_state_helper.h>
#include <drm/display/drm_scdc_helper.h>
@@ -375,14 +376,6 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
drm_atomic_helper_connector_hdmi_hotplug(connector, status);
- if (status == connector_status_disconnected) {
- cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
- return;
- }
-
- cec_s_phys_addr(vc4_hdmi->cec_adap,
- connector->display_info.source_physical_address, false);
-
if (status != connector_status_connected)
return;
@@ -2378,8 +2371,8 @@ static irqreturn_t vc4_cec_irq_handler_rx_thread(int irq, void *priv)
struct vc4_hdmi *vc4_hdmi = priv;
if (vc4_hdmi->cec_rx_msg.len)
- cec_received_msg(vc4_hdmi->cec_adap,
- &vc4_hdmi->cec_rx_msg);
+ drm_connector_hdmi_cec_received_msg(&vc4_hdmi->connector,
+ &vc4_hdmi->cec_rx_msg);
return IRQ_HANDLED;
}
@@ -2389,15 +2382,17 @@ static irqreturn_t vc4_cec_irq_handler_tx_thread(int irq, void *priv)
struct vc4_hdmi *vc4_hdmi = priv;
if (vc4_hdmi->cec_tx_ok) {
- cec_transmit_done(vc4_hdmi->cec_adap, CEC_TX_STATUS_OK,
- 0, 0, 0, 0);
+ drm_connector_hdmi_cec_transmit_done(&vc4_hdmi->connector,
+ CEC_TX_STATUS_OK,
+ 0, 0, 0, 0);
} else {
/*
* This CEC implementation makes 1 retry, so if we
* get a NACK, then that means it made 2 attempts.
*/
- cec_transmit_done(vc4_hdmi->cec_adap, CEC_TX_STATUS_NACK,
- 0, 2, 0, 0);
+ drm_connector_hdmi_cec_transmit_done(&vc4_hdmi->connector,
+ CEC_TX_STATUS_NACK,
+ 0, 2, 0, 0);
}
return IRQ_HANDLED;
}
@@ -2554,9 +2549,9 @@ static irqreturn_t vc4_cec_irq_handler(int irq, void *priv)
return ret;
}
-static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
+static int vc4_hdmi_cec_enable(struct drm_connector *connector)
{
- struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+ struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
struct drm_device *drm = vc4_hdmi->connector.dev;
/* clock period in microseconds */
const u32 usecs = 1000000 / CEC_CLOCK_FREQ;
@@ -2621,9 +2616,9 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
return 0;
}
-static int vc4_hdmi_cec_disable(struct cec_adapter *adap)
+static int vc4_hdmi_cec_disable(struct drm_connector *connector)
{
- struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+ struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
struct drm_device *drm = vc4_hdmi->connector.dev;
unsigned long flags;
int idx;
@@ -2657,17 +2652,17 @@ static int vc4_hdmi_cec_disable(struct cec_adapter *adap)
return 0;
}
-static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
+static int vc4_hdmi_cec_adap_enable(struct drm_connector *connector, bool enable)
{
if (enable)
- return vc4_hdmi_cec_enable(adap);
+ return vc4_hdmi_cec_enable(connector);
else
- return vc4_hdmi_cec_disable(adap);
+ return vc4_hdmi_cec_disable(connector);
}
-static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
+static int vc4_hdmi_cec_adap_log_addr(struct drm_connector *connector, u8 log_addr)
{
- struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+ struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
struct drm_device *drm = vc4_hdmi->connector.dev;
unsigned long flags;
int idx;
@@ -2693,10 +2688,10 @@ static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
return 0;
}
-static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
+static int vc4_hdmi_cec_adap_transmit(struct drm_connector *connector, u8 attempts,
u32 signal_free_time, struct cec_msg *msg)
{
- struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+ struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
struct drm_device *dev = vc4_hdmi->connector.dev;
unsigned long flags;
u32 val;
@@ -2739,84 +2734,65 @@ static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
return 0;
}
-static const struct cec_adap_ops vc4_hdmi_cec_adap_ops = {
- .adap_enable = vc4_hdmi_cec_adap_enable,
- .adap_log_addr = vc4_hdmi_cec_adap_log_addr,
- .adap_transmit = vc4_hdmi_cec_adap_transmit,
-};
-
-static void vc4_hdmi_cec_release(void *ptr)
-{
- struct vc4_hdmi *vc4_hdmi = ptr;
-
- cec_unregister_adapter(vc4_hdmi->cec_adap);
- vc4_hdmi->cec_adap = NULL;
-}
-
-static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
+static int vc4_hdmi_cec_init(struct drm_connector *connector)
{
- struct cec_connector_info conn_info;
+ struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
struct platform_device *pdev = vc4_hdmi->pdev;
struct device *dev = &pdev->dev;
int ret;
- if (!of_property_present(dev->of_node, "interrupts")) {
- dev_warn(dev, "'interrupts' DT property is missing, no CEC\n");
- return 0;
- }
-
- vc4_hdmi->cec_adap = cec_allocate_adapter(&vc4_hdmi_cec_adap_ops,
- vc4_hdmi,
- vc4_hdmi->variant->card_name,
- CEC_CAP_DEFAULTS |
- CEC_CAP_CONNECTOR_INFO, 1);
- ret = PTR_ERR_OR_ZERO(vc4_hdmi->cec_adap);
- if (ret < 0)
- return ret;
-
- cec_fill_conn_info_from_drm(&conn_info, &vc4_hdmi->connector);
- cec_s_conn_info(vc4_hdmi->cec_adap, &conn_info);
-
if (vc4_hdmi->variant->external_irq_controller) {
ret = devm_request_threaded_irq(dev, platform_get_irq_byname(pdev, "cec-rx"),
vc4_cec_irq_handler_rx_bare,
vc4_cec_irq_handler_rx_thread, 0,
"vc4 hdmi cec rx", vc4_hdmi);
if (ret)
- goto err_delete_cec_adap;
+ return ret;
ret = devm_request_threaded_irq(dev, platform_get_irq_byname(pdev, "cec-tx"),
vc4_cec_irq_handler_tx_bare,
vc4_cec_irq_handler_tx_thread, 0,
"vc4 hdmi cec tx", vc4_hdmi);
if (ret)
- goto err_delete_cec_adap;
+ return ret;
} else {
ret = devm_request_threaded_irq(dev, platform_get_irq(pdev, 0),
vc4_cec_irq_handler,
vc4_cec_irq_handler_thread, 0,
"vc4 hdmi cec", vc4_hdmi);
if (ret)
- goto err_delete_cec_adap;
+ return ret;
}
- ret = cec_register_adapter(vc4_hdmi->cec_adap, &pdev->dev);
- if (ret < 0)
- goto err_delete_cec_adap;
+ return 0;
+}
+
+static const struct drm_connector_hdmi_cec_funcs vc4_hdmi_cec_funcs = {
+ .init = vc4_hdmi_cec_init,
+ .enable = vc4_hdmi_cec_adap_enable,
+ .log_addr = vc4_hdmi_cec_adap_log_addr,
+ .transmit = vc4_hdmi_cec_adap_transmit,
+};
+
+static int vc4_hdmi_cec_register(struct vc4_hdmi *vc4_hdmi)
+{
+ struct platform_device *pdev = vc4_hdmi->pdev;
+ struct device *dev = &pdev->dev;
+
+ if (!of_property_present(dev->of_node, "interrupts")) {
+ dev_warn(dev, "'interrupts' DT property is missing, no CEC\n");
+ return 0;
+ }
/*
- * NOTE: Strictly speaking, we should probably use a DRM-managed
- * registration there to avoid removing the CEC adapter by the
- * time the DRM driver doesn't have any user anymore.
+ * NOTE: the CEC adapter will be unregistered by drmm cleanup from
+ * drm_managed_release(), which is called from drm_dev_release()
+ * during device unbind.
*
* However, the CEC framework already cleans up the CEC adapter
* only when the last user has closed its file descriptor, so we
* don't need to handle it in DRM.
*
- * By the time the device-managed hook is executed, we will give
- * up our reference to the CEC adapter and therefore don't
- * really care when it's actually freed.
- *
* There's still a problematic sequence: if we unregister our
* CEC adapter, but the userspace keeps a handle on the CEC
* adapter but not the DRM device for some reason. In such a
@@ -2827,19 +2803,14 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
* the CEC framework already handles this too, by calling
* cec_is_registered() in cec_ioctl() and cec_poll().
*/
- ret = devm_add_action_or_reset(dev, vc4_hdmi_cec_release, vc4_hdmi);
- if (ret)
- return ret;
-
- return 0;
-
-err_delete_cec_adap:
- cec_delete_adapter(vc4_hdmi->cec_adap);
-
- return ret;
+ return drmm_connector_hdmi_cec_register(&vc4_hdmi->connector,
+ &vc4_hdmi_cec_funcs,
+ vc4_hdmi->variant->card_name,
+ 1,
+ &pdev->dev);
}
#else
-static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
+static int vc4_hdmi_cec_register(struct vc4_hdmi *vc4_hdmi)
{
return 0;
}
@@ -3244,7 +3215,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
if (ret)
goto err_put_runtime_pm;
- ret = vc4_hdmi_cec_init(vc4_hdmi);
+ ret = vc4_hdmi_cec_register(vc4_hdmi);
if (ret)
goto err_put_runtime_pm;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index a31157c99bee6b33527c4b558fc72fff65d2a278..8d069718df00d9afc13aadbb12648e2bb75a1721 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -147,7 +147,6 @@ struct vc4_hdmi {
*/
bool disable_wifi_frequencies;
- struct cec_adapter *cec_adap;
struct cec_msg cec_rx_msg;
bool cec_tx_ok;
bool cec_irq_was_rx;
---
base-commit: ae01d3183d2763ed27ab71f4ef5402b683d9ad8a
change-id: 20241223-drm-hdmi-connector-cec-34902ce847b7
Best regards,
--
Dmitry Baryshkov <dmitry.baryshkov at oss.qualcomm.com>
More information about the dri-devel
mailing list