[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <13f93095-c339-4238-843b-1f92ea83018b@microchip.com>
Date: Wed, 25 Jun 2025 03:42:32 +0000
From: <Dharma.B@...rochip.com>
To: <mripard@...nel.org>
CC: <Manikandan.M@...rochip.com>, <andrzej.hajda@...el.com>,
<neil.armstrong@...aro.org>, <rfoss@...nel.org>,
<Laurent.pinchart@...asonboard.com>, <jonas@...boo.se>,
<jernej.skrabec@...il.com>, <maarten.lankhorst@...ux.intel.com>,
<tzimmermann@...e.de>, <airlied@...il.com>, <simona@...ll.ch>,
<dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 2/3] drm/bridge: microchip-lvds: switch to use atomic
variants
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@...rochip.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.
Powered by blists - more mailing lists