[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <25f0cdf7305eac946629895179be8f2b@kernel.org>
Date: Tue, 30 Jan 2024 10:24:00 +0100
From: Michael Walle <mwalle@...nel.org>
To: Dario Binacchi <dario.binacchi@...rulasolutions.com>
Cc: Dave Airlie <airlied@...il.com>, Dmitry Osipenko
<dmitry.osipenko@...labora.com>, Inki Dae <daeinki@...il.com>, Neil
Armstrong <neil.armstrong@...aro.org>, Laurent Pinchart
<Laurent.pinchart@...asonboard.com>, Robert Foss <rfoss@...nel.org>, Frieder
Schrempf <frieder.schrempf@...tron.de>, Jagan Teki
<jagan@...rulasolutions.com>, Andrzej Hajda <andrzej.hajda@...el.com>, Marek
Szyprowski <m.szyprowski@...sung.com>, Jonas Karlman <jonas@...boo.se>,
Jernej Skrabec <jernej.skrabec@...il.com>, Tim Harvey
<tharvey@...eworks.com>, Alexander Stein <alexander.stein@...tq-group.com>,
linux-kernel@...r.kernel.org, DRI mailing list
<dri-devel@...ts.freedesktop.org>, Daniel Vetter <daniel@...ll.ch>, Michael
Trimarchi <michael@...rulasolutions.com>
Subject: Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
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
Powered by blists - more mailing lists