Hi Linus,
Le 23/07/2023 à 18:45, Linus Walleij a écrit :
Hi Arnaud & Molly,
overall the driver looks very good!
On Wed, Jul 19, 2023 at 5:20 PM Arnaud Ferraris
<arnaud.ferraris@xxxxxxxxxxxxx> wrote:
From: Molly Sophia<mollysophia379@xxxxxxxxx>
Novatek NT35596s is a generic DSI IC that drives command and video mode
panels. Add the driver for it. Currently add support for the LCD panel
from JDI connected with this IC, as found on Xiaomi Mi Mix2s phones.
Signed-off-by: Molly Sophia<mollysophia379@xxxxxxxxx>
Signed-off-by: Arnaud Ferraris<arnaud.ferraris@xxxxxxxxxxxxx>
(...)
.../gpu/drm/panel/panel-novatek-nt36672a.c | 251 ++++++++++++++++--
So are you sure the nt35596s panel driver is so similar to nt36672a that
they should share the same driver?
With all the magic number sequences I'm not so sure, do they really share
any of the magic numbers?
Their magic number sequences are quite different, however all the other
bits (power/reset sequences and timings) fit in, which isn't the case of
the other novatek panel drivers. Moreover, they have similar (although
not identical) resolutions.
If not, consider creating a new driver, and then base it on the
nt35510 instead.
I would gladly make this a new driver, but I'd rather base it on the
nt36672a for the reasons mentioned above. I do fear, however, that this
would lead to unnecessary code duplication, as 90% of the driver (magic
number sequences excluded) would be identical to the nt36672a (and to be
fully honest, I don't think I have the needed knowledge to make it
"better").
+static const struct nt36672a_panel_cmd jdi_nt35596s_video_on_cmds[] = {
+ { .data = { 0xff, 0x24 } },
+ { .data = { 0x9d, 0x34 } },
(...)
These are never nice. Do you have a datasheet so you can provide
defines for the magic hex values?
I found an old (2012) datasheet for the NT35596 (without the final "S")
which is marked "Draft", so I'm really unsure this will be enough to
make sense of those numbers.
The construction with these .data seqence array
should be replaces with some open coded sequences
I feel, bit this pattern is already in the driver so I guess it's OK.
Yours,
Linus Walleij
Cheers,
Arnaud
