On Wed, Jul 27, 2016 at 11:03:05AM +0800, Mark yao wrote: > On 2016年07月26日 17:02, Thierry Reding wrote: > > On Tue, Jul 26, 2016 at 10:01:32AM +0800, Mark yao wrote: > > > 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 > > > Cc: Rob Herring > > > Cc: Mark Rutland > > > > > > Signed-off-by: Mark Yao > > > --- > > > .../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. > > > > > 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. > > > > > 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. > > > > > 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? > > > > > 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. > > > > > 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 = ; > delay,ms = <3>; > } > power1 { > regulator = ; > delay,ms = <3>; > } > power2 { > backlight = ; > 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