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@linaro.org>
---
drivers/gpu/drm/vc4/Kconfig | 1 +
drivers/gpu/drm/vc4/vc4_hdmi.c | 92 ++++++++++++++++++++----------------------
drivers/gpu/drm/vc4/vc4_hdmi.h | 1 -
3 files changed, 44 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index 6cc7b7e6294a1bfa54137ca65296cd47e46b1e1e..360fbe755951cc40fecb4f9d643a096a6cf92b8e 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -34,6 +34,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 47d9ada98430634cfd8c1e21c2a4d00d501bab7e..3086c2ad3bb2e8fafdc1f37ba985aa5785d49f9a 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>
@@ -400,16 +401,8 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
* the lock for now.
*/
- if (status == connector_status_disconnected) {
- cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
- return;
- }
-
drm_atomic_helper_connector_hdmi_hotplug(connector, status);
- cec_s_phys_addr(vc4_hdmi->cec_adap,
- connector->display_info.source_physical_address, false);
-
if (status != connector_status_connected)
return;
@@ -2388,7 +2381,7 @@ 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,
+ cec_received_msg(vc4_hdmi->connector.cec.adapter,
&vc4_hdmi->cec_rx_msg);
return IRQ_HANDLED;
@@ -2399,14 +2392,14 @@ 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,
+ cec_transmit_done(vc4_hdmi->connector.cec.adapter, 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,
+ cec_transmit_done(vc4_hdmi->connector.cec.adapter, CEC_TX_STATUS_NACK,
0, 2, 0, 0);
}
return IRQ_HANDLED;
@@ -2566,7 +2559,8 @@ static irqreturn_t vc4_cec_irq_handler(int irq, void *priv)
static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
{
- struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+ struct drm_connector *connector = 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;
@@ -2633,7 +2627,8 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
static int vc4_hdmi_cec_disable(struct cec_adapter *adap)
{
- struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+ struct drm_connector *connector = 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;
@@ -2677,7 +2672,8 @@ static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
{
- struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+ struct drm_connector *connector = 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;
@@ -2706,7 +2702,8 @@ static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
u32 signal_free_time, struct cec_msg *msg)
{
- struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+ struct drm_connector *connector = 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;
@@ -2757,62 +2754,64 @@ static const struct cec_adap_ops vc4_hdmi_cec_adap_ops = {
static void vc4_hdmi_cec_release(void *ptr)
{
- struct vc4_hdmi *vc4_hdmi = ptr;
+ struct drm_connector *connector = ptr;
- cec_unregister_adapter(vc4_hdmi->cec_adap);
- vc4_hdmi->cec_adap = NULL;
+ drm_connector_cec_unregister(connector);
}
-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);
+ return 0;
+}
+
+static int vc4_hdmi_cec_register(struct vc4_hdmi *vc4_hdmi)
+{
+ 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;
+ }
+
+ ret = drm_connector_hdmi_cec_adapter_register(&vc4_hdmi->connector,
+ &vc4_hdmi_cec_adap_ops,
+ vc4_hdmi->variant->card_name,
+ 1,
+ vc4_hdmi_cec_init,
+ NULL,
+ &pdev->dev);
if (ret < 0)
- goto err_delete_cec_adap;
+ return ret;
/*
* NOTE: Strictly speaking, we should probably use a DRM-managed
@@ -2842,14 +2841,9 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
return ret;
return 0;
-
-err_delete_cec_adap:
- cec_delete_adapter(vc4_hdmi->cec_adap);
-
- return ret;
}
#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;
}
@@ -3271,7 +3265,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 e3d989ca302b72533c374dfa3fd0d5bd7fe64a82..5acbe27fb57659d02f32ca571dd3ded4a1a0d9dc 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -146,7 +146,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;
--
2.39.5On Wed, Dec 25, 2024 at 01:10:12AM +0200, Dmitry Baryshkov wrote:
> 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@linaro.org>
> ---
> drivers/gpu/drm/vc4/Kconfig | 1 +
> drivers/gpu/drm/vc4/vc4_hdmi.c | 92 ++++++++++++++++++++----------------------
> drivers/gpu/drm/vc4/vc4_hdmi.h | 1 -
> 3 files changed, 44 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
> index 6cc7b7e6294a1bfa54137ca65296cd47e46b1e1e..360fbe755951cc40fecb4f9d643a096a6cf92b8e 100644
> --- a/drivers/gpu/drm/vc4/Kconfig
> +++ b/drivers/gpu/drm/vc4/Kconfig
> @@ -34,6 +34,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 47d9ada98430634cfd8c1e21c2a4d00d501bab7e..3086c2ad3bb2e8fafdc1f37ba985aa5785d49f9a 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>
> @@ -400,16 +401,8 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
> * the lock for now.
> */
>
> - if (status == connector_status_disconnected) {
> - cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
> - return;
> - }
> -
> drm_atomic_helper_connector_hdmi_hotplug(connector, status);
>
> - cec_s_phys_addr(vc4_hdmi->cec_adap,
> - connector->display_info.source_physical_address, false);
> -
> if (status != connector_status_connected)
> return;
>
> @@ -2388,7 +2381,7 @@ 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,
> + cec_received_msg(vc4_hdmi->connector.cec.adapter,
> &vc4_hdmi->cec_rx_msg);
>
> return IRQ_HANDLED;
> @@ -2399,14 +2392,14 @@ 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,
> + cec_transmit_done(vc4_hdmi->connector.cec.adapter, CEC_TX_STATUS_OK,
> 0, 0, 0, 0);
Shouldn't we create helpers for those just like we did to deal with phys_addr?
Maxime
On Tue, Jan 07, 2025 at 03:34:35PM +0100, Maxime Ripard wrote:
> On Wed, Dec 25, 2024 at 01:10:12AM +0200, Dmitry Baryshkov wrote:
> > 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@linaro.org>
> > ---
> > drivers/gpu/drm/vc4/Kconfig | 1 +
> > drivers/gpu/drm/vc4/vc4_hdmi.c | 92 ++++++++++++++++++++----------------------
> > drivers/gpu/drm/vc4/vc4_hdmi.h | 1 -
> > 3 files changed, 44 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
> > index 6cc7b7e6294a1bfa54137ca65296cd47e46b1e1e..360fbe755951cc40fecb4f9d643a096a6cf92b8e 100644
> > --- a/drivers/gpu/drm/vc4/Kconfig
> > +++ b/drivers/gpu/drm/vc4/Kconfig
> > @@ -34,6 +34,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 47d9ada98430634cfd8c1e21c2a4d00d501bab7e..3086c2ad3bb2e8fafdc1f37ba985aa5785d49f9a 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>
> > @@ -400,16 +401,8 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
> > * the lock for now.
> > */
> >
> > - if (status == connector_status_disconnected) {
> > - cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
> > - return;
> > - }
> > -
> > drm_atomic_helper_connector_hdmi_hotplug(connector, status);
> >
> > - cec_s_phys_addr(vc4_hdmi->cec_adap,
> > - connector->display_info.source_physical_address, false);
> > -
> > if (status != connector_status_connected)
> > return;
> >
> > @@ -2388,7 +2381,7 @@ 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,
> > + cec_received_msg(vc4_hdmi->connector.cec.adapter,
> > &vc4_hdmi->cec_rx_msg);
> >
> > return IRQ_HANDLED;
> > @@ -2399,14 +2392,14 @@ 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,
> > + cec_transmit_done(vc4_hdmi->connector.cec.adapter, CEC_TX_STATUS_OK,
> > 0, 0, 0, 0);
>
> Shouldn't we create helpers for those just like we did to deal with phys_addr?
For phys_addr it was required as we were wrapping locking, adapter and
notifier calls. I can add wrappers for these calls too, of course.
--
With best wishes
Dmitry
© 2016 - 2026 Red Hat, Inc.