[PATCH v5 0/4] drm/bridge: microchip-lvds: clean up and fix bus formats

[PATCH v5 0/4] drm/bridge: microchip-lvds: clean up and fix bus formats

[Dharma Balasubiramani]

This patch series drops the unsed panel field, switches to atomic variants
and adds support to select between the two supported formats (JEIDA and
VESA) by the LVDSC.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
Changes in v5:
- Drop the redundant port node lookup.
- Split the commits adding atomic bridge ops into 2.
- Update commit messages accordingly.
- Link to v4: https://lore.kernel.org/r/20250624-microchip-lvds-v4-0-937d42a420e9@microchip.com

Changes in v4:
- Split the commits into 3.
- Drop <drm/drm_panel.h>
- Link to v3: https://lore.kernel.org/r/20250624-microchip-lvds-v3-1-c3c6f1e40516@microchip.com

Changes in v3:
- Use BIT(0) instead of 1.
- Drop the panel field of the mchp_lvds structure.
- Drop the inner parentheses in write in serialiser_on().
- Link to v2: https://lore.kernel.org/r/20250623-microchip-lvds-v2-1-8ecbabc6abc4@microchip.com

Changes in v2:
- Switch to atomic bridge functions
- Drop custom connector creation
- Use drm_atomic_get_new_connector_for_encoder()
- Link to v1: https://lore.kernel.org/r/20250618-microchip-lvds-v1-1-1eae5acd7a82@microchip.com

---
Dharma Balasubiramani (4):
      drm/bridge: microchip-lvds: Remove unused drm_panel and redundant port node lookup
      drm/bridge: microchip-lvds: migrate to atomic bridge ops
      drm/bridge: microchip-lvds: add atomic pre_enable() and post_disable()
      drm/bridge: microchip-lvds: fix bus format mismatch with VESA displays

 drivers/gpu/drm/bridge/microchip-lvds.c | 80 ++++++++++++++++++++++-----------
 1 file changed, 54 insertions(+), 26 deletions(-)
---
base-commit: 4325743c7e209ae7845293679a4de94b969f2bef
change-id: 20250618-microchip-lvds-b7151d96094a

Best regards,
-- 
Dharma Balasubiramani <dharma.b@microchip.com>

[PATCH v5 1/4] drm/bridge: microchip-lvds: Remove unused drm_panel and redundant port node lookup

[Dharma Balasubiramani]

Drop the unused drm_panel field from the mchp_lvds structure, and remove
the unnecessary port device node lookup, as devm_drm_of_get_bridge()
already performs the required checks internally.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
 drivers/gpu/drm/bridge/microchip-lvds.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
index 9f4ff82bc6b4..06d4169a2d8f 100644
--- a/drivers/gpu/drm/bridge/microchip-lvds.c
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -23,7 +23,6 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_of.h>
-#include <drm/drm_panel.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_simple_kms_helper.h>
@@ -56,7 +55,6 @@ struct mchp_lvds {
 	struct device *dev;
 	void __iomem *regs;
 	struct clk *pclk;
-	struct drm_panel *panel;
 	struct drm_bridge bridge;
 	struct drm_bridge *panel_bridge;
 };
@@ -151,7 +149,6 @@ static int mchp_lvds_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct mchp_lvds *lvds;
-	struct device_node *port;
 	int ret;
 
 	if (!dev->of_node)
@@ -173,19 +170,6 @@ static int mchp_lvds_probe(struct platform_device *pdev)
 		return dev_err_probe(lvds->dev, PTR_ERR(lvds->pclk),
 				"could not get pclk_lvds\n");
 
-	port = of_graph_get_remote_node(dev->of_node, 1, 0);
-	if (!port) {
-		dev_err(dev,
-			"can't find port point, please init lvds panel port!\n");
-		return -ENODEV;
-	}
-
-	lvds->panel = of_drm_find_panel(port);
-	of_node_put(port);
-
-	if (IS_ERR(lvds->panel))
-		return -EPROBE_DEFER;
-
 	lvds->panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
 
 	if (IS_ERR(lvds->panel_bridge))

-- 
2.43.0

Re: [PATCH v5 1/4] drm/bridge: microchip-lvds: Remove unused drm_panel and redundant port node lookup

[Maxime Ripard]

On Wed, 25 Jun 2025 10:26:09 +0530, Dharma Balasubiramani wrote:
> Drop the unused drm_panel field from the mchp_lvds structure, and remove
> the unnecessary port device node lookup, as devm_drm_of_get_bridge()
> already performs the required checks internally.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

[PATCH v5 2/4] drm/bridge: microchip-lvds: migrate to atomic bridge ops

[Dharma Balasubiramani]

Replace legacy .enable and .disable callbacks with their atomic
counterparts .atomic_enable and .atomic_disable.

Also, add turn off the serialiser inside atomic_disable().

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
 drivers/gpu/drm/bridge/microchip-lvds.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
index 06d4169a2d8f..c40c8717f026 100644
--- a/drivers/gpu/drm/bridge/microchip-lvds.c
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -111,7 +111,8 @@ static int mchp_lvds_attach(struct drm_bridge *bridge,
 				 bridge, flags);
 }
 
-static void mchp_lvds_enable(struct drm_bridge *bridge)
+static void mchp_lvds_atomic_enable(struct drm_bridge *bridge,
+				    struct drm_atomic_state *state)
 {
 	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
 	int ret;
@@ -131,18 +132,22 @@ static void mchp_lvds_enable(struct drm_bridge *bridge)
 	lvds_serialiser_on(lvds);
 }
 
-static void mchp_lvds_disable(struct drm_bridge *bridge)
+static void mchp_lvds_atomic_disable(struct drm_bridge *bridge,
+				     struct drm_atomic_state *state)
 {
 	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
 
+	/* Turn off the serialiser */
+	lvds_writel(lvds, LVDSC_CR, 0);
+
 	pm_runtime_put(lvds->dev);
 	clk_disable_unprepare(lvds->pclk);
 }
 
 static const struct drm_bridge_funcs mchp_lvds_bridge_funcs = {
 	.attach = mchp_lvds_attach,
-	.enable = mchp_lvds_enable,
-	.disable = mchp_lvds_disable,
+	.atomic_enable = mchp_lvds_atomic_enable,
+	.atomic_disable = mchp_lvds_atomic_disable,
 };
 
 static int mchp_lvds_probe(struct platform_device *pdev)

-- 
2.43.0

Re: [PATCH v5 2/4] drm/bridge: microchip-lvds: migrate to atomic bridge ops

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 690 bytes --]

On Wed, Jun 25, 2025 at 10:26:10AM +0530, Dharma Balasubiramani wrote:
> Replace legacy .enable and .disable callbacks with their atomic
> counterparts .atomic_enable and .atomic_disable.
> 
> Also, add turn off the serialiser inside atomic_disable().
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>

As a rule of thumb, if you have "Also, do X" in your commit log, you
need a separate patch.

And you need to explain why turning off the serialiser inside
atomic_disable() is needed. It might make sense to you, it's not really
obvious to me from that patch, and it will definitely not be to someone
trying to identify fixes and doing backports.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v5 2/4] drm/bridge: microchip-lvds: migrate to atomic bridge ops

[Dharma.B]

On 25/06/25 12:24 pm, Maxime Ripard wrote:
> On Wed, Jun 25, 2025 at 10:26:10AM +0530, Dharma Balasubiramani wrote:
>> Replace legacy .enable and .disable callbacks with their atomic
>> counterparts .atomic_enable and .atomic_disable.
>>
>> Also, add turn off the serialiser inside atomic_disable().
>>
>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> 
> As a rule of thumb, if you have "Also, do X" in your commit log, you
> need a separate patch.
> 
> And you need to explain why turning off the serialiser inside
> atomic_disable() is needed. It might make sense to you, it's not really
> obvious to me from that patch, and it will definitely not be to someone
> trying to identify fixes and doing backports.

I initially introduced the turning off the serialiser to avoid having an 
empty disable() hook. Now that you've clarified it's perfectly fine to 
sleep in these contexts, I no longer see the need for the split. I'll 
drop both atomic_pre_enable(), atomic_post_disable() and turning off the 
serialiser as well and just keep just atomic_enable() and atomic_disable().

Thanks.
> 
> Maxime


-- 
With Best Regards,
Dharma B.

[PATCH v5 3/4] drm/bridge: microchip-lvds: add atomic pre_enable() and post_disable()

[Dharma Balasubiramani]

pm_runtime_get_sync() and clk_prepare_enable() must be outside the atomic
context, hence move the sleepable operations accordingly.

- atomic_pre_enable() handles pm_runtime and clock preparation
- atomic_enable() enables the serializer based on panel format
- atomic_disable() turns off the serializer
- atomic_post_disable() disables clock and releases runtime PM

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
 drivers/gpu/drm/bridge/microchip-lvds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
index c40c8717f026..b1800f78008c 100644
--- a/drivers/gpu/drm/bridge/microchip-lvds.c
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -111,8 +111,8 @@ static int mchp_lvds_attach(struct drm_bridge *bridge,
 				 bridge, flags);
 }
 
-static void mchp_lvds_atomic_enable(struct drm_bridge *bridge,
-				    struct drm_atomic_state *state)
+static void mchp_lvds_atomic_pre_enable(struct drm_bridge *bridge,
+					struct drm_atomic_state *state)
 {
 	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
 	int ret;
@@ -128,7 +128,12 @@ static void mchp_lvds_atomic_enable(struct drm_bridge *bridge,
 		dev_err(lvds->dev, "failed to get pm runtime: %d\n", ret);
 		return;
 	}
+}
 
+static void mchp_lvds_atomic_enable(struct drm_bridge *bridge,
+				    struct drm_atomic_state *state)
+{
+	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
 	lvds_serialiser_on(lvds);
 }
 
@@ -139,6 +144,12 @@ static void mchp_lvds_atomic_disable(struct drm_bridge *bridge,
 
 	/* Turn off the serialiser */
 	lvds_writel(lvds, LVDSC_CR, 0);
+}
+
+static void mchp_lvds_atomic_post_disable(struct drm_bridge *bridge,
+					  struct drm_atomic_state *state)
+{
+	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
 
 	pm_runtime_put(lvds->dev);
 	clk_disable_unprepare(lvds->pclk);
@@ -146,8 +157,10 @@ static void mchp_lvds_atomic_disable(struct drm_bridge *bridge,
 
 static const struct drm_bridge_funcs mchp_lvds_bridge_funcs = {
 	.attach = mchp_lvds_attach,
+	.atomic_pre_enable = mchp_lvds_atomic_pre_enable,
 	.atomic_enable = mchp_lvds_atomic_enable,
 	.atomic_disable = mchp_lvds_atomic_disable,
+	.atomic_post_disable = mchp_lvds_atomic_post_disable,
 };
 
 static int mchp_lvds_probe(struct platform_device *pdev)

-- 
2.43.0

Re: [PATCH v5 3/4] drm/bridge: microchip-lvds: add atomic pre_enable() and post_disable()

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

On Wed, Jun 25, 2025 at 10:26:11AM +0530, Dharma Balasubiramani wrote:
> pm_runtime_get_sync() and clk_prepare_enable() must be outside the atomic
> context, hence move the sleepable operations accordingly.
> 
> - atomic_pre_enable() handles pm_runtime and clock preparation
> - atomic_enable() enables the serializer based on panel format
> - atomic_disable() turns off the serializer
> - atomic_post_disable() disables clock and releases runtime PM
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>

If that's the reason you wanted to split the enable and disable hooks in
two, you don't need to. You can sleep in any context here.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

[PATCH v5 4/4] drm/bridge: microchip-lvds: fix bus format mismatch with VESA displays

[Dharma Balasubiramani]

The LVDS controller was hardcoded to JEIDA mapping, which leads to
distorted output on panels expecting VESA mapping.

Update the driver to dynamically select the appropriate mapping and
pixel size based on the panel's advertised media bus format. This
ensures compatibility with both JEIDA and VESA displays.

Signed-off-by: Sandeep Sheriker M <sandeep.sheriker@microchip.com>
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
 drivers/gpu/drm/bridge/microchip-lvds.c | 38 +++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
index b1800f78008c..29d18ec00883 100644
--- a/drivers/gpu/drm/bridge/microchip-lvds.c
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -11,6 +11,7 @@
 #include <linux/component.h>
 #include <linux/delay.h>
 #include <linux/jiffies.h>
+#include <linux/media-bus-format.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_graph.h>
 #include <linux/pinctrl/devinfo.h>
@@ -40,9 +41,11 @@
 
 /* Bitfields in LVDSC_CFGR (Configuration Register) */
 #define LVDSC_CFGR_PIXSIZE_24BITS	0
+#define LVDSC_CFGR_PIXSIZE_18BITS	BIT(0)
 #define LVDSC_CFGR_DEN_POL_HIGH		0
 #define LVDSC_CFGR_DC_UNBALANCED	0
 #define LVDSC_CFGR_MAPPING_JEIDA	BIT(6)
+#define LVDSC_CFGR_MAPPING_VESA		0
 
 /*Bitfields in LVDSC_SR */
 #define LVDSC_SR_CS	BIT(0)
@@ -74,9 +77,10 @@ static inline void lvds_writel(struct mchp_lvds *lvds, u32 offset, u32 val)
 	writel_relaxed(val, lvds->regs + offset);
 }
 
-static void lvds_serialiser_on(struct mchp_lvds *lvds)
+static void lvds_serialiser_on(struct mchp_lvds *lvds, u32 bus_format)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(LVDS_POLL_TIMEOUT_MS);
+	u8 map, pix_size;
 
 	/* The LVDSC registers can only be written if WPEN is cleared */
 	lvds_writel(lvds, LVDSC_WPMR, (LVDSC_WPMR_WPKEY_PSSWD &
@@ -91,11 +95,24 @@ static void lvds_serialiser_on(struct mchp_lvds *lvds)
 		usleep_range(1000, 2000);
 	}
 
+	switch (bus_format) {
+	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+		map = LVDSC_CFGR_MAPPING_JEIDA;
+		pix_size = LVDSC_CFGR_PIXSIZE_18BITS;
+		break;
+	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+		map = LVDSC_CFGR_MAPPING_VESA;
+		pix_size = LVDSC_CFGR_PIXSIZE_24BITS;
+		break;
+	default:
+		map = LVDSC_CFGR_MAPPING_JEIDA;
+		pix_size = LVDSC_CFGR_PIXSIZE_24BITS;
+		break;
+	}
+
 	/* Configure the LVDSC */
-	lvds_writel(lvds, LVDSC_CFGR, (LVDSC_CFGR_MAPPING_JEIDA |
-				LVDSC_CFGR_DC_UNBALANCED |
-				LVDSC_CFGR_DEN_POL_HIGH |
-				LVDSC_CFGR_PIXSIZE_24BITS));
+	lvds_writel(lvds, LVDSC_CFGR, map | LVDSC_CFGR_DC_UNBALANCED |
+		    LVDSC_CFGR_DEN_POL_HIGH | pix_size);
 
 	/* Enable the LVDS serializer */
 	lvds_writel(lvds, LVDSC_CR, LVDSC_CR_SER_EN);
@@ -134,7 +151,16 @@ static void mchp_lvds_atomic_enable(struct drm_bridge *bridge,
 				    struct drm_atomic_state *state)
 {
 	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
-	lvds_serialiser_on(lvds);
+	struct drm_connector *connector;
+
+	/* default to jeida-24 */
+	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
+
+	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
+	if (connector && connector->display_info.num_bus_formats)
+		bus_format = connector->display_info.bus_formats[0];
+
+	lvds_serialiser_on(lvds, bus_format);
 }
 
 static void mchp_lvds_atomic_disable(struct drm_bridge *bridge,

-- 
2.43.0

Re: [PATCH v5 4/4] drm/bridge: microchip-lvds: fix bus format mismatch with VESA displays

[Maxime Ripard]

On Wed, 25 Jun 2025 10:26:12 +0530, Dharma Balasubiramani wrote:
> The LVDS controller was hardcoded to JEIDA mapping, which leads to
> distorted output on panels expecting VESA mapping.
> 
> Update the driver to dynamically select the appropriate mapping and
> pixel size based on the panel's advertised media bus format. This
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime