Re: [PATCH V2 0/3] DSI host and peripheral initialisation ordering

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marek.

On Wed, 11 May 2022 at 15:58, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote:
>
> Hi Dave,
>
> 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?

> 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.

> This however fails on Exynos with DSI panels, because when dsi's
> pre_enable is called, the dsi device is not yet powered.

Sorry, I'm not following what the issue is here? DSI lanes being at
LP-11 when the sink isn't powered, so potential for leakage through
the device?
In which case the device should NOT set pre_enable_upstream first, and
the host gets powered up and down with each host_transfer call the
device makes in pre_enable.

(Whilst I can't claim that I know of every single DSI device, most
datasheets I've encountered have required LP-11 on the lanes before
powering up the device).

> I've discussed
> this with Andrzej Hajda and we came to the conclusion that this can be
> resolved by adding the init() callback to the struct mipi_dsi_host_ops.
> Then DSI client (next bridge or panel) would call it after powering self
> on, but before sending any DSI commands in its pre_enable/prepare functions.

You may as well have a mipi_dsi_host_ops call to fully control the DSI
host state, and forget about changing the pre_enable/post_disable
order. However it involves even more changes to all the panel and
bridge drivers.

If you've added an init hook, don't you also need a deinint hook to
ensure that the host is restored to the "power on but not inited"
state before device power off? Currently it feels unbalanced, but
largely as I don't know exactly what you're defining that power on
state to be.

  Dave

> I've prepared a prototype of such solution. This approach finally
> resolved all the initialization issues! The bridge chain finally matches
> the hardware, no hack are needed, and everything is controlled by the
> DRM core. This prototype also includes the Jagan's patches, which add
> IMX support to Samsung DSIM. If one is interested, here is my git repo
> with all the PoC patches:
>
> https://github.com/mszyprow/linux/tree/v5.18-next-20220511-dsi-rework



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux