Hi Sandor, thanks for the updated series. Am Freitag, 13. Oktober 2023, 05:24:20 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. > > Signed-off-by: Sandor Yu <Sandor.yu@xxxxxxx> > --- > v9->v10: > *use mhdp helper driver to replace macro functions, > move maibox access function and mhdp hdmi/dp common API > functions into the driver. > > drivers/gpu/drm/bridge/cadence/Kconfig | 4 > drivers/gpu/drm/bridge/cadence/Makefile | 1 + > .../gpu/drm/bridge/cadence/cdns-mhdp-helper.c | 306 ++++++++++++++ > .../drm/bridge/cadence/cdns-mhdp8546-core.c | 383 +++--------------- > .../drm/bridge/cadence/cdns-mhdp8546-core.h | 44 +- > include/drm/bridge/cdns-mhdp-helper.h | 96 +++++ > 6 files changed, 473 insertions(+), 361 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 ec35215a20034..0b7b4626a7af0 > 100644 > --- a/drivers/gpu/drm/bridge/cadence/Kconfig > +++ b/drivers/gpu/drm/bridge/cadence/Kconfig > @@ -20,6 +20,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 > @@ -27,6 +30,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..2e3eee40494f0 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-helper.c > @@ -0,0 +1,306 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2023 NXP Semiconductor, Inc. > + * > + */ > +#include <drm/bridge/cdns-mhdp-helper.h> > +#include <linux/dev_printk.h> > +#include <linux/module.h> > + > +/* Mailbox helper functions */ > +int cdns_mhdp_mailbox_read(struct cdns_mhdp_base *base) > +{ > + int ret, empty; > + > + WARN_ON(!mutex_is_locked(base->mbox_mutex)); > + > + ret = readx_poll_timeout(readl, base->regs + CDNS_MAILBOX_EMPTY, > + empty, !empty, MAILBOX_RETRY_US, > + MAILBOX_TIMEOUT_US); > + if (ret < 0) > + return ret; > + > + return readl(base->regs + CDNS_MAILBOX_RX_DATA) & 0xff; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_mailbox_read); No need to export this. You can make this function actually static. > + > +int cdns_mhdp_mailbox_write(struct cdns_mhdp_base *base, u8 val) > +{ > + int ret, full; > + > + WARN_ON(!mutex_is_locked(base->mbox_mutex)); > + > + ret = readx_poll_timeout(readl, base->regs + CDNS_MAILBOX_FULL, > + full, !full, MAILBOX_RETRY_US, > + MAILBOX_TIMEOUT_US); > + if (ret < 0) > + return ret; > + > + writel(val, base->regs + CDNS_MAILBOX_TX_DATA); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_mailbox_write); No need to export that one as well. You can make this function actually static as these two functions are only called from the helper itself. > + > +int cdns_mhdp_mailbox_recv_header(struct cdns_mhdp_base *base, > + u8 module_id, u8 opcode, > + u16 req_size) > +{ > + u32 mbox_size, i; > + u8 header[4]; > + int ret; > + > + /* read the header of the message */ > + for (i = 0; i < sizeof(header); i++) { > + ret = cdns_mhdp_mailbox_read(base); > + if (ret < 0) > + return ret; > + > + header[i] = ret; > + } > + > + mbox_size = get_unaligned_be16(header + 2); > + > + if (opcode != header[0] || module_id != header[1] || > + req_size != mbox_size) { > + /* > + * If the message in mailbox is not what we want, we need to > + * clear the mailbox by reading its contents. > + */ > + for (i = 0; i < mbox_size; i++) > + if (cdns_mhdp_mailbox_read(base) < 0) > + break; > + > + return -EINVAL; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_mailbox_recv_header); > + > +int cdns_mhdp_mailbox_recv_data(struct cdns_mhdp_base *base, > + u8 *buff, u16 buff_size) > +{ > + u32 i; > + int ret; > + > + for (i = 0; i < buff_size; i++) { > + ret = cdns_mhdp_mailbox_read(base); > + if (ret < 0) > + return ret; > + > + buff[i] = ret; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_mailbox_recv_data); > + > +int cdns_mhdp_mailbox_send(struct cdns_mhdp_base *base, u8 module_id, > + u8 opcode, u16 size, u8 *message) > +{ > + u8 header[4]; > + int ret, i; > + > + header[0] = opcode; > + header[1] = module_id; > + put_unaligned_be16(size, header + 2); > + > + for (i = 0; i < sizeof(header); i++) { > + ret = cdns_mhdp_mailbox_write(base, header[i]); > + if (ret) > + return ret; > + } > + > + for (i = 0; i < size; i++) { > + ret = cdns_mhdp_mailbox_write(base, message[i]); > + if (ret) > + return ret; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_mailbox_send); > + > +/* General helper functions */ > +int cdns_mhdp_reg_read(struct cdns_mhdp_base *base, u32 addr, u32 *value) > +{ > + u8 msg[4], resp[8]; > + int ret; > + > + put_unaligned_be32(addr, msg); > + > + mutex_lock(base->mbox_mutex); > + > + ret = cdns_mhdp_mailbox_send(base, MB_MODULE_ID_GENERAL, > + GENERAL_REGISTER_READ, > + sizeof(msg), msg); > + if (ret) > + goto out; > + > + ret = cdns_mhdp_mailbox_recv_header(base, MB_MODULE_ID_GENERAL, > + GENERAL_REGISTER_READ, > + sizeof(resp)); > + if (ret) > + goto out; > + > + ret = cdns_mhdp_mailbox_recv_data(base, resp, sizeof(resp)); > + if (ret) > + goto out; > + > + /* Returned address value should be the same as requested */ > + if (memcmp(msg, resp, sizeof(msg))) { > + ret = -EINVAL; > + goto out; > + } > + > + *value = get_unaligned_be32(resp + 4); > + > +out: > + mutex_unlock(base->mbox_mutex); > + if (ret) { > + dev_err(base->dev, "Failed to read register\n"); > + *value = 0; > + } > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_reg_read); > + > +int cdns_mhdp_reg_write(struct cdns_mhdp_base *base, u32 addr, u32 val) > +{ > + u8 msg[8]; > + int ret; > + > + put_unaligned_be32(addr, msg); > + put_unaligned_be32(val, msg + 4); > + > + mutex_lock(base->mbox_mutex); > + > + ret = cdns_mhdp_mailbox_send(base, MB_MODULE_ID_GENERAL, > + GENERAL_REGISTER_WRITE, > + sizeof(msg), msg); > + > + mutex_unlock(base->mbox_mutex); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_reg_write); > + > +/* DPTX helper functions */ > +int cdns_mhdp_dp_reg_write(struct cdns_mhdp_base *base, u16 addr, u32 val) > +{ > + u8 msg[6]; > + int ret; > + > + put_unaligned_be16(addr, msg); > + put_unaligned_be32(val, msg + 2); > + > + mutex_lock(base->mbox_mutex); > + > + ret = cdns_mhdp_mailbox_send(base, MB_MODULE_ID_DP_TX, > + DPTX_WRITE_REGISTER, sizeof(msg), msg); > + > + mutex_unlock(base->mbox_mutex); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_dp_reg_write); > + > +int cdns_mhdp_dp_reg_write_bit(struct cdns_mhdp_base *base, u16 addr, > + u8 start_bit, u8 bits_no, u32 val) > +{ > + u8 field[8]; > + int ret; > + > + put_unaligned_be16(addr, field); > + field[2] = start_bit; > + field[3] = bits_no; > + put_unaligned_be32(val, field + 4); > + > + mutex_lock(base->mbox_mutex); > + > + ret = cdns_mhdp_mailbox_send(base, MB_MODULE_ID_DP_TX, > + DPTX_WRITE_FIELD, sizeof(field), field); > + > + mutex_unlock(base->mbox_mutex); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_dp_reg_write_bit); > + > +int cdns_mhdp_dpcd_read(struct cdns_mhdp_base *base, > + u32 addr, u8 *data, u16 len) > +{ > + u8 msg[5], reg[5]; > + int ret; > + > + put_unaligned_be16(len, msg); > + put_unaligned_be24(addr, msg + 2); > + > + mutex_lock(base->mbox_mutex); > + > + ret = cdns_mhdp_mailbox_send(base, MB_MODULE_ID_DP_TX, > + DPTX_READ_DPCD, sizeof(msg), msg); > + if (ret) > + goto out; > + > + ret = cdns_mhdp_mailbox_recv_header(base, MB_MODULE_ID_DP_TX, > + DPTX_READ_DPCD, > + sizeof(reg) + len); > + if (ret) > + goto out; > + > + ret = cdns_mhdp_mailbox_recv_data(base, reg, sizeof(reg)); > + if (ret) > + goto out; > + > + ret = cdns_mhdp_mailbox_recv_data(base, data, len); > + > +out: > + mutex_unlock(base->mbox_mutex); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_dpcd_read); > + > +int cdns_mhdp_dpcd_write(struct cdns_mhdp_base *base, u32 addr, u8 value) > +{ > + u8 msg[6], reg[5]; > + int ret; > + > + put_unaligned_be16(1, msg); > + put_unaligned_be24(addr, msg + 2); > + msg[5] = value; > + > + mutex_lock(base->mbox_mutex); > + > + ret = cdns_mhdp_mailbox_send(base, MB_MODULE_ID_DP_TX, > + DPTX_WRITE_DPCD, sizeof(msg), msg); > + if (ret) > + goto out; > + > + ret = cdns_mhdp_mailbox_recv_header(base, MB_MODULE_ID_DP_TX, > + DPTX_WRITE_DPCD, sizeof(reg)); > + if (ret) > + goto out; > + > + ret = cdns_mhdp_mailbox_recv_data(base, reg, sizeof(reg)); > + if (ret) > + goto out; > + > + if (addr != get_unaligned_be24(reg + 2)) > + ret = -EINVAL; > + > +out: > + mutex_unlock(base->mbox_mutex); > + > + if (ret) > + dev_err(base->dev, "dpcd write failed: %d\n", ret); > + return ret; > +} > +EXPORT_SYMBOL_GPL(cdns_mhdp_dpcd_write); > + > +MODULE_DESCRIPTION("Cadence MHDP Helper driver"); > +MODULE_AUTHOR("Sandor Yu <Sandor.yu@xxxxxxx>"); > +MODULE_LICENSE("GPL"); > [...] > diff --git a/include/drm/bridge/cdns-mhdp-helper.h > b/include/drm/bridge/cdns-mhdp-helper.h new file mode 100644 > index 0000000000000..b89db9e842266 > --- /dev/null > +++ b/include/drm/bridge/cdns-mhdp-helper.h > @@ -0,0 +1,96 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2023 NXP Semiconductor, Inc. > + */ > +#ifndef __CDNS_MHDP_HELPER_H__ > +#define __CDNS_MHDP_HELPER_H__ > + > +#include <asm/unaligned.h> > +#include <linux/iopoll.h> > + > +/* mailbox regs offset */ > +#define CDNS_MAILBOX_FULL 0x00008 > +#define CDNS_MAILBOX_EMPTY 0x0000c > +#define CDNS_MAILBOX_TX_DATA 0x00010 > +#define CDNS_MAILBOX_RX_DATA 0x00014 > + > +#define MAILBOX_RETRY_US 1000 > +#define MAILBOX_TIMEOUT_US 2000000 > + > +/* Module ID Code */ > +#define MB_MODULE_ID_DP_TX 0x01 > +#define MB_MODULE_ID_HDMI_TX 0x03 > +#define MB_MODULE_ID_HDCP_TX 0x07 > +#define MB_MODULE_ID_HDCP_RX 0x08 > +#define MB_MODULE_ID_HDCP_GENERAL 0x09 > +#define MB_MODULE_ID_GENERAL 0x0A > + > +/* General Commands */ > +#define GENERAL_MAIN_CONTROL 0x01 > +#define GENERAL_TEST_ECHO 0x02 > +#define GENERAL_BUS_SETTINGS 0x03 > +#define GENERAL_TEST_ACCESS 0x04 > +#define GENERAL_REGISTER_WRITE 0x05 > +#define GENERAL_WRITE_FIELD 0x06 > +#define GENERAL_REGISTER_READ 0x07 > +#define GENERAL_GET_HPD_STATE 0x11 > + > +/* DPTX Commands */ > +#define DPTX_SET_POWER_MNG 0x00 > +#define DPTX_SET_HOST_CAPABILITIES 0x01 > +#define DPTX_GET_EDID 0x02 > +#define DPTX_READ_DPCD 0x03 > +#define DPTX_WRITE_DPCD 0x04 > +#define DPTX_ENABLE_EVENT 0x05 > +#define DPTX_WRITE_REGISTER 0x06 > +#define DPTX_READ_REGISTER 0x07 > +#define DPTX_WRITE_FIELD 0x08 > +#define DPTX_TRAINING_CONTROL 0x09 > +#define DPTX_READ_EVENT 0x0a > +#define DPTX_READ_LINK_STAT 0x0b > +#define DPTX_SET_VIDEO 0x0c > +#define DPTX_SET_AUDIO 0x0d > +#define DPTX_GET_LAST_AUX_STAUS 0x0e > +#define DPTX_SET_LINK_BREAK_POINT 0x0f > +#define DPTX_FORCE_LANES 0x10 > +#define DPTX_HPD_STATE 0x11 > +#define DPTX_ADJUST_LT 0x12 > + > +/* HDMI TX Commands */ > +#define HDMI_TX_READ 0x00 > +#define HDMI_TX_WRITE 0x01 > +#define HDMI_TX_UPDATE_READ 0x02 > +#define HDMI_TX_EDID 0x03 > +#define HDMI_TX_EVENTS 0x04 > +#define HDMI_TX_HPD_STATUS 0x05 > + > +struct cdns_mhdp_base { > + struct device *dev; > + void __iomem *regs; > + /* protect mailbox communications with the firmware */ > + struct mutex *mbox_mutex; > +}; > + > +/* Mailbox helper functions */ > +int cdns_mhdp_mailbox_read(struct cdns_mhdp_base *base); > +int cdns_mhdp_mailbox_write(struct cdns_mhdp_base *base, u8 val); You can remove these two declarations. Best regards, Alexander Best regards, > +int cdns_mhdp_mailbox_recv_header(struct cdns_mhdp_base *base, > + u8 module_id, u8 opcode, u16 req_size); > +int cdns_mhdp_mailbox_recv_data(struct cdns_mhdp_base *base, > + u8 *buff, u16 buff_size); > +int cdns_mhdp_mailbox_send(struct cdns_mhdp_base *base, u8 module_id, > + u8 opcode, u16 size, u8 *message); > + > +/* General commands helper functions */ > +int cdns_mhdp_reg_read(struct cdns_mhdp_base *base, u32 addr, u32 *value); > +int cdns_mhdp_reg_write(struct cdns_mhdp_base *base, u32 addr, u32 val); > + > +/* DPTX commands helper functions */ > +int cdns_mhdp_dp_reg_write(struct cdns_mhdp_base *base, u16 addr, u32 val); > +int cdns_mhdp_dp_reg_write_bit(struct cdns_mhdp_base *base, u16 addr, + > u8 start_bit, u8 bits_no, u32 val); > +int cdns_mhdp_dpcd_read(struct cdns_mhdp_base *base, > + u32 addr, u8 *data, u16 len); > +int cdns_mhdp_dpcd_write(struct cdns_mhdp_base *base, u32 addr, u8 value); > + > +#endif /* __CDNS_MHDP_HELPER_H__ */ 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