Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE

From: Michael Walle
Date: Tue Jan 30 2024 - 04:33:58 EST


Hi Dario,

>> Just FYI this conflictted pretty heavily with drm-misc-next changes in
>> the same area, someone should check drm-tip has the correct
>> resolution, I'm not really sure what is definitely should be.
>
> FWIW, this looks rather messy now. The drm-tip doesn't build.
>
> There was a new call to samsung_dsim_set_stop_state() introduced
> in commit b2fe2292624ac (drm: bridge: samsung-dsim: enter display
> mode in the enable() callback).

I had a closer look at the latest linux-next (where somehow my patch
made it into) and tried to apply commit b2fe2292624ac (drm: bridge:
samsung-dsim: enter display mode in the enable() callback). It looks
like only the following hunk is still needed from that patch. Everything
else is covered by this fixes patch.

Dario, could you rebase your commit onto this patch? I had a quick test
with this change and it seems to work fine for our case.

--snip--
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 63a1a0c88be4..92755c90e7d2 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1498,6 +1498,8 @@ static void samsung_dsim_atomic_disable(struct
drm_bridge *bridge,
if (!(dsi->state & DSIM_STATE_ENABLED))
return;

+ samsung_dsim_set_display_enable(dsi, false);
+
dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
}

@@ -1506,8 +1508,6 @@ static void
samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
{
struct samsung_dsim *dsi = bridge_to_dsi(bridge);

- samsung_dsim_set_display_enable(dsi, false);
-
dsi->state &= ~DSIM_STATE_ENABLED;
pm_runtime_put_sync(dsi->dev);
}
--snip--

-michael

I'm sorry, but I didn't understand well what I have to do.

Basically, just rebase your patch (drm: bridge: samsung-dsim:
enter display mode in the enable() callback) on top of
linux-next.

This is what I have done:

git clone https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git
cd linux-next
# add your changes, the ones of the emails
git am --reject 0001-drm-bridge-samsung-dsim-enter-display-mode-in-the-en.patch

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 92755c90e7d2..b47929072583 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1508,6 +1508,9 @@ static void
samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
{
struct samsung_dsim *dsi = bridge_to_dsi(bridge);

+ if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
+ samsung_dsim_set_stop_state(dsi, true);
+

This one should be removed. There is no stop state anymore.
With that hunk, it doesn't compile anyway.

dsi->state &= ~DSIM_STATE_ENABLED;
pm_runtime_put_sync(dsi->dev);
}

And then test the driver for my use case.

Yes. The hunk I've posted above, should be all what's left
of your patch, because as far as I see it, most of your changes
are already contained in my fixes patch. What's left is that
you disable the video mode in .disable() and not in
post_disable().

-michael