On Fri, Aug 21, 2015 at 06:37:37PM -0600, Simon Glass wrote: [...]
I have serious doubts about the wisdom of requiring a contributor to completely re-architect the existing display system in U-Boot. It's a big job. Perhaps we can settle for following along the same lines and not making things worse?
I didn't suggest re-architecting the display system in U-Boot. What I was suggesting was a way to architect Tegra-specific display driver code to make it reusable rather than duplicate display controller programming for each new generation, while the hardware has remained mostly the same.
Perhaps something as simple as:
struct tegra_dc { ... int (*enable)(struct tegra_dc *dc, const struct display_mode *mode); void (*disable)(struct tegra_dc *dc); ... }; struct tegra_output { ... struct tegra_dc *dc; ... int (*enable)(struct tegra_output *output, const struct display_mode *mode); void (*disable)(struct tegra_output *output); ... };would work fine. That's roughly how drivers are implemented in the kernel. Setting up display on an output would be done by determining the mode (typically by parsing EDID if available, or using a hard-coded mode otherwise) and then calling:
output->dc = dc; dc->enable(dc, mode); output->enable(output, mode);You might want to add in an abstraction for panels as well to make sure you have enough flexibility to enable and disable those, too. In that case you'd probably want to complement the above sequence with:
panel->enable(panel);Please don't add function points to structures on an ad-hoc basis. These should use driver model. There is a uclass for display port but not for LCD panels or SOR. You could add a very simple one for a panel if you like. Please take a look at tegra124's display driver for an example.
I don't think the driver model is a good fit here. Abstracting a display port isn't very useful in itself because users don't really care about the type of display, they only care about it being a display. So if you want to usefully abstract you'd do it at a higher level, such as display or screen. Then you have a generic object which users can use to put up a framebuffer onto a physical screen.
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.
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.
Thierry