[U-Boot] [PATCH 3/5] arm: tegra30: video: integrate display driver for t30

[Thierry Reding]

On Tue, Aug 25, 2015 at 10:03:13AM -0600, Simon Glass wrote:
> On 25 August 2015 at 05:02, Thierry Reding <treding at nvidia.com> wrote:
> > On Mon, Aug 24, 2015 at 10:58:48AM -0600, Simon Glass wrote:
> >> On 24 August 2015 at 04:12, Thierry Reding <treding at nvidia.com> wrote:
[...]
> >> > SOR is an even worse abstraction because it's completely Tegra-specific
> >> > and other SoCs will have completely different ways of providing the same
> >> > types of output. You'll end up with a uclass containing a single
> >> > implementation.
> >>
> >> But if it is a single implementation why do you need to add function
> >> pointers? It would just be a normal call in that case. I'm not
> >> suggesting we add uclasses with no generic use.
> >
> > The function pointers are there to allow the display driver to call into
> > the different output drivers. Generally on Tegra what you do to scan out
> > a framebuffer is roughly this:
> >
> >         1) setup display controller to drive a specified display mode
> >         2) enable a window to scan out a given framebuffer
> >         3) setup an output driver to take input from the display
> >            controller and push it over the wire
> >
> > 1) and 2) will be the same no matter what output you use. 3) is specific
> > to the type of output, but can be done with the same software interface.
> >
> > The function pointers help in implementing 3) using the same abstraction
> > which I called tegra_output. A driver for HDMI would implement this in
> > one way, while the driver for DSI would implement it in another. For SOR
> > you would have yet another implementation. But the display driver itself
> > would be able to treat them the same way.
> 
> OK then I think we should create a new video output uclass for this
> purpose. We then end up with a high-level 'interface' uclass (hdmi,
> displayport, etc.) and a low-level 'video transport' uclass.

I don't understand. This is all internal to the display driver, why
should it need to bother with uclasses?

> >> > So, to reiterate, the above wasn't meant to be a generic abstraction for
> >> > a U-Boot-wide display framework, but rather a suggestion on how the
> >> > Tegra driver could internally be structured in order to avoid code
> >> > duplication.
> >> >
> >> >> > Which should work for everything, except maybe DSI, where you may need
> >> >> > some sort of inbetween step for panels that need additional setup using
> >> >> > DCS commands or the like. But I suspect that's a bridge that can be
> >> >> > crossed when we get to it.
> >> >> >
> >> >> > That said, I don't forsee myself having any time to devote to this, but
> >> >> > if anyone ends up spending work on this, feel free to Cc me on patches
> >> >> > or ask if you have questions about the display hardware or the framework
> >> >> > design. I'm sure I can find the time to provide feedback.
> >> >>
> >> >> In which case I suggest we limit the amount of rewrite we ask for in
> >> >> this case...
> >> >
> >> > People asked for my opinion, so I shared. If you prefer code duplication
> >> > over a properly architected driver that's of course your prerogative.
> >>
> >> I am wondering if the problem here is just that I misunderstood your
> >> intent. How about:
> >>
> >> - the display controller code (display.c) should be common across all Tegra SoCs
> >> - the code (which was merged 3 years ago) should move to use the new
> >> device tree bindings (as does tegra124 display support)
> >>
> >> What am I missing?
> >
> > Sounds good to me. My suggestions were targetted at how to decouple some
> > of the code to allow both the RGB (for the existing Tegra20 support) and
> > SOR (for the existing Tegra124 support) outputs to be used, depending on
> > the particular use-case.
> >
> > And yes, this can all be driven from DT. The driver should be able to
> > parse the "primary" output from DT using information that's available
> > (such as which outputs are enabled) and some heuristics in case multiple
> > outputs are enabled (DSI or RGB and HDMI for example). In all cases that
> > I know of the internal panel (RGB, DSI, eDP) will be the primary one, so
> > giving those priority over HDMI should always give you a sane default
> > configuration.
> >
> > Once you have the primary output you can query what mode it should drive
> > (using static display mode information from an internal panel, or
> > preferably EDID), allocate an appropriately sized framebuffer, set the
> > mode and scan out that buffer.
> 
> OK, sounds good. I'm not yet sure who is planning to work on this and
> I would much prefer to do it with another SoC uses the same framework.
> It's normally a bad idea to design a general purpose framework with a
> sample size of 1!

Again, this is about adding infrastructure to the Tegra display driver
to allow code reuse. Why would this need to be done in conjunction with
other SoCs?

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.denx.de/pipermail/u-boot/attachments/20150826/35bfc5d9/attachment.sig>