[PATCH 0/8] Add imx8mp video support

[PATCH 0/8] Add imx8mp video support

[Miquel Raynal]

In order to display a boot picture or an error message, the i.MX8MP
display pipeline must be enabled. The SoC has support for various
interfaces (LVDS, HDMI, DSI). The one supported in this series is the
standard 4-lane LVDS output. The minimal setup is thus composed of:
* An LCD InterFace (LCDIF) with an AXI/APB interface, generating a pixel
  stream
* One LVDS Display Bridge (LDB), also named pixel mapper, which receives
  the pixel stream and route it to one or two (possibly combined) LVDS
  displays.
* All necessary clocks and power controls coming from the MEDIAMIX
  control block.

Patch 1 adds a very useful helper to the core in order to grab devices
through endpoints instead of being limited to phandles. Video pipelines
being often described using graphs endpoints, the introduced helper is
used several times in the serires (there are 3 LCDIF, one of them being
connected to the LDB, itself having 2 ports).

Patch 2 is a fix which is necessary for this series to work properly,
but is way broader than just this use case. In practice, when assigned
clocks are defined in the device tree, the clock uclass tries to assign
the parents first and then sets them to the correct frequency. This only
works if the parents have been enabled themselves. Otherwise we end-up
with a non-clocked parent. I believe this is not the intended behavior
in general, but more importantly on the i.MX8MP, there are "clock
slices" which have pre-requisites in order to be modified and selecting
an ungated parent is one of them.

All the other patches progressively build support for the whole video
pipeline. Regarding the LCDIF driver, there is already a similar driver
for older i.MX SoCs but I didn't manage to get it to work. It was
written more than a decade ago while device-model, clocks and others
were not yet generically supported. Thus, numerous ad-hoc solutions
were implemented, which no longer fit today's requirements. I preferred
to add a new "clean" driver instead of modifying the existing one
because of the too high risk of breaking these platforms. Once proper
clocks/power-domain descriptions will be added to them they might be
converted (and tested) to work with the "new" implementation, but going
the opposite way felt drawback.

Thanks,
Miquèl

Miquel Raynal (8):
  dm: core: Add a helper to retrieve devices through graph endpoints
  clk: Ensure the parent clocks are enabled while reparenting
  clk: imx8mp: Add media related clocks
  imx: power-domain: Describe the i.MX8 MEDIAMIX domain
  imx: power-domain: Add support for the MEDIAMIX control block
  video: imx: Fix Makefile in order to be able to add other imx drivers
  video: imx: Add LDB driver
  video: imx: Add LCDIF driver

 drivers/clk/clk-uclass.c                  |   4 +
 drivers/clk/imx/clk-imx8mp.c              |  35 +++
 drivers/core/uclass.c                     |  62 +++++
 drivers/power/domain/Kconfig              |   7 +
 drivers/power/domain/Makefile             |   1 +
 drivers/power/domain/imx8m-power-domain.c |  17 ++
 drivers/power/domain/imx8mp-mediamix.c    | 185 +++++++++++++
 drivers/video/Makefile                    |   2 +-
 drivers/video/imx/Kconfig                 |   7 +
 drivers/video/imx/Makefile                |   4 +-
 drivers/video/imx/lcdif.c                 | 315 ++++++++++++++++++++++
 drivers/video/imx/ldb.c                   | 251 +++++++++++++++++
 include/dm/uclass.h                       |  21 ++
 13 files changed, 909 insertions(+), 2 deletions(-)
 create mode 100644 drivers/power/domain/imx8mp-mediamix.c
 create mode 100644 drivers/video/imx/lcdif.c
 create mode 100644 drivers/video/imx/ldb.c

-- 
2.43.0

[PATCH 1/8] dm: core: Add a helper to retrieve devices through graph endpoints

[Miquel Raynal]

There are already several helpers to find a udevice based on its
position in a device tree, like getting a child or a node pointed by a
phandle, but there was no support for graph endpoints, which are very
common in display pipelines.

Add a new helper, named uclass_get_device_by_endpoint() which enters the
child graph reprensentation, looks for a specific port, then follows the
remote endpoint, and finally retrieves the first parent of the given
uclass_id.

This is a very handy and straightforward way to get a bridge or a panel
handle.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/core/uclass.c | 62 +++++++++++++++++++++++++++++++++++++++++++
 include/dm/uclass.h   | 21 +++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index e46d5717aa6..5803fd51986 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -572,6 +572,68 @@ int uclass_get_device_by_phandle(enum uclass_id id, struct udevice *parent,
 	ret = uclass_find_device_by_phandle(id, parent, name, &dev);
 	return uclass_get_device_tail(dev, ret, devp);
 }
+
+int uclass_get_device_by_endpoint(enum uclass_id class_id, struct udevice *dev,
+				  u32 ep_idx, struct udevice **devp)
+{
+	ofnode port = ofnode_null(), ep, remote_ep;
+	struct udevice **target;
+	u32 remote_phandle;
+	int ret;
+
+	if (!ep_idx)
+		port = dev_read_subnode(dev, "port");
+
+	if (!ofnode_valid(port)) {
+		char port_name[32];
+
+		port = dev_read_subnode(dev, "ports");
+		if (!ofnode_valid(port)) {
+			debug("%s: no 'port'/'ports' subnode\n",
+			      dev_read_name(dev));
+			return -EINVAL;
+		}
+
+		snprintf(port_name, sizeof(port_name), "port@%x", ep_idx);
+		port = ofnode_find_subnode(port, port_name);
+		if (!ofnode_valid(port)) {
+			debug("%s: no 'port@%x' subnode\n",
+			      dev_read_name(dev), ep_idx);
+			return -EINVAL;
+		}
+	}
+
+	ep = ofnode_find_subnode(port, "endpoint");
+	if (!ofnode_valid(ep)) {
+		debug("%s: no 'endpoint' in %s subnode\n",
+		      ofnode_get_name(port), dev_read_name(dev));
+		return -EINVAL;
+	}
+
+	ret = ofnode_read_u32(ep, "remote-endpoint", &remote_phandle);
+	if (ret)
+		return ret;
+
+	remote_ep = ofnode_get_by_phandle(remote_phandle);
+	if (!ofnode_valid(remote_ep))
+		return -EINVAL;
+
+	while (ofnode_valid(remote_ep)) {
+		remote_ep = ofnode_get_parent(remote_ep);
+		debug("trying subnode: %s\n", ofnode_get_name(remote_ep));
+		if (!ofnode_valid(remote_ep)) {
+			debug("%s: no more remote devices\n",
+			      ofnode_get_name(remote_ep));
+			return -EINVAL;
+		}
+
+		ret = uclass_find_device_by_ofnode(class_id, remote_ep, target);
+		if (!ret)
+			break;
+	};
+
+	return uclass_get_device_tail(*target, ret, devp);
+}
 #endif
 
 /*
diff --git a/include/dm/uclass.h b/include/dm/uclass.h
index 456eef7f2f3..1194174ca9b 100644
--- a/include/dm/uclass.h
+++ b/include/dm/uclass.h
@@ -333,6 +333,27 @@ int uclass_get_device_by_phandle(enum uclass_id id, struct udevice *parent,
 int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv,
 				struct udevice **devp);
 
+/**
+ * uclass_get_device_by_endpoint() - Get a uclass device for a remote endpoint
+ *
+ * This searches through the parents of the specified remote endpoint
+ * for the first device matching the uclass. Said otherwise, this helper
+ * goes through the graph (endpoint) representation and searches for
+ * matching devices. Endpoints can be subnodes of the "port" node or
+ * subnodes of ports identified with a reg property, themselves in a
+ * "ports" container.
+ *
+ * The device is probed to activate it ready for use.
+ *
+ * @class_id: uclass ID to look up
+ * @dev: Device to start from
+ * @ep_idx: Index of the endpoint to follow, 0 if there is none.
+ * @target: Returns pointer to the first device matching the expected uclass.
+ * Return: 0 if OK, -ve on error
+ */
+int uclass_get_device_by_endpoint(enum uclass_id class_id, struct udevice *dev,
+				  u32 ep_idx, struct udevice **target);
+
 /**
  * uclass_first_device() - Get the first device in a uclass
  *
-- 
2.43.0

[PATCH 2/8] clk: Ensure the parent clocks are enabled while reparenting

[Miquel Raynal]

Reparenting a clock C with a new parent P means that C will only
continue clocking if P is already clocking when the mux is updated. In
case the parent is currently disabled, failures (stalls) are likely to
happen.

This is exactly what happens on i.MX8 when enabling the video
pipeline. We tell LCDIF clocks to use the VIDEO PLL as input, while the
VIDEO PLL is currently off. This all happens as part of the
assigned-clocks handling procedure, where the reparenting happens before
the enable() calls. Enabling the parents as part of the reparenting
procedure seems sane and also matches the logic applied in other parts
of the CCM.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/clk/clk-uclass.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index ed6e60bc484..24f801e16f1 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -595,6 +595,10 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
 	if (!ops->set_parent)
 		return -ENOSYS;
 
+	ret = clk_enable(parent);
+	if (ret)
+		return ret;
+
 	ret = ops->set_parent(clk, parent);
 	if (ret)
 		return ret;
-- 
2.43.0

[PATCH 3/8] clk: imx8mp: Add media related clocks

[Miquel Raynal]

These are all the clocks needed to get an LCD panel working, going
through one of the LCDIF and the LDB. The media AXI and APB clocks are
also described.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/clk/imx/clk-imx8mp.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index 7dfc829df2c..92c5d8441c0 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -15,7 +15,10 @@
 
 #include "clk.h"
 
+static u32 share_count_media;
+
 static const char *pll_ref_sels[] = { "clock-osc-24m", "dummy", "dummy", "dummy", };
+static const char *video_pll1_bypass_sels[] = {"video_pll1", "video_pll1_ref_sel", };
 static const char *dram_pll_bypass_sels[] = {"dram_pll", "dram_pll_ref_sel", };
 static const char *arm_pll_bypass_sels[] = {"arm_pll", "arm_pll_ref_sel", };
 static const char *sys_pll1_bypass_sels[] = {"sys_pll1", "sys_pll1_ref_sel", };
@@ -42,6 +45,14 @@ static const char *imx8mp_nand_usdhc_sels[] = {"clock-osc-24m", "sys_pll1_266m",
 					       "sys_pll2_200m", "sys_pll1_133m", "sys_pll3_out",
 					       "sys_pll2_250m", "audio_pll1_out", };
 
+static const char *imx8mp_media_axi_sels[] = {"clock-osc-24m", "sys_pll2_1000m", "sys_pll1_800m",
+					      "sys_pll3_out", "sys_pll1_40m", "audio_pll2_out",
+					      "clk_ext1", "sys_pll2_500m", };
+
+static const char *imx8mp_media_apb_sels[] = {"clock-osc-24m", "sys_pll2_125m", "sys_pll1_800m",
+					      "sys_pll3_out", "sys_pll1_40m", "audio_pll2_out",
+					      "clk_ext1", "sys_pll1_133m", };
+
 static const char *imx8mp_noc_sels[] = {"clock-osc-24m", "sys_pll1_800m", "sys_pll3_out",
 					"sys_pll2_1000m", "sys_pll2_500m", "audio_pll1_out",
 					"video_pll1_out", "audio_pll2_out", };
@@ -174,6 +185,15 @@ static const char *imx8mp_usdhc3_sels[] = {"clock-osc-24m", "sys_pll1_400m", "sy
 					   "sys_pll2_500m", "sys_pll3_out", "sys_pll1_266m",
 					   "audio_pll2_out", "sys_pll1_100m", };
 
+static const char *imx8mp_media_disp_pix_sels[] = {"clock-osc-24m", "video_pll1_out", "audio_pll2_out",
+						   "audio_pll1_out", "sys_pll1_800m",
+						   "sys_pll2_1000m", "sys_pll3_out", "clk_ext4", };
+
+static const char *imx8mp_media_ldb_sels[] = {"clock-osc-24m", "sys_pll2_333m", "sys_pll2_100m",
+						     "sys_pll1_800m", "sys_pll2_1000m",
+						     "clk_ext2", "audio_pll2_out",
+						     "video_pll1_out", };
+
 static const char *imx8mp_enet_ref_sels[] = {"clock-osc-24m", "sys_pll2_125m", "sys_pll2_50m",
 					     "sys_pll2_100m", "sys_pll1_160m", "audio_pll1_out",
 					     "video_pll1_out", "clk_ext4", };
@@ -196,12 +216,15 @@ static int imx8mp_clk_probe(struct udevice *dev)
 
 	base = (void *)ANATOP_BASE_ADDR;
 
+	clk_dm(IMX8MP_VIDEO_PLL1_REF_SEL, imx_clk_mux("video_pll1_ref_sel", base + 0x28, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
 	clk_dm(IMX8MP_DRAM_PLL_REF_SEL, imx_clk_mux("dram_pll_ref_sel", base + 0x50, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
 	clk_dm(IMX8MP_ARM_PLL_REF_SEL, imx_clk_mux("arm_pll_ref_sel", base + 0x84, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
 	clk_dm(IMX8MP_SYS_PLL1_REF_SEL, imx_clk_mux("sys_pll1_ref_sel", base + 0x94, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
 	clk_dm(IMX8MP_SYS_PLL2_REF_SEL, imx_clk_mux("sys_pll2_ref_sel", base + 0x104, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
 	clk_dm(IMX8MP_SYS_PLL3_REF_SEL, imx_clk_mux("sys_pll3_ref_sel", base + 0x114, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
 
+	clk_dm(IMX8MP_VIDEO_PLL1, imx_clk_pll14xx("video_pll1", "video_pll1_ref_sel", base + 0x28,
+						  &imx_1443x_pll));
 	clk_dm(IMX8MP_DRAM_PLL, imx_clk_pll14xx("dram_pll", "dram_pll_ref_sel", base + 0x50,
 						&imx_1443x_dram_pll));
 	clk_dm(IMX8MP_ARM_PLL, imx_clk_pll14xx("arm_pll", "arm_pll_ref_sel", base + 0x84,
@@ -213,12 +236,14 @@ static int imx8mp_clk_probe(struct udevice *dev)
 	clk_dm(IMX8MP_SYS_PLL3, imx_clk_pll14xx("sys_pll3", "sys_pll3_ref_sel", base + 0x114,
 						&imx_1416x_pll));
 
+	clk_dm(IMX8MP_VIDEO_PLL1_BYPASS, imx_clk_mux_flags("video_pll1_bypass", base + 0x28, 16, 1, video_pll1_bypass_sels, ARRAY_SIZE(video_pll1_bypass_sels), CLK_SET_RATE_PARENT));
 	clk_dm(IMX8MP_DRAM_PLL_BYPASS, imx_clk_mux_flags("dram_pll_bypass", base + 0x50, 4, 1, dram_pll_bypass_sels, ARRAY_SIZE(dram_pll_bypass_sels), CLK_SET_RATE_PARENT));
 	clk_dm(IMX8MP_ARM_PLL_BYPASS, imx_clk_mux_flags("arm_pll_bypass", base + 0x84, 4, 1, arm_pll_bypass_sels, ARRAY_SIZE(arm_pll_bypass_sels), CLK_SET_RATE_PARENT));
 	clk_dm(IMX8MP_SYS_PLL1_BYPASS, imx_clk_mux_flags("sys_pll1_bypass", base + 0x94, 4, 1, sys_pll1_bypass_sels, ARRAY_SIZE(sys_pll1_bypass_sels), CLK_SET_RATE_PARENT));
 	clk_dm(IMX8MP_SYS_PLL2_BYPASS, imx_clk_mux_flags("sys_pll2_bypass", base + 0x104, 4, 1, sys_pll2_bypass_sels, ARRAY_SIZE(sys_pll2_bypass_sels), CLK_SET_RATE_PARENT));
 	clk_dm(IMX8MP_SYS_PLL3_BYPASS, imx_clk_mux_flags("sys_pll3_bypass", base + 0x114, 4, 1, sys_pll3_bypass_sels, ARRAY_SIZE(sys_pll3_bypass_sels), CLK_SET_RATE_PARENT));
 
+	clk_dm(IMX8MP_VIDEO_PLL1_OUT, imx_clk_gate("video_pll1_out", "video_pll1_bypass", base + 0x28, 13));
 	clk_dm(IMX8MP_DRAM_PLL_OUT, imx_clk_gate("dram_pll_out", "dram_pll_bypass", base + 0x50, 13));
 	clk_dm(IMX8MP_ARM_PLL_OUT, imx_clk_gate("arm_pll_out", "arm_pll_bypass", base + 0x84, 11));
 	clk_dm(IMX8MP_SYS_PLL1_OUT, imx_clk_gate("sys_pll1_out", "sys_pll1_bypass", base + 0x94, 11));
@@ -267,10 +292,13 @@ static int imx8mp_clk_probe(struct udevice *dev)
 	clk_dm(IMX8MP_CLK_MAIN_AXI, imx8m_clk_composite_critical("main_axi", imx8mp_main_axi_sels, base + 0x8800));
 	clk_dm(IMX8MP_CLK_ENET_AXI, imx8m_clk_composite_critical("enet_axi", imx8mp_enet_axi_sels, base + 0x8880));
 	clk_dm(IMX8MP_CLK_NAND_USDHC_BUS, imx8m_clk_composite_critical("nand_usdhc_bus", imx8mp_nand_usdhc_sels, base + 0x8900));
+	clk_dm(IMX8MP_CLK_MEDIA_AXI, imx8m_clk_composite("media_axi", imx8mp_media_axi_sels, base + 0x8a00));
+	clk_dm(IMX8MP_CLK_MEDIA_APB, imx8m_clk_composite("media_apb", imx8mp_media_apb_sels, base + 0x8a80));
 	clk_dm(IMX8MP_CLK_NOC, imx8m_clk_composite_critical("noc", imx8mp_noc_sels, base + 0x8d00));
 	clk_dm(IMX8MP_CLK_NOC_IO, imx8m_clk_composite_critical("noc_io", imx8mp_noc_io_sels, base + 0x8d80));
 
 	clk_dm(IMX8MP_CLK_AHB, imx8m_clk_composite_critical("ahb_root", imx8mp_ahb_sels, base + 0x9000));
+	clk_dm(IMX8MP_CLK_MEDIA_DISP2_PIX, imx8m_clk_composite("media_disp2_pix", imx8mp_media_disp_pix_sels, base + 0x9300));
 
 	clk_dm(IMX8MP_CLK_IPG_ROOT, imx_clk_divider2("ipg_root", "ahb_root", base + 0x9080, 0, 1));
 
@@ -309,6 +337,8 @@ static int imx8mp_clk_probe(struct udevice *dev)
 
 	clk_dm(IMX8MP_CLK_WDOG, imx8m_clk_composite("wdog", imx8mp_wdog_sels, base + 0xb900));
 	clk_dm(IMX8MP_CLK_USDHC3, imx8m_clk_composite("usdhc3", imx8mp_usdhc3_sels, base + 0xbc80));
+	clk_dm(IMX8MP_CLK_MEDIA_DISP1_PIX, imx8m_clk_composite("media_disp1_pix", imx8mp_media_disp_pix_sels, base + 0xbe00));
+	clk_dm(IMX8MP_CLK_MEDIA_LDB, imx8m_clk_composite("media_ldb", imx8mp_media_ldb_sels, base + 0xbf00));
 
 	clk_dm(IMX8MP_CLK_DRAM_ALT_ROOT, imx_clk_fixed_factor("dram_alt_root", "dram_alt", 1, 4));
 	clk_dm(IMX8MP_CLK_DRAM_CORE, imx_clk_mux2_flags("dram_core_clk", base + 0x9800, 24, 1, imx8mp_dram_core_sels, ARRAY_SIZE(imx8mp_dram_core_sels), CLK_IS_CRITICAL));
@@ -352,6 +382,11 @@ static int imx8mp_clk_probe(struct udevice *dev)
 	clk_dm(IMX8MP_CLK_WDOG2_ROOT, imx_clk_gate4("wdog2_root_clk", "wdog", base + 0x4540, 0));
 	clk_dm(IMX8MP_CLK_WDOG3_ROOT, imx_clk_gate4("wdog3_root_clk", "wdog", base + 0x4550, 0));
 	clk_dm(IMX8MP_CLK_HSIO_ROOT, imx_clk_gate4("hsio_root_clk", "ipg_root", base + 0x45c0, 0));
+	clk_dm(IMX8MP_CLK_MEDIA_APB_ROOT, imx_clk_gate2_shared2("media_apb_root_clk", "media_apb", base + 0x45d0, 0, &share_count_media));
+	clk_dm(IMX8MP_CLK_MEDIA_AXI_ROOT, imx_clk_gate2_shared2("media_axi_root_clk", "media_axi", base + 0x45d0, 0, &share_count_media));
+	clk_dm(IMX8MP_CLK_MEDIA_DISP1_PIX_ROOT, imx_clk_gate2_shared2("media_disp1_pix_root_clk", "media_disp1_pix", base + 0x45d0, 0, &share_count_media));
+	clk_dm(IMX8MP_CLK_MEDIA_DISP2_PIX_ROOT, imx_clk_gate2_shared2("media_disp2_pix_root_clk", "media_disp2_pix", base + 0x45d0, 0, &share_count_media));
+	clk_dm(IMX8MP_CLK_MEDIA_LDB_ROOT, imx_clk_gate2_shared2("media_ldb_root_clk", "media_ldb", base + 0x45d0, 0, &share_count_media));
 
 	clk_dm(IMX8MP_CLK_USDHC3_ROOT, imx_clk_gate4("usdhc3_root_clk", "usdhc3", base + 0x45e0, 0));
 
-- 
2.43.0

[PATCH 4/8] imx: power-domain: Describe the i.MX8 MEDIAMIX domain

[Miquel Raynal]

Add support for the i.MX8 MEDIAMIX domain which is driving the power
over the whole display/rendering pipeline.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/power/domain/imx8m-power-domain.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/power/domain/imx8m-power-domain.c b/drivers/power/domain/imx8m-power-domain.c
index df5d7d69562..18ed3c93835 100644
--- a/drivers/power/domain/imx8m-power-domain.c
+++ b/drivers/power/domain/imx8m-power-domain.c
@@ -41,6 +41,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define IMX8MN_MIPI_A53_DOMAIN			BIT(2)
 
 #define IMX8MP_HSIOMIX_A53_DOMAIN		BIT(19)
+#define IMX8MP_MEDIAMIX_A53_DOMAIN		BIT(12)
 #define IMX8MP_USB2_PHY_A53_DOMAIN		BIT(5)
 #define IMX8MP_USB1_PHY_A53_DOMAIN		BIT(4)
 #define IMX8MP_PCIE_PHY_A53_DOMAIN		BIT(3)
@@ -64,6 +65,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define IMX8MN_MIPI_SW_Pxx_REQ			BIT(0)
 
 #define IMX8MP_HSIOMIX_Pxx_REQ			BIT(17)
+#define IMX8MP_MEDIAMIX_Pxx_REQ                 BIT(10)
 #define IMX8MP_USB2_PHY_Pxx_REQ			BIT(3)
 #define IMX8MP_USB1_PHY_Pxx_REQ			BIT(2)
 #define IMX8MP_PCIE_PHY_SW_Pxx_REQ		BIT(1)
@@ -82,6 +84,9 @@ DECLARE_GLOBAL_DATA_PTR;
 #define IMX8MP_HSIOMIX_PWRDNACKN		BIT(28)
 #define IMX8MP_HSIOMIX_PWRDNREQN		BIT(12)
 
+#define IMX8MP_MEDIAMIX_PWRDNACKN		BIT(30)
+#define IMX8MP_MEDIAMIX_PWRDNREQN		BIT(14)
+
 /*
  * The PGC offset values in Reference Manual
  * (Rev. 1, 01/2018 and the older ones) GPC chapter's
@@ -102,6 +107,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define IMX8MP_PGC_PCIE			13
 #define IMX8MP_PGC_USB1			14
 #define IMX8MP_PGC_USB2			15
+#define IMX8MP_PGC_MEDIAMIX		22
 #define IMX8MP_PGC_HSIOMIX		29
 
 #define GPC_PGC_CTRL(n)			(0x800 + (n) * 0x40)
@@ -304,6 +310,17 @@ static const struct imx_pgc_domain imx8mp_pgc_domains[] = {
 		.pgc = BIT(IMX8MP_PGC_HSIOMIX),
 		.keep_clocks = true,
 	},
+
+	[IMX8MP_POWER_DOMAIN_MEDIAMIX] = {
+		.bits = {
+			.pxx = IMX8MP_MEDIAMIX_Pxx_REQ,
+			.map = IMX8MP_MEDIAMIX_A53_DOMAIN,
+			.hskreq = IMX8MP_MEDIAMIX_PWRDNREQN,
+			.hskack = IMX8MP_MEDIAMIX_PWRDNACKN,
+		},
+		.pgc = BIT(IMX8MP_PGC_MEDIAMIX),
+		.keep_clocks = true,
+	},
 };
 
 static const struct imx_pgc_regs imx8mp_pgc_regs = {
-- 
2.43.0

[PATCH 5/8] imx: power-domain: Add support for the MEDIAMIX control block

[Miquel Raynal]

This block delivers power and clocks to the whole display and rendering
pipeline.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/power/domain/Kconfig           |   7 +
 drivers/power/domain/Makefile          |   1 +
 drivers/power/domain/imx8mp-mediamix.c | 185 +++++++++++++++++++++++++
 3 files changed, 193 insertions(+)
 create mode 100644 drivers/power/domain/imx8mp-mediamix.c

diff --git a/drivers/power/domain/Kconfig b/drivers/power/domain/Kconfig
index bd82d2f7044..5f5218bd8b5 100644
--- a/drivers/power/domain/Kconfig
+++ b/drivers/power/domain/Kconfig
@@ -47,6 +47,13 @@ config IMX8MP_HSIOMIX_BLKCTRL
 	help
 	  Enable support for manipulating NXP i.MX8MP on-SoC HSIOMIX block controller.
 
+config IMX8MP_MEDIAMIX_BLKCTRL
+	bool "Enable i.MX8MP MEDIAMIX domain driver"
+	depends on POWER_DOMAIN && IMX8MP
+	select CLK
+	help
+	  Enable support for manipulating NXP i.MX8MP on-SoC MEDIAMIX block controller.
+
 config MTK_POWER_DOMAIN
 	bool "Enable the MediaTek power domain driver"
 	depends on POWER_DOMAIN && ARCH_MEDIATEK
diff --git a/drivers/power/domain/Makefile b/drivers/power/domain/Makefile
index 2daab73eb75..4e45b6bb3aa 100644
--- a/drivers/power/domain/Makefile
+++ b/drivers/power/domain/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_BCM6328_POWER_DOMAIN) += bcm6328-power-domain.o
 obj-$(CONFIG_IMX8_POWER_DOMAIN) += imx8-power-domain-legacy.o imx8-power-domain.o
 obj-$(CONFIG_IMX8M_POWER_DOMAIN) += imx8m-power-domain.o
 obj-$(CONFIG_IMX8MP_HSIOMIX_BLKCTRL) += imx8mp-hsiomix.o
+obj-$(CONFIG_IMX8MP_MEDIAMIX_BLKCTRL) += imx8mp-mediamix.o
 obj-$(CONFIG_MTK_POWER_DOMAIN) += mtk-power-domain.o
 obj-$(CONFIG_MESON_GX_VPU_POWER_DOMAIN) += meson-gx-pwrc-vpu.o
 obj-$(CONFIG_MESON_EE_POWER_DOMAIN) += meson-ee-pwrc.o
diff --git a/drivers/power/domain/imx8mp-mediamix.c b/drivers/power/domain/imx8mp-mediamix.c
new file mode 100644
index 00000000000..63079ff3430
--- /dev/null
+++ b/drivers/power/domain/imx8mp-mediamix.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * i.MX8 MEDIAMIX control block driver
+ * Copyright (C) 2024 Miquel Raynal <miquel.raynal@bootlin.com>
+ * Inspired from Marek Vasut <marex@denx.de> work on the hsiomix driver.
+ */
+
+#include <asm/io.h>
+#include <clk.h>
+#include <dm.h>
+#include <power-domain-uclass.h>
+#include <linux/delay.h>
+
+#include <dt-bindings/power/imx8mp-power.h>
+
+#define BLK_SFT_RSTN		0x0
+#define BLK_CLK_EN		0x4
+
+struct imx8mp_mediamix_priv {
+	void __iomem *base;
+	struct clk clk_apb;
+	struct clk clk_axi;
+	struct clk clk_disp2;
+	struct power_domain pd_bus;
+	struct power_domain pd_lcdif2;
+};
+
+static int imx8mp_mediamix_on(struct power_domain *power_domain)
+{
+	struct udevice *dev = power_domain->dev;
+	struct imx8mp_mediamix_priv *priv = dev_get_priv(dev);
+	struct power_domain *domain;
+	struct clk *clk;
+	u32 reset;
+	int ret;
+
+	switch (power_domain->id) {
+	case IMX8MP_MEDIABLK_PD_LCDIF_2:
+		domain = &priv->pd_lcdif2;
+		clk = &priv->clk_disp2;
+		reset = BIT(11) | BIT(12) | BIT(24);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Make sure bus domain is awake */
+	ret = power_domain_on(&priv->pd_bus);
+	if (ret)
+		return ret;
+
+	/* Put devices into reset */
+	clrbits_le32(priv->base + BLK_SFT_RSTN, reset);
+
+	/* Enable upstream clocks */
+	ret = clk_enable(&priv->clk_apb);
+	if (ret)
+		goto dis_bus_pd;
+
+	ret = clk_enable(&priv->clk_axi);
+	if (ret)
+		goto dis_bus_pd;
+
+	/* Enable blk-ctrl clock to allow reset to propagate */
+	ret = clk_enable(clk);
+	if (ret)
+		goto dis_bus_pd;
+	setbits_le32(priv->base + BLK_CLK_EN, reset);
+
+	/* Power up upstream GPC domain */
+	ret = power_domain_on(domain);
+	if (ret)
+		goto dis_bus_pd;
+
+	/* Wait for reset to propagate */
+	udelay(5);
+
+	/* Release reset */
+	setbits_le32(priv->base + BLK_SFT_RSTN, reset);
+
+	return 0;
+
+dis_bus_pd:
+	power_domain_off(&priv->pd_bus);
+
+	return ret;
+}
+
+static int imx8mp_mediamix_off(struct power_domain *power_domain)
+{
+	struct udevice *dev = power_domain->dev;
+	struct imx8mp_mediamix_priv *priv = dev_get_priv(dev);
+	struct power_domain *domain;
+	u32 reset;
+
+	switch (power_domain->id) {
+	case IMX8MP_MEDIABLK_PD_LCDIF_2:
+		domain = &priv->pd_lcdif2;
+		reset = BIT(11) | BIT(12) | BIT(24);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Put devices into reset and disable clocks */
+	clrbits_le32(priv->base + BLK_SFT_RSTN, reset);
+	clrbits_le32(priv->base + BLK_CLK_EN, reset);
+
+	/* Power down upstream GPC domain */
+	power_domain_off(domain);
+
+	/* Allow bus domain to suspend */
+	power_domain_off(&priv->pd_bus);
+
+	return 0;
+}
+
+static int imx8mp_mediamix_of_xlate(struct power_domain *power_domain,
+				    struct ofnode_phandle_args *args)
+{
+	power_domain->id = args->args[0];
+
+	return 0;
+}
+
+static int imx8mp_mediamix_bind(struct udevice *dev)
+{
+	/* Bind child lcdif */
+	return dm_scan_fdt_dev(dev);
+}
+
+static int imx8mp_mediamix_probe(struct udevice *dev)
+{
+	struct imx8mp_mediamix_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	priv->base = dev_read_addr_ptr(dev);
+
+	ret = clk_get_by_name(dev, "apb", &priv->clk_apb);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_get_by_name(dev, "axi", &priv->clk_axi);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_get_by_name(dev, "disp2", &priv->clk_disp2);
+	if (ret < 0)
+		return ret;
+
+	ret = power_domain_get_by_name(dev, &priv->pd_bus, "bus");
+	if (ret < 0)
+		return ret;
+
+	ret = power_domain_get_by_name(dev, &priv->pd_lcdif2, "lcdif2");
+	if (ret < 0)
+		goto free_bus_pd;
+
+	return 0;
+
+free_bus_pd:
+	power_domain_free(&priv->pd_bus);
+	return ret;
+}
+
+static const struct udevice_id imx8mp_mediamix_ids[] = {
+	{ .compatible = "fsl,imx8mp-media-blk-ctrl" },
+	{ }
+};
+
+struct power_domain_ops imx8mp_mediamix_ops = {
+	.on = imx8mp_mediamix_on,
+	.off = imx8mp_mediamix_off,
+	.of_xlate = imx8mp_mediamix_of_xlate,
+};
+
+U_BOOT_DRIVER(imx8mp_mediamix) = {
+	.name		= "imx8mp_mediamix",
+	.id		= UCLASS_POWER_DOMAIN,
+	.of_match	= imx8mp_mediamix_ids,
+	.bind		= imx8mp_mediamix_bind,
+	.probe		= imx8mp_mediamix_probe,
+	.priv_auto	= sizeof(struct imx8mp_mediamix_priv),
+	.ops		= &imx8mp_mediamix_ops,
+};
-- 
2.43.0

[PATCH 6/8] video: imx: Fix Makefile in order to be able to add other imx drivers

[Miquel Raynal]

The IPUv3 is one IP part of the imx world, there are others, and
selecting the whole imx/ folder based on such a specific Kconfig symbol
is sub-optimal. Let's always enter the imx/ folder, and then selectively
compile parts of the folder based on the configuration.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/video/Makefile     | 2 +-
 drivers/video/imx/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index f3f70cd04a1..20854b7081e 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -52,7 +52,7 @@ obj-$(CONFIG_VIDEO_COREBOOT) += coreboot.o
 obj-$(CONFIG_VIDEO_DW_HDMI) += dw_hdmi.o
 obj-$(CONFIG_VIDEO_DW_MIPI_DSI) += dw_mipi_dsi.o
 obj-$(CONFIG_VIDEO_EFI) += efi.o
-obj-$(CONFIG_VIDEO_IPUV3) += imx/
+obj-y += imx/
 obj-$(CONFIG_VIDEO_IVYBRIDGE_IGD) += ivybridge_igd.o
 obj-$(CONFIG_VIDEO_LCD_ANX9804) += anx9804.o
 obj-$(CONFIG_VIDEO_LCD_ENDEAVORU) += endeavoru-panel.o
diff --git a/drivers/video/imx/Makefile b/drivers/video/imx/Makefile
index 179ea651fe8..46cc44d64b0 100644
--- a/drivers/video/imx/Makefile
+++ b/drivers/video/imx/Makefile
@@ -3,4 +3,4 @@
 # (C) Copyright 2000-2007
 # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
 
-obj-y += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
+obj-$(CONFIG_VIDEO_IPUV3) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
-- 
2.43.0

[PATCH 7/8] video: imx: Add LDB driver

[Miquel Raynal]

Add support for the LVDS Display Bridge (LDB) found on i.MX8.

When attached, the bridge driver looks for panels connected to one of
its two outputs and adapts its own configuration to use them. There is
currently no support for merged/split displays.

Note regarding the clock configuration:
The LDB output clock should be absolutely identical to the LCDIF output
clock so both blocks can talk to each other synchronously. However, the
LDB clock has an internal divisor of 7 (respectively 3.5 in dual
configuration) which means the LDB input clock must be explicitly set
once we know the configuration.

This driver was tested on i.MX8MP using a single panel connected to the
LVDS2 interface.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/video/imx/Kconfig  |   4 +
 drivers/video/imx/Makefile |   1 +
 drivers/video/imx/ldb.c    | 251 +++++++++++++++++++++++++++++++++++++
 3 files changed, 256 insertions(+)
 create mode 100644 drivers/video/imx/ldb.c

diff --git a/drivers/video/imx/Kconfig b/drivers/video/imx/Kconfig
index 34e8b640595..f8e0e3b23b3 100644
--- a/drivers/video/imx/Kconfig
+++ b/drivers/video/imx/Kconfig
@@ -13,3 +13,7 @@ config IMX_VIDEO_SKIP
 config IMX_HDMI
 	bool "Enable HDMI support in IPUv3"
 	depends on VIDEO_IPUV3
+
+config IMX_LDB
+	bool "i.MX LVDS display bridge (LDB)"
+	depends on VIDEO_BRIDGE
diff --git a/drivers/video/imx/Makefile b/drivers/video/imx/Makefile
index 46cc44d64b0..8d106589b75 100644
--- a/drivers/video/imx/Makefile
+++ b/drivers/video/imx/Makefile
@@ -4,3 +4,4 @@
 # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
 
 obj-$(CONFIG_VIDEO_IPUV3) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
+obj-$(CONFIG_IMX_LDB) += ldb.o
diff --git a/drivers/video/imx/ldb.c b/drivers/video/imx/ldb.c
new file mode 100644
index 00000000000..10be4421cb5
--- /dev/null
+++ b/drivers/video/imx/ldb.c
@@ -0,0 +1,251 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Derived work from:
+ *   Philippe Cornu <philippe.cornu@st.com>
+ *   Yannick Fertre <yannick.fertre@st.com>
+ * Adapted by Miquel Raynal <miquel.raynal@bootlin.com>
+ */
+
+#define LOG_CATEGORY UCLASS_VIDEO_BRIDGE
+
+#include <clk.h>
+#include <dm.h>
+#include <log.h>
+#include <panel.h>
+#include <video_bridge.h>
+#include <asm/io.h>
+#include <linux/delay.h>
+
+#define LDB_CTRL_CH0_ENABLE BIT(0)
+#define LDB_CTRL_CH1_ENABLE BIT(2)
+#define LDB_CTRL_CH0_DATA_WIDTH BIT(5)
+#define LDB_CTRL_CH0_BIT_MAPPING BIT(6)
+#define LDB_CTRL_CH1_DATA_WIDTH BIT(7)
+#define LDB_CTRL_CH1_BIT_MAPPING BIT(8)
+#define LDB_CTRL_DI0_VSYNC_POLARITY BIT(9)
+#define LDB_CTRL_DI1_VSYNC_POLARITY BIT(10)
+
+#define LVDS_CTRL_CH0_EN BIT(0)
+#define LVDS_CTRL_CH1_EN BIT(1)
+#define LVDS_CTRL_VBG_EN BIT(2)
+#define LVDS_CTRL_PRE_EMPH_EN BIT(4)
+#define LVDS_CTRL_PRE_EMPH_ADJ(n) (((n) & 0x7) << 5)
+#define LVDS_CTRL_CC_ADJ(n) (((n) & 0x7) << 11)
+
+struct imx_ldb_priv {
+	struct clk ldb_clk;
+	void __iomem *ldb_ctrl;
+	void __iomem *lvds_ctrl;
+	struct udevice *lvds1;
+	struct udevice *lvds2;
+};
+
+static int imx_ldb_set_backlight(struct udevice *dev, int percent)
+{
+	struct imx_ldb_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	if (priv->lvds1) {
+		ret = panel_enable_backlight(priv->lvds1);
+		if (ret) {
+			debug("ldb: Cannot enable lvds1 backlight\n");
+			return ret;
+		}
+
+		ret = panel_set_backlight(priv->lvds1, percent);
+		if (ret)
+			return ret;
+	}
+
+	if (priv->lvds2) {
+		ret = panel_enable_backlight(priv->lvds2);
+		if (ret) {
+			debug("ldb: Cannot enable lvds2 backlight\n");
+			return ret;
+		}
+
+		ret = panel_set_backlight(priv->lvds2, percent);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int imx_ldb_of_to_plat(struct udevice *dev)
+{
+	struct imx_ldb_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	uclass_get_device_by_endpoint(UCLASS_PANEL, dev, 1, &priv->lvds1);
+	uclass_get_device_by_endpoint(UCLASS_PANEL, dev, 2, &priv->lvds2);
+	if (!priv->lvds1 && !priv->lvds2) {
+		debug("ldb: No remote panel for '%s' (ret=%d)\n",
+		      dev_read_name(dev), ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* The block has a mysterious x7 internal divisor (x3.5 in dual configuration) */
+#define IMX_LDB_INTERNAL_DIVISOR(x) (((x) * 70) / 10)
+#define IMX_LDB_INTERNAL_DIVISOR_DUAL(x) (((x) * 35) / 10)
+
+static ulong imx_ldb_input_rate(struct imx_ldb_priv *priv,
+				struct display_timing *timings)
+{
+	ulong target_rate = timings->pixelclock.typ;
+
+	if (priv->lvds1 && priv->lvds2)
+		return IMX_LDB_INTERNAL_DIVISOR_DUAL(target_rate);
+
+	return IMX_LDB_INTERNAL_DIVISOR(target_rate);
+}
+
+static int imx_ldb_attach(struct udevice *dev)
+{
+	struct imx_ldb_priv *priv = dev_get_priv(dev);
+	struct display_timing timings;
+	bool format_jeida = false;
+	bool format_24bpp = true;
+	u32 ldb_ctrl = 0, lvds_ctrl;
+	ulong ldb_rate;
+	int ret;
+
+	/* TODO: update the 24bpp/jeida booleans with proper checks when they
+	 * will be supported.
+	 */
+	if (priv->lvds1) {
+		ret = panel_get_display_timing(priv->lvds1, &timings);
+		if (ret) {
+			ret = ofnode_decode_display_timing(dev_ofnode(priv->lvds1),
+							   0, &timings);
+			if (ret) {
+				printf("Cannot decode lvds1 timings (%d)\n", ret);
+				return ret;
+			}
+		}
+
+		ldb_ctrl |= LDB_CTRL_CH0_ENABLE;
+		if (format_24bpp)
+			ldb_ctrl |= LDB_CTRL_CH0_DATA_WIDTH;
+		if (format_jeida)
+			ldb_ctrl |= LDB_CTRL_CH0_BIT_MAPPING;
+		if (timings.flags & DISPLAY_FLAGS_VSYNC_HIGH)
+			ldb_ctrl |= LDB_CTRL_DI0_VSYNC_POLARITY;
+	}
+
+	if (priv->lvds2) {
+		ret = panel_get_display_timing(priv->lvds2, &timings);
+		if (ret) {
+			ret = ofnode_decode_display_timing(dev_ofnode(priv->lvds2),
+							   0, &timings);
+			if (ret) {
+				printf("Cannot decode lvds2 timings (%d)\n", ret);
+				return ret;
+			}
+		}
+
+		ldb_ctrl |= LDB_CTRL_CH1_ENABLE;
+		if (format_24bpp)
+			ldb_ctrl |= LDB_CTRL_CH1_DATA_WIDTH;
+		if (format_jeida)
+			ldb_ctrl |= LDB_CTRL_CH1_BIT_MAPPING;
+		if (timings.flags & DISPLAY_FLAGS_VSYNC_HIGH)
+			ldb_ctrl |= LDB_CTRL_DI1_VSYNC_POLARITY;
+	}
+
+	/*
+	 * Not all pixel clocks will work, as the final rate (after internal
+	 * integer division) should be identical to the LCDIF clock, otherwise
+	 * the rendering will appear resized/shimmering.
+	 */
+	ldb_rate = imx_ldb_input_rate(priv, &timings);
+	clk_set_rate(&priv->ldb_clk, ldb_rate);
+
+	writel(ldb_ctrl, priv->ldb_ctrl);
+
+	lvds_ctrl = LVDS_CTRL_CC_ADJ(2) | LVDS_CTRL_PRE_EMPH_EN |
+		    LVDS_CTRL_PRE_EMPH_ADJ(3) | LVDS_CTRL_VBG_EN;
+	writel(lvds_ctrl, priv->lvds_ctrl);
+
+	/* Wait for VBG to stabilize. */
+	udelay(15);
+
+	if (priv->lvds1)
+		lvds_ctrl |= LVDS_CTRL_CH0_EN;
+	if (priv->lvds2)
+		lvds_ctrl |= LVDS_CTRL_CH1_EN;
+
+	writel(lvds_ctrl, priv->lvds_ctrl);
+
+	return 0;
+}
+
+static int imx_ldb_probe(struct udevice *dev)
+{
+	struct imx_ldb_priv *priv = dev_get_priv(dev);
+	struct udevice *parent = dev_get_parent(dev);
+	fdt_addr_t parent_addr, child_addr;
+	int ret;
+
+	ret = clk_get_by_name(dev, "ldb", &priv->ldb_clk);
+	if (ret < 0)
+		return ret;
+
+	parent_addr = dev_read_addr(parent);
+	if (parent_addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	child_addr = dev_read_addr_name(dev, "ldb");
+	if (child_addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	priv->ldb_ctrl = map_physmem(parent_addr + child_addr, 0, MAP_NOCACHE);
+	if (!priv->ldb_ctrl)
+		return -EINVAL;
+
+	child_addr = dev_read_addr_name(dev, "lvds");
+	if (child_addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	priv->lvds_ctrl = map_physmem(parent_addr + child_addr, 0, MAP_NOCACHE);
+	if (!priv->lvds_ctrl)
+		return -EINVAL;
+
+	ret = clk_enable(&priv->ldb_clk);
+	if (ret)
+		return ret;
+
+	ret = video_bridge_set_active(dev, true);
+	if (ret)
+		goto dis_clk;
+
+	return 0;
+
+dis_clk:
+	clk_disable(&priv->ldb_clk);
+
+	return ret;
+}
+
+struct video_bridge_ops imx_ldb_ops = {
+	.attach = imx_ldb_attach,
+	.set_backlight	= imx_ldb_set_backlight,
+};
+
+static const struct udevice_id imx_ldb_ids[] = {
+	{ .compatible = "fsl,imx8mp-ldb"},
+	{ }
+};
+
+U_BOOT_DRIVER(imx_ldb) = {
+	.name = "imx-lvds-display-bridge",
+	.id = UCLASS_VIDEO_BRIDGE,
+	.of_match = imx_ldb_ids,
+	.probe = imx_ldb_probe,
+	.of_to_plat = imx_ldb_of_to_plat,
+	.ops = &imx_ldb_ops,
+	.priv_auto = sizeof(struct imx_ldb_priv),
+};
-- 
2.43.0

[PATCH 8/8] video: imx: Add LCDIF driver

[Miquel Raynal]

Add support for the LCD interfaces (LCDIF1/2). When probed, these
interfaces request numerous clocks and power doomains, attach the bridge
and look for a panel in order to retrieve its capabilities and
properties.

There is a similar driver existing in the upper folder for other i.MX
targets, I discovered this driver a bit late. It is not tergetting the
i.MX8 and I have no idea how different can the LCDIF be on this SoC, but
I did not manage to get it work, especially because it is not fully
compliant with the device-model, especially on the clocks/power
management side which is all ad-hoc. This is normal though, it was
contributed more than ten years ago.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/video/imx/Kconfig  |   3 +
 drivers/video/imx/Makefile |   1 +
 drivers/video/imx/lcdif.c  | 315 +++++++++++++++++++++++++++++++++++++
 3 files changed, 319 insertions(+)
 create mode 100644 drivers/video/imx/lcdif.c

diff --git a/drivers/video/imx/Kconfig b/drivers/video/imx/Kconfig
index f8e0e3b23b3..1a36d907e0c 100644
--- a/drivers/video/imx/Kconfig
+++ b/drivers/video/imx/Kconfig
@@ -17,3 +17,6 @@ config IMX_HDMI
 config IMX_LDB
 	bool "i.MX LVDS display bridge (LDB)"
 	depends on VIDEO_BRIDGE
+
+config IMX_LCDIF
+	bool "i.MX LCD interface"
diff --git a/drivers/video/imx/Makefile b/drivers/video/imx/Makefile
index 8d106589b75..1edf5a6bdf0 100644
--- a/drivers/video/imx/Makefile
+++ b/drivers/video/imx/Makefile
@@ -5,3 +5,4 @@
 
 obj-$(CONFIG_VIDEO_IPUV3) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
 obj-$(CONFIG_IMX_LDB) += ldb.o
+obj-$(CONFIG_IMX_LCDIF) += lcdif.o
diff --git a/drivers/video/imx/lcdif.c b/drivers/video/imx/lcdif.c
new file mode 100644
index 00000000000..7871bba8d90
--- /dev/null
+++ b/drivers/video/imx/lcdif.c
@@ -0,0 +1,315 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * i.MX8 LCD interface driver inspired from the Linux driver
+ * Copyright 2019 NXP
+ * Copyright 2024 Bootlin
+ * Adapted by Miquel Raynal <miquel.raynal@bootlin.com>
+ */
+
+#include <asm/io.h>
+#include <asm/mach-imx/dma.h>
+#include <clk.h>
+#include <dm.h>
+#include <panel.h>
+#include <power-domain.h>
+#include <video.h>
+#include <video_bridge.h>
+#include <linux/delay.h>
+
+#include "../videomodes.h"
+
+#define LCDIFV3_CTRL 0x0
+#define LCDIFV3_CTRL_SET 0x4
+#define LCDIFV3_CTRL_CLR 0x8
+#define   CTRL_INV_HS BIT(0)
+#define   CTRL_INV_VS BIT(1)
+#define   CTRL_INV_DE BIT(2)
+#define   CTRL_INV_PXCK BIT(3)
+#define   CTRL_CLK_GATE BIT(30)
+#define   CTRL_SW_RESET BIT(31)
+
+#define LCDIFV3_DISP_PARA 0x10
+#define   DISP_PARA_DISP_MODE_NORMAL 0
+#define   DISP_PARA_LINE_PATTERN_RGB_YUV 0
+#define   DISP_PARA_DISP_ON BIT(31)
+
+#define LCDIFV3_DISP_SIZE 0x14
+#define   DISP_SIZE_DELTA_X(x) ((x) & 0xffff)
+#define   DISP_SIZE_DELTA_Y(x) ((x) << 16)
+
+#define LCDIFV3_HSYN_PARA 0x18
+#define   HSYN_PARA_FP_H(x) ((x) & 0xffff)
+#define   HSYN_PARA_BP_H(x) ((x) << 16)
+
+#define LCDIFV3_VSYN_PARA 0x1C
+#define   VSYN_PARA_FP_V(x) ((x) & 0xffff)
+#define   VSYN_PARA_BP_V(x) ((x) << 16)
+
+#define LCDIFV3_VSYN_HSYN_WIDTH 0x20
+#define   VSYN_HSYN_PW_H(x) ((x) & 0xffff)
+#define   VSYN_HSYN_PW_V(x) ((x) << 16)
+
+#define LCDIFV3_CTRLDESCL0_1 0x200
+#define   CTRLDESCL0_1_WIDTH(x) ((x) & 0xffff)
+#define   CTRLDESCL0_1_HEIGHT(x) ((x) << 16)
+
+#define LCDIFV3_CTRLDESCL0_3 0x208
+#define   CTRLDESCL0_3_PITCH(x) ((x) & 0xFFFF)
+
+#define LCDIFV3_CTRLDESCL_LOW0_4 0x20C
+#define LCDIFV3_CTRLDESCL_HIGH0_4 0x210
+
+#define LCDIFV3_CTRLDESCL0_5 0x214
+#define   CTRLDESCL0_5_YUV_FORMAT(x) (((x) & 0x3) << 14)
+#define   CTRLDESCL0_5_BPP(x) (((x) & 0xf) << 24)
+#define     BPP32_ARGB8888 0x9
+#define   CTRLDESCL0_5_SHADOW_LOAD_EN BIT(30)
+#define   CTRLDESCL0_5_EN BIT(31)
+
+struct lcdifv3_priv {
+	void __iomem *base;
+	struct clk pix_clk;
+	struct power_domain pd;
+	struct udevice *panel;
+	struct udevice *bridge;
+};
+
+static void lcdifv3_set_mode(struct lcdifv3_priv *priv,
+			     struct display_timing *timings)
+{
+	u32 reg;
+
+	writel(DISP_SIZE_DELTA_X(timings->hactive.typ) |
+	       DISP_SIZE_DELTA_Y(timings->vactive.typ),
+	       priv->base + LCDIFV3_DISP_SIZE);
+
+	writel(HSYN_PARA_FP_H(timings->hfront_porch.typ) |
+	       HSYN_PARA_BP_H(timings->hback_porch.typ),
+	       priv->base + LCDIFV3_HSYN_PARA);
+
+	writel(VSYN_PARA_BP_V(timings->vback_porch.typ) |
+	       VSYN_PARA_FP_V(timings->vfront_porch.typ),
+	       priv->base + LCDIFV3_VSYN_PARA);
+
+	writel(VSYN_HSYN_PW_H(timings->hsync_len.typ) |
+	       VSYN_HSYN_PW_V(timings->vsync_len.typ),
+	       priv->base + LCDIFV3_VSYN_HSYN_WIDTH);
+
+	writel(CTRLDESCL0_1_WIDTH(timings->hactive.typ) |
+	       CTRLDESCL0_1_HEIGHT(timings->vactive.typ),
+	       priv->base + LCDIFV3_CTRLDESCL0_1);
+
+	if (timings->flags & DISPLAY_FLAGS_HSYNC_LOW)
+		writel(CTRL_INV_HS, priv->base + LCDIFV3_CTRL_SET);
+	else
+		writel(CTRL_INV_HS, priv->base + LCDIFV3_CTRL_CLR);
+
+	if (timings->flags & DISPLAY_FLAGS_VSYNC_LOW)
+		writel(CTRL_INV_VS, priv->base + LCDIFV3_CTRL_SET);
+	else
+		writel(CTRL_INV_VS, priv->base + LCDIFV3_CTRL_CLR);
+
+	if (timings->flags & DISPLAY_FLAGS_DE_LOW)
+		writel(CTRL_INV_DE, priv->base + LCDIFV3_CTRL_SET);
+	else
+		writel(CTRL_INV_DE, priv->base + LCDIFV3_CTRL_CLR);
+
+	if (timings->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+		writel(CTRL_INV_PXCK, priv->base + LCDIFV3_CTRL_SET);
+	else
+		writel(CTRL_INV_PXCK, priv->base + LCDIFV3_CTRL_CLR);
+
+	writel(0, priv->base + LCDIFV3_DISP_PARA);
+
+	reg = readl(priv->base + LCDIFV3_CTRLDESCL0_5);
+	reg &= ~(CTRLDESCL0_5_BPP(0xf) | CTRLDESCL0_5_YUV_FORMAT(0x3));
+	reg |= CTRLDESCL0_5_BPP(BPP32_ARGB8888);
+	writel(reg, priv->base + LCDIFV3_CTRLDESCL0_5);
+}
+
+static void lcdifv3_enable_controller(struct lcdifv3_priv *priv)
+{
+	u32 reg;
+
+	reg = readl(priv->base + LCDIFV3_DISP_PARA);
+	reg |= DISP_PARA_DISP_ON;
+	writel(reg, priv->base + LCDIFV3_DISP_PARA);
+
+	reg = readl(priv->base + LCDIFV3_CTRLDESCL0_5);
+	reg |= CTRLDESCL0_5_EN;
+	writel(reg, priv->base + LCDIFV3_CTRLDESCL0_5);
+}
+
+static int lcdifv3_video_sync(struct udevice *dev)
+{
+	struct lcdifv3_priv *priv = dev_get_priv(dev);
+	u32 reg;
+
+	reg = readl(priv->base + LCDIFV3_CTRLDESCL0_5);
+	reg |= CTRLDESCL0_5_SHADOW_LOAD_EN;
+	writel(reg, priv->base + LCDIFV3_CTRLDESCL0_5);
+
+	return 0;
+}
+
+static void lcdifv3_init(struct udevice *dev, struct display_timing *timings)
+{
+	struct video_uc_plat *plat = dev_get_uclass_plat(dev);
+	struct lcdifv3_priv *priv = dev_get_priv(dev);
+
+	clk_set_rate(&priv->pix_clk, timings->pixelclock.typ);
+
+	writel(CTRL_SW_RESET | CTRL_CLK_GATE, priv->base + LCDIFV3_CTRL_CLR);
+	udelay(10);
+
+	lcdifv3_set_mode(priv, timings);
+
+	writel(plat->base & 0xFFFFFFFF, priv->base + LCDIFV3_CTRLDESCL_LOW0_4);
+	writel(plat->base >> 32, priv->base + LCDIFV3_CTRLDESCL_HIGH0_4);
+
+	writel(CTRLDESCL0_3_PITCH(timings->hactive.typ * 4), /* 32bpp */
+	       priv->base + LCDIFV3_CTRLDESCL0_3);
+
+	lcdifv3_enable_controller(priv);
+}
+
+static int lcdifv3_video_probe(struct udevice *dev)
+{
+	struct video_uc_plat *plat = dev_get_uclass_plat(dev);
+	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
+	struct lcdifv3_priv *priv = dev_get_priv(dev);
+	struct clk axi_clk, disp_axi_clk;
+	struct display_timing timings;
+	u32 fb_start, fb_end;
+	int ret;
+
+	ret = power_domain_get(dev, &priv->pd);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_get_by_name(dev, "pix", &priv->pix_clk);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_get_by_name(dev, "axi", &axi_clk);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_get_by_name(dev, "disp_axi", &disp_axi_clk);
+	if (ret < 0)
+		return ret;
+
+	ret = power_domain_on(&priv->pd);
+	if (ret)
+		return ret;
+
+	ret = clk_enable(&priv->pix_clk);
+	if (ret)
+		goto dis_pd;
+
+	ret = clk_enable(&axi_clk);
+	if (ret)
+		goto dis_pix_clk;
+
+	ret = clk_enable(&disp_axi_clk);
+	if (ret)
+		goto dis_axi_clk;
+
+	priv->base = dev_remap_addr(dev);
+	if (!priv->base) {
+		ret = -EINVAL;
+		goto dis_clks;
+	}
+
+	/* Attach bridge */
+	ret = uclass_get_device_by_endpoint(UCLASS_VIDEO_BRIDGE, dev,
+					    0, &priv->bridge);
+	if (ret)
+		goto dis_clks;
+
+	ret = video_bridge_attach(priv->bridge);
+	if (ret)
+		goto dis_clks;
+
+	ret = video_bridge_set_backlight(priv->bridge, 80);
+	if (ret)
+		goto dis_clks;
+
+	/* Attach panels */
+	ret = uclass_get_device_by_endpoint(UCLASS_PANEL, priv->bridge,
+					    1, &priv->panel);
+	if (ret) {
+		ret = uclass_get_device_by_endpoint(UCLASS_PANEL, priv->bridge,
+						    2, &priv->panel);
+		if (ret)
+			goto dis_clks;
+	}
+
+	ret = panel_get_display_timing(priv->panel, &timings);
+	if (ret) {
+		ret = ofnode_decode_display_timing(dev_ofnode(priv->panel),
+						   0, &timings);
+		if (ret) {
+			printf("Cannot decode panel timings (%d)\n", ret);
+			goto dis_clks;
+		}
+	}
+
+	lcdifv3_init(dev, &timings);
+
+	/* Only support 32bpp for now */
+	uc_priv->bpix = VIDEO_BPP32;
+	uc_priv->xsize = timings.hactive.typ;
+	uc_priv->ysize = timings.vactive.typ;
+
+	/* Enable dcache for the frame buffer */
+	fb_start = plat->base & ~(MMU_SECTION_SIZE - 1);
+	fb_end = ALIGN(plat->base + plat->size, 1 << MMU_SECTION_SHIFT);
+	mmu_set_region_dcache_behaviour(fb_start, fb_end - fb_start,
+					DCACHE_WRITEBACK);
+	video_set_flush_dcache(dev, true);
+	gd->fb_base = plat->base;
+
+	return 0;
+
+dis_clks:
+	clk_disable(&disp_axi_clk);
+dis_axi_clk:
+	clk_disable(&axi_clk);
+dis_pix_clk:
+	clk_disable(&priv->pix_clk);
+dis_pd:
+	power_domain_off(&priv->pd);
+
+	return ret;
+}
+
+static int lcdifv3_video_bind(struct udevice *dev)
+{
+	struct video_uc_plat *plat = dev_get_uclass_plat(dev);
+
+	/* Max size supported by LCDIF */
+	plat->size = 1920 * 1080 * VNBYTES(VIDEO_BPP32);
+
+	return 0;
+}
+
+static const struct udevice_id lcdifv3_video_ids[] = {
+	{ .compatible = "fsl,imx8mp-lcdif" },
+	{ }
+};
+
+static struct video_ops lcdifv3_video_ops = {
+	.video_sync = lcdifv3_video_sync,
+};
+
+U_BOOT_DRIVER(lcdifv3_video) = {
+	.name = "lcdif",
+	.id = UCLASS_VIDEO,
+	.of_match = lcdifv3_video_ids,
+	.bind = lcdifv3_video_bind,
+	.ops = &lcdifv3_video_ops,
+	.probe = lcdifv3_video_probe,
+	.priv_auto = sizeof(struct lcdifv3_priv),
+	.flags = DM_FLAG_PRE_RELOC | DM_FLAG_OS_PREPARE,
+};
-- 
2.43.0

Re: [PATCH 0/8] Add imx8mp video support

[Michael Nazzareno Trimarchi]

Hi Miquel

On Tue, Sep 10, 2024 at 12:13 PM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
>
> In order to display a boot picture or an error message, the i.MX8MP
> display pipeline must be enabled. The SoC has support for various
> interfaces (LVDS, HDMI, DSI). The one supported in this series is the
> standard 4-lane LVDS output. The minimal setup is thus composed of:
> * An LCD InterFace (LCDIF) with an AXI/APB interface, generating a pixel
>   stream
> * One LVDS Display Bridge (LDB), also named pixel mapper, which receives
>   the pixel stream and route it to one or two (possibly combined) LVDS
>   displays.
> * All necessary clocks and power controls coming from the MEDIAMIX
>   control block.
>
> Patch 1 adds a very useful helper to the core in order to grab devices
> through endpoints instead of being limited to phandles. Video pipelines
> being often described using graphs endpoints, the introduced helper is
> used several times in the serires (there are 3 LCDIF, one of them being
> connected to the LDB, itself having 2 ports).
>
> Patch 2 is a fix which is necessary for this series to work properly,
> but is way broader than just this use case. In practice, when assigned
> clocks are defined in the device tree, the clock uclass tries to assign
> the parents first and then sets them to the correct frequency. This only
> works if the parents have been enabled themselves. Otherwise we end-up
> with a non-clocked parent. I believe this is not the intended behavior
> in general, but more importantly on the i.MX8MP, there are "clock
> slices" which have pre-requisites in order to be modified and selecting
> an ungated parent is one of them.
>
> All the other patches progressively build support for the whole video
> pipeline. Regarding the LCDIF driver, there is already a similar driver
> for older i.MX SoCs but I didn't manage to get it to work. It was
> written more than a decade ago while device-model, clocks and others
> were not yet generically supported. Thus, numerous ad-hoc solutions
> were implemented, which no longer fit today's requirements. I preferred
> to add a new "clean" driver instead of modifying the existing one
> because of the too high risk of breaking these platforms. Once proper
> clocks/power-domain descriptions will be added to them they might be
> converted (and tested) to work with the "new" implementation, but going
> the opposite way felt drawback.
>

Thank you for adding those patches. We are working on mipi support
here and some of the clock patches
are there too. I will try to look and rebase our patchset

https://patchwork.amarulasolutions.com/patch/3401/

Michael


> Thanks,
> Miquèl
>
> Miquel Raynal (8):
>   dm: core: Add a helper to retrieve devices through graph endpoints
>   clk: Ensure the parent clocks are enabled while reparenting
>   clk: imx8mp: Add media related clocks
>   imx: power-domain: Describe the i.MX8 MEDIAMIX domain
>   imx: power-domain: Add support for the MEDIAMIX control block
>   video: imx: Fix Makefile in order to be able to add other imx drivers
>   video: imx: Add LDB driver
>   video: imx: Add LCDIF driver
>
>  drivers/clk/clk-uclass.c                  |   4 +
>  drivers/clk/imx/clk-imx8mp.c              |  35 +++
>  drivers/core/uclass.c                     |  62 +++++
>  drivers/power/domain/Kconfig              |   7 +
>  drivers/power/domain/Makefile             |   1 +
>  drivers/power/domain/imx8m-power-domain.c |  17 ++
>  drivers/power/domain/imx8mp-mediamix.c    | 185 +++++++++++++
>  drivers/video/Makefile                    |   2 +-
>  drivers/video/imx/Kconfig                 |   7 +
>  drivers/video/imx/Makefile                |   4 +-
>  drivers/video/imx/lcdif.c                 | 315 ++++++++++++++++++++++
>  drivers/video/imx/ldb.c                   | 251 +++++++++++++++++
>  include/dm/uclass.h                       |  21 ++
>  13 files changed, 909 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/power/domain/imx8mp-mediamix.c
>  create mode 100644 drivers/video/imx/lcdif.c
>  create mode 100644 drivers/video/imx/ldb.c
>
> --
> 2.43.0
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH 0/8] Add imx8mp video support

[Miquel Raynal]

Hi Michael,

michael@amarulasolutions.com wrote on Tue, 10 Sep 2024 12:30:42 +0200:

> Hi Miquel
> 
> On Tue, Sep 10, 2024 at 12:13 PM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> >
> > In order to display a boot picture or an error message, the i.MX8MP
> > display pipeline must be enabled. The SoC has support for various
> > interfaces (LVDS, HDMI, DSI). The one supported in this series is the
> > standard 4-lane LVDS output. The minimal setup is thus composed of:
> > * An LCD InterFace (LCDIF) with an AXI/APB interface, generating a pixel
> >   stream
> > * One LVDS Display Bridge (LDB), also named pixel mapper, which receives
> >   the pixel stream and route it to one or two (possibly combined) LVDS
> >   displays.
> > * All necessary clocks and power controls coming from the MEDIAMIX
> >   control block.
> >
> > Patch 1 adds a very useful helper to the core in order to grab devices
> > through endpoints instead of being limited to phandles. Video pipelines
> > being often described using graphs endpoints, the introduced helper is
> > used several times in the serires (there are 3 LCDIF, one of them being
> > connected to the LDB, itself having 2 ports).
> >
> > Patch 2 is a fix which is necessary for this series to work properly,
> > but is way broader than just this use case. In practice, when assigned
> > clocks are defined in the device tree, the clock uclass tries to assign
> > the parents first and then sets them to the correct frequency. This only
> > works if the parents have been enabled themselves. Otherwise we end-up
> > with a non-clocked parent. I believe this is not the intended behavior
> > in general, but more importantly on the i.MX8MP, there are "clock
> > slices" which have pre-requisites in order to be modified and selecting
> > an ungated parent is one of them.
> >
> > All the other patches progressively build support for the whole video
> > pipeline. Regarding the LCDIF driver, there is already a similar driver
> > for older i.MX SoCs but I didn't manage to get it to work. It was
> > written more than a decade ago while device-model, clocks and others
> > were not yet generically supported. Thus, numerous ad-hoc solutions
> > were implemented, which no longer fit today's requirements. I preferred
> > to add a new "clean" driver instead of modifying the existing one
> > because of the too high risk of breaking these platforms. Once proper
> > clocks/power-domain descriptions will be added to them they might be
> > converted (and tested) to work with the "new" implementation, but going
> > the opposite way felt drawback.
> >  
> 
> Thank you for adding those patches. We are working on mipi support
> here and some of the clock patches
> are there too. I will try to look and rebase our patchset
> 
> https://patchwork.amarulasolutions.com/patch/3401/

Thanks for letting me know. Indeed there are a couple of conflicts.
- My patch 2 can easily be replaced by your approach. 
- Regarding the power domain we had two different approaches, I
  didn't look in details, but again one or the other seems fine, I
  guess.
- The LDB driver in my series is new.
- Finally regarding the LCDIF changes, the approach taken on your
  side follows the somewhat too ad-hoc logic and would probably benefit
  from being migrated to the driver I propose, which does make use of
  the (DM) clock, power domain and DT API. I am really happy with the
  core helper I've added retrieving a device in front of a graph
  endpoint, which makes the whole "get panel/bridge" approach much more
  flexible and adapted to today's needs.

What is the status of your patchset? How shall we handle the conflicts?

Thanks,
Miquèl

Re: [PATCH 0/8] Add imx8mp video support

[Michael Nazzareno Trimarchi]

Hi Miquel

On Tue, Sep 10, 2024 at 2:56 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Michael,
>
> michael@amarulasolutions.com wrote on Tue, 10 Sep 2024 12:30:42 +0200:
>
> > Hi Miquel
> >
> > On Tue, Sep 10, 2024 at 12:13 PM Miquel Raynal
> > <miquel.raynal@bootlin.com> wrote:
> > >
> > > In order to display a boot picture or an error message, the i.MX8MP
> > > display pipeline must be enabled. The SoC has support for various
> > > interfaces (LVDS, HDMI, DSI). The one supported in this series is the
> > > standard 4-lane LVDS output. The minimal setup is thus composed of:
> > > * An LCD InterFace (LCDIF) with an AXI/APB interface, generating a pixel
> > >   stream
> > > * One LVDS Display Bridge (LDB), also named pixel mapper, which receives
> > >   the pixel stream and route it to one or two (possibly combined) LVDS
> > >   displays.
> > > * All necessary clocks and power controls coming from the MEDIAMIX
> > >   control block.
> > >
> > > Patch 1 adds a very useful helper to the core in order to grab devices
> > > through endpoints instead of being limited to phandles. Video pipelines
> > > being often described using graphs endpoints, the introduced helper is
> > > used several times in the serires (there are 3 LCDIF, one of them being
> > > connected to the LDB, itself having 2 ports).
> > >
> > > Patch 2 is a fix which is necessary for this series to work properly,
> > > but is way broader than just this use case. In practice, when assigned
> > > clocks are defined in the device tree, the clock uclass tries to assign
> > > the parents first and then sets them to the correct frequency. This only
> > > works if the parents have been enabled themselves. Otherwise we end-up
> > > with a non-clocked parent. I believe this is not the intended behavior
> > > in general, but more importantly on the i.MX8MP, there are "clock
> > > slices" which have pre-requisites in order to be modified and selecting
> > > an ungated parent is one of them.
> > >
> > > All the other patches progressively build support for the whole video
> > > pipeline. Regarding the LCDIF driver, there is already a similar driver
> > > for older i.MX SoCs but I didn't manage to get it to work. It was
> > > written more than a decade ago while device-model, clocks and others
> > > were not yet generically supported. Thus, numerous ad-hoc solutions
> > > were implemented, which no longer fit today's requirements. I preferred
> > > to add a new "clean" driver instead of modifying the existing one
> > > because of the too high risk of breaking these platforms. Once proper
> > > clocks/power-domain descriptions will be added to them they might be
> > > converted (and tested) to work with the "new" implementation, but going
> > > the opposite way felt drawback.
> > >
> >
> > Thank you for adding those patches. We are working on mipi support
> > here and some of the clock patches
> > are there too. I will try to look and rebase our patchset
> >
> > https://patchwork.amarulasolutions.com/patch/3401/
>
> Thanks for letting me know. Indeed there are a couple of conflicts.
> - My patch 2 can easily be replaced by your approach.
> - Regarding the power domain we had two different approaches, I
>   didn't look in details, but again one or the other seems fine, I
>   guess.
> - The LDB driver in my series is new.
> - Finally regarding the LCDIF changes, the approach taken on your
>   side follows the somewhat too ad-hoc logic and would probably benefit
>   from being migrated to the driver I propose, which does make use of
>   the (DM) clock, power domain and DT API. I am really happy with the
>   core helper I've added retrieving a device in front of a graph
>   endpoint, which makes the whole "get panel/bridge" approach much more
>   flexible and adapted to today's needs.
>

My idea is to work on top of your patches and try to migrate them and
cover conflicts.
What be helpful is to get some of our clock patches and include in
your series and then I will try
to move our work on it

Michael

> What is the status of your patchset? How shall we handle the conflicts?
>
> Thanks,
> Miquèl



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH 3/8] clk: imx8mp: Add media related clocks

[Fabio Estevam]

Hi Miquel,

On Tue, Sep 10, 2024 at 7:14 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> These are all the clocks needed to get an LCD panel working, going
> through one of the LCDIF and the LDB. The media AXI and APB clocks are
> also described.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

This one does not apply. Please rebase it against the next branch.

Re: [PATCH 0/8] Add imx8mp video support

[Fabio Estevam]

Hi Miquel,

Thanks for the series.

On Tue, Sep 10, 2024 at 7:21 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Miquel Raynal (8):
>   dm: core: Add a helper to retrieve devices through graph endpoints
>   clk: Ensure the parent clocks are enabled while reparenting
>   clk: imx8mp: Add media related clocks
>   imx: power-domain: Describe the i.MX8 MEDIAMIX domain
>   imx: power-domain: Add support for the MEDIAMIX control block
>   video: imx: Fix Makefile in order to be able to add other imx drivers
>   video: imx: Add LDB driver
>   video: imx: Add LCDIF driver

Please also add the newly added drivers to some defconfig so we can
have an in-tree user. Otherwise, it becomes dead-code.

Thanks

Re: [PATCH 7/8] video: imx: Add LDB driver

[Fabio Estevam]

On Tue, Sep 10, 2024 at 7:21 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Add support for the LVDS Display Bridge (LDB) found on i.MX8.

i.MX8MP for clarity.

> +config IMX_LDB
> +       bool "i.MX LVDS display bridge (LDB)"

In Linux, we have:

config DRM_FSL_LDB
tristate "Freescale i.MX8MP LDB bridge"

Please follow the same description text.

Re: [PATCH 8/8] video: imx: Add LCDIF driver

[Fabio Estevam]

On Tue, Sep 10, 2024 at 7:51 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Add support for the LCD interfaces (LCDIF1/2). When probed, these
> interfaces request numerous clocks and power doomains, attach the bridge

Typo "domains"

> and look for a panel in order to retrieve its capabilities and
> properties.
>
> There is a similar driver existing in the upper folder for other i.MX

"similar existing driver"

> targets, I discovered this driver a bit late. It is not tergetting the

targeting.

> i.MX8 and I have no idea how different can the LCDIF be on this SoC, but

i.MX8MP.

Re: [PATCH 1/8] dm: core: Add a helper to retrieve devices through graph endpoints

[Simon Glass]

Hi Miquel,

On Tue, 10 Sept 2024 at 04:13, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> There are already several helpers to find a udevice based on its
> position in a device tree, like getting a child or a node pointed by a
> phandle, but there was no support for graph endpoints, which are very
> common in display pipelines.
>
> Add a new helper, named uclass_get_device_by_endpoint() which enters the
> child graph reprensentation, looks for a specific port, then follows the
> remote endpoint, and finally retrieves the first parent of the given
> uclass_id.
>
> This is a very handy and straightforward way to get a bridge or a panel
> handle.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/core/uclass.c | 62 +++++++++++++++++++++++++++++++++++++++++++
>  include/dm/uclass.h   | 21 +++++++++++++++
>  2 files changed, 83 insertions(+)

This is fine, but please add a test when changing driver model.

Perhaps it is in another patch? I don't seem to have the series so I
wonder if I have been unsubscribed again...

>
> diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
> index e46d5717aa6..5803fd51986 100644
> --- a/drivers/core/uclass.c
> +++ b/drivers/core/uclass.c
> @@ -572,6 +572,68 @@ int uclass_get_device_by_phandle(enum uclass_id id, struct udevice *parent,
>         ret = uclass_find_device_by_phandle(id, parent, name, &dev);
>         return uclass_get_device_tail(dev, ret, devp);
>  }
> +
> +int uclass_get_device_by_endpoint(enum uclass_id class_id, struct udevice *dev,
> +                                 u32 ep_idx, struct udevice **devp)
> +{
> +       ofnode port = ofnode_null(), ep, remote_ep;
> +       struct udevice **target;
> +       u32 remote_phandle;
> +       int ret;
> +
> +       if (!ep_idx)
> +               port = dev_read_subnode(dev, "port");
> +
> +       if (!ofnode_valid(port)) {
> +               char port_name[32];
> +
> +               port = dev_read_subnode(dev, "ports");
> +               if (!ofnode_valid(port)) {
> +                       debug("%s: no 'port'/'ports' subnode\n",
> +                             dev_read_name(dev));
> +                       return -EINVAL;
> +               }
> +
> +               snprintf(port_name, sizeof(port_name), "port@%x", ep_idx);
> +               port = ofnode_find_subnode(port, port_name);
> +               if (!ofnode_valid(port)) {
> +                       debug("%s: no 'port@%x' subnode\n",
> +                             dev_read_name(dev), ep_idx);
> +                       return -EINVAL;
> +               }
> +       }
> +
> +       ep = ofnode_find_subnode(port, "endpoint");
> +       if (!ofnode_valid(ep)) {
> +               debug("%s: no 'endpoint' in %s subnode\n",
> +                     ofnode_get_name(port), dev_read_name(dev));
> +               return -EINVAL;
> +       }
> +
> +       ret = ofnode_read_u32(ep, "remote-endpoint", &remote_phandle);
> +       if (ret)
> +               return ret;
> +
> +       remote_ep = ofnode_get_by_phandle(remote_phandle);
> +       if (!ofnode_valid(remote_ep))
> +               return -EINVAL;
> +
> +       while (ofnode_valid(remote_ep)) {
> +               remote_ep = ofnode_get_parent(remote_ep);
> +               debug("trying subnode: %s\n", ofnode_get_name(remote_ep));
> +               if (!ofnode_valid(remote_ep)) {
> +                       debug("%s: no more remote devices\n",
> +                             ofnode_get_name(remote_ep));
> +                       return -EINVAL;
> +               }
> +
> +               ret = uclass_find_device_by_ofnode(class_id, remote_ep, target);
> +               if (!ret)
> +                       break;
> +       };
> +
> +       return uclass_get_device_tail(*target, ret, devp);
> +}
>  #endif
>
>  /*
> diff --git a/include/dm/uclass.h b/include/dm/uclass.h
> index 456eef7f2f3..1194174ca9b 100644
> --- a/include/dm/uclass.h
> +++ b/include/dm/uclass.h
> @@ -333,6 +333,27 @@ int uclass_get_device_by_phandle(enum uclass_id id, struct udevice *parent,
>  int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv,
>                                 struct udevice **devp);
>
> +/**
> + * uclass_get_device_by_endpoint() - Get a uclass device for a remote endpoint
> + *
> + * This searches through the parents of the specified remote endpoint
> + * for the first device matching the uclass. Said otherwise, this helper
> + * goes through the graph (endpoint) representation and searches for
> + * matching devices. Endpoints can be subnodes of the "port" node or
> + * subnodes of ports identified with a reg property, themselves in a
> + * "ports" container.
> + *
> + * The device is probed to activate it ready for use.
> + *
> + * @class_id: uclass ID to look up
> + * @dev: Device to start from
> + * @ep_idx: Index of the endpoint to follow, 0 if there is none.
> + * @target: Returns pointer to the first device matching the expected uclass.
> + * Return: 0 if OK, -ve on error
> + */
> +int uclass_get_device_by_endpoint(enum uclass_id class_id, struct udevice *dev,
> +                                 u32 ep_idx, struct udevice **target);
> +
>  /**
>   * uclass_first_device() - Get the first device in a uclass
>   *
> --
> 2.43.0
>

Regards,
SImon

Re: [PATCH 3/8] clk: imx8mp: Add media related clocks

[Fabio Estevam]

Hi Miquel,

On Wed, Sep 11, 2024 at 4:48 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Miquel,
>
> On Tue, Sep 10, 2024 at 7:14 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > These are all the clocks needed to get an LCD panel working, going
> > through one of the LCDIF and the LDB. The media AXI and APB clocks are
> > also described.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>
> This one does not apply. Please rebase it against the next branch.

Also, shouldn't we restrict registering these clocks only if DM_VIDEO
is selected?

clk-imx8mm.c and drivers/clk/imx/clk-imx8mn.c guard some clocks with
their respective CONFIG_ options.

We should do the same here to avoid unnecessary code growth for those
who don't need
splash screen on i.MX8MP.

Re: [PATCH 4/8] imx: power-domain: Describe the i.MX8 MEDIAMIX domain

[Fabio Estevam]

On Tue, Sep 10, 2024 at 7:31 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> +       [IMX8MP_POWER_DOMAIN_MEDIAMIX] = {
> +               .bits = {
> +                       .pxx = IMX8MP_MEDIAMIX_Pxx_REQ,
> +                       .map = IMX8MP_MEDIAMIX_A53_DOMAIN,
> +                       .hskreq = IMX8MP_MEDIAMIX_PWRDNREQN,
> +                       .hskack = IMX8MP_MEDIAMIX_PWRDNACKN,
> +               },
> +               .pgc = BIT(IMX8MP_PGC_MEDIAMIX),
> +               .keep_clocks = true,
> +       },

Great, this matches the drivers/pmdomain/imx/gpcv2.c kernel driver:

Reviewed-by: Fabio Estevam <festevam@gmail.com>

Re: [PATCH 5/8] imx: power-domain: Add support for the MEDIAMIX control block

[Fabio Estevam]

On Tue, Sep 10, 2024 at 7:21 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> +       /* Make sure bus domain is awake */
> +       ret = power_domain_on(&priv->pd_bus);
> +       if (ret)
> +               return ret;
> +
> +       /* Put devices into reset */
> +       clrbits_le32(priv->base + BLK_SFT_RSTN, reset);
> +
> +       /* Enable upstream clocks */
> +       ret = clk_enable(&priv->clk_apb);
> +       if (ret)
> +               goto dis_bus_pd;
> +
> +       ret = clk_enable(&priv->clk_axi);
> +       if (ret)
> +               goto dis_bus_pd;
> +
> +       /* Enable blk-ctrl clock to allow reset to propagate */
> +       ret = clk_enable(clk);
> +       if (ret)
> +               goto dis_bus_pd;
> +       setbits_le32(priv->base + BLK_CLK_EN, reset);
> +
> +       /* Power up upstream GPC domain */
> +       ret = power_domain_on(domain);
> +       if (ret)
> +               goto dis_bus_pd;

The previously enabled clocks should be disabled on the error paths.

Could you use clk_enable_bulk()?

Re: [PATCH 3/8] clk: imx8mp: Add media related clocks

[Michael Nazzareno Trimarchi]

Hi Fabio

On Thu, Sep 12, 2024 at 2:30 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Miquel,
>
> On Wed, Sep 11, 2024 at 4:48 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Miquel,
> >
> > On Tue, Sep 10, 2024 at 7:14 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > These are all the clocks needed to get an LCD panel working, going
> > > through one of the LCDIF and the LDB. The media AXI and APB clocks are
> > > also described.
> > >
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> >
> > This one does not apply. Please rebase it against the next branch.
>
> Also, shouldn't we restrict registering these clocks only if DM_VIDEO
> is selected?
>
> clk-imx8mm.c and drivers/clk/imx/clk-imx8mn.c guard some clocks with
> their respective CONFIG_ options.
>
> We should do the same here to avoid unnecessary code growth for those
> who don't need
> splash screen on i.MX8MP.

I think that we can wait a bit on those patches, I will send my video series
and try to arrange it back and rebase and work together with Miquel

Michael


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH 0/8] Add imx8mp video support

[Miquel Raynal]

Hi Michael,

> > > Thank you for adding those patches. We are working on mipi support
> > > here and some of the clock patches
> > > are there too. I will try to look and rebase our patchset
> > >
> > > https://patchwork.amarulasolutions.com/patch/3401/  
> >
> > Thanks for letting me know. Indeed there are a couple of conflicts.
> > - My patch 2 can easily be replaced by your approach.
> > - Regarding the power domain we had two different approaches, I
> >   didn't look in details, but again one or the other seems fine, I
> >   guess.
> > - The LDB driver in my series is new.
> > - Finally regarding the LCDIF changes, the approach taken on your
> >   side follows the somewhat too ad-hoc logic and would probably benefit
> >   from being migrated to the driver I propose, which does make use of
> >   the (DM) clock, power domain and DT API. I am really happy with the
> >   core helper I've added retrieving a device in front of a graph
> >   endpoint, which makes the whole "get panel/bridge" approach much more
> >   flexible and adapted to today's needs.
> >  
> 
> My idea is to work on top of your patches and try to migrate them and
> cover conflicts.
> What be helpful is to get some of our clock patches and include in
> your series and then I will try
> to move our work on it

Of course, no problem. I'll be ready to test them again on i.MX8MP.

Fabio reported several typos which are easy to fix, but for instance
the DM helper needs a test, so I 'll work on it and share it with you.

Do you have a rough timeline for the next iteration?

Thanks,
Miquèl

Re: [PATCH 3/8] clk: imx8mp: Add media related clocks

[Adam Ford]

On Tue, Sep 10, 2024 at 5:14 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> These are all the clocks needed to get an LCD panel working, going
> through one of the LCDIF and the LDB. The media AXI and APB clocks are
> also described.

Are these clocks going to be enumerated in SPL?  I am concerned it
might bloat the SPL phase if they do.  If that's the case, can we
encapsulate these new clocks inside #if config_is_enabled() so they
are only enabled when needed?

adam
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/clk/imx/clk-imx8mp.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
> index 7dfc829df2c..92c5d8441c0 100644
> --- a/drivers/clk/imx/clk-imx8mp.c
> +++ b/drivers/clk/imx/clk-imx8mp.c
> @@ -15,7 +15,10 @@
>
>  #include "clk.h"
>
> +static u32 share_count_media;
> +
>  static const char *pll_ref_sels[] = { "clock-osc-24m", "dummy", "dummy", "dummy", };
> +static const char *video_pll1_bypass_sels[] = {"video_pll1", "video_pll1_ref_sel", };
>  static const char *dram_pll_bypass_sels[] = {"dram_pll", "dram_pll_ref_sel", };
>  static const char *arm_pll_bypass_sels[] = {"arm_pll", "arm_pll_ref_sel", };
>  static const char *sys_pll1_bypass_sels[] = {"sys_pll1", "sys_pll1_ref_sel", };
> @@ -42,6 +45,14 @@ static const char *imx8mp_nand_usdhc_sels[] = {"clock-osc-24m", "sys_pll1_266m",
>                                                "sys_pll2_200m", "sys_pll1_133m", "sys_pll3_out",
>                                                "sys_pll2_250m", "audio_pll1_out", };
>
> +static const char *imx8mp_media_axi_sels[] = {"clock-osc-24m", "sys_pll2_1000m", "sys_pll1_800m",
> +                                             "sys_pll3_out", "sys_pll1_40m", "audio_pll2_out",
> +                                             "clk_ext1", "sys_pll2_500m", };
> +
> +static const char *imx8mp_media_apb_sels[] = {"clock-osc-24m", "sys_pll2_125m", "sys_pll1_800m",
> +                                             "sys_pll3_out", "sys_pll1_40m", "audio_pll2_out",
> +                                             "clk_ext1", "sys_pll1_133m", };
> +
>  static const char *imx8mp_noc_sels[] = {"clock-osc-24m", "sys_pll1_800m", "sys_pll3_out",
>                                         "sys_pll2_1000m", "sys_pll2_500m", "audio_pll1_out",
>                                         "video_pll1_out", "audio_pll2_out", };
> @@ -174,6 +185,15 @@ static const char *imx8mp_usdhc3_sels[] = {"clock-osc-24m", "sys_pll1_400m", "sy
>                                            "sys_pll2_500m", "sys_pll3_out", "sys_pll1_266m",
>                                            "audio_pll2_out", "sys_pll1_100m", };
>
> +static const char *imx8mp_media_disp_pix_sels[] = {"clock-osc-24m", "video_pll1_out", "audio_pll2_out",
> +                                                  "audio_pll1_out", "sys_pll1_800m",
> +                                                  "sys_pll2_1000m", "sys_pll3_out", "clk_ext4", };
> +
> +static const char *imx8mp_media_ldb_sels[] = {"clock-osc-24m", "sys_pll2_333m", "sys_pll2_100m",
> +                                                    "sys_pll1_800m", "sys_pll2_1000m",
> +                                                    "clk_ext2", "audio_pll2_out",
> +                                                    "video_pll1_out", };
> +
>  static const char *imx8mp_enet_ref_sels[] = {"clock-osc-24m", "sys_pll2_125m", "sys_pll2_50m",
>                                              "sys_pll2_100m", "sys_pll1_160m", "audio_pll1_out",
>                                              "video_pll1_out", "clk_ext4", };
> @@ -196,12 +216,15 @@ static int imx8mp_clk_probe(struct udevice *dev)
>
>         base = (void *)ANATOP_BASE_ADDR;
>
> +       clk_dm(IMX8MP_VIDEO_PLL1_REF_SEL, imx_clk_mux("video_pll1_ref_sel", base + 0x28, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
>         clk_dm(IMX8MP_DRAM_PLL_REF_SEL, imx_clk_mux("dram_pll_ref_sel", base + 0x50, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
>         clk_dm(IMX8MP_ARM_PLL_REF_SEL, imx_clk_mux("arm_pll_ref_sel", base + 0x84, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
>         clk_dm(IMX8MP_SYS_PLL1_REF_SEL, imx_clk_mux("sys_pll1_ref_sel", base + 0x94, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
>         clk_dm(IMX8MP_SYS_PLL2_REF_SEL, imx_clk_mux("sys_pll2_ref_sel", base + 0x104, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
>         clk_dm(IMX8MP_SYS_PLL3_REF_SEL, imx_clk_mux("sys_pll3_ref_sel", base + 0x114, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
>
> +       clk_dm(IMX8MP_VIDEO_PLL1, imx_clk_pll14xx("video_pll1", "video_pll1_ref_sel", base + 0x28,
> +                                                 &imx_1443x_pll));
>         clk_dm(IMX8MP_DRAM_PLL, imx_clk_pll14xx("dram_pll", "dram_pll_ref_sel", base + 0x50,
>                                                 &imx_1443x_dram_pll));
>         clk_dm(IMX8MP_ARM_PLL, imx_clk_pll14xx("arm_pll", "arm_pll_ref_sel", base + 0x84,
> @@ -213,12 +236,14 @@ static int imx8mp_clk_probe(struct udevice *dev)
>         clk_dm(IMX8MP_SYS_PLL3, imx_clk_pll14xx("sys_pll3", "sys_pll3_ref_sel", base + 0x114,
>                                                 &imx_1416x_pll));
>
> +       clk_dm(IMX8MP_VIDEO_PLL1_BYPASS, imx_clk_mux_flags("video_pll1_bypass", base + 0x28, 16, 1, video_pll1_bypass_sels, ARRAY_SIZE(video_pll1_bypass_sels), CLK_SET_RATE_PARENT));
>         clk_dm(IMX8MP_DRAM_PLL_BYPASS, imx_clk_mux_flags("dram_pll_bypass", base + 0x50, 4, 1, dram_pll_bypass_sels, ARRAY_SIZE(dram_pll_bypass_sels), CLK_SET_RATE_PARENT));
>         clk_dm(IMX8MP_ARM_PLL_BYPASS, imx_clk_mux_flags("arm_pll_bypass", base + 0x84, 4, 1, arm_pll_bypass_sels, ARRAY_SIZE(arm_pll_bypass_sels), CLK_SET_RATE_PARENT));
>         clk_dm(IMX8MP_SYS_PLL1_BYPASS, imx_clk_mux_flags("sys_pll1_bypass", base + 0x94, 4, 1, sys_pll1_bypass_sels, ARRAY_SIZE(sys_pll1_bypass_sels), CLK_SET_RATE_PARENT));
>         clk_dm(IMX8MP_SYS_PLL2_BYPASS, imx_clk_mux_flags("sys_pll2_bypass", base + 0x104, 4, 1, sys_pll2_bypass_sels, ARRAY_SIZE(sys_pll2_bypass_sels), CLK_SET_RATE_PARENT));
>         clk_dm(IMX8MP_SYS_PLL3_BYPASS, imx_clk_mux_flags("sys_pll3_bypass", base + 0x114, 4, 1, sys_pll3_bypass_sels, ARRAY_SIZE(sys_pll3_bypass_sels), CLK_SET_RATE_PARENT));
>
> +       clk_dm(IMX8MP_VIDEO_PLL1_OUT, imx_clk_gate("video_pll1_out", "video_pll1_bypass", base + 0x28, 13));
>         clk_dm(IMX8MP_DRAM_PLL_OUT, imx_clk_gate("dram_pll_out", "dram_pll_bypass", base + 0x50, 13));
>         clk_dm(IMX8MP_ARM_PLL_OUT, imx_clk_gate("arm_pll_out", "arm_pll_bypass", base + 0x84, 11));
>         clk_dm(IMX8MP_SYS_PLL1_OUT, imx_clk_gate("sys_pll1_out", "sys_pll1_bypass", base + 0x94, 11));
> @@ -267,10 +292,13 @@ static int imx8mp_clk_probe(struct udevice *dev)
>         clk_dm(IMX8MP_CLK_MAIN_AXI, imx8m_clk_composite_critical("main_axi", imx8mp_main_axi_sels, base + 0x8800));
>         clk_dm(IMX8MP_CLK_ENET_AXI, imx8m_clk_composite_critical("enet_axi", imx8mp_enet_axi_sels, base + 0x8880));
>         clk_dm(IMX8MP_CLK_NAND_USDHC_BUS, imx8m_clk_composite_critical("nand_usdhc_bus", imx8mp_nand_usdhc_sels, base + 0x8900));
> +       clk_dm(IMX8MP_CLK_MEDIA_AXI, imx8m_clk_composite("media_axi", imx8mp_media_axi_sels, base + 0x8a00));
> +       clk_dm(IMX8MP_CLK_MEDIA_APB, imx8m_clk_composite("media_apb", imx8mp_media_apb_sels, base + 0x8a80));
>         clk_dm(IMX8MP_CLK_NOC, imx8m_clk_composite_critical("noc", imx8mp_noc_sels, base + 0x8d00));
>         clk_dm(IMX8MP_CLK_NOC_IO, imx8m_clk_composite_critical("noc_io", imx8mp_noc_io_sels, base + 0x8d80));
>
>         clk_dm(IMX8MP_CLK_AHB, imx8m_clk_composite_critical("ahb_root", imx8mp_ahb_sels, base + 0x9000));
> +       clk_dm(IMX8MP_CLK_MEDIA_DISP2_PIX, imx8m_clk_composite("media_disp2_pix", imx8mp_media_disp_pix_sels, base + 0x9300));
>
>         clk_dm(IMX8MP_CLK_IPG_ROOT, imx_clk_divider2("ipg_root", "ahb_root", base + 0x9080, 0, 1));
>
> @@ -309,6 +337,8 @@ static int imx8mp_clk_probe(struct udevice *dev)
>
>         clk_dm(IMX8MP_CLK_WDOG, imx8m_clk_composite("wdog", imx8mp_wdog_sels, base + 0xb900));
>         clk_dm(IMX8MP_CLK_USDHC3, imx8m_clk_composite("usdhc3", imx8mp_usdhc3_sels, base + 0xbc80));
> +       clk_dm(IMX8MP_CLK_MEDIA_DISP1_PIX, imx8m_clk_composite("media_disp1_pix", imx8mp_media_disp_pix_sels, base + 0xbe00));
> +       clk_dm(IMX8MP_CLK_MEDIA_LDB, imx8m_clk_composite("media_ldb", imx8mp_media_ldb_sels, base + 0xbf00));
>
>         clk_dm(IMX8MP_CLK_DRAM_ALT_ROOT, imx_clk_fixed_factor("dram_alt_root", "dram_alt", 1, 4));
>         clk_dm(IMX8MP_CLK_DRAM_CORE, imx_clk_mux2_flags("dram_core_clk", base + 0x9800, 24, 1, imx8mp_dram_core_sels, ARRAY_SIZE(imx8mp_dram_core_sels), CLK_IS_CRITICAL));
> @@ -352,6 +382,11 @@ static int imx8mp_clk_probe(struct udevice *dev)
>         clk_dm(IMX8MP_CLK_WDOG2_ROOT, imx_clk_gate4("wdog2_root_clk", "wdog", base + 0x4540, 0));
>         clk_dm(IMX8MP_CLK_WDOG3_ROOT, imx_clk_gate4("wdog3_root_clk", "wdog", base + 0x4550, 0));
>         clk_dm(IMX8MP_CLK_HSIO_ROOT, imx_clk_gate4("hsio_root_clk", "ipg_root", base + 0x45c0, 0));
> +       clk_dm(IMX8MP_CLK_MEDIA_APB_ROOT, imx_clk_gate2_shared2("media_apb_root_clk", "media_apb", base + 0x45d0, 0, &share_count_media));
> +       clk_dm(IMX8MP_CLK_MEDIA_AXI_ROOT, imx_clk_gate2_shared2("media_axi_root_clk", "media_axi", base + 0x45d0, 0, &share_count_media));
> +       clk_dm(IMX8MP_CLK_MEDIA_DISP1_PIX_ROOT, imx_clk_gate2_shared2("media_disp1_pix_root_clk", "media_disp1_pix", base + 0x45d0, 0, &share_count_media));
> +       clk_dm(IMX8MP_CLK_MEDIA_DISP2_PIX_ROOT, imx_clk_gate2_shared2("media_disp2_pix_root_clk", "media_disp2_pix", base + 0x45d0, 0, &share_count_media));
> +       clk_dm(IMX8MP_CLK_MEDIA_LDB_ROOT, imx_clk_gate2_shared2("media_ldb_root_clk", "media_ldb", base + 0x45d0, 0, &share_count_media));
>
>         clk_dm(IMX8MP_CLK_USDHC3_ROOT, imx_clk_gate4("usdhc3_root_clk", "usdhc3", base + 0x45e0, 0));
>
> --
> 2.43.0
>

Re: [PATCH 5/8] imx: power-domain: Add support for the MEDIAMIX control block

[Miquel Raynal]

Hi Fabio,

First, sorry for the delay and thanks a lot for the review.

On 12/09/2024 at 09:40:32 -03, Fabio Estevam <festevam@gmail.com> wrote:

> On Tue, Sep 10, 2024 at 7:21 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
>> +       /* Make sure bus domain is awake */
>> +       ret = power_domain_on(&priv->pd_bus);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* Put devices into reset */
>> +       clrbits_le32(priv->base + BLK_SFT_RSTN, reset);
>> +
>> +       /* Enable upstream clocks */
>> +       ret = clk_enable(&priv->clk_apb);
>> +       if (ret)
>> +               goto dis_bus_pd;
>> +
>> +       ret = clk_enable(&priv->clk_axi);
>> +       if (ret)
>> +               goto dis_bus_pd;
>> +
>> +       /* Enable blk-ctrl clock to allow reset to propagate */
>> +       ret = clk_enable(clk);
>> +       if (ret)
>> +               goto dis_bus_pd;
>> +       setbits_le32(priv->base + BLK_CLK_EN, reset);
>> +
>> +       /* Power up upstream GPC domain */
>> +       ret = power_domain_on(domain);
>> +       if (ret)
>> +               goto dis_bus_pd;
>
> The previously enabled clocks should be disabled on the error paths.

That's right, I will.

> Could you use clk_enable_bulk()?

This not very convenient and not future-proof, because, while we just
want to grab and enable the apb and axi clocks, the media_disp1_pix and
media_disp2_pix will be used depending on the configuration. For not
I've not included support for media_disp1_pix because I do not use it
and cannot test it, but it is a legitimate addition that someone will
soon or later want to bring. Hence I believe using the _bulk() helper is
not really appropriate in this case.

Thanks!
Miquèl