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

[Jagan Teki]

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.

Jagan.