[PATCH 2/2] dt-bindings: add simple-panel-dsi and simple-panel

On Wed, Jul 27, 2016 at 11:03:05AM +0800, Mark yao wrote:

On 2016?07?26? 17:02, Thierry Reding wrote:

quoted

On Tue, Jul 26, 2016 at 10:01:32AM +0800, Mark yao wrote:

quoted

On 2016?07?25? 23:21, Thierry Reding wrote:

     On Wed, Jul 20, 2016 at 11:18:50AM +0800, Mark Yao wrote:

         Allow user add display timing on device tree with simple-panel-dsi
         or simple-panel.

         Cc: Thierry Reding [off-list ref]
         Cc: Rob Herring [off-list ref]
         Cc: Mark Rutland [off-list ref]

         Signed-off-by: Mark Yao [off-list ref]
         ---
          .../bindings/display/panel/simple-panel.txt        | 48 ++++++++++++++++++++++
          1 file changed, 48 insertions(+)

     Sorry, not going to happen. Read this for an explanation of why not:

             https://sietch-tagr.blogspot.de/2016/04/display-panels-are-not-special.html

     Thierry


Hi Thierry

The blog actually not persuade me why can't use display timing on
device tree.

Okay, perhaps read it again, it addresses most of your points below.

quoted

1, Binding panel as a simple string on device tree seems simple on device tree,
but it's complex on kernel code, and kernel code would became bigger and
bigger.

I don't think the video timings in the simple-panel driver are very
complex. They also don't use very much space. And if you're really
concerned about space you can always use conditional compilation and
Kconfig symbols to remove timings for panels that you don't use.

Also, panels are characterized by much more than just video timings.
There were attempts, way back, to fully describe panels in device tree
and that failed. What you propose here is a partial solution to a much
more complex problem.

This is all explained in the blog post.

quoted

2, Our customer always ask me, where is the display timing? They only find a
simple panel string on device tree,  need search the kernel code to find the
actually timing. They are used to find all device info on device tree, but
panel timing info is not, this would confuse them. They don't want to know how
code work, just want a easier interface.

That's not a very good argument. There's plenty of data that's not in
device tree for other devices, why should panels be different? Also, I
would hope that any customer of yours knows their way around kernel
code and can therefore easily add video timings for new panels. It's
quite trivial to do, and there are many examples on how to do it.

quoted

3, I think device tree not only can use for kernel, other module also can use
it. on our project, we use uboot + kernel, the uboot support fdt, that function
can parsing device tree. So if describe the display timing on device tree, both
uboot and kernel can share same display timing, not need to describe twice, it
would save work and not easy to make mistake.

That's a bit of a stretch. Video timings is fairly straightforward data
and can be easily added to any other piece of code that you want to run.
Yes you will have to duplicate the data, but how is that different from
duplicating all the driver code?

quoted

4, For differentiation product, we face many different panel, every once in a
while, need to add a new panel, we can't convert all the panel , code the panel
on kernel seems too bad, and the kernel image became bigger and bigger.

Why can't you convert all the panels? We already support a bunch of them
and haven't yet run into any problems. If you do encounter any issues
trying to port panels to the DRM panel infrastructure, please let me
know and I can help sort them out.

The kernel image size isn't a problem either. In any modern kernel the
video timing data in the panel driver is tiny compared to the rest.

quoted

Generally, Our customer don't want to do any modify on kernel, they just modify
device tree to bring up their device. Describe the panel timing on device tree,
would make customer easy to use and reuse it.

Yes, that would perhaps make it easier for them to bring up the device.
But soon after they'll notice that there are glitches when turning the
panel on and off, and then they'll realize that they can't fix that
using their simple device tree.

About the panel on and off, I don't think the panel-simple do the good
enough.

panel-simple only have one gpio and one regulator, and their sequence is
hard code, Why not a panel have two gpio or two regulator? On our project,
we find many customer don't use the RC to do panel reset, they directly use
gpio reset, so they need a gpio to do panel reset.

The driver is called panel-simple for a reason. It's not meant to cover
all possible use-cases, only the simple ones.

the device tree panel's on and off function is what the next step I want to
upstream, on our downstream kernel, we do like that:

panel {
    power_ctrl {
          power0 {
                  gpios = <xxx>;
                  delay,ms = <3>;
          }
          power1 {
                  regulator = <xxx>;
                  delay,ms = <3>;
          }
          power2 {
                  backlight = <xxx>;
                  delay,ms = <3>;
          }
    }
}
and on driver, power on sequence with power0->power1->power2, power down
with power2->power1->power0.
if user want to swap the power, can easy do that by  adjust dts power
sequence.

this method can easy order the gpio, regulator, backlight sequence, judge
the delay time and add new regulator or gpio.
I think this panel power on and off method is better than panel-simple
driver currently using.

That's almost exactly like the power sequences that Alex Courbot had
proposed about three years ago. The concept of such a thing was rejected
by device tree binding maintainers, which is why we ended up with what
we have now. I'm sure you can find a link to the discussion if you want
the details about why it got rejected.

All of that's described in the blog, by the way.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160728/1e412141/attachment.sig>