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

[Simon Glass]

Hi,

On 26 August 2015 at 00:39, Thierry Reding <treding at nvidia.com> wrote:
> On Mon, Aug 24, 2015 at 10:03:35PM +0000, Marcel Ziswiler wrote:
>> On 21 Aug 2015 11:29, Thierry Reding <treding at nvidia.com> wrote:
>> > Perhaps a good idea would be to simply copy what we have in the kernel
>> > and see where (if at all) U-Boot breaks down and fix it to work properly
>> > with "upstream" DTBs.
>>
>> I can certainly give this a try on our hardware however most NVIDIA
>> boards are far or at least were far from public so I can't test those
>> as I don't have any access.
>
> I have access to most boards, so I can help with the testing. Tom and
> Stephen may be able to help as well, so we can spread the load. I'm
> fairly confident, though, that if we get things tested on one board per
> Tegra generation it will work fine on other boards of the same
> generation as well. There isn't all that much difference between boards
> (at least as far as U-Boot cares).
>
>> > There really isn't much reason for
>> > duplicating the drivers since the display controllers haven't changed
>> > very much over the various SoC generations. And especially in U-Boot I
>> > don't think you'll need fancy features like color conversion or gamma
>> > correction, or smart dimming, which are really the only areas where
>> > there have been changes.
>> >
>> > If you look at the Linux kernel driver we get away with very minimal
>> > parameterization, and I think the same should be possible for U-Boot.
>>
>> Agreed, just wondering about the lack of any documentation thereof.
>
> I don't understand this comment. The display controller is fairly well
> documented in the TRM.
>
>> > In a similar vein I think it should be possible to write unified drivers
>> > for each type of output (RGB, HDMI, DSI, SOR). Those require even less
>> > parameterization since there have been almost no changes to those since
>> > their introduction, the rare exception being fancy features that I don't
>> > think would be needed for U-Boot.
>> >
>> > Granted, U-Boot doesn't give you much of a framework to work with, so
>> > there's a lot of code that would need to be written to abstract things,
>> > but I think that's effort well spent, much better than simply
>> > duplicating for each new generation.
>>
>> As mentioned before my doubts go as deep as the actual functional split thereof.
>
> Which functional split are you referring to?
>
>> > Frankly I think it should all move into a separate "tegra" subdirectory
>> > in drivers/video/. There's likely going to be quite a few files if we
>> > want to support several types of outputs, and a subdirectory will help
>> > keep things organized.
>> >
>> > I think a framework for U-Boot could be fairly simple and doesn't have
>> > to have the complexity of DRM/KMS in the kernel. I'd expect the U-Boot
>> > configuration to be statically defined, and if the "framework" is Tegra
>> > specific it doesn't need to be as generic either.
>> >
>> > 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);
>> >
>> > 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.
>>
>> I also doubt finding that kind of time. My goal was simply to make T30
>> usable to the same extend as the T20 before it gets completely lost.
>
> You don't have to do all of the above to get Tegra30 to the same point
> where Tegra20 is now. Perhaps a good middle-ground for now would be to
> simply avoid duplicating the display driver completely and rather look
> at reusing the Tegra20 code on both Tegra20 and Tegra30. That would be
> not making things worse and should get you what you want.

If you can make this work I can sign up for moving Tegra124 over.

Regards,
Simon