On Tue, Nov 8, 2022 at 3:53 PM Jagan Teki <jagan@xxxxxxxxxx> wrote: > On Tue, 8 Nov 2022 at 20:18, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > On Tue, Nov 8, 2022 at 3:12 PM Jagan Teki <jagan@xxxxxxxxxx> wrote: > > > On Tue, 8 Nov 2022 at 19:31, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > > > > > > > On Thu, Nov 3, 2022 at 3:12 PM Jagan Teki <jagan@xxxxxxxxxx> wrote: > > > > > > > > > Jadard JD9365DA-H3 is WXGA MIPI DSI panel and it support TFT > > > > > dot matrix LCD with 800RGBx1280 dots at maximum. > > > > > > > > > > Add support for it. > > > > > > > > > > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > > > > > Signed-off-by: Jagan Teki <jagan@xxxxxxxxxx> > > > > > --- > > > > > Changes for v3: > > > > > - updatd to WXGA > > > > > - use JD9365DA_CMD_DCS and JD9365DA_CMD_DELAY > > > > > > > > My comments on v2 have not been addressed, for example I asked to > > > > remove the delay from sequences and just use an explicit delay and > > > > to then use the existing sequence sending macro. > > > > > > True, I responded on the same day [1], since I didn't get the reply I > > > have posted by assuming my comment is valid. Would you please check > > > and respond? > > > > > > [1] https://lore.kernel.org/all/CA+VMnFz0w-6O=wt3iuJo1BhQgPZ2XbpX6JdDz6vg_JW9nHTR2A@xxxxxxxxxxxxxx/ > > > > OK I see, sorry for not reading close. > > > > The driver just supports one single variant. > > > > What you are doing is preparing the ground for more variants > > that may or may not exist. This creates the antipattern "big upfront design" > > i.e. abstractions added for things that do not yet exist. > > > > I think it is better to strip it down to just open coding the delay after > > the init sequence. When the next variant appears, if ever, they can > > add abstraction. Maybe they need the same delay in the same > > place? Who knows... > > I understand your point, but delays are strictly mentioned by the > panel vendor init sequence, cz101b4001 do you think adding in the > generic or common code is still valid since we have code added for > cz101b4001 specifically via driver data? I would instead of encoding a sequence into the driver data encode a per-variant callback that sends the sequence (which can then be just static array) and then ends with an explicit delay. Yours, Linus Walleij
