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

[Linus Walleij <linus.walleij@linaro.org>]

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

Yours,
Linus Walleij