Received-SPF: pass (google.com: domain of linux-kernel+bounces-140074-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240410071439.2152588-1-yangcong5@huaqin.corp-partner.google.com>
 <20240410071439.2152588-3-yangcong5@huaqin.corp-partner.google.com> <CAD=FV=V2J=Tth2zhpo-kPo4uvESt70mFneO2V6TV-haac0VZuQ@mail.gmail.com>
In-Reply-To: <CAD=FV=V2J=Tth2zhpo-kPo4uvESt70mFneO2V6TV-haac0VZuQ@mail.gmail.com>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Thu, 11 Apr 2024 10:25:30 +0200
Message-ID: <CACRpkdYtM=5jdQddCqRFgBRXvcJEjk1ULJNKKFz7jhhkGxV59Q@mail.gmail.com>
Subject: Re: [PATCH v1 2/4] drm/panel: boe-tv101wum-nl6: Support for BOE
 nv110wum-l60 MIPI-DSI panel
To: Doug Anderson <dianders@chromium.org>
Cc: Cong Yang <yangcong5@huaqin.corp-partner.google.com>, sam@ravnborg.org, 
	neil.armstrong@linaro.org, daniel@ffwll.ch, airlied@gmail.com, 
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, 
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 11, 2024 at 9:40=E2=80=AFAM Doug Anderson <dianders@chromium.or=
g> wrote:
> On Wed, Apr 10, 2024 at 12:15=E2=80=AFAM Cong Yang
> <yangcong5@huaqin.corp-partner.google.com> wrote:
> >
> > The BOE nv110wum-l60 is a 11.0" WUXGA TFT LCD panel, which fits in nice=
ly
> > with the existing panel-boe-tv101wum-nl6 driver. Hence, we add a new
> > compatible with panel specific config.
>
> I guess we have the same question we've had with this driver in the
> past: do we add more tables here, or do we break this out into a
> separate driver like we ended up doing with "ili9882t". I guess the
> question is: what is the display controller used with this panel and
> is it the same (or nearly the same) display controller as other panels
> in this driver or is it a completely different display controller.
> Maybe you could provide this information in the commit message to help
> reviewers understand.

I think at a minimum we need to split out any identifiable display controll=
ers
to their own drivers.

Then what developers see is that the code sequence is very similar
between two completely different display controllers so they have this
urge to shoehorn several displays into the same driver for this
reason.

The latter is not good code reuse, what we need to do here is to split
out a sequencing library, like if we had
drivers/gpu/drm/panel/cmd-seqence-lib.c|.h with a bool Kconfig and
some helpful symbols to do the same seqences in different drivers,
so the same order can be obtained in different display controller
drivers that would be great.

I'm thinking something along the line of

panel_seq_exit_sleep_mode(unsigned int delay_after_exit_sleep,
    u8 *cmd_seq_after_exit_sleep,
    unsigned int delay_after_cmd_seq,
    unsigned int delay_after_set_display_on);

That will call mipi_dsi_dcs_exit_sleep_mode(), delay, send
command sequence, delay, call mipi_dsi_dcs_set_display_on()
and delay where any delay can be 0.

This achieves the same goal without messing up the whole place,
but requires some tinkering with how to pass a sequence the right
way etc.

Are Google & partners interested in the job? ;)

Yours,
Linus Walleij