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

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

Posted by Dharma Balasubiramani 5 months, 3 weeks ago

From: Sandeep Sheriker M <sandeep.sheriker@microchip.com>

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.

Modernize the bridge ops to use atomic_enable/disable, and retrieve
the bus format from the connector via the atomic bridge state.

Signed-off-by: Sandeep Sheriker M <sandeep.sheriker@microchip.com>
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
Note: Tested the changes on newvision 10.1 VESA display.

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
---
 drivers/gpu/drm/bridge/microchip-lvds.c | 64 +++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
index 9f4ff82bc6b4..b71478aa36e9 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>
@@ -41,9 +42,11 @@
 
 /* Bitfields in LVDSC_CFGR (Configuration Register) */
 #define LVDSC_CFGR_PIXSIZE_24BITS	0
+#define LVDSC_CFGR_PIXSIZE_18BITS	1
 #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)
@@ -76,9 +79,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 &
@@ -93,11 +97,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);
@@ -113,7 +130,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_pre_enable(struct drm_bridge *bridge,
+					struct drm_atomic_state *state)
 {
 	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
 	int ret;
@@ -129,11 +147,35 @@ static void mchp_lvds_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);
+	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,
+				     struct drm_atomic_state *state)
+{
+	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
 
-	lvds_serialiser_on(lvds);
+	/* Turn off the serialiser */
+	lvds_writel(lvds, LVDSC_CR, 0);
 }
 
-static void mchp_lvds_disable(struct drm_bridge *bridge)
+static void mchp_lvds_atomic_post_disable(struct drm_bridge *bridge,
+					  struct drm_atomic_state *state)
 {
 	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
 
@@ -143,8 +185,10 @@ static void mchp_lvds_disable(struct drm_bridge *bridge)
 
 static const struct drm_bridge_funcs mchp_lvds_bridge_funcs = {
 	.attach = mchp_lvds_attach,
-	.enable = mchp_lvds_enable,
-	.disable = mchp_lvds_disable,
+	.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)

---
base-commit: 4325743c7e209ae7845293679a4de94b969f2bef
change-id: 20250618-microchip-lvds-b7151d96094a

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

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

Posted by Laurent Pinchart 5 months, 3 weeks ago

Hi Dharma,

Thank you for the patch.

On Mon, Jun 23, 2025 at 04:20:20PM +0530, Dharma Balasubiramani wrote:
> From: Sandeep Sheriker M <sandeep.sheriker@microchip.com>
> 
> 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.
> 
> Modernize the bridge ops to use atomic_enable/disable, and retrieve
> the bus format from the connector via the atomic bridge state.
> 
> Signed-off-by: Sandeep Sheriker M <sandeep.sheriker@microchip.com>
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> ---
> Note: Tested the changes on newvision 10.1 VESA display.
> 
> 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
> ---
>  drivers/gpu/drm/bridge/microchip-lvds.c | 64 +++++++++++++++++++++++++++------
>  1 file changed, 54 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
> index 9f4ff82bc6b4..b71478aa36e9 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>
> @@ -41,9 +42,11 @@
>  
>  /* Bitfields in LVDSC_CFGR (Configuration Register) */
>  #define LVDSC_CFGR_PIXSIZE_24BITS	0
> +#define LVDSC_CFGR_PIXSIZE_18BITS	1

#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)

I think you can drop the panel field of the mchp_lvds structure in the
same patch.

> @@ -76,9 +79,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 &
> @@ -93,11 +97,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));

You can drop the inner parentheses.

>  
>  	/* Enable the LVDS serializer */
>  	lvds_writel(lvds, LVDSC_CR, LVDSC_CR_SER_EN);
> @@ -113,7 +130,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_pre_enable(struct drm_bridge *bridge,
> +					struct drm_atomic_state *state)
>  {
>  	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
>  	int ret;
> @@ -129,11 +147,35 @@ static void mchp_lvds_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);
> +	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,
> +				     struct drm_atomic_state *state)
> +{
> +	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
>  
> -	lvds_serialiser_on(lvds);
> +	/* Turn off the serialiser */
> +	lvds_writel(lvds, LVDSC_CR, 0);
>  }
>  
> -static void mchp_lvds_disable(struct drm_bridge *bridge)
> +static void mchp_lvds_atomic_post_disable(struct drm_bridge *bridge,
> +					  struct drm_atomic_state *state)
>  {
>  	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
>  
> @@ -143,8 +185,10 @@ static void mchp_lvds_disable(struct drm_bridge *bridge)
>  
>  static const struct drm_bridge_funcs mchp_lvds_bridge_funcs = {
>  	.attach = mchp_lvds_attach,
> -	.enable = mchp_lvds_enable,
> -	.disable = mchp_lvds_disable,
> +	.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)
> 
> ---
> base-commit: 4325743c7e209ae7845293679a4de94b969f2bef
> change-id: 20250618-microchip-lvds-b7151d96094a

-- 
Regards,

Laurent Pinchart