<div dir="ltr"><div>Hi,Doug</div><div>   Thank you for <font color="#5f6368" face="Roboto, -apple-system, BlinkMacSystemFont, Segoe UI, Helvetica, Arial, sans-serif, Apple Color Emoji, Segoe UI Emoji, Segoe UI Symbol, Roboto, Arial, sans-serif"><span style="font-size:14px">reviewer ,new init code </span></font>match DSI <span style="color:rgb(80,0,80)">BURST mode </span> seems to support hs mode, the init register and porch settings are provided by novate.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Sep 14, 2021 at 3:23 AM Doug Anderson <<a href="mailto:dianders@google.com">dianders@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
On Mon, Sep 13, 2021 at 3:59 AM yangcong<br>
<<a href="mailto:yangcong5@huaqin.corp-partner.google.com" target="_blank">yangcong5@huaqin.corp-partner.google.com</a>> wrote:<br>
><br>
> This is an incell IC, TDDI use time division multiplexing.<br>
> Init code effect touch sensing.<br>
> Support touch function we needed to handle were:<br>
> a) Update init code for the panel driver, adjust the porch value.<br>
> b) After update init code these two panels can support DSI HS mode.<br>
><br>
> Signed-off-by: yangcong <<a href="mailto:yangcong5@huaqin.corp-partner.google.com" target="_blank">yangcong5@huaqin.corp-partner.google.com</a>><br>
> ---<br>
>  .../gpu/drm/panel/panel-boe-tv101wum-nl6.c    | 399 +++++-------------<br>
>  1 file changed, 110 insertions(+), 289 deletions(-)<br>
<br>
Please squash this with patch #3 in the series. Even though you have<br>
landed patch #3 in the series in a Chrome OS tree it doesn't matter.<br>
Since patch #3 hasn't landed upstream there is no reason to post the<br>
"wrong" code and then fix it up in the same series. The Chrome OS tree<br>
can always take a revert of the old version and a re-pick of the new<br>
version.<br>
<br>
<br>
> @@ -36,7 +36,6 @@ struct panel_desc {<br>
>         const struct panel_init_cmd *init_cmds;<br>
>         unsigned int lanes;<br>
>         bool discharge_on_disable;<br>
> -       bool unsupport_dsi_hs_mode;<br>
<br>
Not that I'm complaining, but how did this suddenly get supported?<br>
<br>
<br>
> @@ -84,6 +83,8 @@ static const struct panel_init_cmd boe_tv110c9m_init_cmd[] = {<br>
>         _INIT_DCS_CMD(0x07, 0x78),<br>
>         _INIT_DCS_CMD(0x08, 0x5A),<br>
>         _INIT_DCS_CMD(0x0D, 0x63),<br>
> +       _INIT_DCS_CMD(0x0E, 0x91),<br>
> +       _INIT_DCS_CMD(0x0F, 0x73),<br>
<br>
I _really_ don't like this opaque list of commands and the fact that<br>
apparently they aren't fixed but need to change depending on how<br>
you're configuring the panel, but I won't personally block this patch<br>
just because of it since it matches what other panels in this driver<br>
are doing.<br>
<br>
If someone more familiar with MIPI panels wants to chime in though,<br>
I'm all ears.<br>
<br>
<br>
> @@ -1442,15 +1260,15 @@ static int boe_panel_enable(struct drm_panel *panel)<br>
>  }<br>
><br>
>  static const struct drm_display_mode boe_tv110c9m_default_mode = {<br>
> -       .clock = 162383,<br>
> +       .clock = 166594,<br>
>         .hdisplay = 1200,<br>
>         .hsync_start = 1200 + 40,<br>
>         .hsync_end = 1200 + 40 + 8,<br>
>         .htotal = 1200 + 40 + 8 + 28,<br>
>         .vdisplay = 2000,<br>
>         .vsync_start = 2000 + 26,<br>
> -       .vsync_end = 2000 + 26 + 1,<br>
> -       .vtotal = 2000 + 26 + 1 + 94,<br>
> +       .vsync_end = 2000 + 26 + 2,<br>
> +       .vtotal = 2000 + 26 + 2 + 148,<br>
>         .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,<br>
>  };<br>
><br>
> @@ -1463,14 +1281,15 @@ static const struct panel_desc boe_tv110c9m_desc = {<br>
>         },<br>
>         .lanes = 4,<br>
>         .format = MIPI_DSI_FMT_RGB888,<br>
> -       .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |<br>
> -                     MIPI_DSI_MODE_LPM,<br>
> +       .mode_flags = MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_VIDEO<br>
> +                       | MIPI_DSI_MODE_VIDEO_HSE<br>
> +                       | MIPI_DSI_CLOCK_NON_CONTINUOUS<br>
> +                       | MIPI_DSI_MODE_VIDEO_BURST,<br>
>         .init_cmds = boe_tv110c9m_init_cmd,<br>
> -       .unsupport_dsi_hs_mode = true,<br>
>  };<br>
><br>
>  static const struct drm_display_mode inx_hj110iz_default_mode = {<br>
> -       .clock = 162383,<br>
> +       .clock = 166594,<br>
>         .hdisplay = 1200,<br>
>         .hsync_start = 1200 + 40,<br>
>         .hsync_end = 1200 + 40 + 8,<br>
> @@ -1478,7 +1297,7 @@ static const struct drm_display_mode inx_hj110iz_default_mode = {<br>
>         .vdisplay = 2000,<br>
>         .vsync_start = 2000 + 26,<br>
>         .vsync_end = 2000 + 26 + 1,<br>
> -       .vtotal = 2000 + 26 + 1 + 94,<br>
> +       .vtotal = 2000 + 26 + 1 + 149,<br>
<br>
It seems really odd that the two panels have _exactly_ the same timing<br>
except that one of them now has:<br>
<br>
.vsync_end = 2000 + 26 + 2,<br>
<br>
...and the other:<br>
<br>
.vsync_end = 2000 + 26 + 1,<br>
<br>
Do they really need to be different?<br>
<br>
-Doug<br>
</blockquote></div></div>