Skip to contentSkip to conversation

arm: tegra30: video: integrate display driver for t30

PATCH New Marcel Ziswiler 3 participants 3 messages Aug 24, 2015 – Aug 26, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 3/5 arm: tegra30: video: integrate display driver for t30

Quoted 3 lines · Thierry Reding

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.

Quoted 5 lines

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...

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 3 lines

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?

Quoted 9 lines

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.

Quoted 10 lines

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.

Quoted 48 lines

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.

Quoted 4 lines

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.

#1 lore raw

PATCH 3/5 arm: tegra30: video: integrate display driver for t30

Quoted 8 lines · Marcel Ziswiler
Quoted 3 lines · Thierry Reding

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).

Quoted 11 lines
Quoted 9 lines

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.

Quoted 6 lines
Quoted 6 lines

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.

Quoted 6 lines
Quoted 4 lines

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?

Quoted 48 lines
Quoted 48 lines

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.

Quoted 7 lines
Quoted 4 lines

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.

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/aac5f182/attachment.sig>

#2 lore raw

PATCH 3/5 arm: tegra30: video: integrate display driver for t30

Hi,

Quoted 100 lines · Thierry Reding
Quoted 8 lines · Marcel Ziswiler
Quoted 3 lines · Thierry Reding

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).

Quoted 11 lines
Quoted 9 lines

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.

Quoted 6 lines
Quoted 6 lines

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.

Quoted 6 lines
Quoted 4 lines

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?

Quoted 53 lines
Quoted 52 lines

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.

Quoted 8 lines
Quoted 2 lines

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