Hi Alexander,
Thanks your comments,
>
> 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.
OK, I will change it to static in the next version.
>
> > +
> > +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.
mhdp8546 driver need this function.
>
> > +
> > +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.
cdns_mhdp_mailbox_read will be removed. Thanks!
B.R
Sandor
>
> 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
>
>
