??????[PATCH] drm: bridge: it66121: Added it66121 chip external screen status judgment.

[??????]

OK??I will modify the message. Thanks a lot.

---------------????????---------------
??????: Robert Foss<robert.f...@linaro.org&gt;
????????: 2021-09-20(????) 17:47
??????: Yunlongli<liyunlo...@uniontech.com&gt;
??????: Phong LE<p...@baylibre.com&gt;, Neil 
Armstrong<narmstr...@baylibre.com&gt;, Andrzej Hajda<a.ha...@samsung.com&gt;, 
David Airlie<airl...@linux.ie&gt;, Daniel Vetter<dan...@ffwll.ch&gt;, Laurent 
Pinchart<laurent.pinch...@ideasonboard.com&gt;, Jonas 
Karlman<jo...@kwiboo.se&gt;, Jernej Skrabec<jernej.skra...@gmail.com&gt;, 
dri-devel<dri-devel@lists.freedesktop.org&gt;, 
linux-kernel<linux-ker...@vger.kernel.org&gt;
????: Re: [PATCH] drm: bridge: it66121: Added it66121 chip external screen 
status judgment.

Hey Yunlongli,

Thanks for submitting this fix.

On Sat, 18 Sept 2021 at 05:51, Yunlongli <liyunlo...@uniontech.com&gt; wrote:

The formatting of this commit message is a bit unusual, let's try to
change it to the normal formatting.

Remove the dot from the commit title:
"drm: bridge: it66121: Added it66121 chip external screen status
judgment." -&gt; "drm: bridge: it66121: Added it66121 chip external
screen status judgment"


&gt;
&gt; fix: Add further confirm if external screens are involved.

The "fix:" tag is not needed. However if this commit fixes a bug
introduced in an earlier commit a machine readable tag like the the
one below could be added after the commit message.

Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver")

&gt;
&gt; log: In the actual tests,&nbsp; the IT66121 chip sometimes misjudged 
whether
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; it had an external screen, so, reference the 
it66121_user_guid.pdf
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; about Audio/Video data is stable or not A 
typical initialization
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; of HDMI link should be based on interrupt 
signal and appropriate
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; register probing. Recommended flow is 
detailed in IT66121
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Programming Guide. Simply put, the 
microcontroller should monitor
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; the HPD status first. Upon valid HPD event, 
move on to check
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; RxSENDetect register to see if the receiver 
chip is ready for
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; further handshaking. When RxSENDetect is 
asserted, start reading EDID
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; data through DDC channels and carry on the 
rest of the handshaking
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; subsequently.If the micro-controller makes 
no use of the interrupt
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; signal as well as the above-mentioned 
status&nbsp; registers, the link
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; establishment might fail. Please do follow 
the suggested
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; initialization flow recommended in IT66121 
Programming Guide.
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; So, I add the IT66121_SYS_STATUS_SENDECTECT 
register status detection.
&gt;

The "log:" prefix is not needed, and neither is the indentation of the text.

Secondly maybe it would be nice to format the above chunk of text into
paragraphs just to make it easier to read.

&gt; Signed-off-by: Yunlongli <liyunlo...@uniontech.com&gt;
&gt; ---
&gt;&nbsp; drivers/gpu/drm/bridge/ite-it66121.c | 2 +-
&gt;&nbsp; 1 file changed, 1 insertion(+), 1 deletion(-)
&gt;
&gt; diff --git a/drivers/gpu/drm/bridge/ite-it66121.c 
b/drivers/gpu/drm/bridge/ite-it66121.c
&gt; index 2f2a09adb4bc..9ed4fa298d11 100644
&gt; --- a/drivers/gpu/drm/bridge/ite-it66121.c
&gt; +++ b/drivers/gpu/drm/bridge/ite-it66121.c
&gt; @@ -523,7 +523,7 @@ static bool it66121_is_hpd_detect(struct it66121_ctx 
*ctx)
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; if 
(regmap_read(ctx-&gt;regmap, IT66121_SYS_STATUS_REG, &amp;val))
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 return false;
&gt;
&gt; -&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; return val &amp; 
IT66121_SYS_STATUS_HPDETECT;
&gt; +&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; return ((val &amp; 
IT66121_SYS_STATUS_HPDETECT) &amp;&amp; (val &amp; 
IT66121_SYS_STATUS_SENDECTECT));
&gt;&nbsp; }
&gt;
&gt;&nbsp; static int it66121_bridge_attach(struct drm_bridge *bridge,
&gt; --
&gt; 2.20.1
&gt;
&gt;
&gt;

With the above suggestions fixed, feel free to add my r-b and submit a
v2 of this patch.
Reviewed-by: Robert Foss <robert.f...@linaro.org&gt;