[PATCH v9 10/18] drm: bridge: samsung-dsim: Init exynos host for first DSI transfer

[Jagan Teki]

Hi Marek,

On Tue, Dec 13, 2022 at 2:28 PM Marek Szyprowski
<m.szyprowski at samsung.com> wrote:
>
> On 12.12.2022 16:33, Jagan Teki wrote:
>
> On Mon, Dec 12, 2022 at 8:52 PM Marek Szyprowski
> <m.szyprowski at samsung.com> wrote:
>
> On 12.12.2022 09:43, Marek Szyprowski wrote:
>
> On 12.12.2022 09:32, Jagan Teki wrote:
>
> On Mon, Dec 12, 2022 at 1:56 PM Marek Szyprowski
> <m.szyprowski at samsung.com> wrote:
>
> Hi Jagan,
>
> On 09.12.2022 16:23, Jagan Teki wrote:
>
> The existing drm panels and bridges in Exynos required host
> initialization during the first DSI command transfer even though
> the initialization was done before.
>
> This host reinitialization is handled via DSIM_STATE_REINITIALIZED
> flag and triggers from host transfer.
>
> Do this exclusively for Exynos.
>
> Initial logic is derived from Marek Szyprowski changes.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski at samsung.com>
> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
> ---
> Changes from v9:
> - derived from v8
> - added comments
>
>    drivers/gpu/drm/bridge/samsung-dsim.c | 15 ++++++++++++++-
>    include/drm/bridge/samsung-dsim.h     |  5 +++--
>    2 files changed, 17 insertions(+), 3 deletions(-)
>
> The following chunk is missing compared to v8:
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 6e9ad955ebd3..6a9403cb92ae 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1315,7 +1315,9 @@ static int samsung_dsim_init(struct samsung_dsim
> *dsi, unsigned int flag)
>                   return 0;
>
>           samsung_dsim_reset(dsi);
> -       samsung_dsim_enable_irq(dsi);
> +
> +       if (!(dsi->state & DSIM_STATE_INITIALIZED))
> +               samsung_dsim_enable_irq(dsi);
>
> Is this really required? does it make sure that the IRQ does not
> enable twice?
>
> That's what that check does. Without the 'if (!(dsi->state &
> DSIM_STATE_INITIALIZED))' check, the irqs will be enabled twice (first
> from pre_enable, then from the first transfer), what leads to a
> warning from irq core.
>
> I've just noticed that we also would need to clear the
> DSIM_STATE_REINITIALIZED flag in dsim_suspend.
>
> However I've found that a bit simpler patch would keep the current code
> flow for Exynos instead of this reinitialization hack. This can be
> applied on the "[PATCH v9 09/18] drm: bridge: samsung-dsim: Add host
> init in pre_enable" patch:
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 0b2e52585485..acc95c61ae45 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1291,9 +1291,11 @@ static void samsung_dsim_atomic_pre_enable(struct
> drm_bridge *bridge,
>
>          dsi->state |= DSIM_STATE_ENABLED;
>
> -       ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
> -       if (ret)
> -               return;
> +       if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> +               ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
> +               if (ret)
> +                       return;
> +       }
>
> Sorry, I don't understand this. Does it mean Exynos doesn't need to
> init host in pre_enable? If I remember correctly even though the host
> is initialized it has to reinitialize during the first transfer - This
> is what the Exynos requirement is. Please correct or explain here.
>
> This is a matter of enabling power regulator(s) in the right order and doing the host initialization in the right moment. It was never a matter of re-initialization. See the current code for the reference (it uses the same approach as my above change). I've already explained that here:
>
> https://lore.kernel.org/all/e96197f9-948a-997e-5453-9f9d179b5f5a@samsung.com/
>
> If you would like to see the exact proper moment of the dsi host initialization on the Exynos see the code here:
>
> https://github.com/mszyprow/linux/tree/v5.18-next-20220511-dsi-rework and patches adding mipi_dsi_host_init() to panel/bridge drivers.

As I said before, the downstream bridge needs an explicit call to host
init via mipi_dsi_host_init - this is indeed not a usual use-case
scenario. Let's handle this with a REINIT fix and see if we can update
this later to handle both scenarios.

Would you please test this repo, I have included all.

https://gitlab.com/openedev/kernel/-/commits/imx8mm-dsi-v10

Thanks,
Jagan.