[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACRpkdZpiQ7E_v-Gfk6vFcUEiMazvixYaL0ksKeP=Tq3O6Fh=Q@mail.gmail.com>
Date: Tue, 26 May 2020 13:32:25 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: Ondrej Jirman <megous@...ous.com>
Cc: linux-sunxi <linux-sunxi@...glegroups.com>,
Thierry Reding <thierry.reding@...il.com>,
Sam Ravnborg <sam@...nborg.org>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
Rob Herring <robh+dt@...nel.org>,
Maxime Ripard <mripard@...nel.org>,
Chen-Yu Tsai <wens@...e.org>, Icenowy Zheng <icenowy@...c.io>,
"open list:DRM PANEL DRIVERS" <dri-devel@...ts.freedesktop.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Samuel Holland <samuel@...lland.org>,
Martijn Braam <martijn@...xit.nl>, Luca Weiss <luca@...tu.xyz>,
Bhushan Shah <bshah@....org>
Subject: Re: [PATCH v3 3/5] drm: panel: Add Xingbangda XBD599 panel (ST7703 controller)
Hi Ondrej,
I see you took over this driver submission from
Icenowy.
On Wed, May 13, 2020 at 11:24 PM Ondrej Jirman <megous@...ous.com> wrote:
> From: Icenowy Zheng <icenowy@...c.io>
>
> Xingbangda XBD599 is a 5.99" 720x1440 MIPI-DSI IPS LCD panel made by
> Xingbangda, which is used on PinePhone final assembled phones.
>
> It is based on Sitronix ST7703 LCD controller.
>
> Add support for it.
>
> Signed-off-by: Icenowy Zheng <icenowy@...c.io>
> Signed-off-by: Ondrej Jirman <megous@...ous.com>
(...)
> create mode 100644 drivers/gpu/drm/panel/panel-sitronix-st7703.c
Nice!
> + /*
> + * Init sequence was supplied by the panel vendor.
> + */
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETEXTC,
> + 0xF1, 0x12, 0x83);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETMIPI,
> + 0x33, 0x81, 0x05, 0xF9, 0x0E, 0x0E, 0x20, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x44, 0x25,
> + 0x00, 0x91, 0x0a, 0x00, 0x00, 0x02, 0x4F, 0x11,
> + 0x00, 0x00, 0x37);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETPOWER_EXT,
> + 0x25, 0x22, 0x20, 0x03);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETRGBIF,
> + 0x10, 0x10, 0x05, 0x05, 0x03, 0xFF, 0x00, 0x00,
> + 0x00, 0x00);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETSCR,
> + 0x73, 0x73, 0x50, 0x50, 0x00, 0xC0, 0x08, 0x70,
> + 0x00);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETVDC, 0x4E);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETPANEL, 0x0B);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETCYC, 0x80);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETDISP, 0xF0, 0x12, 0xF0);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETEQ,
> + 0x00, 0x00, 0x0B, 0x0B, 0x10, 0x10, 0x00, 0x00,
> + 0x00, 0x00, 0xFF, 0x00, 0xC0, 0x10);
> + dsi_dcs_write_seq(dsi, 0xC6, 0x01, 0x00, 0xFF, 0xFF, 0x00);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETPOWER,
> + 0x74, 0x00, 0x32, 0x32, 0x77, 0xF1, 0xFF, 0xFF,
> + 0xCC, 0xCC, 0x77, 0x77);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETBGP, 0x07, 0x07);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETVCOM, 0x2C, 0x2C);
> + dsi_dcs_write_seq(dsi, 0xBF, 0x02, 0x11, 0x00);
> +
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETGIP1,
> + 0x82, 0x10, 0x06, 0x05, 0xA2, 0x0A, 0xA5, 0x12,
> + 0x31, 0x23, 0x37, 0x83, 0x04, 0xBC, 0x27, 0x38,
> + 0x0C, 0x00, 0x03, 0x00, 0x00, 0x00, 0x0C, 0x00,
> + 0x03, 0x00, 0x00, 0x00, 0x75, 0x75, 0x31, 0x88,
> + 0x88, 0x88, 0x88, 0x88, 0x88, 0x13, 0x88, 0x64,
> + 0x64, 0x20, 0x88, 0x88, 0x88, 0x88, 0x88, 0x88,
> + 0x02, 0x88, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETGIP2,
> + 0x02, 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x02, 0x46, 0x02, 0x88,
> + 0x88, 0x88, 0x88, 0x88, 0x88, 0x64, 0x88, 0x13,
> + 0x57, 0x13, 0x88, 0x88, 0x88, 0x88, 0x88, 0x88,
> + 0x75, 0x88, 0x23, 0x14, 0x00, 0x00, 0x02, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x0A,
> + 0xA5, 0x00, 0x00, 0x00, 0x00);
> + dsi_dcs_write_seq(dsi, ST7703_CMD_SETGAMMA,
> + 0x00, 0x09, 0x0D, 0x23, 0x27, 0x3C, 0x41, 0x35,
> + 0x07, 0x0D, 0x0E, 0x12, 0x13, 0x10, 0x12, 0x12,
> + 0x18, 0x00, 0x09, 0x0D, 0x23, 0x27, 0x3C, 0x41,
> + 0x35, 0x07, 0x0D, 0x0E, 0x12, 0x13, 0x10, 0x12,
> + 0x12, 0x18);
> + msleep(20);
This stuff is really hard or impossible to understand without the
datasheet.
In my previous review I wrote:
It appears that the Himax HX8363 is using the same display controller
if you look at the datasheet:
http://www.datasheet-pdf.com/PDF/HX8369-A-Datasheet-Himax-729024
There you find an explanation to some of the commands.
That means, try to get rid of as much of the magic bytes as you can
and use proper #defines. I know it takes some work but the result
is so much more useful and readable.
Further I wrote:
You should definately insert code to read the MTP bytes:
0xDA manufacturer
0xDB driver version
0xDC LCD module/driver
And print these, se e.g. my newly added NT35510 driver or
the Sony ACX424AKP driver.
So please do that.
Yours,
Linus Walleij
Powered by blists - more mailing lists