OK,I will modify the message. Thanks a lot.<div><br><br><div style="padding:2px 0;">---------------原始邮件---------------</div><div style="background:#f0f0f0;color:#212121;padding:8px;border-radius:4px"><div><b>发件人: </b>Robert Foss<robert.foss@linaro.org></div><div><b>发送时间: </b>2021-09-20(周一) 17:47</div><div><b>收件人: </b>Yunlongli<liyunlonga@uniontech.com></div><div><b>抄 送: </b>Phong LE<ple@baylibre.com>, Neil Armstrong<narmstrong@baylibre.com>, Andrzej Hajda<a.hajda@samsung.com>, David Airlie<airlied@linux.ie>, Daniel Vetter<daniel@ffwll.ch>, Laurent Pinchart<Laurent.pinchart@ideasonboard.com>, Jonas Karlman<jonas@kwiboo.se>, Jernej Skrabec<jernej.skrabec@gmail.com>, dri-devel<dri-devel@lists.freedesktop.org>, linux-kernel<linux-kernel@vger.kernel.org></div><div><b>主题: </b>Re: [PATCH] drm: bridge: it66121: Added it66121 chip external screen status judgment.</div></div>Hey Yunlongli,<br><br>Thanks for submitting this fix.<br><br>On Sat, 18 Sept 2021 at 05:51, Yunlongli <liyunlonga@uniontech.com> wrote:<br><br>The formatting of this commit message is a bit unusual, let's try to<br>change it to the normal formatting.<br><br>Remove the dot from the commit title:<br>"drm: bridge: it66121: Added it66121 chip external screen status<br>judgment." -> "drm: bridge: it66121: Added it66121 chip external<br>screen status judgment"<br><br><br>><br>> fix: Add further confirm if external screens are involved.<br><br>The "fix:" tag is not needed. However if this commit fixes a bug<br>introduced in an earlier commit a machine readable tag like the the<br>one below could be added after the commit message.<br><br>Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver")<br><br>><br>> log: In the actual tests,  the IT66121 chip sometimes misjudged whether<br>>      it had an external screen, so, reference the it66121_user_guid.pdf<br>>      about Audio/Video data is stable or not A typical initialization<br>>      of HDMI link should be based on interrupt signal and appropriate<br>>      register probing. Recommended flow is detailed in IT66121<br>>      Programming Guide. Simply put, the microcontroller should monitor<br>>      the HPD status first. Upon valid HPD event, move on to check<br>>      RxSENDetect register to see if the receiver chip is ready for<br>>      further handshaking. When RxSENDetect is asserted, start reading EDID<br>>      data through DDC channels and carry on the rest of the handshaking<br>>      subsequently.If the micro-controller makes no use of the interrupt<br>>      signal as well as the above-mentioned status  registers, the link<br>>      establishment might fail. Please do follow the suggested<br>>      initialization flow recommended in IT66121 Programming Guide.<br>>      So, I add the IT66121_SYS_STATUS_SENDECTECT register status detection.<br>><br><br>The "log:" prefix is not needed, and neither is the indentation of the text.<br><br>Secondly maybe it would be nice to format the above chunk of text into<br>paragraphs just to make it easier to read.<br><br>> Signed-off-by: Yunlongli <liyunlonga@uniontech.com><br>> ---<br>>  drivers/gpu/drm/bridge/ite-it66121.c | 2 +-<br>>  1 file changed, 1 insertion(+), 1 deletion(-)<br>><br>> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c<br>> index 2f2a09adb4bc..9ed4fa298d11 100644<br>> --- a/drivers/gpu/drm/bridge/ite-it66121.c<br>> +++ b/drivers/gpu/drm/bridge/ite-it66121.c<br>> @@ -523,7 +523,7 @@ static bool it66121_is_hpd_detect(struct it66121_ctx *ctx)<br>>         if (regmap_read(ctx->regmap, IT66121_SYS_STATUS_REG, &val))<br>>                 return false;<br>><br>> -       return val & IT66121_SYS_STATUS_HPDETECT;<br>> +       return ((val & IT66121_SYS_STATUS_HPDETECT) && (val & IT66121_SYS_STATUS_SENDECTECT));<br>>  }<br>><br>>  static int it66121_bridge_attach(struct drm_bridge *bridge,<br>> --<br>> 2.20.1<br>><br>><br>><br><br>With the above suggestions fixed, feel free to add my r-b and submit a<br>v2 of this patch.<br>Reviewed-by: Robert Foss <robert.foss@linaro.org><br></div>