Hi Marek, On Wed, May 18, 2022 at 7:35 PM Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: > > Hi Dave, > > On 11.05.2022 17:47, Dave Stevenson wrote: > > On Wed, 11 May 2022 at 15:58, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: > >> On 05.04.2022 13:43, Dave Stevenson wrote: > >>> On Fri, 18 Mar 2022 at 12:25, Dave Stevenson > >>> <dave.stevenson@xxxxxxxxxxxxxxx> wrote: > >>>> On Fri, 4 Mar 2022 at 15:18, Dave Stevenson > >>>> <dave.stevenson@xxxxxxxxxxxxxxx> wrote: > >>>>> Hi All > >>>> A gentle ping on this series. Any comments on the approach? > >>>> Thanks. > >>> I realise the merge window has just closed and therefore folks have > >>> been busy, but no responses on this after a month? > >>> > >>> Do I give up and submit a patch to document that DSI is broken and no one cares? > >> Thanks for pointing this patchset in the 'drm: bridge: Add Samsung MIPI > >> DSIM bridge' thread, otherwise I would miss it since I'm not involved > >> much in the DRM development. > >> > >> This resolves most of the issues in the Exynos DSI and its recent > >> conversion to the drm bridge framework. I've added the needed > >> prepare_upstream_first flags to the panels and everything works fine > >> without the bridge chain order hacks. > >> > >> Feel free to add: > >> > >> Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> > > Thanks for testing it. I was almost at the stage of abandoning the patch set. > > > >> The only remaining thing to resolve is the moment of enabling DSI host. > >> The proper sequence is: > >> > >> 1. host power on, 2. device power on, 3. host init, 4. device init, 5. > >> video enable. > >> > >> #1 is done in dsi's pre_enable, #2 is done in panel's prepare. #3 was so > >> far done in the first host transfer call, which usually happens in > >> panel's prepare, then the #4 happens. Then video enable is done in the > >> enable callbacks. > > What's your definition of host power on and host init here? What state > > are you defining the DSI interface to be in after each operation? > > Well, lets start from the point that I'm not a DSI specialist nor I'm > not the exynos-dsi author. I just played a bit with the code trying to > restore proper driver operation on the various Exynos based boards I have. > > By the host/device power on I mean enabling their power regulators. By > host init I mean executing the samsung_dsim_init() function, which > basically sets the lp-11 state if I understand it right. > > > >> Jagan wants to move it to the dsi host pre_enable() to let it work with > >> DSI bridges controlled over different interfaces > >> (https://lore.kernel.org/all/20220504114021.33265-6-jagan@xxxxxxxxxxxxxxxxxxxx/ > >> ). > > I think I'm in agreement with Jagan. > > As documented in patch 4/4: > > + * A DSI host should keep the PHY powered down until the pre_enable > > operation is > > + * called. All lanes are in an undefined idle state up to this point, and it > > + * must not be assumed that it is LP-11. > > + * pre_enable should initialise the PHY, set the data lanes to LP-11, and the > > + * clock lane to either LP-11 or HS depending on the mode_flag > > + * %MIPI_DSI_CLOCK_NON_CONTINUOUS. > > Right, this theory makes sense. > > However Exynos DSI for some reasons did the host initialization in the > first call of the samsung_dsim_host_transfer(). If I moved the host > initialization to pre_enable (before powering the panel on), executing > DSI commands failed (timeout). This issue happens on all boards I have > access (Trats, Trats2, Arndale, TM2e), so this must be an issue with > Exynos DSI host itself not related to particular panel/bridge. > > If I call samsung_dsim_init() once again, before issuing the first DSI > command, then everything works fine. I've tried to check which part of > that function is needed to be executed before transferring the commands, > but it turned out that the complete host reset and (re)configuration is > necessary. It looks that the initialization will need to be done twice, > first time in the pre_enable to satisfy Jagan case, then on the first > dsi transfer to make it work with real DSI panels. > > Here is a git repo with such change: > https://github.com/mszyprow/linux/tree/v5.18-next-20220511-dsi-rework-v2 This is worthy check, I will test it on imx8mm and integrate it into the next version - hope this is fine for you? Jagan.
