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

[cong yang]

Hi,

Linus Walleij <linus.walleij@xxxxxxxxxx> 于2024年4月11日周四 16:25写道：

>
> On Thu, Apr 11, 2024 at 9:40 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
> > On Wed, Apr 10, 2024 at 12:15 AM Cong Yang
> > <yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> 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.
>
> I think at a minimum we need to split out any identifiable display controllers
> to their own drivers.
>
> Then what developers see is that the code sequence is very similar
> between two completely different display controllers so they have this
> urge to shoehorn several displays into the same driver for this
> reason.
>
> The latter is not good code reuse, what we need to do here is to split
> out a sequencing library, like if we had
> drivers/gpu/drm/panel/cmd-seqence-lib.c|.h with a bool Kconfig and
> some helpful symbols to do the same seqences in different drivers,
> so the same order can be obtained in different display controller
> drivers that would be great.
>
> I'm thinking something along the line of
>
> panel_seq_exit_sleep_mode(unsigned int delay_after_exit_sleep,
>     u8 *cmd_seq_after_exit_sleep,
>     unsigned int delay_after_cmd_seq,
>     unsigned int delay_after_set_display_on);
>
> That will call mipi_dsi_dcs_exit_sleep_mode(), delay, send
> command sequence, delay, call mipi_dsi_dcs_set_display_on()
> and delay where any delay can be 0.
>
> This achieves the same goal without messing up the whole place,
> but requires some tinkering with how to pass a sequence the right
> way etc.
>
> Are Google & partners interested in the job? ;)
>
> Yours,
> Linus Walleij

I learned from himax that even if the same controller is used with
different glasses, the corresponding parameters are not fixed.

For example: _INIT_DCS_CMD(0xB9, 0x83, 0x10, 0x21, 0x55, 0x00),

even in the group initial code, the same register will be loaded with
parameters twice.

 example is the same“0xB4” , but the specific implementation functions
are also different.
_INIT_DCS_CMD(0xB4, 0x35, 0x35, 0x43, 0x43, 0x35, 0x35, 0x30, 0x7A,
0x30, 0x7A, 0x01, 0x9D),
 . .
 . .
_INIT_DCS_CMD(0xB4, 0x03, 0xFF, 0xF8),

So assuming that the registers of the two screens is the same now,
it cannot be set as a common parameter.
Otherwise, it may be a bit troublesome for the maintainers.

If necessary, I can break out starry_himax83102_j02, boe_nv110wum and
ivo_t109nw41
as separate driver. Then add some define to these registers.

#define HX83102_SETPOWER 0xb1
#define HX83102_SETDISP 0xb2
#define HX83102_SETCYC 0xb4
#define HX83102_SETEXTC 0xb9
#define HX83102_SETMIPI 0xba
#define HX83102_SETVDC 0xbc
#define HX83102_SETBANK 0xbd
#define HX83102_SETPTBA 0xbf
#define HX83102_SETSTBA 0xc0
#define HX83102_SETTCON 0xc7
#define HX83102_SETRAMDMY 0xc8
#define HX83102_SETPWM 0xc9
#define HX83102_SETCLOCK 0xcb
#define HX83102_SETPANEL 0xcc
#define HX83102_SETCASCADE 0xd0
#define HX83102_SETPCTRL 0xd1
#define HX83102_SETGIP0 0xd3
#define HX83102_SETGIP1 0xd5
#define HX83102_SETGIP3 0xd8
#define HX83102_SETTP1 0xe7
#define HX83102_SETSPCCMD 0xe9

Thanks！