All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Walle <mwalle@kernel.org>
To: Inki Dae <inki.dae@samsung.com>,
	Jagan Teki <jagan@amarulasolutions.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Robert Foss <rfoss@kernel.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	Frieder Schrempf <frieder.schrempf@kontron.de>,
	Tim Harvey <tharvey@gateworks.com>,
	Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
Date: Fri, 01 Dec 2023 10:04:33 +0100	[thread overview]
Message-ID: <631fe35a2a3b00781231e4f3f5094fae@kernel.org> (raw)
In-Reply-To: <20231113164344.1612602-1-mwalle@kernel.org>

> The FORCE_STOP_STATE bit is unsuitable to force the DSI link into LP-11
> mode. It seems the bridge internally queues DSI packets and when the
> FORCE_STOP_STATE bit is cleared, they are sent in close succession
> without any useful timing (this also means that the DSI lanes won't go
> into LP-11 mode). The length of this gibberish varies between 1ms and
> 5ms. This sometimes breaks an attached bridge (TI SN65DSI84 in this
> case). In our case, the bridge will fail in about 1 per 500 reboots.
> 
> The FORCE_STOP_STATE handling was introduced to have the DSI lanes in
> LP-11 state during the .pre_enable phase. But as it turns out, none of
> this is needed at all. Between samsung_dsim_init() and
> samsung_dsim_set_display_enable() the lanes are already in LP-11 mode.
> The code as it was before commit 20c827683de0 ("drm: bridge:
> samsung-dsim: Fix init during host transfer") and 0c14d3130654 ("drm:
> bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec") was correct
> in this regard.
> 
> This patch basically reverts both commits. It was tested on an i.MX8M
> SoC with an SN65DSI84 bridge. The signals were probed and the DSI
> packets were decoded during initialization and link start-up. After 
> this
> patch the first DSI packet on the link is a VSYNC packet and the timing
> is correct.
> 
> Command mode between .pre_enable and .enable was also briefly tested by
> a quick hack. There was no DSI link partner which would have responded,
> but it was made sure the DSI packet was send on the link. As a side
> note, the command mode seems to just work in HS mode. I couldn't find
> that the bridge will handle commands in LP mode.
> 
> Fixes: 20c827683de0 ("drm: bridge: samsung-dsim: Fix init during host 
> transfer")
> Fixes: 0c14d3130654 ("drm: bridge: samsung-dsim: Fix i.MX8M enable flow 
> to meet spec")
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> Let me know wether this should be two commits each reverting one, but 
> both
> commits appeared first in kernel 6.5.

Are there any news?

-michael

WARNING: multiple messages have this Message-ID (diff)
From: Michael Walle <mwalle@kernel.org>
To: Inki Dae <inki.dae@samsung.com>,
	Jagan Teki <jagan@amarulasolutions.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Robert Foss <rfoss@kernel.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	Frieder Schrempf <frieder.schrempf@kontron.de>,
	Tim Harvey <tharvey@gateworks.com>,
	Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
Date: Fri, 01 Dec 2023 10:04:33 +0100	[thread overview]
Message-ID: <631fe35a2a3b00781231e4f3f5094fae@kernel.org> (raw)
In-Reply-To: <20231113164344.1612602-1-mwalle@kernel.org>

> The FORCE_STOP_STATE bit is unsuitable to force the DSI link into LP-11
> mode. It seems the bridge internally queues DSI packets and when the
> FORCE_STOP_STATE bit is cleared, they are sent in close succession
> without any useful timing (this also means that the DSI lanes won't go
> into LP-11 mode). The length of this gibberish varies between 1ms and
> 5ms. This sometimes breaks an attached bridge (TI SN65DSI84 in this
> case). In our case, the bridge will fail in about 1 per 500 reboots.
> 
> The FORCE_STOP_STATE handling was introduced to have the DSI lanes in
> LP-11 state during the .pre_enable phase. But as it turns out, none of
> this is needed at all. Between samsung_dsim_init() and
> samsung_dsim_set_display_enable() the lanes are already in LP-11 mode.
> The code as it was before commit 20c827683de0 ("drm: bridge:
> samsung-dsim: Fix init during host transfer") and 0c14d3130654 ("drm:
> bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec") was correct
> in this regard.
> 
> This patch basically reverts both commits. It was tested on an i.MX8M
> SoC with an SN65DSI84 bridge. The signals were probed and the DSI
> packets were decoded during initialization and link start-up. After 
> this
> patch the first DSI packet on the link is a VSYNC packet and the timing
> is correct.
> 
> Command mode between .pre_enable and .enable was also briefly tested by
> a quick hack. There was no DSI link partner which would have responded,
> but it was made sure the DSI packet was send on the link. As a side
> note, the command mode seems to just work in HS mode. I couldn't find
> that the bridge will handle commands in LP mode.
> 
> Fixes: 20c827683de0 ("drm: bridge: samsung-dsim: Fix init during host 
> transfer")
> Fixes: 0c14d3130654 ("drm: bridge: samsung-dsim: Fix i.MX8M enable flow 
> to meet spec")
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> Let me know wether this should be two commits each reverting one, but 
> both
> commits appeared first in kernel 6.5.

Are there any news?

-michael

  parent reply	other threads:[~2023-12-01  9:04 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-13 16:43 [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE Michael Walle
2023-11-13 16:43 ` Michael Walle
2023-11-14  7:15 ` Alexander Stein
2023-11-14  7:15   ` Alexander Stein
2023-11-14  8:52   ` Michael Walle
2023-11-14  8:52     ` Michael Walle
2023-11-14 14:29 ` Frieder Schrempf
2023-11-14 14:29   ` Frieder Schrempf
2023-11-14 15:53   ` Michael Walle
2023-11-14 15:53     ` Michael Walle
2023-12-01  9:04 ` Michael Walle [this message]
2023-12-01  9:04   ` Michael Walle
2023-12-18 11:24   ` Frieder Schrempf
2023-12-18 11:24     ` Frieder Schrempf
2023-12-21  4:23     ` Inki Dae
2024-01-09  8:47       ` Michael Walle
2024-01-09  8:47         ` Michael Walle
2024-01-09 12:50         ` Daniel Vetter
2024-01-09 12:50           ` Daniel Vetter
2024-01-19  6:36           ` Inki Dae
2024-01-26 18:28             ` Dave Airlie
2024-01-26 18:28               ` Dave Airlie
2024-01-29  9:20               ` Frieder Schrempf
2024-01-29  9:20                 ` Frieder Schrempf
2024-01-29 16:51                 ` Frieder Schrempf
2024-01-29 16:51                   ` Frieder Schrempf
2024-01-29 10:32               ` Michael Walle
2024-01-29 10:32                 ` Michael Walle
2024-01-29 10:39                 ` Michael Walle
2024-01-29 10:39                   ` Michael Walle
2024-01-29 16:06                 ` Michael Walle
2024-01-29 16:06                   ` Michael Walle
2024-01-30  9:11                   ` Dario Binacchi
2024-01-30  9:11                     ` Dario Binacchi
2024-01-30  9:24                     ` Michael Walle
2024-01-30  9:24                       ` Michael Walle

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=631fe35a2a3b00781231e4f3f5094fae@kernel.org \
    --to=mwalle@kernel.org \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@gmail.com \
    --cc=alexander.stein@ew.tq-group.com \
    --cc=andrzej.hajda@intel.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=frieder.schrempf@kontron.de \
    --cc=inki.dae@samsung.com \
    --cc=jagan@amarulasolutions.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=neil.armstrong@linaro.org \
    --cc=rfoss@kernel.org \
    --cc=tharvey@gateworks.com \
    /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.