Re: [PATCH v4 2/2] drm: rcar-du: Add RZ/G2L DSI driver

2022-07-22 Thread Sam Ravnborg
Hi Biju,

driver looks good - you can add my:
Acked-by: Sam Ravnborg 


I have one general question - that is not related to this driver, but is
directed to the bridge people on this mail.

> +static void rzg2l_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
> +  struct drm_bridge_state 
> *old_bridge_state)
> +{
> + struct drm_atomic_state *state = old_bridge_state->base.state;
> + struct rzg2l_mipi_dsi *dsi = bridge_to_rzg2l_mipi_dsi(bridge);
> + const struct drm_display_mode *mode;
> + struct drm_connector *connector;
> + struct drm_crtc *crtc;
> + int ret;
> +
> + connector = drm_atomic_get_new_connector_for_encoder(state, 
> bridge->encoder);
> + crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
> + mode = _atomic_get_new_crtc_state(state, crtc)->adjusted_mode;

It is relative often we see the need to access the new crtc_state in
the atomic_enable() operation. Could we add it as a parameter to
atomic_enable() and fish it out in the caller?
That would save some boilerplate code.

I once had a helper cooked up for the above and could dig it up
again - but the parameter idea seems better?!?

Sam


[PATCH v4 2/2] drm: rcar-du: Add RZ/G2L DSI driver

2022-07-22 Thread Biju Das
This driver supports the MIPI DSI encoder found in the RZ/G2L
SoC. It currently supports DSI mode only.

Signed-off-by: Biju Das 
---
v3->v4:
 * Updated error handling in rzg2l_mipi_dsi_startup() and 
rzg2l_mipi_dsi_atomic_enable()
v2->v3:
 * pass rzg2l_mipi_dsi pointer to {Link,Phy} register rd/wr function instead
   of the memory pointer
 * Fixed the comment in rzg2l_mipi_dsi_startup()
 * Removed unnecessary dbg message from rzg2l_mipi_dsi_start_video()
 * DRM bridge parameter initialization moved to probe
 * Replaced dev_dbg->dev_err in rzg2l_mipi_dsi_parse_dt()
 * Inserted the missing blank lane after return in probe()
 * Added missing MODULE_DEVICE_TABLE
 * Added include linux/bits.h in header file
 * Fixed various macros in header file.
 * Reorder the make file for DSI, so that it is no more dependent
   on RZ/G2L DU patch series.
v1->v2:
 * Rework based on dt-binding change (DSI + D-PHY) as single block
 * Replaced link_mmio and phy_mmio with mmio in struct rzg2l_mipi_dsi
 * Replaced rzg2l_mipi_phy_write with rzg2l_mipi_dsi_phy_write
   and rzg2l_mipi_dsi_link_write
 * Replaced rzg2l_mipi_phy_read->rzg2l_mipi_dsi_link_read
RFC->v1:
 * Added "depends on ARCH_RENESAS || COMPILE_TEST" on KCONFIG
   and dropped DRM as it is implied by DRM_BRIDGE
 * Used devm_reset_control_get_exclusive() for reset handle
 * Removed bool hsclkmode from struct rzg2l_mipi_dsi
 * Added error check for pm, using pm_runtime_resume_and_get() instead of
   pm_runtime_get_sync()
 * Added check for unsupported formats in rzg2l_mipi_dsi_host_attach()
 * Avoided read-modify-write stopping hsclock
 * Used devm_platform_ioremap_resource for resource allocation
 * Removed unnecessary assert call from probe and remove.
 * wrap the line after the PTR_ERR() in probe()
 * Updated reset failure messages in probe
 * Fixed the typo arstc->prstc
 * Made hex constants to lower case.
RFC:
 * 
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220112174612.10773-23-biju.das...@bp.renesas.com/
---
 drivers/gpu/drm/rcar-du/Kconfig   |   8 +
 drivers/gpu/drm/rcar-du/Makefile  |   2 +
 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c  | 712 ++
 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h | 151 
 4 files changed, 873 insertions(+)
 create mode 100644 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
 create mode 100644 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index f6e6a6d5d987..3e59c7c213f5 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -51,6 +51,14 @@ config DRM_RCAR_MIPI_DSI
help
  Enable support for the R-Car Display Unit embedded MIPI DSI encoders.
 
+config DRM_RZG2L_MIPI_DSI
+   tristate "RZ/G2L MIPI DSI Encoder Support"
+   depends on DRM_BRIDGE && OF
+   depends on ARCH_RENESAS || COMPILE_TEST
+   select DRM_MIPI_DSI
+   help
+ Enable support for the RZ/G2L Display Unit embedded MIPI DSI encoders.
+
 config DRM_RCAR_VSP
bool "R-Car DU VSP Compositor Support" if ARM
default y if ARM64
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index e7275b5e7ec8..14a3fa88cc0b 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -15,6 +15,8 @@ obj-$(CONFIG_DRM_RCAR_DW_HDMI)+= 
rcar_dw_hdmi.o
 obj-$(CONFIG_DRM_RCAR_LVDS)+= rcar_lvds.o
 obj-$(CONFIG_DRM_RCAR_MIPI_DSI)+= rcar_mipi_dsi.o
 
+obj-$(CONFIG_DRM_RZG2L_MIPI_DSI)   += rzg2l_mipi_dsi.o
+
 # 'remote-endpoint' is fixed up at run-time
 DTC_FLAGS_rcar_du_of_lvds_r8a7790 += -Wno-graph_endpoint
 DTC_FLAGS_rcar_du_of_lvds_r8a7791 += -Wno-graph_endpoint
diff --git a/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c 
b/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
new file mode 100644
index ..e81b7a64ea10
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
@@ -0,0 +1,712 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RZ/G2L MIPI DSI Encoder Driver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "rzg2l_mipi_dsi_regs.h"
+
+struct rzg2l_mipi_dsi {
+   struct device *dev;
+   void __iomem *mmio;
+
+   struct reset_control *rstc;
+   struct reset_control *arstc;
+   struct reset_control *prstc;
+
+   struct mipi_dsi_host host;
+   struct drm_bridge bridge;
+   struct drm_bridge *next_bridge;
+
+   struct clk *vclk;
+
+   enum mipi_dsi_pixel_format format;
+   unsigned int num_data_lanes;
+   unsigned int lanes;
+   unsigned long mode_flags;
+};
+
+static inline struct rzg2l_mipi_dsi *
+bridge_to_rzg2l_mipi_dsi(struct drm_bridge *bridge)
+{
+   return container_of(bridge, struct