Re: [PATCH v17 1/8] drm: bridge: Cadence: Create mhdp helper driver
From: Dmitry Baryshkov <hidden>
Date: 2024-09-24 09:31:05
Also in:
dri-devel, linux-arm-kernel, linux-phy, lkml
On Tue, Sep 24, 2024 at 10:41:26AM GMT, Alexander Stein wrote:
Hi Sandor, Am Dienstag, 24. September 2024, 09:36:46 CEST schrieb Sandor Yu:quoted
MHDP8546 mailbox access functions will be share to other mhdp driver and Cadence HDP-TX HDMI/DP PHY drivers. Create a new mhdp helper driver and move all those functions into. cdns_mhdp_reg_write() is renamed to cdns_mhdp_dp_reg_write(), because it use the DPTX command ID DPTX_WRITE_REGISTER. New cdns_mhdp_reg_write() is created with the general command ID GENERAL_REGISTER_WRITE. Rewrite cdns_mhdp_set_firmware_active() in mhdp8546 core driver, use cdns_mhdp_mailbox_send() to replace cdns_mhdp_mailbox_write() same as the other mailbox access functions. Replaces the local mutex mbox_mutex with a global mutex mhdp_mailbox_mutex to prevent race conditions in mailbox access by multi drivers. Signed-off-by: Sandor Yu <redacted> --- v16->v17: - Replaces the local mutex mbox_mutex with a global mutex mhdp_mailbox_mutex v12->v16: *No change. V11->v12: - Move status initialize out of mbox_mutex. - Reorder API functions in alphabetical. - Add notes for malibox access functions. - Add year 2024 to copyright. drivers/gpu/drm/bridge/cadence/Kconfig | 4 + drivers/gpu/drm/bridge/cadence/Makefile | 1 + .../gpu/drm/bridge/cadence/cdns-mhdp-helper.c | 307 +++++++++++++ .../drm/bridge/cadence/cdns-mhdp8546-core.c | 425 ++++-------------- .../drm/bridge/cadence/cdns-mhdp8546-core.h | 47 +- .../drm/bridge/cadence/cdns-mhdp8546-hdcp.c | 36 +- include/drm/bridge/cdns-mhdp-helper.h | 94 ++++ 7 files changed, 507 insertions(+), 407 deletions(-) create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp-helper.c create mode 100644 include/drm/bridge/cdns-mhdp-helper.hdiff --git a/drivers/gpu/drm/bridge/cadence/Kconfig b/drivers/gpu/drm/bridge/cadence/Kconfig index cced81633ddcd..e0973339e9e33 100644 --- a/drivers/gpu/drm/bridge/cadence/Kconfig +++ b/drivers/gpu/drm/bridge/cadence/Kconfig@@ -21,6 +21,9 @@ config DRM_CDNS_DSI_J721E the routing of the DSS DPI signal to the Cadence DSI. endif +config CDNS_MHDP_HELPER + tristate + config DRM_CDNS_MHDP8546 tristate "Cadence DPI/DP bridge" select DRM_DISPLAY_DP_HELPER@@ -28,6 +31,7 @@ config DRM_CDNS_MHDP8546 select DRM_DISPLAY_HELPER select DRM_KMS_HELPER select DRM_PANEL_BRIDGE + select CDNS_MHDP_HELPER depends on OF help Support Cadence DPI to DP bridge. This is an internaldiff --git a/drivers/gpu/drm/bridge/cadence/Makefile b/drivers/gpu/drm/bridge/cadence/Makefile index c95fd5b81d137..087dc074820d7 100644 --- a/drivers/gpu/drm/bridge/cadence/Makefile +++ b/drivers/gpu/drm/bridge/cadence/Makefile@@ -2,6 +2,7 @@ obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o cdns-dsi-y := cdns-dsi-core.o cdns-dsi-$(CONFIG_DRM_CDNS_DSI_J721E) += cdns-dsi-j721e.o +obj-$(CONFIG_CDNS_MHDP_HELPER) += cdns-mhdp-helper.o obj-$(CONFIG_DRM_CDNS_MHDP8546) += cdns-mhdp8546.o cdns-mhdp8546-y := cdns-mhdp8546-core.o cdns-mhdp8546-hdcp.o cdns-mhdp8546-$(CONFIG_DRM_CDNS_MHDP8546_J721E) += cdns-mhdp8546-j721e.odiff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp-helper.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-helper.c new file mode 100644 index 0000000000000..c60a6b69a5343 --- /dev/null +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-helper.c@@ -0,0 +1,307 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2023, 2024 NXP Semiconductor, Inc. + * + */ +#include <drm/bridge/cdns-mhdp-helper.h> +#include <linux/dev_printk.h> +#include <linux/module.h> + +/* Protects mailbox communications with the firmware */ +DEFINE_MUTEX(mhdp_mailbox_mutex);This is not enough if the driver is built as a module:quoted
ERROR: modpost: "mhdp_mailbox_mutex" [drivers/gpu/drm/bridge/cadence/cdns-mhdp8546.ko] undefined! ERROR: modpost: "mhdp_mailbox_mutex" [drivers/gpu/drm/bridge/cadence/cdns-mhdp8501.ko] undefined!Not sure if EXPORT_SYMBOL_GPL() on a mutex is considered good style.
Absolutely no. The API should wrap register access, using the mutex whether required, rather than exporting the mutex for everybody else to use.
Best regards, Alexanderquoted
[snip]-- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider http://www.tq-group.com/
-- With best wishes Dmitry