<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<div class="moz-cite-prefix">On 12.12.2022 16:33, Jagan Teki wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAMty3ZBzpmwAV7e7wdBu+GOmg6M7xqqc46QtGzuLsnv2kT0Zdw@mail.gmail.com">
<pre class="moz-quote-pre" wrap="">On Mon, Dec 12, 2022 at 8:52 PM Marek Szyprowski
<a class="moz-txt-link-rfc2396E" href="mailto:m.szyprowski@samsung.com"><m.szyprowski@samsung.com></a> wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On 12.12.2022 09:43, Marek Szyprowski wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On 12.12.2022 09:32, Jagan Teki wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Mon, Dec 12, 2022 at 1:56 PM Marek Szyprowski
<a class="moz-txt-link-rfc2396E" href="mailto:m.szyprowski@samsung.com"><m.szyprowski@samsung.com></a> wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Hi Jagan,
On 09.12.2022 16:23, Jagan Teki wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">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 <a class="moz-txt-link-rfc2396E" href="mailto:m.szyprowski@samsung.com"><m.szyprowski@samsung.com></a>
Signed-off-by: Jagan Teki <a class="moz-txt-link-rfc2396E" href="mailto:jagan@amarulasolutions.com"><jagan@amarulasolutions.com></a>
---
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(-)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">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);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Is this really required? does it make sure that the IRQ does not
enable twice?
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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;
+ }
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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.</pre>
</blockquote>
<p>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:</p>
<p><a class="moz-txt-link-freetext" href="https://lore.kernel.org/all/e96197f9-948a-997e-5453-9f9d179b5f5a@samsung.com/">https://lore.kernel.org/all/e96197f9-948a-997e-5453-9f9d179b5f5a@samsung.com/</a></p>
<p>If you would like to see the exact proper moment of the dsi host
initialization on the Exynos see the code here:</p>
<pre id="b"><a href="https://github.com/mszyprow/linux/tree/v5.18-next-20220511-dsi-rework" class="moz-txt-link-freetext">https://github.com/mszyprow/linux/tree/v5.18-next-20220511-dsi-rework</a> and patches adding <span class="blob-code-inner blob-code-marker js-code-nav-pass " data-code-marker="+"><span class="pl-c1">mipi_dsi_host_init</span>() to panel/bridge drivers.
</span></pre>
<p></p>
<pre class="moz-signature" cols="72">Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland</pre>
<table id=bannersignimg data-cui-lock="true" namo_lock><tr><td><p> </p>
</td></tr></table><table id=confidentialsignimg data-cui-lock="true" namo_lock><tr><td><p> <img style="border: 0px solid currentColor; border-image: none; width: 520px; height: 144px; display: inline-block;" unselectable="on" data-cui-image="true" src="cid:cafe_image_0@s-core.co.kr"> </p>
</td></tr></table></body>
</html>
<table style='display: none;'><tbody><tr><td><img style='display: none;' border=0 src='http://ext.w1.samsung.net/mail/ext/v1/external/status/update?userid=m.szyprowski&do=bWFpbElEPTIwMjIxMjEzMDg1ODE5ZXVjYXMxcDFhYzU2NjkxNmE1NTE2OGY1NDRjYzM4OTVmZjkzMWY1ZSZyZWNpcGllbnRBZGRyZXNzPWRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmc_' width=0 height=0></td></tr></tbody></table>