Re: [PATCH v5 2/3] drm: bridge: Add support for Cadence MHDP DPI/DP bridge

2020-02-13 Thread Tomi Valkeinen

On 12/02/2020 06:26, Yuti Amonkar wrote:

This patch adds new DRM driver for Cadence MHDP DPTX IP used on J721e SoC.
MHDP DPTX IP is the component that complies with VESA DisplayPort (DP) and
embedded Display Port (eDP) standards. It integrates uCPU running the
embedded Firmware(FW) interfaced over APB interface.
Basically, it takes a DPI stream as input and output it encoded in DP
format. Currently, it supports only SST mode.

Signed-off-by: Yuti Amonkar 
Signed-off-by: Jyri Sarha 
---
  drivers/gpu/drm/bridge/Kconfig  |   11 +
  drivers/gpu/drm/bridge/Makefile |3 +
  drivers/gpu/drm/bridge/cdns-mhdp-core.c | 2206 +++
  drivers/gpu/drm/bridge/cdns-mhdp-core.h |  380 
  4 files changed, 2600 insertions(+)
  create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-core.c
  create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-core.h

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 8397bf72d2f3..c66f2ef04f71 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -27,6 +27,17 @@ config DRM_CDNS_DSI
  Support Cadence DPI to DSI bridge. This is an internal
  bridge and is meant to be directly embedded in a SoC.
  
+config DRM_CDNS_MHDP

+   tristate "Cadence DPI/DP bridge"
+   select DRM_KMS_HELPER
+   select DRM_PANEL_BRIDGE
+   depends on OF
+   help
+ Support Cadence DPI to DP bridge. This is an internal
+ bridge and is meant to be directly embedded in a SoC.
+ It takes a DPI stream as input and output it encoded
+ in DP format.
+
  config DRM_DUMB_VGA_DAC
tristate "Dumb VGA DAC Bridge support"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 1eb5376c5d68..71019088d257 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -15,6 +15,9 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
  obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
  obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
+obj-$(CONFIG_DRM_CDNS_MHDP) += cdns-mhdp.o
  
  obj-y += analogix/

  obj-y += synopsys/
+
+cdns-mhdp-objs := cdns-mhdp-core.o


I think it's better to keep the two lines added in this change together, instead of having those 
other obj-y lines in between.



diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.c 
b/drivers/gpu/drm/bridge/cdns-mhdp-core.c
new file mode 100644
index ..51ed9cdee161
--- /dev/null
+++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.c
@@ -0,0 +1,2206 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Cadence MHDP DP bridge driver.
+ *
+ * Copyright: 2019 Cadence Design Systems, Inc.
+ *
+ * Author: Quentin Schulz 


Author is Quentin, but there's no signed-off-by from him? There are also many names in 
MODULE_AUTHOR, without signed-off-bys.


I think the rule is that you should have signed-off-bys for people who have contributed. I'm not 
sure how strict that rule is, and what to do if the people cannot be reached anymore.





+static int mhdp_probe(struct platform_device *pdev)
+{
+   const struct of_device_id *match;
+   struct resource *regs;
+   struct cdns_mhdp_device *mhdp;
+   struct clk *clk;
+   int ret;
+   unsigned long rate;
+   int irq;
+   u32 lanes_prop;
+   unsigned int link_rate;
+
+   mhdp = devm_kzalloc(>dev, sizeof(struct cdns_mhdp_device),
+   GFP_KERNEL);
+   if (!mhdp)
+   return -ENOMEM;
+
+   clk = devm_clk_get(>dev, NULL);
+   if (IS_ERR(clk)) {
+   dev_err(>dev, "couldn't get clk: %ld\n", PTR_ERR(clk));
+   return PTR_ERR(clk);
+   }
+
+   mhdp->clk = clk;
+   mhdp->dev = >dev;
+   mhdp->conn_bus_flags_defaults = DRM_BUS_FLAG_DE_HIGH;
+   mutex_init(>mbox_mutex);
+   spin_lock_init(>start_lock);
+   dev_set_drvdata(>dev, mhdp);
+
+   drm_dp_aux_init(>aux);
+   mhdp->aux.dev = >dev;
+   mhdp->aux.transfer = mhdp_transfer;
+
+   regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   mhdp->regs = devm_ioremap_resource(>dev, regs);
+   if (IS_ERR(mhdp->regs))
+   return PTR_ERR(mhdp->regs);
+
+   mhdp->phy = devm_of_phy_get_by_index(>dev, pdev->dev.of_node, 0);
+   if (IS_ERR(mhdp->phy)) {
+   dev_err(>dev, "no PHY configured\n");
+   return PTR_ERR(mhdp->phy);
+   }
+
+   platform_set_drvdata(pdev, mhdp);
+
+   clk_prepare_enable(clk);
+
+   match = of_match_device(mhdp_ids, >dev);
+   if (!match)
+   return -ENODEV;
+   mhdp->ops = (struct mhdp_platform_ops *)match->data;
+
+   pm_runtime_enable(>dev);
+   ret = pm_runtime_get_sync(>dev);
+   if (ret < 0) {
+   dev_err(>dev, "pm_runtime_get_sync failed\n");
+   pm_runtime_disable(>dev);
+   goto clk_disable;
+   }
+
+   if 

[PATCH v5 2/3] drm: bridge: Add support for Cadence MHDP DPI/DP bridge

2020-02-12 Thread Yuti Amonkar
This patch adds new DRM driver for Cadence MHDP DPTX IP used on J721e SoC.
MHDP DPTX IP is the component that complies with VESA DisplayPort (DP) and
embedded Display Port (eDP) standards. It integrates uCPU running the
embedded Firmware(FW) interfaced over APB interface.
Basically, it takes a DPI stream as input and output it encoded in DP
format. Currently, it supports only SST mode.

Signed-off-by: Yuti Amonkar 
Signed-off-by: Jyri Sarha 
---
 drivers/gpu/drm/bridge/Kconfig  |   11 +
 drivers/gpu/drm/bridge/Makefile |3 +
 drivers/gpu/drm/bridge/cdns-mhdp-core.c | 2206 +++
 drivers/gpu/drm/bridge/cdns-mhdp-core.h |  380 
 4 files changed, 2600 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-core.c
 create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-core.h

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 8397bf72d2f3..c66f2ef04f71 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -27,6 +27,17 @@ config DRM_CDNS_DSI
  Support Cadence DPI to DSI bridge. This is an internal
  bridge and is meant to be directly embedded in a SoC.
 
+config DRM_CDNS_MHDP
+   tristate "Cadence DPI/DP bridge"
+   select DRM_KMS_HELPER
+   select DRM_PANEL_BRIDGE
+   depends on OF
+   help
+ Support Cadence DPI to DP bridge. This is an internal
+ bridge and is meant to be directly embedded in a SoC.
+ It takes a DPI stream as input and output it encoded
+ in DP format.
+
 config DRM_DUMB_VGA_DAC
tristate "Dumb VGA DAC Bridge support"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 1eb5376c5d68..71019088d257 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -15,6 +15,9 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
 obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
 obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
+obj-$(CONFIG_DRM_CDNS_MHDP) += cdns-mhdp.o
 
 obj-y += analogix/
 obj-y += synopsys/
+
+cdns-mhdp-objs := cdns-mhdp-core.o
diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.c 
b/drivers/gpu/drm/bridge/cdns-mhdp-core.c
new file mode 100644
index ..51ed9cdee161
--- /dev/null
+++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.c
@@ -0,0 +1,2206 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Cadence MHDP DP bridge driver.
+ *
+ * Copyright: 2019 Cadence Design Systems, Inc.
+ *
+ * Author: Quentin Schulz 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "cdns-mhdp-core.h"
+
+static const struct of_device_id mhdp_ids[] = {
+   { .compatible = "cdns,mhdp8546", },
+   { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mhdp_ids);
+
+static inline u32 get_unaligned_be24(const void *p)
+{
+   const u8 *_p = p;
+
+   return _p[0] << 16 | _p[1] << 8 | _p[2];
+}
+
+static inline void put_unaligned_be24(u32 val, void *p)
+{
+   u8 *_p = p;
+
+   _p[0] = val >> 16;
+   _p[1] = val >> 8;
+   _p[2] = val;
+}
+
+static int cdns_mhdp_mailbox_read(struct cdns_mhdp_device *mhdp)
+{
+   int val, ret;
+
+   WARN_ON(!mutex_is_locked(>mbox_mutex));
+
+   ret = readx_poll_timeout(readl, mhdp->regs + CDNS_MAILBOX_EMPTY,
+val, !val, MAILBOX_RETRY_US,
+MAILBOX_TIMEOUT_US);
+   if (ret < 0)
+   return ret;
+
+   return readl(mhdp->regs + CDNS_MAILBOX_RX_DATA) & 0xff;
+}
+
+static int cdns_mhdp_mailbox_write(struct cdns_mhdp_device *mhdp, u8 val)
+{
+   int ret, full;
+
+   WARN_ON(!mutex_is_locked(>mbox_mutex));
+
+   ret = readx_poll_timeout(readl, mhdp->regs + CDNS_MAILBOX_FULL,
+full, !full, MAILBOX_RETRY_US,
+MAILBOX_TIMEOUT_US);
+   if (ret < 0)
+   return ret;
+
+   writel(val, mhdp->regs + CDNS_MAILBOX_TX_DATA);
+
+   return 0;
+}
+
+static int cdns_mhdp_mailbox_validate_receive(struct cdns_mhdp_device *mhdp,
+ 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 < 4; i++) {
+   ret = cdns_mhdp_mailbox_read(mhdp);
+   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