diff mbox series

[v6,5/6] drm: lcdif: Add multiple encoders and first bridges support

Message ID 20230510092450.4024730-6-victor.liu@nxp.com
State New
Headers
Series drm: lcdif: Add i.MX93 LCDIF support |

Commit Message

Liu Ying May 10, 2023, 9:24 a.m. UTC 
  The single LCDIF embedded in i.MX93 SoC may drive multiple displays
simultaneously.  Look at LCDIF output port's remote port parents to
find all enabled first bridges.  Add an encoder for each found bridge
and attach the bridge to the encoder.  This is a preparation for
adding i.MX93 LCDIF support.

Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v5->v6:
* Drop MAX_DISPLAYS macro. (Marek)
* Drop the encoder member in struct lcdif_drm_private.
* Drop endpoint id check.
* Allocate encoders by calling devm_kzalloc().

v4->v5:
* Rebase upon v6.4-rc1 and resolve a trivial conflict.
* Add Alexander's A-b and T-b tags.

v3->v4:
* Improve warning message when ignoring invalid LCDIF OF endpoint ids.
  (Alexander)

v2->v3:
* No change.

v1->v2:
* Split from patch 2/2 in v1. (Marek, Alexander)
* Drop '!remote ||' from lcdif_attach_bridge(). (Lothar)
* Drop unneeded 'bridges' member from lcdif_drm_private structure.

 drivers/gpu/drm/mxsfb/lcdif_drv.c | 70 +++++++++++++++++++++++++++----
 drivers/gpu/drm/mxsfb/lcdif_drv.h |  2 -
 drivers/gpu/drm/mxsfb/lcdif_kms.c | 21 ++--------
 3 files changed, 65 insertions(+), 28 deletions(-)

Comments

Marek Vasut May 10, 2023, 4:24 p.m. UTC | #1 
On 5/10/23 11:24, Liu Ying wrote:
> The single LCDIF embedded in i.MX93 SoC may drive multiple displays
> simultaneously.  Look at LCDIF output port's remote port parents to
> find all enabled first bridges.  Add an encoder for each found bridge
> and attach the bridge to the encoder.  This is a preparation for
> adding i.MX93 LCDIF support.
> 
> Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> v5->v6:
> * Drop MAX_DISPLAYS macro. (Marek)
> * Drop the encoder member in struct lcdif_drm_private.
> * Drop endpoint id check.

It might be nice to check (based on driver data for each IP variant) the 
encoder count, but that can be a separate patch.

Reviewed-by: Marek Vasut <marex@denx.de>

Thanks !

btw if this doesn't get picked by someone in like a week or two, let me 
know and I'll apply this via drm-misc .
Ying Liu May 25, 2023, 1:45 a.m. UTC | #2 
Hi Marek,

On Thu, May 11, 2023 at 12:24 AM Marek Vasut <marex@denx.de> wrote:
> On 5/10/23 11:24, Liu Ying wrote:
> > The single LCDIF embedded in i.MX93 SoC may drive multiple displays
> > simultaneously.  Look at LCDIF output port's remote port parents to
> > find all enabled first bridges.  Add an encoder for each found bridge
> > and attach the bridge to the encoder.  This is a preparation for
> > adding i.MX93 LCDIF support.
> >
> > Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > ---
> > v5->v6:
> > * Drop MAX_DISPLAYS macro. (Marek)
> > * Drop the encoder member in struct lcdif_drm_private.
> > * Drop endpoint id check.
>
> It might be nice to check (based on driver data for each IP variant) the
> encoder count, but that can be a separate patch.
>
> Reviewed-by: Marek Vasut <marex@denx.de>
>
> Thanks !
>
> btw if this doesn't get picked by someone in like a week or two, let me
> know and I'll apply this via drm-misc .

Thanks for your review.  I don't see any more comments in two weeks.
Can you please go ahead to apply this series?

Regards,
Liu Ying
Marek Vasut May 26, 2023, 5:09 p.m. UTC | #3 
On 5/25/23 03:45, Ying Liu wrote:
> Hi Marek,

Hi,

> On Thu, May 11, 2023 at 12:24 AM Marek Vasut <marex@denx.de> wrote:
>> On 5/10/23 11:24, Liu Ying wrote:
>>> The single LCDIF embedded in i.MX93 SoC may drive multiple displays
>>> simultaneously.  Look at LCDIF output port's remote port parents to
>>> find all enabled first bridges.  Add an encoder for each found bridge
>>> and attach the bridge to the encoder.  This is a preparation for
>>> adding i.MX93 LCDIF support.
>>>
>>> Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>>> Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>>> ---
>>> v5->v6:
>>> * Drop MAX_DISPLAYS macro. (Marek)
>>> * Drop the encoder member in struct lcdif_drm_private.
>>> * Drop endpoint id check.
>>
>> It might be nice to check (based on driver data for each IP variant) the
>> encoder count, but that can be a separate patch.
>>
>> Reviewed-by: Marek Vasut <marex@denx.de>
>>
>> Thanks !
>>
>> btw if this doesn't get picked by someone in like a week or two, let me
>> know and I'll apply this via drm-misc .
> 
> Thanks for your review.  I don't see any more comments in two weeks.
> Can you please go ahead to apply this series?

Done, applied to drm-misc-next , thanks !

Thanks for the reminder too .
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index e816f87828fb..dc85498b81cd 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -9,13 +9,16 @@ 
 #include <linux/dma-mapping.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_encoder.h>
 #include <drm/drm_fbdev_dma.h>
 #include <drm/drm_gem_dma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -38,19 +41,70 @@  static const struct drm_mode_config_helper_funcs lcdif_mode_config_helpers = {
 	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
 };
 
+static const struct drm_encoder_funcs lcdif_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
 static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 {
-	struct drm_device *drm = lcdif->drm;
+	struct device *dev = lcdif->drm->dev;
+	struct device_node *ep;
 	struct drm_bridge *bridge;
 	int ret;
 
-	bridge = devm_drm_of_get_bridge(drm->dev, drm->dev->of_node, 0, 0);
-	if (IS_ERR(bridge))
-		return PTR_ERR(bridge);
-
-	ret = drm_bridge_attach(&lcdif->encoder, bridge, NULL, 0);
-	if (ret)
-		return dev_err_probe(drm->dev, ret, "Failed to attach bridge\n");
+	for_each_endpoint_of_node(dev->of_node, ep) {
+		struct device_node *remote;
+		struct of_endpoint of_ep;
+		struct drm_encoder *encoder;
+
+		remote = of_graph_get_remote_port_parent(ep);
+		if (!of_device_is_available(remote)) {
+			of_node_put(remote);
+			continue;
+		}
+		of_node_put(remote);
+
+		ret = of_graph_parse_endpoint(ep, &of_ep);
+		if (ret < 0) {
+			dev_err(dev, "Failed to parse endpoint %pOF\n", ep);
+			of_node_put(ep);
+			return ret;
+		}
+
+		bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, of_ep.id);
+		if (IS_ERR(bridge)) {
+			of_node_put(ep);
+			return dev_err_probe(dev, PTR_ERR(bridge),
+					     "Failed to get bridge for endpoint%u\n",
+					     of_ep.id);
+		}
+
+		encoder = devm_kzalloc(dev, sizeof(*encoder), GFP_KERNEL);
+		if (!encoder) {
+			dev_err(dev, "Failed to allocate encoder for endpoint%u\n",
+				of_ep.id);
+			of_node_put(ep);
+			return -ENOMEM;
+		}
+
+		encoder->possible_crtcs = drm_crtc_mask(&lcdif->crtc);
+		ret = drm_encoder_init(lcdif->drm, encoder, &lcdif_encoder_funcs,
+				       DRM_MODE_ENCODER_NONE, NULL);
+		if (ret) {
+			dev_err(dev, "Failed to initialize encoder for endpoint%u: %d\n",
+				of_ep.id, ret);
+			of_node_put(ep);
+			return ret;
+		}
+
+		ret = drm_bridge_attach(encoder, bridge, NULL, 0);
+		if (ret) {
+			of_node_put(ep);
+			return dev_err_probe(dev, ret,
+					     "Failed to attach bridge for endpoint%u\n",
+					     of_ep.id);
+		}
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.h b/drivers/gpu/drm/mxsfb/lcdif_drv.h
index aa6d099a1897..61483a4e058d 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.h
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.h
@@ -11,7 +11,6 @@ 
 #include <drm/drm_bridge.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_device.h>
-#include <drm/drm_encoder.h>
 #include <drm/drm_plane.h>
 
 struct clk;
@@ -30,7 +29,6 @@  struct lcdif_drm_private {
 		/* i.MXRT does support overlay planes, add them here. */
 	} planes;
 	struct drm_crtc			crtc;
-	struct drm_encoder		encoder;
 };
 
 static inline struct lcdif_drm_private *
diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index e13382beb53e..2541d2de4e45 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -659,14 +659,6 @@  static const struct drm_crtc_funcs lcdif_crtc_funcs = {
 	.disable_vblank = lcdif_crtc_disable_vblank,
 };
 
-/* -----------------------------------------------------------------------------
- * Encoder
- */
-
-static const struct drm_encoder_funcs lcdif_encoder_funcs = {
-	.destroy = drm_encoder_cleanup,
-};
-
 /* -----------------------------------------------------------------------------
  * Planes
  */
@@ -759,7 +751,6 @@  int lcdif_kms_init(struct lcdif_drm_private *lcdif)
 					BIT(DRM_COLOR_YCBCR_BT2020);
 	const u32 supported_ranges = BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
 				     BIT(DRM_COLOR_YCBCR_FULL_RANGE);
-	struct drm_encoder *encoder = &lcdif->encoder;
 	struct drm_crtc *crtc = &lcdif->crtc;
 	int ret;
 
@@ -783,13 +774,7 @@  int lcdif_kms_init(struct lcdif_drm_private *lcdif)
 		return ret;
 
 	drm_crtc_helper_add(crtc, &lcdif_crtc_helper_funcs);
-	ret = drm_crtc_init_with_planes(lcdif->drm, crtc,
-					&lcdif->planes.primary, NULL,
-					&lcdif_crtc_funcs, NULL);
-	if (ret)
-		return ret;
-
-	encoder->possible_crtcs = drm_crtc_mask(crtc);
-	return drm_encoder_init(lcdif->drm, encoder, &lcdif_encoder_funcs,
-				DRM_MODE_ENCODER_NONE, NULL);
+	return drm_crtc_init_with_planes(lcdif->drm, crtc,
+					 &lcdif->planes.primary, NULL,
+					 &lcdif_crtc_funcs, NULL);
 }