[U-Boot] [PATCH 0/4] tegra: Move tegra20 towards the 'new' display bindings

[Simon Glass]

Hi,

On 20 January 2016 at 08:37, Thierry Reding <thierry.reding at gmail.com> wrote:
> On Tue, Jan 19, 2016 at 09:47:11AM -0700, Stephen Warren wrote:
>> On 01/18/2016 06:58 PM, Simon Glass wrote:
>> >Hi Stephen,
>> >
>> >On 18 January 2016 at 12:52, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> >>On 01/14/2016 04:12 PM, Simon Glass wrote:
>> >>>
>> >>>Hi Lucas,
>> >>>
>> >>>On 14 January 2016 at 13:34, Lucas Stach <dev at lynxeye.de> wrote:
>> >>>>
>> >>>>Am Donnerstag, den 14.01.2016, 13:26 -0700 schrieb Simon Glass:
>> >>>>>
>> >>>>>The original tegra20 display driver was written before Linux had
>> >>>>>device tree
>> >>>>>bindings for display. Since then Linux has developed a robust set of
>> >>>>>bindings
>> >>>>>covering various aspects of enabling a display.
>> >>>>>
>> >>>>>This series moves closer to those bindings by using the panel and
>> >>>>>backlight
>> >>>>>as separate drivers. The device tree files for seaboard, ventana and
>> >>>>>harmony
>> >>>>>thereby become almost the same as Linux.
>> >>>>>
>> >>>>>Unfortunately this breaks the other boards, which will need a similar
>> >>>>>sync.
>> >>>>>So I'm not sure how easy it will be to accept this series. Still, it
>> >>>>>seems
>> >>>>>worth sending it out in the hope that board maintainers can help. I
>> >>>>>have
>> >>>>>kept this series separate so that it can progress separately.
>> >>>>>
>> >>>>By pushing display timings into the DT you are actually diverging from
>> >>>>mainline, as mainline doesn't require this, but instead infers the
>> >>>>timings from the panel compatible. Is this a desired goal?
>> >>>
>> >>>
>> >>>This is not divergence.
>> >>
>> >>
>> >>Really? The DT content is different. Isn't that the definition of
>> >>divergence?
>> >
>> >You might infer that from the movie, but divergence really means that
>> >things are getting further apart. So I think here we have convergence.
>> >
>> >>
>> >>>Please take a look at the patch series. The
>> >>>device tree files are very close to the same now. The existing U-Boot
>> >>>support has display timings in the device tree too, so this is not
>> >>>being added.
>> >>>
>> >>>The display timings are a small part of the work, but in the back of
>> >>>my mind is that we don't want to have a big table of display panel
>> >>>timings as exists in Linux. This is a waste of space when a board will
>> >>>only use one panel.
>> >>
>> >>
>> >>That was rather the point of the panel-specific compatible values: To force
>> >>the DT to contain a semantic definition of the type of panel, rather than a
>> >>"generic" definition of timings. A benefit of the semantic representation is
>> >>that if we later find bugs that need to be fixed on certain panels, if we
>> >>know the panel type, then bug fixes can be applied. Equally, if we enhance
>> >>the SW to require more data about the panel, that can be added to a driver
>> >>without the need to change the DT, thus allowing old DTs to continue to
>> >>work. More semantic rather than purely "syntactic" knowledge is available.
>> >>However, if we only have a generic timing definition (or other data suitable
>> >>for current SW features or code-paths), then panel-specific bug fixes will
>> >>never be possible since SW can't know the identify of the panel. The
>> >>disadvantage of requiring a mapping table between panel type and display
>> >>timings was considered reasonable for SW stacks at which DT was targeted
>> >>(i.e. main OSs rather than HW-specific bootloaders). Even so, to avoid the
>> >>bloat issue, you can always just #ifdef the mapping table and end up with
>> >>the same code size; even less perhaps since no timing DT parsing code is
>> >>required.
>> >>
>> >>At least, that was the reasoning when the DT bindings for Tegra panels were
>> >>first created; IIRC there was discussion of bindings for generic panels,
>> >>timings, panel power sequences, etc., and they were rejected for the reason
>> >>I explained above and perhaps others. However, it does seem someone has
>> >>changed their mind again given that the generic panel-timings binding does
>> >>exist now. This is one of the many things that sucks about DT; no decision
>> >>is ever kept, so consistency in design and implementation isn't possible:-(
>> >
>> >Sounds a bit contrived to me.
>>
>> I believe this approach has been taken after years of experience with DT and
>> having to retro-fit bug-fixes and new features into existing DTs.
>>
>> Still, you'd get the best background from the Linux kernel DT maintainers on
>> this topic, since they have experience with DT over more years than me. I'm
>> simply reporting the arguments they've presented.
>
> To be fair, the panel bindings came about after the display-timings
> binding. Perhaps the main reason why the panel bindings are what they
> are today is because I consider them devices no different at all from
> peripherals and the like. Back at the time, most of the pushback was
> because people believed that panels were somehow special in that they
> would simply require the video timings to make them work, and hence
> having some generic helpers to parse them from DT would be enough and
> is what many drivers ended up implementing.
>
> But it turned out that there's more to panels than just the video
> timings. Many panels do have a varying number of inputs that need to
> be controlled before you can use them. There are voltage supplies and
> reset or powerdown GPIOs, PWM inputs for backlight etc. Like Stephen
> said it was at some point attempted to describe this completely
> generically in DT using power sequences, but that effort turned out to
> be much too complicated even considering only a subset of relatively
> simple panels.
>
> Taking into account the various types of smart panels out there things
> get even more complicated. All of a sudden you need a sort of register
> programming sequence to enable or disable the panel. Of course with a
> generic panel binding you need to find a way to put that into the DT
> along with everything else. And then you need a way to specify where
> to write these registers in the enable/disable sequence, in some cases
> you might even need to split it up into several chunks, etc.
>
> So the end result was that while you had a generic way for describing
> the video timings, every driver had to implement its own support
> infrastructure for the various resources that you need to turn on. In
> fact this became quite the opposite of generic.
>
> Now when you consider panels to be devices and give them a proper
> compatible string things actually get almost trivial. Now each panel
> can have a binding that fully describes the supplies and GPIOs and the
> driver that binds to the device can contain the enable and disable
> sequences in code. This is completely normal for any other kind of
> device, why would panels need to be special? Now that a panel is simply
> a device like any other you can make things truly generic, create the
> abstractions you need and simply hook up outputs to panels and make
> them work together.
>
> There is still some commonality in many panels, so in Linux we ended up
> implementing a "simple" panel driver, which can drive panels that have a
> backlight, a power supply and an enable GPIO, with an optional DDC bus
> that can be used to read EDID. That driver supports 34 different panels
> at the time of this writing and it's still fairly small. But there is a
> slew of options to reduce that size even further if you really need to.
> You could for example have Kconfig options for panel vendors, or even
> per-panel Kconfig options to minimize the data used for the panel. I
> suspect that you might end up with even less data than the equivalent DT
> content.

So far I've just implemented simple-panel, but the option is there to
create your own driver in UCLASS_PANEL if needed. I have not needed
any special panel features yet, but we'll see how it goes. I suppose a
lot of panels are pretty simple/similar.

I didn't see any delay stuff in Linux - or at least it is there but
was zero for seaboard. So probably I was worrying about that
unnecessarily.

Regards,
Simon