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

[PATCH v4 0/3] 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 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 (3):
      drm/bridge: microchip-lvds: drop unused drm_panel
      drm/bridge: microchip-lvds: switch to use atomic variants
      drm/bridge: microchip-lvds: fix bus format mismatch with VESA displays

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

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

[PATCH v4 1/3] drm/bridge: microchip-lvds: drop unused drm_panel

[Dharma Balasubiramani]

Drop the drm_panel field of the mchp_lvds struct as it is unused.

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

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
index 9f4ff82bc6b4..42751124b868 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;
 };
@@ -179,13 +177,8 @@ static int mchp_lvds_probe(struct platform_device *pdev)
 			"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

[PATCH v4 2/3] drm/bridge: microchip-lvds: switch to use atomic variants

[Dharma Balasubiramani]

Modernize the bridge ops to use atomic_enable/disable.

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

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
index 42751124b868..e4ff46b03d54 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_pre_enable(struct drm_bridge *bridge,
+					struct drm_atomic_state *state)
 {
 	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
 	int ret;
@@ -127,11 +128,26 @@ 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);
 	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);
+}
+
+static void mchp_lvds_atomic_post_disable(struct drm_bridge *bridge,
+					  struct drm_atomic_state *state)
 {
 	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
 
@@ -141,8 +157,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)

-- 
2.43.0

[PATCH v4 3/3] 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 e4ff46b03d54..ce52b794a9c6 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 v4 1/3] drm/bridge: microchip-lvds: drop unused drm_panel

[Laurent Pinchart]

On Tue, Jun 24, 2025 at 02:54:14PM +0530, Dharma Balasubiramani wrote:
> Drop the drm_panel field of the mchp_lvds struct as it is unused.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> ---
>  drivers/gpu/drm/bridge/microchip-lvds.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
> index 9f4ff82bc6b4..42751124b868 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;
>  };
> @@ -179,13 +177,8 @@ static int mchp_lvds_probe(struct platform_device *pdev)
>  			"can't find port point, please init lvds panel port!\n");
>  		return -ENODEV;
>  	}

The above code seems unneeded now, I think you can drop it too.

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

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 1/3] drm/bridge: microchip-lvds: drop unused drm_panel

[Dharma.B]

On 24/06/25 4:12 pm, Laurent Pinchart wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue, Jun 24, 2025 at 02:54:14PM +0530, Dharma Balasubiramani wrote:
>> Drop the drm_panel field of the mchp_lvds struct as it is unused.
>>
>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>> ---
>>   drivers/gpu/drm/bridge/microchip-lvds.c | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
>> index 9f4ff82bc6b4..42751124b868 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;
>>   };
>> @@ -179,13 +177,8 @@ static int mchp_lvds_probe(struct platform_device *pdev)
>>                        "can't find port point, please init lvds panel port!\n");
>>                return -ENODEV;
>>        }
> 
> The above code seems unneeded now, I think you can drop it too.

Yes, I considered removing it as well. However, I was wondering if it 
might still be valuable to return early when the endpoint isn't found, 
to clearly indicate that the panel port is missing.

> 
>> -
>> -     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))
> 
> --
> Regards,
> 
> Laurent Pinchart


-- 
With Best Regards,
Dharma B.

Re: [PATCH v4 1/3] drm/bridge: microchip-lvds: drop unused drm_panel

[Laurent Pinchart]

On Tue, Jun 24, 2025 at 10:46:26AM +0000, Dharma.B@microchip.com wrote:
> On 24/06/25 4:12 pm, Laurent Pinchart wrote:
> > On Tue, Jun 24, 2025 at 02:54:14PM +0530, Dharma Balasubiramani wrote:
> >> Drop the drm_panel field of the mchp_lvds struct as it is unused.
> >>
> >> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> >> ---
> >>   drivers/gpu/drm/bridge/microchip-lvds.c | 7 -------
> >>   1 file changed, 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
> >> index 9f4ff82bc6b4..42751124b868 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;
> >>   };
> >> @@ -179,13 +177,8 @@ static int mchp_lvds_probe(struct platform_device *pdev)
> >>                        "can't find port point, please init lvds panel port!\n");
> >>                return -ENODEV;
> >>        }
> > 
> > The above code seems unneeded now, I think you can drop it too.
> 
> Yes, I considered removing it as well. However, I was wondering if it 
> might still be valuable to return early when the endpoint isn't found, 
> to clearly indicate that the panel port is missing.

That error is covered by the devm_drm_of_get_bridge() call below, so I
think you can drop the code above.

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

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 1/3] drm/bridge: microchip-lvds: drop unused drm_panel

[Dharma.B]

On 24/06/25 4:20 pm, Laurent Pinchart wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue, Jun 24, 2025 at 10:46:26AM +0000, Dharma.B@microchip.com wrote:
>> On 24/06/25 4:12 pm, Laurent Pinchart wrote:
>>> On Tue, Jun 24, 2025 at 02:54:14PM +0530, Dharma Balasubiramani wrote:
>>>> Drop the drm_panel field of the mchp_lvds struct as it is unused.
>>>>
>>>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>>>> ---
>>>>    drivers/gpu/drm/bridge/microchip-lvds.c | 7 -------
>>>>    1 file changed, 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
>>>> index 9f4ff82bc6b4..42751124b868 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;
>>>>    };
>>>> @@ -179,13 +177,8 @@ static int mchp_lvds_probe(struct platform_device *pdev)
>>>>                         "can't find port point, please init lvds panel port!\n");
>>>>                 return -ENODEV;
>>>>         }
>>>
>>> The above code seems unneeded now, I think you can drop it too.
>>
>> Yes, I considered removing it as well. However, I was wondering if it
>> might still be valuable to return early when the endpoint isn't found,
>> to clearly indicate that the panel port is missing.
> 
> That error is covered by the devm_drm_of_get_bridge() call below, so I
> think you can drop the code above.

Sure, I will drop the device node "port" as well.

> 
>>>> -
>>>> -     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))
> 
> --
> Regards,
> 
> Laurent Pinchart


-- 
With Best Regards,
Dharma B.

Re: [PATCH v4 2/3] drm/bridge: microchip-lvds: switch to use atomic variants

[Maxime Ripard]

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

On Tue, Jun 24, 2025 at 02:54:15PM +0530, Dharma Balasubiramani wrote:
> Modernize the bridge ops to use atomic_enable/disable.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> ---
>  drivers/gpu/drm/bridge/microchip-lvds.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
> index 42751124b868..e4ff46b03d54 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_pre_enable(struct drm_bridge *bridge,
> +					struct drm_atomic_state *state)
>  {
>  	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
>  	int ret;
> @@ -127,11 +128,26 @@ 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);
>  	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);
> +}
> +
> +static void mchp_lvds_atomic_post_disable(struct drm_bridge *bridge,
> +					  struct drm_atomic_state *state)
>  {
>  	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
>  
> @@ -141,8 +157,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,
>  };

Like I said to you earlier today, it's not just what you claim it is.
You're splitting enable into atomic_pre_enable and atomic_enable, and
disable into atomic_disable and atomic_post_disable.

At the *very* least this should be explained in your commit log, and it
would be much better if it was done in another patch.

Maxime

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

Re: [PATCH v4 2/3] drm/bridge: microchip-lvds: switch to use atomic variants

[Dharma.B]

On 24/06/25 4:32 pm, Maxime Ripard wrote:
> On Tue, Jun 24, 2025 at 02:54:15PM +0530, Dharma Balasubiramani wrote:
>> Modernize the bridge ops to use atomic_enable/disable.
>>
>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>> ---
>>   drivers/gpu/drm/bridge/microchip-lvds.c | 26 ++++++++++++++++++++++----
>>   1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
>> index 42751124b868..e4ff46b03d54 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_pre_enable(struct drm_bridge *bridge,
>> +					struct drm_atomic_state *state)
>>   {
>>   	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
>>   	int ret;
>> @@ -127,11 +128,26 @@ 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);
>>   	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);
>> +}
>> +
>> +static void mchp_lvds_atomic_post_disable(struct drm_bridge *bridge,
>> +					  struct drm_atomic_state *state)
>>   {
>>   	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
>>   
>> @@ -141,8 +157,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,
>>   };
> 
> Like I said to you earlier today, it's not just what you claim it is.
> You're splitting enable into atomic_pre_enable and atomic_enable, and
> disable into atomic_disable and atomic_post_disable.
> 
> At the *very* least this should be explained in your commit log, and it
> would be much better if it was done in another patch.

Sure, I’ll split the changes into two commits:

1. Convert .enable → atomic_enable() and .disable → atomic_disable()

2. Introduce atomic_pre_enable() and atomic_post_disable(), moving the 
sleepable operations accordingly.


> 
> Maxime


-- 
With Best Regards,
Dharma B.