On Sun, Jan 26, 2025 at 03:29:07PM +0200, Dmitry Baryshkov wrote:
> Add generic CEC helpers to be used by HDMI drivers. Both notifier and
> and adapter are supported for registration. Once registered, the driver
> can call common set of functions to update physical address, to
> invalidate it or to unregister CEC data.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.barysh...@linaro.org>
> ---
>  drivers/gpu/drm/display/Kconfig               |   5 +
>  drivers/gpu/drm/display/Makefile              |   2 +
>  drivers/gpu/drm/display/drm_hdmi_cec_helper.c | 209 
> ++++++++++++++++++++++++++
>  include/drm/display/drm_hdmi_cec_helper.h     |  61 ++++++++
>  4 files changed, 277 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
> index 
> 8d22b7627d41f7bc015decf24ae02a05bc00f055..49da9b768acf3e5f84f2cefae4bb042cfd57a50c
>  100644
> --- a/drivers/gpu/drm/display/Kconfig
> +++ b/drivers/gpu/drm/display/Kconfig
> @@ -82,6 +82,11 @@ config DRM_DISPLAY_HDMI_AUDIO_HELPER
>         DRM display helpers for HDMI Audio functionality (generic HDMI Codec
>         implementation).
>  
> +config DRM_DISPLAY_HDMI_CEC_HELPER
> +     bool
> +     help
> +       DRM display helpers for HDMI CEC implementation.
> +
>  config DRM_DISPLAY_HDMI_HELPER
>       bool
>       help
> diff --git a/drivers/gpu/drm/display/Makefile 
> b/drivers/gpu/drm/display/Makefile
> index 
> b17879b957d5401721396e247fa346387cf6c48a..2cd078e2b81c1a9e6b336c4187b444bcb8a50e51
>  100644
> --- a/drivers/gpu/drm/display/Makefile
> +++ b/drivers/gpu/drm/display/Makefile
> @@ -16,6 +16,8 @@ drm_display_helper-$(CONFIG_DRM_DISPLAY_DSC_HELPER) += \
>  drm_display_helper-$(CONFIG_DRM_DISPLAY_HDCP_HELPER) += drm_hdcp_helper.o
>  drm_display_helper-$(CONFIG_DRM_DISPLAY_HDMI_AUDIO_HELPER) += \
>       drm_hdmi_audio_helper.o
> +drm_display_helper-$(CONFIG_DRM_DISPLAY_HDMI_CEC_HELPER) += \
> +     drm_hdmi_cec_helper.o
>  drm_display_helper-$(CONFIG_DRM_DISPLAY_HDMI_HELPER) += \
>       drm_hdmi_helper.o \
>       drm_scdc_helper.o
> diff --git a/drivers/gpu/drm/display/drm_hdmi_cec_helper.c 
> b/drivers/gpu/drm/display/drm_hdmi_cec_helper.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..a6ed5f0fc3835b013a83308f5285ea0819c5702c
> --- /dev/null
> +++ b/drivers/gpu/drm/display/drm_hdmi_cec_helper.c
> @@ -0,0 +1,209 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright (c) 2024 Linaro Ltd
> + */
> +
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_connector.h>
> +#include <drm/display/drm_hdmi_cec_helper.h>
> +
> +#include <linux/mutex.h>
> +
> +#include <media/cec.h>
> +#include <media/cec-notifier.h>
> +
> +void drm_connector_hdmi_cec_unregister(struct drm_connector *connector)
> +{
> +     cec_unregister_adapter(connector->cec.adapter);
> +     connector->cec.adapter = NULL;
> +
> +     cec_notifier_conn_unregister(connector->cec.notifier);
> +     connector->cec.notifier = NULL;
> +
> +     connector->cec.funcs = NULL;
> +}
> +EXPORT_SYMBOL(drm_connector_hdmi_cec_unregister);
> +
> +static const struct drm_connector_cec_funcs drm_connector_hdmi_cec_funcs = {
> +     .unregister = drm_connector_hdmi_cec_unregister,
> +};
> +
> +int drm_connector_hdmi_cec_notifier_register(struct drm_connector *connector,
> +                                          const char *port_name,
> +                                          struct device *dev)
> +{
> +     struct cec_connector_info conn_info;
> +     struct cec_notifier *notifier;
> +     int ret;
> +
> +     mutex_lock(&connector->cec.mutex);
> +
> +     if (connector->cec.funcs) {
> +             ret = -EBUSY;
> +             goto err_unlock;
> +     }
> +
> +     cec_fill_conn_info_from_drm(&conn_info, connector);
> +
> +     notifier = cec_notifier_conn_register(dev, port_name, &conn_info);
> +     if (!notifier) {
> +             ret = -ENOMEM;
> +             goto err_unlock;
> +     }
> +
> +     connector->cec.notifier = notifier;
> +     connector->cec.funcs = &drm_connector_hdmi_cec_funcs;
> +
> +     mutex_unlock(&connector->cec.mutex);
> +
> +     return 0;
> +
> +err_unlock:
> +     mutex_unlock(&connector->cec.mutex);
> +
> +     return ret;
> +}
> +EXPORT_SYMBOL(drm_connector_hdmi_cec_notifier_register);
> +
> +#define to_hdmi_cec_adapter_ops(ops) \
> +     container_of(ops, struct drm_connector_hdmi_cec_adapter_ops, base)
> +
> +static int drm_connector_hdmi_cec_adap_enable(struct cec_adapter *adap, bool 
> enable)
> +{
> +     struct drm_connector *connector = cec_get_drvdata(adap);
> +     struct drm_connector_hdmi_cec_adapter_ops *ops =
> +             to_hdmi_cec_adapter_ops(connector->cec.funcs);
> +
> +     return ops->enable(connector, enable);
> +}
> +
> +static int drm_connector_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 
> logical_addr)
> +{
> +     struct drm_connector *connector = cec_get_drvdata(adap);
> +     struct drm_connector_hdmi_cec_adapter_ops *ops =
> +             to_hdmi_cec_adapter_ops(connector->cec.funcs);
> +
> +     return ops->log_addr(connector, logical_addr);
> +}
> +
> +static int drm_connector_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 
> attempts,
> +                                             u32 signal_free_time, struct 
> cec_msg *msg)
> +{
> +     struct drm_connector *connector = cec_get_drvdata(adap);
> +     struct drm_connector_hdmi_cec_adapter_ops *ops =
> +             to_hdmi_cec_adapter_ops(connector->cec.funcs);
> +
> +     return ops->transmit(connector, attempts, signal_free_time, msg);
> +}
> +
> +static const struct cec_adap_ops drm_connector_hdmi_cec_adap_ops = {
> +     .adap_enable = drm_connector_hdmi_cec_adap_enable,
> +     .adap_log_addr = drm_connector_hdmi_cec_adap_log_addr,
> +     .adap_transmit = drm_connector_hdmi_cec_adap_transmit,
> +};
> +
> +int drm_connector_hdmi_cec_register(struct drm_connector *connector,
> +                                 const struct 
> drm_connector_hdmi_cec_adapter_ops *ops,
> +                                 const char *name,
> +                                 u8 available_las,
> +                                 struct device *dev)
> +{
> +     struct cec_connector_info conn_info;
> +     struct cec_adapter *cec_adap;
> +     int ret;
> +
> +     if (!ops->base.unregister ||
> +         !ops->init || !ops->enable || !ops->log_addr || !ops->transmit)
> +             return -EINVAL;
> +
> +     mutex_lock(&connector->cec.mutex);
> +
> +     if (connector->cec.funcs) {
> +             ret = -EBUSY;
> +             goto err_unlock;
> +     }
> +
> +     cec_adap = cec_allocate_adapter(&drm_connector_hdmi_cec_adap_ops, 
> connector, name,
> +                                     CEC_CAP_DEFAULTS | 
> CEC_CAP_CONNECTOR_INFO,
> +                                     available_las ? : CEC_MAX_LOG_ADDRS);
> +     ret = PTR_ERR_OR_ZERO(cec_adap);
> +     if (ret < 0)
> +             goto err_unlock;
> +
> +     cec_fill_conn_info_from_drm(&conn_info, connector);
> +     cec_s_conn_info(cec_adap, &conn_info);
> +
> +     connector->cec.adapter = cec_adap;
> +     connector->cec.funcs = &ops->base;
> +
> +     ret = ops->init(connector);
> +     if (ret < 0)
> +             goto err_delete_adapter;
> +
> +     ret = cec_register_adapter(cec_adap, dev);
> +     if (ret < 0)
> +             goto err_delete_adapter;
> +
> +     mutex_unlock(&connector->cec.mutex);
> +
> +     return 0;
> +
> +err_delete_adapter:
> +     cec_delete_adapter(cec_adap);
> +
> +     connector->cec.adapter = NULL;
> +
> +err_unlock:
> +     mutex_unlock(&connector->cec.mutex);
> +
> +     return ret;
> +}
> +EXPORT_SYMBOL(drm_connector_hdmi_cec_register);
> +
> +void drm_connector_hdmi_cec_received_msg(struct drm_connector *connector,
> +                                      struct cec_msg *msg)
> +{
> +     cec_received_msg(connector->cec.adapter, msg);
> +}
> +EXPORT_SYMBOL(drm_connector_hdmi_cec_received_msg);
> +
> +void drm_connector_hdmi_cec_transmit_attempt_done(struct drm_connector 
> *connector,
> +                                               u8 status)
> +{
> +     cec_transmit_attempt_done(connector->cec.adapter, status);
> +}
> +EXPORT_SYMBOL(drm_connector_hdmi_cec_transmit_attempt_done);
> +
> +void drm_connector_hdmi_cec_transmit_done(struct drm_connector *connector,
> +                                       u8 status,
> +                                       u8 arb_lost_cnt, u8 nack_cnt,
> +                                       u8 low_drive_cnt, u8 error_cnt)
> +{
> +     cec_transmit_done(connector->cec.adapter, status,
> +                       arb_lost_cnt, nack_cnt, low_drive_cnt, error_cnt);
> +}
> +EXPORT_SYMBOL(drm_connector_hdmi_cec_transmit_done);
> +
> +void drm_connector_hdmi_cec_phys_addr_invalidate(struct drm_connector 
> *connector)
> +{
> +     mutex_lock(&connector->cec.mutex);
> +
> +     cec_phys_addr_invalidate(connector->cec.adapter);
> +     cec_notifier_phys_addr_invalidate(connector->cec.notifier);
> +
> +     mutex_unlock(&connector->cec.mutex);
> +}
> +EXPORT_SYMBOL(drm_connector_hdmi_cec_phys_addr_invalidate);
> +
> +void drm_connector_hdmi_cec_phys_addr_set(struct drm_connector *connector)
> +{
> +     mutex_lock(&connector->cec.mutex);
> +
> +     cec_s_phys_addr(connector->cec.adapter,
> +                     connector->display_info.source_physical_address, false);
> +     cec_notifier_set_phys_addr(connector->cec.notifier,
> +                                
> connector->display_info.source_physical_address);
> +
> +     mutex_unlock(&connector->cec.mutex);
> +}
> +EXPORT_SYMBOL(drm_connector_hdmi_cec_phys_addr_set);
> diff --git a/include/drm/display/drm_hdmi_cec_helper.h 
> b/include/drm/display/drm_hdmi_cec_helper.h
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..cd6274e4ee9b3e41a2d85289c4a420b854340e19
> --- /dev/null
> +++ b/include/drm/display/drm_hdmi_cec_helper.h
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +#ifndef DRM_DISPLAY_HDMI_CEC_HELPER
> +#define DRM_DISPLAY_HDMI_CEC_HELPER
> +
> +#include <drm/drm_connector.h>
> +
> +#include <linux/types.h>
> +
> +struct drm_connector;
> +
> +struct cec_msg;
> +struct device;
> +
> +struct drm_connector_hdmi_cec_adapter_ops {
> +     struct drm_connector_cec_funcs base;
> +
> +     int (*init)(struct drm_connector *connector);
> +     void (*uninit)(struct drm_connector *connector);
> +
> +     int (*enable)(struct drm_connector *connector, bool enable);
> +     int (*log_addr)(struct drm_connector *connector, u8 logical_addr);
> +     int (*transmit)(struct drm_connector *connector, u8 attempts,
> +                     u32 signal_free_time, struct cec_msg *msg);
> +};

Why can't we merge drm_connector_cec_funcs and
drm_connector_cec_adapter_ops? They look equivalent to me?

We should also document those hooks.

Aside from the mutex discussion, the rest of the patch looks good to me.

Maxime

Attachment: signature.asc
Description: PGP signature

Reply via email to