Re: [PATCH v8 3/3] drm: bridge: cdns-mhdp: Add j721e wrapper

2020-08-10 Thread Laurent Pinchart
Hi Swapnil,

Thank you for the patch.

On Thu, Aug 06, 2020 at 01:34:32PM +0200, Swapnil Jakhade wrote:
> Add j721e wrapper for mhdp, which sets up the clock and data muxes.
> 
> Signed-off-by: Jyri Sarha 
> Signed-off-by: Yuti Amonkar 
> Signed-off-by: Swapnil Jakhade 
> Reviewed-by: Tomi Valkeinen 
> Reviewed-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/bridge/Kconfig   | 13 +
>  drivers/gpu/drm/bridge/Makefile  |  2 +
>  drivers/gpu/drm/bridge/cdns-mhdp-core.c  | 15 +
>  drivers/gpu/drm/bridge/cdns-mhdp-core.h  |  1 +
>  drivers/gpu/drm/bridge/cdns-mhdp-j721e.c | 72 
>  drivers/gpu/drm/bridge/cdns-mhdp-j721e.h | 19 +++
>  6 files changed, 122 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-j721e.c
>  create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-j721e.h
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 6a4c324302a8..8c1738653b7e 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -38,6 +38,19 @@ config DRM_CDNS_MHDP
> It takes a DPI stream as input and outputs it encoded
> in DP format.
>  
> +if DRM_CDNS_MHDP
> +
> +config DRM_CDNS_MHDP_J721E
> + depends on ARCH_K3_J721E_SOC

depends on ARCH_K3_J721E_SOC || COMPILE_TEST

> + bool "J721E Cadence DPI/DP wrapper support"
> + default y
> + help
> +   Support J721E Cadence DPI/DP wrapper. This is a wrapper
> +   which adds support for J721E related platform ops. It
> +   initializes the J721e Display Port and sets up the
> +   clock and data muxes.
> +endif
> +
>  config DRM_CHRONTEL_CH7033
>   tristate "Chrontel CH7033 Video Encoder"
>   depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 7046bf077603..be92ebf620b6 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -2,6 +2,8 @@
>  obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
>  obj-$(CONFIG_DRM_CDNS_MHDP) += cdns-mhdp.o
>  cdns-mhdp-y := cdns-mhdp-core.o
> +cdns-mhdp-$(CONFIG_DRM_CDNS_MHDP_J721E) += cdns-mhdp-j721e.o
> +
>  obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
>  obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o
>  obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
> diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.c 
> b/drivers/gpu/drm/bridge/cdns-mhdp-core.c
> index d47187ab358b..53c25f6ecddf 100644
> --- a/drivers/gpu/drm/bridge/cdns-mhdp-core.c
> +++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.c
> @@ -42,6 +42,8 @@
>  
>  #include "cdns-mhdp-core.h"
>  
> +#include "cdns-mhdp-j721e.h"
> +
>  static DECLARE_WAIT_QUEUE_HEAD(fw_load_wq);
>  
>  static int cdns_mhdp_mailbox_read(struct cdns_mhdp_device *mhdp)
> @@ -1702,6 +1704,16 @@ static int cdns_mhdp_connector_init(struct 
> cdns_mhdp_device *mhdp)
>  
>   conn->display_info.bus_flags = DRM_BUS_FLAG_DE_HIGH;
>  
> + if (of_device_is_compatible(mhdp->dev->of_node, "ti,j721e-mhdp8546"))
> + /*
> +  * DP is internal to J7 SoC and we need to use DRIVE_POSEDGE
> +  * in the display controller. This is achieved for the time being
> +  * by defining SAMPLE_NEGEDGE here.
> +  */

The indentation is wrong. You can adjust it or move it before the if (...).

> + conn->display_info.bus_flags |=
> + DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
> + DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE;

This should be set in drm_bridge_timings.input_bus_flags instead, and
the tidss should use those when available instead of getting the value
from the connector.

> +
>   ret = drm_connector_attach_encoder(conn, bridge->encoder);
>   if (ret) {
>   DRM_ERROR("Failed to attach connector to encoder\n");
> @@ -2521,6 +2533,9 @@ static int cdns_mhdp_remove(struct platform_device 
> *pdev)
>  
>  static const struct of_device_id mhdp_ids[] = {
>   { .compatible = "cdns,mhdp8546", },
> +#ifdef CONFIG_DRM_CDNS_MHDP_J721E
> + { .compatible = "ti,j721e-mhdp8546", .data = _ti_j721e_ops },
> +#endif
>   { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, mhdp_ids);
> diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.h 
> b/drivers/gpu/drm/bridge/cdns-mhdp-core.h
> index bd97a7aeb28b..d40a0f8615a4 100644
> --- a/drivers/gpu/drm/bridge/cdns-mhdp-core.h
> +++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.h
> @@ -343,6 +343,7 @@ struct cdns_mhdp_bridge_state {
>  
>  struct cdns_mhdp_device {
>   void __iomem *regs;
> + void __iomem *j721e_regs;
>  
>   struct device *dev;
>   struct clk *clk;
> diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c 
> b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c
> new file mode 100644
> index ..cc33c9afb5bb
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TI j721e Cadence MHDP DP wrapper
> + *
> + * 

[PATCH v8 3/3] drm: bridge: cdns-mhdp: Add j721e wrapper

2020-08-07 Thread Swapnil Jakhade
Add j721e wrapper for mhdp, which sets up the clock and data muxes.

Signed-off-by: Jyri Sarha 
Signed-off-by: Yuti Amonkar 
Signed-off-by: Swapnil Jakhade 
Reviewed-by: Tomi Valkeinen 
Reviewed-by: Laurent Pinchart 
---
 drivers/gpu/drm/bridge/Kconfig   | 13 +
 drivers/gpu/drm/bridge/Makefile  |  2 +
 drivers/gpu/drm/bridge/cdns-mhdp-core.c  | 15 +
 drivers/gpu/drm/bridge/cdns-mhdp-core.h  |  1 +
 drivers/gpu/drm/bridge/cdns-mhdp-j721e.c | 72 
 drivers/gpu/drm/bridge/cdns-mhdp-j721e.h | 19 +++
 6 files changed, 122 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-j721e.c
 create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-j721e.h

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 6a4c324302a8..8c1738653b7e 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -38,6 +38,19 @@ config DRM_CDNS_MHDP
  It takes a DPI stream as input and outputs it encoded
  in DP format.
 
+if DRM_CDNS_MHDP
+
+config DRM_CDNS_MHDP_J721E
+   depends on ARCH_K3_J721E_SOC
+   bool "J721E Cadence DPI/DP wrapper support"
+   default y
+   help
+ Support J721E Cadence DPI/DP wrapper. This is a wrapper
+ which adds support for J721E related platform ops. It
+ initializes the J721e Display Port and sets up the
+ clock and data muxes.
+endif
+
 config DRM_CHRONTEL_CH7033
tristate "Chrontel CH7033 Video Encoder"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 7046bf077603..be92ebf620b6 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -2,6 +2,8 @@
 obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
 obj-$(CONFIG_DRM_CDNS_MHDP) += cdns-mhdp.o
 cdns-mhdp-y := cdns-mhdp-core.o
+cdns-mhdp-$(CONFIG_DRM_CDNS_MHDP_J721E) += cdns-mhdp-j721e.o
+
 obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
 obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o
 obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.c 
b/drivers/gpu/drm/bridge/cdns-mhdp-core.c
index d47187ab358b..53c25f6ecddf 100644
--- a/drivers/gpu/drm/bridge/cdns-mhdp-core.c
+++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.c
@@ -42,6 +42,8 @@
 
 #include "cdns-mhdp-core.h"
 
+#include "cdns-mhdp-j721e.h"
+
 static DECLARE_WAIT_QUEUE_HEAD(fw_load_wq);
 
 static int cdns_mhdp_mailbox_read(struct cdns_mhdp_device *mhdp)
@@ -1702,6 +1704,16 @@ static int cdns_mhdp_connector_init(struct 
cdns_mhdp_device *mhdp)
 
conn->display_info.bus_flags = DRM_BUS_FLAG_DE_HIGH;
 
+   if (of_device_is_compatible(mhdp->dev->of_node, "ti,j721e-mhdp8546"))
+   /*
+* DP is internal to J7 SoC and we need to use DRIVE_POSEDGE
+* in the display controller. This is achieved for the time being
+* by defining SAMPLE_NEGEDGE here.
+*/
+   conn->display_info.bus_flags |=
+   DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
+   DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE;
+
ret = drm_connector_attach_encoder(conn, bridge->encoder);
if (ret) {
DRM_ERROR("Failed to attach connector to encoder\n");
@@ -2521,6 +2533,9 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
 
 static const struct of_device_id mhdp_ids[] = {
{ .compatible = "cdns,mhdp8546", },
+#ifdef CONFIG_DRM_CDNS_MHDP_J721E
+   { .compatible = "ti,j721e-mhdp8546", .data = _ti_j721e_ops },
+#endif
{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, mhdp_ids);
diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.h 
b/drivers/gpu/drm/bridge/cdns-mhdp-core.h
index bd97a7aeb28b..d40a0f8615a4 100644
--- a/drivers/gpu/drm/bridge/cdns-mhdp-core.h
+++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.h
@@ -343,6 +343,7 @@ struct cdns_mhdp_bridge_state {
 
 struct cdns_mhdp_device {
void __iomem *regs;
+   void __iomem *j721e_regs;
 
struct device *dev;
struct clk *clk;
diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c 
b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c
new file mode 100644
index ..cc33c9afb5bb
--- /dev/null
+++ b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI j721e Cadence MHDP DP wrapper
+ *
+ * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Jyri Sarha 
+#include 
+
+#include "cdns-mhdp-j721e.h"
+
+#defineREVISION0x00
+#defineDPTX_IPCFG  0x04
+#defineECC_MEM_CFG 0x08
+#defineDPTX_DSC_CFG0x0c
+#defineDPTX_SRC_CFG0x10
+#defineDPTX_VIF_SECURE_MODE_CFG0x14
+#defineDPTX_VIF_CONN_STATUS0x18
+#definePHY_CLK_STATUS  0x1c
+
+#define