linux-kernel - Re: [PATCH v1 2/4] drm/panel: boe-tv101wum-nl6: Support for BOE nv110wum-l60 MIPI-DSI panel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHwB_NLgZDwX4PSrSDH+qPkjq=060Vx35BdX76iX3mpMRkHDcQ@mail.gmail.com>
Date: Thu, 11 Apr 2024 16:13:57 +0800
From: cong yang <yangcong5@...qin.corp-partner.google.com>
To: Doug Anderson <dianders@...omium.org>
Cc: LinusW <linus.walleij@...aro.org>, sam@...nborg.org, neil.armstrong@...aro.org, 
	daniel@...ll.ch, airlied@...il.com, robh+dt@...nel.org, 
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, 
	dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/4] drm/panel: boe-tv101wum-nl6: Support for BOE
 nv110wum-l60 MIPI-DSI panel

Hi,

Doug Anderson <dianders@...omium.org> 于2024年4月11日周四 15:48写道：
>
> Hi,
>
> On Wed, Apr 10, 2024 at 12:15 AM Cong Yang
> <yangcong5@...qin.corp-partner.google.com> wrote:
> >
> > The BOE nv110wum-l60 is a 11.0" WUXGA TFT LCD panel, which fits in nicely
> > with the existing panel-boe-tv101wum-nl6 driver. Hence, we add a new
> > compatible with panel specific config.
>
> I guess we have the same question we've had with this driver in the
> past: do we add more tables here, or do we break this out into a
> separate driver like we ended up doing with "ili9882t". I guess the
> question is: what is the display controller used with this panel and
> is it the same (or nearly the same) display controller as other panels
> in this driver or is it a completely different display controller.
> Maybe you could provide this information in the commit message to help
> reviewers understand.

okay, I will add detailed information in V2 patch.Thanks.
>
>
> > Signed-off-by: Cong Yang <yangcong5@...qin.corp-partner.google.com>
> > ---
> >  .../gpu/drm/panel/panel-boe-tv101wum-nl6.c    | 115 ++++++++++++++++++
> >  1 file changed, 115 insertions(+)
>
> Maybe add Linus W to your patches since he has had opinions on this
> driver in the past. I've added him as CC here but you should make sure
> to CC him on future versions unless he says not to. ;-)

Got it,thanks.

>
>
> > diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> > index 0ffe8f8c01de..f91827e1548c 100644
> > --- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> > +++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> > @@ -1368,6 +1368,91 @@ static const struct panel_init_cmd starry_himax83102_j02_init_cmd[] = {
> >         {},
> >  };
> >
> > +static const struct panel_init_cmd boe_nv110wum_init_cmd[] = {
> > +       _INIT_DELAY_CMD(60),
> > +       _INIT_DCS_CMD(0xB9, 0x83, 0x10, 0x21, 0x55, 0x00),
>
> Given that the first command of "(0xB9, 0x83, 0x10, 0x21, 0x55, 0x00)"
> seems to be the same as "starry_himax83102_j02" maybe those two are
> the same controller? I'm just guessing, but if those are the same
> controller as the two new ones you're adding in this series, maybe all
> 3 of them should be in their own driver? Maybe we can do something to
> make more sense of some of these commands too? There certainly seem to
> be a lot of commonalities in the init sequences of all 3 and if we can
> define the init sequence more logically then we can share more of the
> code between the different panels and we don't have a giant duplicated
> blob.

Yes, your guess is correct. boe_nv110wum and ivo_t109nw41 and
starry_himax83102_j02
are the same controller (himax83102). They are equipped with different
glass panels (BOE/IVO/starry),
so there will be some differences in initial code and porch.

>
>
> > +       _INIT_DCS_CMD(0xB9, 0x00, 0x00, 0x00),
> > +       _INIT_DELAY_CMD(50),
> > +       _INIT_DCS_CMD(0x11),
> > +       _INIT_DELAY_CMD(110),
> > +       _INIT_DCS_CMD(0x29),
> > +       _INIT_DELAY_CMD(25),
> > +       {},
> > +};
> >  static inline struct boe_panel *to_boe_panel(struct drm_panel *panel)
>
> nit: should have a blank line between the end of your struct and the
> next function.

Got it,thanks.

>
>
> > +static const struct panel_desc boe_nv110wum_desc = {
> > +       .modes = &boe_tv110wum_default_mode,
> > +       .bpc = 8,
> > +       .size = {
> > +               .width_mm = 147,
> > +               .height_mm = 235,
> > +       },
> > +       .lanes = 4,
> > +       .format = MIPI_DSI_FMT_RGB888,
> > +       .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> > +                     MIPI_DSI_MODE_LPM,
> > +       .init_cmds = boe_nv110wum_init_cmd,
> > +       .lp11_before_reset = true,
> > +};
> >  static int boe_panel_get_modes(struct drm_panel *panel,
> >                                struct drm_connector *connector)
>
> nit: should have a blank line between the end of your struct and the
> next function.
>
>
> > @@ -1973,6 +2085,9 @@ static const struct of_device_id boe_of_match[] = {
> >         { .compatible = "starry,himax83102-j02",
> >           .data = &starry_himax83102_j02_desc
> >         },
> > +       { .compatible = "boe,nv110wum-l60",
> > +         .data = &boe_nv110wum_desc
> > +       },
>
> nit: the existing panels that are supported are sorted alphabetically.
> Please sort things alphabetically throughout your patch series.

Got it, fx net patch. Thanks.

>
> -Doug