On 21 Aug 2015 11:29, Thierry Reding <tred...@nvidia.com> wrote:
> I agree with Stephen that U-Boot should use an exact copy of the DTS
> files in the kernel. The bits in the kernel get much more review and
> have been known to work well for a number of years.
I do not oppose however I doubt this has ever been the case. At least for the
Tegras this is far from the truth.
> U-Boot depending on the device tree or not is really an orthogonal
> issue. If we keep a delta between U-Boot and Linux DTS files, we'll risk
> breaking things badly if we ever do end up putting device tree sources
> into a common source tree, because we'd be exposing U-Boot to something
> that it wasn't tested against.
I'm not even that sure whether using device trees in U-Boot is really that
smart at all especially considering SPL and such. But that's a different
thematic...
> That's certainly a true statement. There really should never have been
> such a disconnect as we currently have. If we can't agree on using the
> very same DTS files for both U-Boot and Linux we might as well not use
> device tree at all (in U-Boot).
Exactly.
> 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 can't obviously force you to do all that work to fix past mistakes,
> but I'd like to see at least new DTS content in U-Boot deviate from what
> we have upstream in the kernel.
OK, fair enough.
> It looks to me like this adds even a third copy. There's already a
> duplicate of the display controller bits in drivers/video/tegra124 along
> with some new code to support eDP.
Me wondering how that came along then?
> 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.
> 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.
> 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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
