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: > > 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 <Sandor.yu@xxxxxxx> > > --- > > 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.h > > > > diff --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 internal > > diff --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.o > > diff --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: > > > 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, > Alexander > > > [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