On 12.12.2022 16:33, Jagan Teki wrote: > On Mon, Dec 12, 2022 at 8:52 PM Marek Szyprowski > <m.szyprow...@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.szyprow...@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.szyprow...@samsung.com> >>>>>> Signed-off-by: Jagan Teki<ja...@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-9f9d179b5...@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 addingmipi_dsi_host_init() to panel/bridge drivers. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland