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

Dharma Balasubiramani posted 4 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v5 2/4] drm/bridge: microchip-lvds: migrate to atomic bridge ops
Posted by Dharma Balasubiramani 3 months, 2 weeks ago
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
Posted by Maxime Ripard 3 months, 2 weeks ago
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
Re: [PATCH v5 2/4] drm/bridge: microchip-lvds: migrate to atomic bridge ops
Posted by Dharma.B@microchip.com 3 months, 2 weeks ago
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.