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 <[email protected]> >> --- >> 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.
