All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Stevenson <dave.stevenson@raspberrypi.com>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Marek Vasut <marex@denx.de>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Neil Armstrong <narmstrong@baylibre.com>,
	David Airlie <airlied@linux.ie>,
	Robert Foss <robert.foss@linaro.org>,
	Jonas Karlman <jonas@kwiboo.se>,
	Douglas Anderson <dianders@chromium.org>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	Andrzej Hajda <andrzej.hajda@gmail.com>,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Jagan Teki <jagan@amarulasolutions.com>
Subject: Re: [PATCH V2 0/3] DSI host and peripheral initialisation ordering
Date: Wed, 11 May 2022 16:47:57 +0100	[thread overview]
Message-ID: <CAPY8ntDR54vaBuwQN2F_j4_rxYgkFmsxc+mKYBn0YLe8=CtxtQ@mail.gmail.com> (raw)
In-Reply-To: <e96197f9-948a-997e-5453-9f9d179b5f5a@samsung.com>

Hi Marek.

On Wed, 11 May 2022 at 15:58, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi Dave,
>
> On 05.04.2022 13:43, Dave Stevenson wrote:
> > On Fri, 18 Mar 2022 at 12:25, Dave Stevenson
> > <dave.stevenson@raspberrypi.com>  wrote:
> >> On Fri, 4 Mar 2022 at 15:18, Dave Stevenson
> >> <dave.stevenson@raspberrypi.com>  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@samsung.com>

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@amarulasolutions.com/
> ).

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

  parent reply	other threads:[~2022-05-11 15:48 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04 15:17 [PATCH V2 0/3] DSI host and peripheral initialisation ordering Dave Stevenson
2022-03-04 15:17 ` [PATCH V2 1/4] drm/bridge: Remove duplication from drm_bridge and drm_atomic_bridge chains Dave Stevenson
2022-06-08 11:00   ` Dmitry Baryshkov
2022-07-18 18:16     ` Sam Ravnborg
2022-03-04 15:17 ` [PATCH V2 2/4] drm/bridge: Introduce pre_enable_upstream_first to alter bridge init order Dave Stevenson
2022-09-14  9:46   ` Jagan Teki
2022-03-04 15:17 ` [PATCH V2 3/4] drm/panel: Add prepare_upstream_first flag to drm_panel Dave Stevenson
2022-06-08 11:02   ` Dmitry Baryshkov
2022-10-06 14:25   ` Jagan Teki
2022-10-07 12:55     ` Dave Stevenson
2022-10-17  2:44       ` Jagan Teki
2022-10-18 17:10         ` Dave Stevenson
2022-03-04 15:17 ` [PATCH V2 4/4] drm/bridge: Document the expected behaviour of DSI host controllers Dave Stevenson
2022-03-18 12:25 ` [PATCH V2 0/3] DSI host and peripheral initialisation ordering Dave Stevenson
2022-04-05 11:43   ` Dave Stevenson
2022-05-11 14:58     ` Marek Szyprowski
2022-05-11 15:47       ` Marek Vasut
2022-05-11 17:29         ` Marek Szyprowski
2022-05-13 14:01         ` Jagan Teki
2022-05-11 15:47       ` Dave Stevenson [this message]
2022-05-18 14:05         ` Marek Szyprowski
2022-05-18 22:53           ` Andrzej Hajda
2022-05-19 12:56             ` Dave Stevenson
2022-06-08 10:49               ` Dave Stevenson
2022-06-08 10:57                 ` Jagan Teki
2022-06-07 19:46           ` Jagan Teki
2022-05-13 13:57       ` Jagan Teki
2022-06-10  7:52       ` Lucas Stach
2022-07-06  7:09         ` Frieder Schrempf
2022-07-06  7:13           ` Jagan Teki
2022-07-06 10:27           ` Dave Stevenson
2022-07-06 10:43             ` Frieder Schrempf
2022-07-19 14:36         ` Dave Stevenson
2022-07-18 20:52 ` Sam Ravnborg
2022-07-19 13:45   ` Dave Stevenson
2022-11-13 13:06     ` Dmitry Baryshkov
2022-11-15 14:14       ` Dave Stevenson
2022-11-15 14:21         ` Dmitry Baryshkov
2022-11-15 14:38           ` Dave Stevenson
2022-11-17 13:24             ` Dmitry Baryshkov
2022-11-17 14:34               ` Maxime Ripard
2022-11-17 14:35                 ` Dmitry Baryshkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPY8ntDR54vaBuwQN2F_j4_rxYgkFmsxc+mKYBn0YLe8=CtxtQ@mail.gmail.com' \
    --to=dave.stevenson@raspberrypi.com \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@linux.ie \
    --cc=andrzej.hajda@gmail.com \
    --cc=andrzej.hajda@intel.com \
    --cc=dianders@chromium.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jagan@amarulasolutions.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=m.szyprowski@samsung.com \
    --cc=marex@denx.de \
    --cc=narmstrong@baylibre.com \
    --cc=robert.foss@linaro.org \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.