On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut <ma...@denx.de> 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.szyprow...@samsung.com> > >>> Signed-off-by: Jagan Teki <ja...@amarulasolutions.com> > >>> --- > >>> 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.