On Mon, Feb 04, 2019 at 10:27:09AM -0600, Rob Herring wrote:
On Mon, Feb 4, 2019 at 2:24 AM Thierry Reding [off-list ref] wrote:
quoted
On Mon, Feb 04, 2019 at 12:13:55AM -0800, Vasily Khoruzhick wrote:
quoted
On Sun, Feb 3, 2019 at 11:43 PM Thierry Reding [off-list ref] wrote:
quoted
On Sun, Feb 03, 2019 at 10:54:57AM -0800, Vasily Khoruzhick wrote:
quoted
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 <redacted>
---
.../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.
simple-panel would probably work if you stuck in some mostly compatible
string and provided a ddc-i2c-bus property in the device tree node. The
generic-ish fallback case could be implemented by providing a fallback
compatible string (we used to have "simple-panel", which I think would
still be adequate for this I suppose) and adding a dummy descriptor in
the driver, perhaps one with pre-defined delays that could be adjusted
to work for all cases, or they could just be 0. At least that way we'd
be explicitly documenting that we support this as a fallback.
I'm not sure how you'd want to enforce that people provide the right
compatible strings that way. Currently there's no way to make your panel
work without adding a panel driver, so people are forced to write the
DT bindings and a driver, or add support to an existing one. If we open
this backdoor, I suspect many people will just take the easy route and
rely on the fallback. And then what do we do when we get bug reports
about panels not working, or requiring some quirks. How do we go and
find out what the right compatible strings are for each board, or how to
fix things for something we don't have access to.
This all sounds like a Pandora's box to me.
Thierry
On Mon, Feb 4, 2019 at 8:56 AM Thierry Reding [off-list ref] wrote:
quoted
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.
simple-panel would probably work if you stuck in some mostly compatible
string and provided a ddc-i2c-bus property in the device tree node. The
generic-ish fallback case could be implemented by providing a fallback
compatible string (we used to have "simple-panel", which I think would
still be adequate for this I suppose) and adding a dummy descriptor in
the driver, perhaps one with pre-defined delays that could be adjusted
to work for all cases, or they could just be 0. At least that way we'd
be explicitly documenting that we support this as a fallback.
I'm not sure how you'd want to enforce that people provide the right
compatible strings that way. Currently there's no way to make your panel
work without adding a panel driver, so people are forced to write the
DT bindings and a driver, or add support to an existing one. If we open
this backdoor, I suspect many people will just take the easy route and
rely on the fallback. And then what do we do when we get bug reports
about panels not working, or requiring some quirks. How do we go and
find out what the right compatible strings are for each board, or how to
fix things for something we don't have access to.
This all sounds like a Pandora's box to me.
OK, just give me an option that will work on this platform with a
single software image (keep in mind that u-boot aka "firmware" is part
of this image) and that is acceptable for upstream and I'll try to
implement it.
Thierry
On Mon, Feb 4, 2019 at 10:56 AM Thierry Reding [off-list ref] wrote:
On Mon, Feb 04, 2019 at 10:27:09AM -0600, Rob Herring wrote:
quoted
On Mon, Feb 4, 2019 at 2:24 AM Thierry Reding [off-list ref] wrote:
quoted
On Mon, Feb 04, 2019 at 12:13:55AM -0800, Vasily Khoruzhick wrote:
quoted
On Sun, Feb 3, 2019 at 11:43 PM Thierry Reding [off-list ref] wrote:
quoted
On Sun, Feb 03, 2019 at 10:54:57AM -0800, Vasily Khoruzhick wrote:
quoted
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.
simple-panel would probably work if you stuck in some mostly compatible
string and provided a ddc-i2c-bus property in the device tree node. The
generic-ish fallback case could be implemented by providing a fallback
compatible string (we used to have "simple-panel", which I think would
still be adequate for this I suppose) and adding a dummy descriptor in
the driver, perhaps one with pre-defined delays that could be adjusted
to work for all cases, or they could just be 0. At least that way we'd
be explicitly documenting that we support this as a fallback.
I'd like something more specific than 'simple-panel' that at least
implies it has EDID and whatever else we think it should imply.
Looking into this a bit more, why don't we just do a connector here?
eDP has a standard connector (with power). It's just like other
connectors, but with power and no hotplug. If someone does their own
interface, then they should do their own connector or panel binding.
I'm not sure how you'd want to enforce that people provide the right
compatible strings that way. Currently there's no way to make your panel
work without adding a panel driver, so people are forced to write the
DT bindings and a driver, or add support to an existing one. If we open
this backdoor, I suspect many people will just take the easy route and
rely on the fallback. And then what do we do when we get bug reports
about panels not working, or requiring some quirks. How do we go and
find out what the right compatible strings are for each board, or how to
fix things for something we don't have access to.
We'll simply reject anything that should be implied by compatible. And
now we can enforce with schema that DTs aren't populated with
fallbacks alone. Of course, we can't make anyone pass all schema
checks before shipping a dtb.
Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel