Re: [PATCH RESEND v2 08/12] dt-bindings: add binding for generic eDP panel

From: Rob Herring <robh+dt@kernel.org>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree <devicetree@vger.kernel.org>,
	David Airlie <airlied@linux.ie>,
	linux-sunxi <linux-sunxi@googlegroups.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Vasily Khoruzhick <anarsoul@gmail.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Chen-Yu Tsai <wens@csie.org>, Sean Paul <seanpaul@chromium.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	arm-linux <linux-arm-kernel@lists.infradead.org>,
	Icenowy Zheng <icenowy@aosc.io>
Subject: Re: [PATCH RESEND v2 08/12] dt-bindings: add binding for generic eDP panel
Date: Mon, 4 Feb 2019 10:27:09 -0600	[thread overview]
Message-ID: <CAL_JsqKt1kOXmRNvkYsQ6AL69+TPZURKpDbKkkEUb2+KZ5hKjQ@mail.gmail.com> (raw)
In-Reply-To: <20190204082353.GE19087@ulmo>

On Mon, Feb 4, 2019 at 2:24 AM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Mon, Feb 04, 2019 at 12:13:55AM -0800, Vasily Khoruzhick wrote:
> > On Sun, Feb 3, 2019 at 11:43 PM Thierry Reding <thierry.reding@gmail.com> wrote:
> > >
> > > On Sun, Feb 03, 2019 at 10:54:57AM -0800, Vasily Khoruzhick wrote:
> > > > eDP panels usually have EDID EEPROM, so there's no need to define panel
> > > > width/height or any modes/timings in dts. But this panel still may have
> > > > regulator and/or backlight.
> > > >
> > > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > > > ---
> > > >  .../devicetree/bindings/display/panel/panel-edp.txt        | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-edp.txt
> > >
> > > Please don't try to make panels look more generic than they really are.
> > > You're going to have to provide a compatible string for your device that
> > > is more specific than "panel-edp". You claim that you don't need any
> > > extra information that is panel specific, but you don't know that now.
> > > We have in the past thought that we didn't need things like prepare
> > > delay, but then we ran into situations where we did need them.
> > >
> > > Just do what everybody else does. Provide a specific compatible string
> > > and match on that in the panel-simple driver. Even if you can read all
> > > the video timings from an EDID EEPROM, you can still provide a mode in
> > > the panel descriptor to serve as a fallback if for example the EEPROM
> > > is faulty on some device.
> >
> > Pinebook used several 768p panels that have slightly different timings
> > and recent batch uses 1080p panel.
> >
> > What panel descriptor should I use as fallback?
>
> You don't use panel descriptors as fallback. The simple-panel driver
> will bind to a panel device and use the corresponding descriptor. If
> your device tree contains the correct information, the descriptor is
> correct for the panel you have.
>
> In other words you need to ensure that you have the correct panel in
> device tree for the board that you're using. This is exactly the same
> thing as for other devices.
>
> One way to to this is to have separate device trees for each variant
> of the board that you want to support. Another variant may be to have
> a common device tree and then have some early firmware update the DTB
> with the correct panel information.

That can be a pain to manage especially if panels are swapped run to
run with 2nd sources.

I think it is perfectly fine to have a generic-ish fallback as long as
it is just that, a fallback. If the panel has quirks, then you'd
better make sure the firmware is stuffing in the right compatibles or
that you can update the firmware.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel