On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut <marex@xxxxxxx> wrote:
>
> On 11/17/22 14:04, Marek Szyprowski wrote:
> > On 17.11.2022 05:58, Marek Vasut wrote:
> >> On 11/10/22 19:38, Jagan Teki wrote:
> >>> DSI host initialization handling in previous exynos dsi driver has
> >>> some pitfalls. It initializes the host during host transfer() hook
> >>> that is indeed not the desired call flow for I2C and any other DSI
> >>> configured downstream bridges.
> >>>
> >>> Host transfer() is usually triggered for downstream DSI panels or
> >>> bridges and I2C-configured-DSI bridges miss these host initialization
> >>> as these downstream bridges use bridge operations hooks like pre_enable,
> >>> and enable in order to initialize or set up the host.
> >>>
> >>> This patch is trying to handle the host init handler to satisfy all
> >>> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
> >>> flag to ensure that host init is also done on first cmd transfer, this
> >>> helps existing DSI panels work on exynos platform (form Marek
> >>> Szyprowski).
> >>>
> >>> v8, v7, v6, v5:
> >>> * none
> >>>
> >>> v4:
> >>> * update init handling to ensure host init done on first cmd transfer
> >>>
> >>> v3:
> >>> * none
> >>>
> >>> v2:
> >>> * check initialized state in samsung_dsim_init
> >>>
> >>> v1:
> >>> * keep DSI init in host transfer
> >>>
> >>> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> >>> Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> >>> ---
> >>> drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++++++++++++++++--------
> >>> include/drm/bridge/samsung-dsim.h | 5 +++--
> >>> 2 files changed, 20 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> b/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> index bb1f45fd5a88..ec7e01ae02ea 100644
> >>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
> >>> samsung_dsim *dsi)
> >>> disable_irq(dsi->irq);
> >>> }
> >>> -static int samsung_dsim_init(struct samsung_dsim *dsi)
> >>> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
> >>> flag)
> >>> {
> >>> const struct samsung_dsim_driver_data *driver_data =
> >>> dsi->driver_data;
> >>> + if (dsi->state & flag)
> >>> + return 0;
> >>> +
> >>> samsung_dsim_reset(dsi);
> >>> - samsung_dsim_enable_irq(dsi);
> >>> +
> >>> + if (!(dsi->state & DSIM_STATE_INITIALIZED))
> >>> + samsung_dsim_enable_irq(dsi);
> >>> if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
> >>> samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
> >>> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
> >>> samsung_dsim *dsi)
> >>> samsung_dsim_set_phy_ctrl(dsi);
> >>> samsung_dsim_init_link(dsi);
> >>> + dsi->state |= flag;
> >>> +
> >>> return 0;
> >>> }
> >>> @@ -1269,6 +1276,10 @@ 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;
> >>> }
> >>> static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
> >>> @@ -1458,12 +1469,9 @@ static ssize_t
> >>> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
> >>> if (!(dsi->state & DSIM_STATE_ENABLED))
> >>> return -EINVAL;
> >>> - if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
> >>> - ret = samsung_dsim_init(dsi);
> >>> - if (ret)
> >>> - return ret;
> >>> - dsi->state |= DSIM_STATE_INITIALIZED;
> >>> - }
> >>> + ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
> >>
> >> This triggers full controller reset and reprogramming upon first
> >> command transfer, is such heavy handed reload really necessary ?
> >
> > Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
> > DSI. If this is a real issue for you, then maybe the driver could do the
> > initialization conditionally, in prepare() callback in case of IMX and
> > on the first transfer in case of Exynos?
>
> That's odd , it does actually break panel support for me, without this
> double reset the panel works again. But I have to wonder, why would such
> a full reset be necessary at all , even on the exynos ?
Is it breaking samsung_dsim_reset from host_transfer? maybe checking
whether a reset is required before calling it might fix the issue. I
agree with double initialization is odd but it seems it is required on
some panels in Exynos, I think tweaking them and adjusting the panel
code might resolve this discrepancy.
Jagan.
