[PATCH v3 4/4] drm: panel: Add Jadard JD9365DA-H3 DSI panel

[Jagan Teki]

On Tue, 8 Nov 2022 at 20:33, Linus Walleij <linus.walleij at linaro.org> wrote:
>
> On Tue, Nov 8, 2022 at 3:53 PM Jagan Teki <jagan at edgeble.ai> wrote:
> > On Tue, 8 Nov 2022 at 20:18, Linus Walleij <linus.walleij at linaro.org> wrote:
> > > On Tue, Nov 8, 2022 at 3:12 PM Jagan Teki <jagan at edgeble.ai> wrote:
> > > > On Tue, 8 Nov 2022 at 19:31, Linus Walleij <linus.walleij at linaro.org> wrote:
> > > > >
> > > > > On Thu, Nov 3, 2022 at 3:12 PM Jagan Teki <jagan at edgeble.ai> 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 at lists.freedesktop.org
> > > > > > Signed-off-by: Jagan Teki <jagan at edgeble.ai>
> > > > > > ---
> > > > > > 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@mail.gmail.com/
> > >
> > > 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.

Okay. I will wrap up that delay to make it explicit and update the dcs
packet send sequence.

Thanks,
Jagan.