lkml.org 
[lkml]   [2023]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 2/2] drm/panel: Add driver for Novatek NT36523
    On Tue, Mar 07, 2023 at 11:34:55PM +0100, Linus Walleij wrote:
    > Hi Jianhua,
    >
    > thanks for your patch!
    >
    > It appears Konrad is working on a very similar driver, so I suggest merging
    > them into one Novatek NT36523 driver.
    >
    > Possibly we can fix this up first and then add Konrads Lenovo-panel with
    > a patch on top.
    >
    > On Mon, Feb 20, 2023 at 1:13 PM Jianhua Lu <lujianhua000@gmail.com> wrote:
    >
    > > Add a driver for panels using the Novatek NT36523 display driver IC.
    > >
    > > Signed-off-by: Jianhua Lu <lujianhua000@gmail.com>
    >
    > (...)
    >
    > I like how you abstract the panel with init commands in the panel info.
    >
    > > +enum dsi_cmd_type {
    > > + INIT_DCS_CMD,
    > > + DELAY_CMD,
    > > +};
    > > +
    > > +struct panel_init_cmd {
    > > + enum dsi_cmd_type type;
    > > + size_t len;
    > > + const char *data;
    > > +};
    > > +
    > > +#define _INIT_DCS_CMD(...) { \
    > > + .type = INIT_DCS_CMD, \
    > > + .len = sizeof((char[]){__VA_ARGS__}), \
    > > + .data = (char[]){__VA_ARGS__} }
    > > +
    > > +#define _INIT_DELAY_CMD(...) { \
    > > + .type = DELAY_CMD,\
    > > + .len = sizeof((char[]){__VA_ARGS__}), \
    > > + .data = (char[]){__VA_ARGS__} }
    >
    > I have seen this type of reinvented wheels a few times now. Don't do this.
    >
    > Look into other recently merged drivers and look how they do it, for example
    > drivers/gpu/drm/panel/panel-himax-hx8394.c
    >
    > For example:
    >
    > - Use mipi_dsi_dcs_write_seq()
    >
    > - If the delay is just used at one point in the sequence, do not invent
    > a command language like above for it, open code the delay instead
    >
    > - Try to decode as much magic as possible, if you look in Konrads
    > driver you clearly see some standard MIPI commands, I bet you have
    > some too.
    >
    > - Maybe use callbacks to send sequences instead of tables, like in
    > the himax driver?
    I use init cmd tables in order to send init sequences to dsi0 and dsi1 at the
    same time (sync dual dsi mode). I don't found a convenient way to use
    mipi_dsi_dcs_write_seq() for sync dual dsi mode.
    >
    > Other than that it seems like something that could also handle the Lenovo
    > display, or the other way around, I don't know which driver is the best
    > starting point, but this one has the right Novatek name at least.
    >
    > Yours,
    > Linus Walleij

    \
     
     \ /
      Last update: 2023-03-27 00:48    [W:6.336 / U:0.140 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site