| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ylahtg54vvrpg5rzp3z5oyi37mtblj3hn4pzwylcimfakrzy3m@idqczwb3hvxl>
Date: Tue, 28 Jan 2025 11:36:06 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Dave Stevenson <dave.stevenson@...pberrypi.com>,
Maíra Canal <mcanal@...lia.com>, Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>,
Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong <neil.armstrong@...aro.org>,
Robert Foss <rfoss@...nel.org>, Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 02/10] drm/display: add CEC helpers code
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.baryshkov@...aro.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
Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)
Powered by blists - more mailing lists