Tegra DRM with HDMI support (\o/)

Tegra DRM with HDMI support (\o/)

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1660 bytes --]

Hi,

I've finally managed to get HDMI working on Tegra20. Unfortunately the
xf86-video-modesetting driver doesn't work on top of it yet, but I think
that's a different error somewhere else and I'm still trying to figure
out what exactly is going wrong. However, a framebuffer console can be
run on top of it, as does a small test program that I've written for
testing.

The branch that contains all of this is the tegra/next branch on my
personal Linux kernel tree on gitorious[0]. Some other things are
included as well, such as the latest PCIe driver, an experimental draft
of a NAND driver (loosely based on the U-Boot driver by Jim Lin and
Simon Glass) as well as various fixes and hacks. Some of it is probably
also junk that I'll throw away at some point. The branch isn't very
clean, but it should work well enough for anybody who wants to give it a
try.

My plan was to get LVDS and HDMI to work on Tegra20 with CMA for GEM
object allocations before submitting for mainline inclusion. Since all
of these are now working I will clean up the branch, provide a branch
with only the DRM bits into a separate branch and submit the patches for
review, with the hope of getting them included in 3.8. A lot of the HDMI
work has been contributed by NVIDIA's Mark Zhang, who is also busy (and
apparently very close to) getting Tegra30 support working. If he can
make it work, Tegra30 support can be included as well.

All of that said, I invite everyone with access to Tegra hardware to
give the DRM driver a spin and report back if you encounter any
problems. Feel free to also drop a note on success.

Thierry

[0]: git://gitorious.org/thierryreding/linux.git

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Tegra DRM with HDMI support (\o/)

[Stephen Warren]

On 10/11/2012 02:07 PM, Thierry Reding wrote:
> Hi,
> 
> I've finally managed to get HDMI working on Tegra20. Unfortunately
> the xf86-video-modesetting driver doesn't work on top of it yet,
> but I think that's a different error somewhere else and I'm still
> trying to figure out what exactly is going wrong. However, a
> framebuffer console can be run on top of it, as does a small test
> program that I've written for testing.

I should start out by saying that this is all very excellent news!
Thanks so much for your hard work on tegra-drm.

I have managed to get Harmony HDMI working, mostly just by copying the
obvious device tree entries from tegra20-trimslice.dts. This didn't
quite work immediately though, and while debugging this I found a few
issues...

(most files I mention below are in drivers/gpu/drm/tegra/)

1)

clock.c:tegra_clock_setup() calls clk_set_rate() on the PLL. However,
there is no error-checking, so any failure is completely silent and
simply results in the display not working without much clue as to why.
In my case, my monitor's preferred mode was 1920x1080 with a 154MHz
pixel clock, which required a 616MHz clock for pll_d. However,
mach-tegra/tegra20_clocks_data.c didn't have an entry for that rate,
so clk_set_rate() failed.

For reference, the following entry in tegra_pll_d_freq_table[] solves
this for me:

	{ 12000000, 616000000, 616, 12, 1, 8},

(although I have no idea if the "cpcon" field there is correct)

I imagine that output.c:tegra_connector_mode_valid() should be
querying the clock API to determine whether the desired mode clock is
actually possible, and rejecting the mode if not. In my case, I assume
that would have caused the code to fall back to some other mode, and
hopefully found one that did (my monitor apparently lists 51 modes in
the EDID after all).

2)

The code in clock.c uses lots of clk_get_sys() calls with hard-coded
clock names. We really should be using the common clock DT bindings
for this instead of hard-coding names. This is especially true since
the names differ on different SoCs, so there's a ton of
of_device_is_compatible(output->dev->of_node, "nvidia,tegra30-hdmi")
in this code.

3)

It sucks that our clock driver can't just calculate the PLL m and n
values; without this, we will end up with an almost infinitely large
tegra_pll_d_freq_table[]. I'll try and find out whether this can be fixed.

Re: Tegra DRM with HDMI support (\o/)

[Mark Zhang]

On Fri, 2012-10-12 at 07:36 +0800, Stephen Warren wrote:
> On 10/11/2012 02:07 PM, Thierry Reding wrote:
> > Hi,
> > 
> > I've finally managed to get HDMI working on Tegra20. Unfortunately
> > the xf86-video-modesetting driver doesn't work on top of it yet,
> > but I think that's a different error somewhere else and I'm still
> > trying to figure out what exactly is going wrong. However, a
> > framebuffer console can be run on top of it, as does a small test
> > program that I've written for testing.
> 
> I should start out by saying that this is all very excellent news!
> Thanks so much for your hard work on tegra-drm.
> 
> I have managed to get Harmony HDMI working, mostly just by copying the
> obvious device tree entries from tegra20-trimslice.dts. This didn't
> quite work immediately though, and while debugging this I found a few
> issues...
> 
> (most files I mention below are in drivers/gpu/drm/tegra/)
> 
> 1)
> 
> clock.c:tegra_clock_setup() calls clk_set_rate() on the PLL. However,
> there is no error-checking, so any failure is completely silent and
> simply results in the display not working without much clue as to why.
> In my case, my monitor's preferred mode was 1920x1080 with a 154MHz
> pixel clock, which required a 616MHz clock for pll_d. However,
> mach-tegra/tegra20_clocks_data.c didn't have an entry for that rate,
> so clk_set_rate() failed.
> 
> For reference, the following entry in tegra_pll_d_freq_table[] solves
> this for me:
> 
> 	{ 12000000, 616000000, 616, 12, 1, 8},
> 
> (although I have no idea if the "cpcon" field there is correct)
> 
> I imagine that output.c:tegra_connector_mode_valid() should be
> querying the clock API to determine whether the desired mode clock is
> actually possible, and rejecting the mode if not. In my case, I assume
> that would have caused the code to fall back to some other mode, and
> hopefully found one that did (my monitor apparently lists 51 modes in
> the EDID after all).
> 
> 2)
> 
> The code in clock.c uses lots of clk_get_sys() calls with hard-coded
> clock names. We really should be using the common clock DT bindings
> for this instead of hard-coding names. This is especially true since
> the names differ on different SoCs, so there's a ton of
> of_device_is_compatible(output->dev->of_node, "nvidia,tegra30-hdmi")
> in this code.
> 

Agree. But I've no idea how to utilize this common clock DT bindings in
code. Could you give me some example codes or some reference docs?

> 3)
> 
> It sucks that our clock driver can't just calculate the PLL m and n
> values; without this, we will end up with an almost infinitely large
> tegra_pll_d_freq_table[]. I'll try and find out whether this can be fixed.
> --

Absolutely. This is a problem when I debug HDMI issues. HDMI devices
usually have a lot of video modes which require a lot of clock
definitions. Seems in our downstream kernel, the clock driver will
calculate the m/n values if the fixed clock table can't meet the
requirement. I just feel strange that why we don't upstream this part of
codes...

> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Tegra DRM with HDMI support (\o/)

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3722 bytes --]

On Thu, Oct 11, 2012 at 05:36:35PM -0600, Stephen Warren wrote:
> On 10/11/2012 02:07 PM, Thierry Reding wrote:
> > Hi,
> > 
> > I've finally managed to get HDMI working on Tegra20. Unfortunately
> > the xf86-video-modesetting driver doesn't work on top of it yet,
> > but I think that's a different error somewhere else and I'm still
> > trying to figure out what exactly is going wrong. However, a
> > framebuffer console can be run on top of it, as does a small test
> > program that I've written for testing.
> 
> I should start out by saying that this is all very excellent news!
> Thanks so much for your hard work on tegra-drm.
> 
> I have managed to get Harmony HDMI working, mostly just by copying the
> obvious device tree entries from tegra20-trimslice.dts. This didn't
> quite work immediately though, and while debugging this I found a few
> issues...
> 
> (most files I mention below are in drivers/gpu/drm/tegra/)
> 
> 1)
> 
> clock.c:tegra_clock_setup() calls clk_set_rate() on the PLL. However,
> there is no error-checking, so any failure is completely silent and
> simply results in the display not working without much clue as to why.
> In my case, my monitor's preferred mode was 1920x1080 with a 154MHz
> pixel clock, which required a 616MHz clock for pll_d. However,
> mach-tegra/tegra20_clocks_data.c didn't have an entry for that rate,
> so clk_set_rate() failed.

I'm not entirely happy with how this is currently implemented. One thing
I was going to experiment with now that I have a working reference is to
move clock setup into the respective outputs in order to get rid of the
various if branches. This may turn out to not be much better, but it
will allow the clock setup to be more closely located to the actual user
of the code and therefore increase maintainability.

Even if that doesn't work out, error checking is still a must.

> For reference, the following entry in tegra_pll_d_freq_table[] solves
> this for me:
> 
> 	{ 12000000, 616000000, 616, 12, 1, 8},
> 
> (although I have no idea if the "cpcon" field there is correct)
> 
> I imagine that output.c:tegra_connector_mode_valid() should be
> querying the clock API to determine whether the desired mode clock is
> actually possible, and rejecting the mode if not. In my case, I assume
> that would have caused the code to fall back to some other mode, and
> hopefully found one that did (my monitor apparently lists 51 modes in
> the EDID after all).

Yes, that's definitely something we should cleanup. If the output can
determine that the clock can't be set to the rate required by the mode
it should mark the mode as invalide.

> 2)
> 
> The code in clock.c uses lots of clk_get_sys() calls with hard-coded
> clock names. We really should be using the common clock DT bindings
> for this instead of hard-coding names. This is especially true since
> the names differ on different SoCs, so there's a ton of
> of_device_is_compatible(output->dev->of_node, "nvidia,tegra30-hdmi")
> in this code.

I don't quite see how that is supposed to work. Wouldn't that mean that
we needed to setup various entries in the clock tables to be able to
look the clocks up by generic names? Like pll_d would need to be the
"parent" clock of "tegra-hdmi" or similar. In that case we could call
clk_get(hdmi->dev, "parent") instead. Is that what you had in mind?

> 3)
> 
> It sucks that our clock driver can't just calculate the PLL m and n
> values; without this, we will end up with an almost infinitely large
> tegra_pll_d_freq_table[]. I'll try and find out whether this can be fixed.

If this can be calculated it is definitely the preferred way.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Tegra DRM with HDMI support (\o/)

[Stephen Warren]

On 10/11/2012 11:09 PM, Thierry Reding wrote:
> On Thu, Oct 11, 2012 at 05:36:35PM -0600, Stephen Warren wrote:
>> On 10/11/2012 02:07 PM, Thierry Reding wrote:
>>> Hi,
>>> 
>>> I've finally managed to get HDMI working on Tegra20.
>>> Unfortunately the xf86-video-modesetting driver doesn't work on
>>> top of it yet, but I think that's a different error somewhere
>>> else and I'm still trying to figure out what exactly is going
>>> wrong. However, a framebuffer console can be run on top of it,
>>> as does a small test program that I've written for testing.
>> 
>> I should start out by saying that this is all very excellent
>> news! Thanks so much for your hard work on tegra-drm.
>> 
>> I have managed to get Harmony HDMI working, mostly just by
>> copying the obvious device tree entries from
>> tegra20-trimslice.dts. This didn't quite work immediately though,
>> and while debugging this I found a few issues...
...
>> The code in clock.c uses lots of clk_get_sys() calls with
>> hard-coded clock names. We really should be using the common
>> clock DT bindings for this instead of hard-coding names. This is
>> especially true since the names differ on different SoCs, so
>> there's a ton of of_device_is_compatible(output->dev->of_node,
>> "nvidia,tegra30-hdmi") in this code.
> 
> I don't quite see how that is supposed to work. Wouldn't that mean
> that we needed to setup various entries in the clock tables to be
> able to look the clocks up by generic names? Like pll_d would need
> to be the "parent" clock of "tegra-hdmi" or similar. In that case
> we could call clk_get(hdmi->dev, "parent") instead. Is that what
> you had in mind?

There are two ways this could work:

1)

All clocks needed could be represented in the node of that device (or
perhaps in the DC node?) For example, perhaps hdmi might contain:

clocks = <&car TEGRA_CLK_PLL_D>
         <&car TEGRA_CLK_PLL_D_OUT_0>
         <&car TEGRA_CLK_HDMI>
         ...;
clock-names = "pll_d", "pll_d_out_0", "hdmi", ...;

That should work (I think) with pretty much no modification to the
current code, other than calling clk_get(dev, "hdmi") rather than
clk_get_sys(NULL, "hdmi").

However, I'm not sure this is the best way or not; this still requires
the HDMI driver to implement all the clocking policy (e.g. is the HDMI
clock a child of PLL_D, PLL_C, PLL_P, ... and what rate should the
parent PLL be set to, etc.)

For example, what if the clock frequency HDMI needs can be generated
by PLL_C, so we make PLL_C the parent of HDMI (so as to save power by
not running PLL_D, or to use PLL_D for a second display head), then
something else comes along and /has/ to use PLL_C. If the two childs'
frequency requirements are the same, everything is fine. If the two
child's frequency requirements are not compatible, we might need to
spin up PLL_D and reparent HDMI onto it. More complex scenarios are
possible where PLL_C needs to be reprogrammed, yet can be in a way
that supports both HDMI and some other requirement, so we only need to
temporarily move HDMI onto PLL_D, then reprogram PLL_C, then move HDMI
back to it and spin PLL_D down. All these decisions should be made
centrally in the Tegra clock driver, or at least some kind of
centralized policy management point, and not by the HDMI driver:

2)

The HDMI device node only knows about the clocks used directly by the
HDMI module:

clocks = <&car TEGRA_CLK_HDMI>;
clock-names = "hdmi";

(note: that's just an example; I don't know off the top of my head
exactly which clocks that module consumes directly)

Then, the HDMI driver will simply perform a single clk = clk_get(dev,
"hdmi"), and later call clk_set_rate(clk, rate) as appropriate. All
the policy decisions then get made internally to the Tegra clock
driver (since it's a central nexus for all clock outputs from the
Clock And Reset controller (CAR).

A similar question exists for audio too. Currently, our audio drivers
do something similar to what tegra-drm is doing; (1) above. However, I
have a feeling we may want to convert audio over to model (2) above too.

Oh, and the cpufreq driver has a similar for centralized policy, since
at 216MHz the CPU clocks are derived from one PLL, but for anything
faster we use PLL_X to generate them. Right now, this is implemented
in the cpufreq driver, but again should really be centrally managed.

Re: Tegra DRM with HDMI support (\o/)

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 5387 bytes --]

On Fri, Oct 12, 2012 at 03:17:04PM -0600, Stephen Warren wrote:
> On 10/11/2012 11:09 PM, Thierry Reding wrote:
> > On Thu, Oct 11, 2012 at 05:36:35PM -0600, Stephen Warren wrote:
[...]
> >> The code in clock.c uses lots of clk_get_sys() calls with
> >> hard-coded clock names. We really should be using the common
> >> clock DT bindings for this instead of hard-coding names. This is
> >> especially true since the names differ on different SoCs, so
> >> there's a ton of of_device_is_compatible(output->dev->of_node,
> >> "nvidia,tegra30-hdmi") in this code.
> > 
> > I don't quite see how that is supposed to work. Wouldn't that mean
> > that we needed to setup various entries in the clock tables to be
> > able to look the clocks up by generic names? Like pll_d would need
> > to be the "parent" clock of "tegra-hdmi" or similar. In that case
> > we could call clk_get(hdmi->dev, "parent") instead. Is that what
> > you had in mind?
> 
> There are two ways this could work:
> 
> 1)
> 
> All clocks needed could be represented in the node of that device (or
> perhaps in the DC node?) For example, perhaps hdmi might contain:
> 
> clocks = <&car TEGRA_CLK_PLL_D>
>          <&car TEGRA_CLK_PLL_D_OUT_0>
>          <&car TEGRA_CLK_HDMI>
>          ...;
> clock-names = "pll_d", "pll_d_out_0", "hdmi", ...;
> 
> That should work (I think) with pretty much no modification to the
> current code, other than calling clk_get(dev, "hdmi") rather than
> clk_get_sys(NULL, "hdmi").

I thought DT support didn't work for Tegra yet. If that works we should
probably get rid of the DRM related entries in the clock tables and put
the information in the DT regardless of which of the alternatives is
implemented.

> However, I'm not sure this is the best way or not; this still requires
> the HDMI driver to implement all the clocking policy (e.g. is the HDMI
> clock a child of PLL_D, PLL_C, PLL_P, ... and what rate should the
> parent PLL be set to, etc.)

Yes, it's pretty ugly and as you mentioned before it requires many
special cases to be handled depending on the SoC. I'm presuming that
future Tegra generations will be supported by the same driver and the
number of special cases will only increase.

> For example, what if the clock frequency HDMI needs can be generated
> by PLL_C, so we make PLL_C the parent of HDMI (so as to save power by
> not running PLL_D, or to use PLL_D for a second display head), then
> something else comes along and /has/ to use PLL_C. If the two childs'
> frequency requirements are the same, everything is fine. If the two
> child's frequency requirements are not compatible, we might need to
> spin up PLL_D and reparent HDMI onto it. More complex scenarios are
> possible where PLL_C needs to be reprogrammed, yet can be in a way
> that supports both HDMI and some other requirement, so we only need to
> temporarily move HDMI onto PLL_D, then reprogram PLL_C, then move HDMI
> back to it and spin PLL_D down. All these decisions should be made
> centrally in the Tegra clock driver, or at least some kind of
> centralized policy management point, and not by the HDMI driver:

That does sound awfully complex. However given that it would allow us to
use only a single clock and program it in whatever way we need in the
drivers is a very strong point in favour of this alternative.

Furthermore, given the above scenarios, it won't be possible to do that
without a central implementation because all clock usage information
needs to be available to make the right decisions.

> 2)
> 
> The HDMI device node only knows about the clocks used directly by the
> HDMI module:
> 
> clocks = <&car TEGRA_CLK_HDMI>;
> clock-names = "hdmi";
> 
> (note: that's just an example; I don't know off the top of my head
> exactly which clocks that module consumes directly)
> 
> Then, the HDMI driver will simply perform a single clk = clk_get(dev,
> "hdmi"), and later call clk_set_rate(clk, rate) as appropriate. All
> the policy decisions then get made internally to the Tegra clock
> driver (since it's a central nexus for all clock outputs from the
> Clock And Reset controller (CAR).

From an abstraction or design point of view, the CAR indeed seems the
best place to implement this kind of policy. User drivers shouldn't have
to worry or know about any of these details. The big win here being that
most of the special casing could be removed, therefore making the driver
a whole lot simpler and easier to maintain.

> A similar question exists for audio too. Currently, our audio drivers
> do something similar to what tegra-drm is doing; (1) above. However, I
> have a feeling we may want to convert audio over to model (2) above too.
> 
> Oh, and the cpufreq driver has a similar for centralized policy, since
> at 216MHz the CPU clocks are derived from one PLL, but for anything
> faster we use PLL_X to generate them. Right now, this is implemented
> in the cpufreq driver, but again should really be centrally managed.

After reading all of the above I'm definitely in favour of alternative
2). If you think we can pull it off I'd say let's go for it. I'm willing
to spend some time on helping with this, but I must admit I'm not overly
familiar with the clock framework. Maybe Prashant would be better suited
to have a shot at it.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Tegra DRM with HDMI support (\o/)

[Stephen Warren]

On 10/13/2012 02:42 PM, Thierry Reding wrote:
> On Fri, Oct 12, 2012 at 03:17:04PM -0600, Stephen Warren wrote:
>> On 10/11/2012 11:09 PM, Thierry Reding wrote:
>>> On Thu, Oct 11, 2012 at 05:36:35PM -0600, Stephen Warren
>>> wrote:
> [...]
>>>> The code in clock.c uses lots of clk_get_sys() calls with 
>>>> hard-coded clock names. We really should be using the common 
>>>> clock DT bindings for this instead of hard-coding names. This
>>>> is especially true since the names differ on different SoCs,
>>>> so there's a ton of
>>>> of_device_is_compatible(output->dev->of_node, 
>>>> "nvidia,tegra30-hdmi") in this code.
>>> 
>>> I don't quite see how that is supposed to work. Wouldn't that
>>> mean that we needed to setup various entries in the clock
>>> tables to be able to look the clocks up by generic names? Like
>>> pll_d would need to be the "parent" clock of "tegra-hdmi" or
>>> similar. In that case we could call clk_get(hdmi->dev,
>>> "parent") instead. Is that what you had in mind?
>> 
>> There are two ways this could work:
>> 
>> 1)
>> 
>> All clocks needed could be represented in the node of that device
>> (or perhaps in the DC node?) For example, perhaps hdmi might
>> contain:
>> 
>> clocks = <&car TEGRA_CLK_PLL_D> <&car TEGRA_CLK_PLL_D_OUT_0> 
>> <&car TEGRA_CLK_HDMI> ...; clock-names = "pll_d", "pll_d_out_0",
>> "hdmi", ...;
>> 
>> That should work (I think) with pretty much no modification to
>> the current code, other than calling clk_get(dev, "hdmi") rather
>> than clk_get_sys(NULL, "hdmi").
> 
> I thought DT support didn't work for Tegra yet. If that works we
> should probably get rid of the DRM related entries in the clock
> tables and put the information in the DT regardless of which of the
> alternatives is implemented.

Certainly, the Tegra clock driver itself doesn't support DT, i.e.
isn't a DT clock provider. So, this won't work completely right now. I
believe Prashant had patches to implement this though. Prashant,
what's the status on those?

It looks like the core common clock DT support did make it into 3.7,
so any dependencies are probably contained within the Tegra tree.

Still, perhaps this is something we can clean up later; we'd probably
benefit more from having some form of tegra-drm in place earlier
rather than having it later with perfect clock usage, although DT
binding churn:-(

Re: Tegra DRM with HDMI support (\o/)

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2730 bytes --]

On Mon, Oct 15, 2012 at 10:11:50AM -0600, Stephen Warren wrote:
> On 10/13/2012 02:42 PM, Thierry Reding wrote:
> > On Fri, Oct 12, 2012 at 03:17:04PM -0600, Stephen Warren wrote:
> >> On 10/11/2012 11:09 PM, Thierry Reding wrote:
> >>> On Thu, Oct 11, 2012 at 05:36:35PM -0600, Stephen Warren
> >>> wrote:
> > [...]
> >>>> The code in clock.c uses lots of clk_get_sys() calls with 
> >>>> hard-coded clock names. We really should be using the common 
> >>>> clock DT bindings for this instead of hard-coding names. This
> >>>> is especially true since the names differ on different SoCs,
> >>>> so there's a ton of
> >>>> of_device_is_compatible(output->dev->of_node, 
> >>>> "nvidia,tegra30-hdmi") in this code.
> >>> 
> >>> I don't quite see how that is supposed to work. Wouldn't that
> >>> mean that we needed to setup various entries in the clock
> >>> tables to be able to look the clocks up by generic names? Like
> >>> pll_d would need to be the "parent" clock of "tegra-hdmi" or
> >>> similar. In that case we could call clk_get(hdmi->dev,
> >>> "parent") instead. Is that what you had in mind?
> >> 
> >> There are two ways this could work:
> >> 
> >> 1)
> >> 
> >> All clocks needed could be represented in the node of that device
> >> (or perhaps in the DC node?) For example, perhaps hdmi might
> >> contain:
> >> 
> >> clocks = <&car TEGRA_CLK_PLL_D> <&car TEGRA_CLK_PLL_D_OUT_0> 
> >> <&car TEGRA_CLK_HDMI> ...; clock-names = "pll_d", "pll_d_out_0",
> >> "hdmi", ...;
> >> 
> >> That should work (I think) with pretty much no modification to
> >> the current code, other than calling clk_get(dev, "hdmi") rather
> >> than clk_get_sys(NULL, "hdmi").
> > 
> > I thought DT support didn't work for Tegra yet. If that works we
> > should probably get rid of the DRM related entries in the clock
> > tables and put the information in the DT regardless of which of the
> > alternatives is implemented.
> 
> Certainly, the Tegra clock driver itself doesn't support DT, i.e.
> isn't a DT clock provider. So, this won't work completely right now. I
> believe Prashant had patches to implement this though. Prashant,
> what's the status on those?
> 
> It looks like the core common clock DT support did make it into 3.7,
> so any dependencies are probably contained within the Tegra tree.
> 
> Still, perhaps this is something we can clean up later; we'd probably
> benefit more from having some form of tegra-drm in place earlier
> rather than having it later with perfect clock usage, although DT
> binding churn:-(

Yes, I agree. Unless of course if we can get the clock DT support ready
soon, then we might yet be able to fold support into tegra-drm.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Tegra DRM with HDMI support (\o/)

[Peter De Schrijver]

> 
> There are two ways this could work:
> 
> 1)
> 
> All clocks needed could be represented in the node of that device (or
> perhaps in the DC node?) For example, perhaps hdmi might contain:
> 
> clocks = <&car TEGRA_CLK_PLL_D>
>          <&car TEGRA_CLK_PLL_D_OUT_0>
>          <&car TEGRA_CLK_HDMI>
>          ...;
> clock-names = "pll_d", "pll_d_out_0", "hdmi", ...;
> 
> That should work (I think) with pretty much no modification to the
> current code, other than calling clk_get(dev, "hdmi") rather than
> clk_get_sys(NULL, "hdmi").
> 
> However, I'm not sure this is the best way or not; this still requires
> the HDMI driver to implement all the clocking policy (e.g. is the HDMI
> clock a child of PLL_D, PLL_C, PLL_P, ... and what rate should the
> parent PLL be set to, etc.)
> 
> For example, what if the clock frequency HDMI needs can be generated
> by PLL_C, so we make PLL_C the parent of HDMI (so as to save power by
> not running PLL_D, or to use PLL_D for a second display head), then
> something else comes along and /has/ to use PLL_C. If the two childs'
> frequency requirements are the same, everything is fine. If the two
> child's frequency requirements are not compatible, we might need to
> spin up PLL_D and reparent HDMI onto it. More complex scenarios are
> possible where PLL_C needs to be reprogrammed, yet can be in a way
> that supports both HDMI and some other requirement, so we only need to
> temporarily move HDMI onto PLL_D, then reprogram PLL_C, then move HDMI
> back to it and spin PLL_D down. All these decisions should be made
> centrally in the Tegra clock driver, or at least some kind of
> centralized policy management point, and not by the HDMI driver:
> 

This assumes we can glitch free provide a new parent clock to eg. HDMI, I
doubt the hardware can do that.

Cheers,

Peter.

Re: Tegra DRM with HDMI support (\o/)

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3051 bytes --]

On Tue, Oct 16, 2012 at 11:28:27AM +0300, Peter De Schrijver wrote:
> > 
> > There are two ways this could work:
> > 
> > 1)
> > 
> > All clocks needed could be represented in the node of that device (or
> > perhaps in the DC node?) For example, perhaps hdmi might contain:
> > 
> > clocks = <&car TEGRA_CLK_PLL_D>
> >          <&car TEGRA_CLK_PLL_D_OUT_0>
> >          <&car TEGRA_CLK_HDMI>
> >          ...;
> > clock-names = "pll_d", "pll_d_out_0", "hdmi", ...;
> > 
> > That should work (I think) with pretty much no modification to the
> > current code, other than calling clk_get(dev, "hdmi") rather than
> > clk_get_sys(NULL, "hdmi").
> > 
> > However, I'm not sure this is the best way or not; this still requires
> > the HDMI driver to implement all the clocking policy (e.g. is the HDMI
> > clock a child of PLL_D, PLL_C, PLL_P, ... and what rate should the
> > parent PLL be set to, etc.)
> > 
> > For example, what if the clock frequency HDMI needs can be generated
> > by PLL_C, so we make PLL_C the parent of HDMI (so as to save power by
> > not running PLL_D, or to use PLL_D for a second display head), then
> > something else comes along and /has/ to use PLL_C. If the two childs'
> > frequency requirements are the same, everything is fine. If the two
> > child's frequency requirements are not compatible, we might need to
> > spin up PLL_D and reparent HDMI onto it. More complex scenarios are
> > possible where PLL_C needs to be reprogrammed, yet can be in a way
> > that supports both HDMI and some other requirement, so we only need to
> > temporarily move HDMI onto PLL_D, then reprogram PLL_C, then move HDMI
> > back to it and spin PLL_D down. All these decisions should be made
> > centrally in the Tegra clock driver, or at least some kind of
> > centralized policy management point, and not by the HDMI driver:
> > 
> 
> This assumes we can glitch free provide a new parent clock to eg. HDMI, I
> doubt the hardware can do that.

I think for starters it would be enough if the clock code was smart
enough to choose an appropriate parent. We could start thinking about
power optimizations later. I took a brief look at the current code and
it seems like all the information is already there, so we just need an
algorithm that figures out the proper parent that can provide the right
frequency and select it as parent in clk_set_rate() automatically. Such
an algorithm could be as simple as this:

	if (current parent supports frequency) {
		set parent frequency
	} else {
		foreach (parent clock) {
			if (clock supports frequency)
				select as new parent
		}

		if (no parent found)
			abort
	}

Checking for frequency support of a new parent would also involve
checking for conflicts with other children of that clock. For some
rudimentary power saving it should be enough to prefer already enabled
clocks when looking for a new parent, so that if a compatible match is
found no new clocks need to be enabled.

Does that sound reasonable?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Tegra DRM with HDMI support (\o/)

[Stephen Warren]

On 10/17/2012 12:55 AM, Thierry Reding wrote:
> On Tue, Oct 16, 2012 at 11:28:27AM +0300, Peter De Schrijver
> wrote:
>>> 
>>> There are two ways this could work:
>>> 
>>> 1)
>>> 
>>> All clocks needed could be represented in the node of that
>>> device (or perhaps in the DC node?) For example, perhaps hdmi
>>> might contain:
>>> 
>>> clocks = <&car TEGRA_CLK_PLL_D> <&car TEGRA_CLK_PLL_D_OUT_0> 
>>> <&car TEGRA_CLK_HDMI> ...; clock-names = "pll_d",
>>> "pll_d_out_0", "hdmi", ...;
>>> 
>>> That should work (I think) with pretty much no modification to
>>> the current code, other than calling clk_get(dev, "hdmi")
>>> rather than clk_get_sys(NULL, "hdmi").
>>> 
>>> However, I'm not sure this is the best way or not; this still
>>> requires the HDMI driver to implement all the clocking policy
>>> (e.g. is the HDMI clock a child of PLL_D, PLL_C, PLL_P, ... and
>>> what rate should the parent PLL be set to, etc.)
>>> 
>>> For example, what if the clock frequency HDMI needs can be
>>> generated by PLL_C, so we make PLL_C the parent of HDMI (so as
>>> to save power by not running PLL_D, or to use PLL_D for a
>>> second display head), then something else comes along and /has/
>>> to use PLL_C. If the two childs' frequency requirements are the
>>> same, everything is fine. If the two child's frequency
>>> requirements are not compatible, we might need to spin up PLL_D
>>> and reparent HDMI onto it. More complex scenarios are possible
>>> where PLL_C needs to be reprogrammed, yet can be in a way that
>>> supports both HDMI and some other requirement, so we only need
>>> to temporarily move HDMI onto PLL_D, then reprogram PLL_C, then
>>> move HDMI back to it and spin PLL_D down. All these decisions
>>> should be made centrally in the Tegra clock driver, or at least
>>> some kind of centralized policy management point, and not by
>>> the HDMI driver:
>>> 
>> 
>> This assumes we can glitch free provide a new parent clock to eg.
>> HDMI, I doubt the hardware can do that.
> 
> I think for starters it would be enough if the clock code was
> smart enough to choose an appropriate parent. We could start
> thinking about power optimizations later. I took a brief look at
> the current code and it seems like all the information is already
> there, so we just need an algorithm that figures out the proper
> parent that can provide the right frequency and select it as parent
> in clk_set_rate() automatically. Such an algorithm could be as
> simple as this:
> 
> if (current parent supports frequency) { set parent frequency }
> else { foreach (parent clock) { if (clock supports frequency) 
> select as new parent }
> 
> if (no parent found) abort }
> 
> Checking for frequency support of a new parent would also involve 
> checking for conflicts with other children of that clock. For some 
> rudimentary power saving it should be enough to prefer already
> enabled clocks when looking for a new parent, so that if a
> compatible match is found no new clocks need to be enabled.
> 
> Does that sound reasonable?

At a high level it sounds OK to me. I'm sure there are many details.
Jon Mayo (now CC'd) (and other NVIDIA people already CC'd) should
chime in with any comments or objections though.

RE: Tegra DRM with HDMI support (\o/)

[Jon Mayo]

> -----Original Message-----
> From: Stephen Warren [mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org]
> Sent: Wednesday, October 17, 2012 8:21 AM
> To: Thierry Reding; Jon Mayo
> Cc: Peter De Schrijver; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Prashant Gaikwad;
> Joseph Lo
> Subject: Re: Tegra DRM with HDMI support (\o/)
> 
> On 10/17/2012 12:55 AM, Thierry Reding wrote:
> > On Tue, Oct 16, 2012 at 11:28:27AM +0300, Peter De Schrijver
> > wrote:
> >>>
> >>> There are two ways this could work:
> >>>
> >>> 1)
> >>>
> >>> All clocks needed could be represented in the node of that
> >>> device (or perhaps in the DC node?) For example, perhaps hdmi
> >>> might contain:
> >>>
> >>> clocks = <&car TEGRA_CLK_PLL_D> <&car TEGRA_CLK_PLL_D_OUT_0>
> >>> <&car TEGRA_CLK_HDMI> ...; clock-names = "pll_d",
> >>> "pll_d_out_0", "hdmi", ...;
> >>>
> >>> That should work (I think) with pretty much no modification to
> >>> the current code, other than calling clk_get(dev, "hdmi")
> >>> rather than clk_get_sys(NULL, "hdmi").
> >>>
> >>> However, I'm not sure this is the best way or not; this still
> >>> requires the HDMI driver to implement all the clocking policy
> >>> (e.g. is the HDMI clock a child of PLL_D, PLL_C, PLL_P, ... and
> >>> what rate should the parent PLL be set to, etc.)
> >>>

the output setup for HDMI should do setparent on hdmi and dc to attach the proper pll and setup the divider (the divider is in the DC HW and not in the clock, one of the unusual cases). A default policy of always using pll_d2 / pll_d2_out_0 on T30 and later for HDMI is going to cover every case we have today. Then we can defer making it a configurable thing until someone actually needs to do the esoteric configuration. 

On T30, HDMI will work with pll_d, but I'm tipping my hand and telling you that if you always use pll_d2 you won't have to mess with this code in the next few chips. :)

for T20, you'll want to use pll_d for HDMI, unless you need DSI, then you'll have to use pll_c for HDMI. That might be where the configuration comes in handy. If you want, you can pretend that DSI-based T20 devices aren't important enough to support now.

> >>> For example, what if the clock frequency HDMI needs can be
> >>> generated by PLL_C, so we make PLL_C the parent of HDMI (so as
> >>> to save power by not running PLL_D, or to use PLL_D for a
> >>> second display head), then something else comes along and /has/
> >>> to use PLL_C. If the two childs' frequency requirements are the
> >>> same, everything is fine. If the two child's frequency
> >>> requirements are not compatible, we might need to spin up PLL_D
> >>> and reparent HDMI onto it. More complex scenarios are possible
> >>> where PLL_C needs to be reprogrammed, yet can be in a way that
> >>> supports both HDMI and some other requirement, so we only need
> >>> to temporarily move HDMI onto PLL_D, then reprogram PLL_C, then
> >>> move HDMI back to it and spin PLL_D down. All these decisions
> >>> should be made centrally in the Tegra clock driver, or at least
> >>> some kind of centralized policy management point, and not by
> >>> the HDMI driver:
> >>>
> >>
> >> This assumes we can glitch free provide a new parent clock to eg.
> >> HDMI, I doubt the hardware can do that.
> >
> > I think for starters it would be enough if the clock code was
> > smart enough to choose an appropriate parent. We could start
> > thinking about power optimizations later. I took a brief look at
> > the current code and it seems like all the information is already
> > there, so we just need an algorithm that figures out the proper
> > parent that can provide the right frequency and select it as parent
> > in clk_set_rate() automatically. Such an algorithm could be as
> > simple as this:
> >
> > if (current parent supports frequency) { set parent frequency }
> > else { foreach (parent clock) { if (clock supports frequency)
> > select as new parent }
> >

> > if (no parent found) abort }
> >
> > Checking for frequency support of a new parent would also involve
> > checking for conflicts with other children of that clock. For some
> > rudimentary power saving it should be enough to prefer already
> > enabled clocks when looking for a new parent, so that if a
> > compatible match is found no new clocks need to be enabled.
> >
> > Does that sound reasonable?
> 
> At a high level it sounds OK to me. I'm sure there are many details.
> Jon Mayo (now CC'd) (and other NVIDIA people already CC'd) should
> chime in with any comments or objections though.


Why bother trying to do some kind of best fit formula for clock frequencies? T30 and beyond have two PLLs dedicated to two DCs. 

You will never get a general purpose setup working on T20 if you have DSI. You can only make DSI and HDMI work together if you carefully choose what pixelclocks you wish to support on HDMI, as you will be sharing a clock with other unrelated peripherials. If you have LVDS panel on your T20 device (harmony and ventana do), then using pll_p for the LCD and pll_d for HDMI give you the most flexibility. But you may have to underclock the LCD panel if pll_p doesn't divide to something optiminal for that panel.

Re: Tegra DRM with HDMI support (\o/)

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 6942 bytes --]

On Wed, Oct 17, 2012 at 11:04:05AM -0700, Jon Mayo wrote:
> > -----Original Message-----
> > From: Stephen Warren [mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org]
> > Sent: Wednesday, October 17, 2012 8:21 AM
> > To: Thierry Reding; Jon Mayo
> > Cc: Peter De Schrijver; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Prashant Gaikwad;
> > Joseph Lo
> > Subject: Re: Tegra DRM with HDMI support (\o/)
> > 
> > On 10/17/2012 12:55 AM, Thierry Reding wrote:
> > > On Tue, Oct 16, 2012 at 11:28:27AM +0300, Peter De Schrijver
> > > wrote:
> > >>>
> > >>> There are two ways this could work:
> > >>>
> > >>> 1)
> > >>>
> > >>> All clocks needed could be represented in the node of that
> > >>> device (or perhaps in the DC node?) For example, perhaps hdmi
> > >>> might contain:
> > >>>
> > >>> clocks = <&car TEGRA_CLK_PLL_D> <&car TEGRA_CLK_PLL_D_OUT_0>
> > >>> <&car TEGRA_CLK_HDMI> ...; clock-names = "pll_d",
> > >>> "pll_d_out_0", "hdmi", ...;
> > >>>
> > >>> That should work (I think) with pretty much no modification to
> > >>> the current code, other than calling clk_get(dev, "hdmi")
> > >>> rather than clk_get_sys(NULL, "hdmi").
> > >>>
> > >>> However, I'm not sure this is the best way or not; this still
> > >>> requires the HDMI driver to implement all the clocking policy
> > >>> (e.g. is the HDMI clock a child of PLL_D, PLL_C, PLL_P, ... and
> > >>> what rate should the parent PLL be set to, etc.)
> > >>>
> 
> the output setup for HDMI should do setparent on hdmi and dc to attach the
> proper pll and setup the divider (the divider is in the DC HW and not in the
> clock, one of the unusual cases). A default policy of always using pll_d2 /
> pll_d2_out_0 on T30 and later for HDMI is going to cover every case we have
> today. Then we can defer making it a configurable thing until someone
> actually needs to do the esoteric configuration.

I had been thinking about what Stephen mentioned earlier about sorting
out probed modes for which the DC cannot generate an appropriate
frequency. That won't work currently because the reparenting is done
later than the mode probing. If using pll_d and pll_d2 on Tegra20 and
Tegra30 respectively is a safe default we could actually set up the
clock at module probe time so it will be properly configured when the
modes are probed and a call to clk_round_rate() should be enough to
verify that the required frequency can be set.

> On T30, HDMI will work with pll_d, but I'm tipping my hand and telling you
> that if you always use pll_d2 you won't have to mess with this code in the
> next few chips. :)

If we make this configurable in the device tree, we could provide both
the "hdmi" and "base-clock" clocks, where base-clock defaults to pll_d
or pll_d2 depending on the Tegra generation. If some board requires a
more exotic setup it can override the clocks in the board DTS.

> for T20, you'll want to use pll_d for HDMI, unless you need DSI, then you'll
> have to use pll_c for HDMI. That might be where the configuration comes in
> handy. If you want, you can pretend that DSI-based T20 devices aren't
> important enough to support now.

I think we can safely ignore DSI for now since the driver doesn't
support it yet anyway. The driver is just a dummy that doesn't do
anything useful right now. But as I mentioned above, it should be
straightforward to override the clock setup on a per-board basis.

> > >>> For example, what if the clock frequency HDMI needs can be
> > >>> generated by PLL_C, so we make PLL_C the parent of HDMI (so as
> > >>> to save power by not running PLL_D, or to use PLL_D for a
> > >>> second display head), then something else comes along and /has/
> > >>> to use PLL_C. If the two childs' frequency requirements are the
> > >>> same, everything is fine. If the two child's frequency
> > >>> requirements are not compatible, we might need to spin up PLL_D
> > >>> and reparent HDMI onto it. More complex scenarios are possible
> > >>> where PLL_C needs to be reprogrammed, yet can be in a way that
> > >>> supports both HDMI and some other requirement, so we only need
> > >>> to temporarily move HDMI onto PLL_D, then reprogram PLL_C, then
> > >>> move HDMI back to it and spin PLL_D down. All these decisions
> > >>> should be made centrally in the Tegra clock driver, or at least
> > >>> some kind of centralized policy management point, and not by
> > >>> the HDMI driver:
> > >>>
> > >>
> > >> This assumes we can glitch free provide a new parent clock to eg.
> > >> HDMI, I doubt the hardware can do that.
> > >
> > > I think for starters it would be enough if the clock code was
> > > smart enough to choose an appropriate parent. We could start
> > > thinking about power optimizations later. I took a brief look at
> > > the current code and it seems like all the information is already
> > > there, so we just need an algorithm that figures out the proper
> > > parent that can provide the right frequency and select it as parent
> > > in clk_set_rate() automatically. Such an algorithm could be as
> > > simple as this:
> > >
> > > if (current parent supports frequency) { set parent frequency }
> > > else { foreach (parent clock) { if (clock supports frequency)
> > > select as new parent }
> > >
> 
> > > if (no parent found) abort }
> > >
> > > Checking for frequency support of a new parent would also involve
> > > checking for conflicts with other children of that clock. For some
> > > rudimentary power saving it should be enough to prefer already
> > > enabled clocks when looking for a new parent, so that if a
> > > compatible match is found no new clocks need to be enabled.
> > >
> > > Does that sound reasonable?
> > 
> > At a high level it sounds OK to me. I'm sure there are many details.
> > Jon Mayo (now CC'd) (and other NVIDIA people already CC'd) should
> > chime in with any comments or objections though.
> 
> 
> Why bother trying to do some kind of best fit formula for clock
> frequencies? T30 and beyond have two PLLs dedicated to two DCs.
> 
> You will never get a general purpose setup working on T20 if you have
> DSI. You can only make DSI and HDMI work together if you carefully
> choose what pixelclocks you wish to support on HDMI, as you will be
> sharing a clock with other unrelated peripherials. If you have LVDS
> panel on your T20 device (harmony and ventana do), then using pll_p
> for the LCD and pll_d for HDMI give you the most flexibility. But you
> may have to underclock the LCD panel if pll_p doesn't divide to
> something optiminal for that panel.

Okay, that sounds like a fixed mapping would be a better alternative. It
obviously would preclude any of the power-saving that Stephen proposed.
However given that it keeps us from trouble further down the road
perhaps it really is preferable.

Stephen, what do you think?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Tegra DRM with HDMI support (\o/)

[Stephen Warren]

On 10/17/2012 01:12 PM, Thierry Reding wrote:
> On Wed, Oct 17, 2012 at 11:04:05AM -0700, Jon Mayo wrote:
>>> -----Original Message----- From: Stephen Warren
>>> [mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org] Sent: Wednesday, October 17,
>>> 2012 8:21 AM To: Thierry Reding; Jon Mayo Cc: Peter De
>>> Schrijver; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Prashant Gaikwad; 
>>> Joseph Lo Subject: Re: Tegra DRM with HDMI support (\o/)
>>> 
>>> On 10/17/2012 12:55 AM, Thierry Reding wrote:
>>>> On Tue, Oct 16, 2012 at 11:28:27AM +0300, Peter De Schrijver 
>>>> wrote:
>>>>>> 
>>>>>> There are two ways this could work:
>>>>>> 
>>>>>> 1)
>>>>>> 
>>>>>> All clocks needed could be represented in the node of
>>>>>> that device (or perhaps in the DC node?) For example,
>>>>>> perhaps hdmi might contain:
>>>>>> 
>>>>>> clocks = <&car TEGRA_CLK_PLL_D> <&car
>>>>>> TEGRA_CLK_PLL_D_OUT_0> <&car TEGRA_CLK_HDMI> ...;
>>>>>> clock-names = "pll_d", "pll_d_out_0", "hdmi", ...;
>>>>>> 
>>>>>> That should work (I think) with pretty much no
>>>>>> modification to the current code, other than calling
>>>>>> clk_get(dev, "hdmi") rather than clk_get_sys(NULL,
>>>>>> "hdmi").
>>>>>> 
>>>>>> However, I'm not sure this is the best way or not; this
>>>>>> still requires the HDMI driver to implement all the
>>>>>> clocking policy (e.g. is the HDMI clock a child of PLL_D,
>>>>>> PLL_C, PLL_P, ... and what rate should the parent PLL be
>>>>>> set to, etc.)
>>>>>> 
>> 
>> the output setup for HDMI should do setparent on hdmi and dc to
>> attach the proper pll and setup the divider (the divider is in
>> the DC HW and not in the clock, one of the unusual cases). A
>> default policy of always using pll_d2 / pll_d2_out_0 on T30 and
>> later for HDMI is going to cover every case we have today. Then
>> we can defer making it a configurable thing until someone 
>> actually needs to do the esoteric configuration.
> 
> I had been thinking about what Stephen mentioned earlier about
> sorting out probed modes for which the DC cannot generate an
> appropriate frequency. That won't work currently because the
> reparenting is done later than the mode probing. If using pll_d and
> pll_d2 on Tegra20 and Tegra30 respectively is a safe default we
> could actually set up the clock at module probe time so it will be
> properly configured when the modes are probed and a call to
> clk_round_rate() should be enough to verify that the required
> frequency can be set.
> 
>> On T30, HDMI will work with pll_d, but I'm tipping my hand and
>> telling you that if you always use pll_d2 you won't have to mess
>> with this code in the next few chips. :)
> 
> If we make this configurable in the device tree, we could provide
> both the "hdmi" and "base-clock" clocks, where base-clock defaults
> to pll_d or pll_d2 depending on the Tegra generation. If some board
> requires a more exotic setup it can override the clocks in the
> board DTS.

I think if the parenting is going to be static, lets not have tegradrm
do it, but rather encode it into one of
arch/arm/mach-tegra/{common,board-dt-tegra[23]0}.c. Eventually, we
wish to replace the clock setup tables in those files with tables in
device tree, so they can differ easily per-board, which would also
handle the DSI situation well.

If tegradrm does later gain the ability to dynamically fiddle with the
clock parenting, we can add entries in the (DT) list of clocks that
the DCs or ORs needs; when just 1 is specified it's e.g. the "hdmi"
clock and tegradrm never does any reparenting, whereas in the future
we could allow 3 to be specified; "hdmi", parent-a, parent-b, and
"unlock" any reparenting support.

>> for T20, you'll want to use pll_d for HDMI, unless you need DSI,
>> then you'll have to use pll_c for HDMI. That might be where the
>> configuration comes in handy. If you want, you can pretend that
>> DSI-based T20 devices aren't important enough to support now.
> 
> I think we can safely ignore DSI for now since the driver doesn't 
> support it yet anyway. The driver is just a dummy that doesn't do 
> anything useful right now. But as I mentioned above, it should be 
> straightforward to override the clock setup on a per-board basis.

We definitely don't want to preclude DSI from working, so we should
consider it. We do support Whistler upstream, and that has a DSI
panel, and there were some noises about getting it working sometime.

>>>>>> For example, what if the clock frequency HDMI needs can
>>>>>> be generated by PLL_C, so we make PLL_C the parent of
>>>>>> HDMI (so as to save power by not running PLL_D, or to use
>>>>>> PLL_D for a second display head), then something else
>>>>>> comes along and /has/ to use PLL_C. If the two childs'
>>>>>> frequency requirements are the same, everything is fine.
>>>>>> If the two child's frequency requirements are not
>>>>>> compatible, we might need to spin up PLL_D and reparent
>>>>>> HDMI onto it. More complex scenarios are possible where
>>>>>> PLL_C needs to be reprogrammed, yet can be in a way that 
>>>>>> supports both HDMI and some other requirement, so we only
>>>>>> need to temporarily move HDMI onto PLL_D, then reprogram
>>>>>> PLL_C, then move HDMI back to it and spin PLL_D down. All
>>>>>> these decisions should be made centrally in the Tegra
>>>>>> clock driver, or at least some kind of centralized policy
>>>>>> management point, and not by the HDMI driver:
>>>>>> 
>>>>> 
>>>>> This assumes we can glitch free provide a new parent clock
>>>>> to eg. HDMI, I doubt the hardware can do that.
>>>> 
>>>> I think for starters it would be enough if the clock code
>>>> was smart enough to choose an appropriate parent. We could
>>>> start thinking about power optimizations later. I took a
>>>> brief look at the current code and it seems like all the
>>>> information is already there, so we just need an algorithm
>>>> that figures out the proper parent that can provide the right
>>>> frequency and select it as parent in clk_set_rate()
>>>> automatically. Such an algorithm could be as simple as this:
>>>> 
>>>> if (current parent supports frequency) { set parent frequency
>>>> } else { foreach (parent clock) { if (clock supports
>>>> frequency) select as new parent }
>>>> 
>> 
>>>> if (no parent found) abort }
>>>> 
>>>> Checking for frequency support of a new parent would also
>>>> involve checking for conflicts with other children of that
>>>> clock. For some rudimentary power saving it should be enough
>>>> to prefer already enabled clocks when looking for a new
>>>> parent, so that if a compatible match is found no new clocks
>>>> need to be enabled.
>>>> 
>>>> Does that sound reasonable?
>>> 
>>> At a high level it sounds OK to me. I'm sure there are many
>>> details. Jon Mayo (now CC'd) (and other NVIDIA people already
>>> CC'd) should chime in with any comments or objections though.
>> 
>> Why bother trying to do some kind of best fit formula for clock 
>> frequencies? T30 and beyond have two PLLs dedicated to two DCs.
>> 
>> You will never get a general purpose setup working on T20 if you
>> have DSI. You can only make DSI and HDMI work together if you
>> carefully choose what pixelclocks you wish to support on HDMI, as
>> you will be sharing a clock with other unrelated peripherials. If
>> you have LVDS panel on your T20 device (harmony and ventana do),
>> then using pll_p for the LCD and pll_d for HDMI give you the most
>> flexibility. But you may have to underclock the LCD panel if
>> pll_p doesn't divide to something optiminal for that panel.
> 
> Okay, that sounds like a fixed mapping would be a better
> alternative. It obviously would preclude any of the power-saving
> that Stephen proposed. However given that it keeps us from trouble
> further down the road perhaps it really is preferable.

Considering a system with dual HDMI outputs and no built-in panel, and
with mode timings on both outputs that could both be derived from a
single clock, wouldn't using the same PLL for both outputs save power
since the second PLL can be shut down? I'm not sure how much power
this would save though; perhaps the power savings of a PLL aren't that
important vs. the power savings from clock-gating or power-gating the
logic that's clocked by the PLL.

> Stephen, what do you think?

Overall though, a static clock setup, at least initially, sounds
reasonable to me.

RE: Tegra DRM with HDMI support (\o/)

[Jon Mayo]

> -----Original Message-----
> From: Stephen Warren [mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org]
> Sent: Wednesday, October 17, 2012 12:42 PM
> To: Thierry Reding
> Cc: Jon Mayo; Peter De Schrijver; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Prashant
> Gaikwad; Joseph Lo
> Subject: Re: Tegra DRM with HDMI support (\o/)
> 
> On 10/17/2012 01:12 PM, Thierry Reding wrote:
> > On Wed, Oct 17, 2012 at 11:04:05AM -0700, Jon Mayo wrote:
> >>> -----Original Message----- From: Stephen Warren
> >>> [mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org] Sent: Wednesday, October 17,
> >>> 2012 8:21 AM To: Thierry Reding; Jon Mayo Cc: Peter De
> >>> Schrijver; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Prashant Gaikwad;
> >>> Joseph Lo Subject: Re: Tegra DRM with HDMI support (\o/)
> >>>
> >>> On 10/17/2012 12:55 AM, Thierry Reding wrote:
> >>>> On Tue, Oct 16, 2012 at 11:28:27AM +0300, Peter De Schrijver
> >>>> wrote:
> >>>>>>
> >>>>>> There are two ways this could work:
> >>>>>>
> >>>>>> 1)
> >>>>>>
> >>>>>> All clocks needed could be represented in the node of
> >>>>>> that device (or perhaps in the DC node?) For example,
> >>>>>> perhaps hdmi might contain:
> >>>>>>
> >>>>>> clocks = <&car TEGRA_CLK_PLL_D> <&car
> >>>>>> TEGRA_CLK_PLL_D_OUT_0> <&car TEGRA_CLK_HDMI> ...;
> >>>>>> clock-names = "pll_d", "pll_d_out_0", "hdmi", ...;
> >>>>>>
> >>>>>> That should work (I think) with pretty much no
> >>>>>> modification to the current code, other than calling
> >>>>>> clk_get(dev, "hdmi") rather than clk_get_sys(NULL,
> >>>>>> "hdmi").
> >>>>>>
> >>>>>> However, I'm not sure this is the best way or not; this
> >>>>>> still requires the HDMI driver to implement all the
> >>>>>> clocking policy (e.g. is the HDMI clock a child of PLL_D,
> >>>>>> PLL_C, PLL_P, ... and what rate should the parent PLL be
> >>>>>> set to, etc.)
> >>>>>>
> >>
> >> the output setup for HDMI should do setparent on hdmi and dc to
> >> attach the proper pll and setup the divider (the divider is in
> >> the DC HW and not in the clock, one of the unusual cases). A
> >> default policy of always using pll_d2 / pll_d2_out_0 on T30 and
> >> later for HDMI is going to cover every case we have today. Then
> >> we can defer making it a configurable thing until someone
> >> actually needs to do the esoteric configuration.
> >
> > I had been thinking about what Stephen mentioned earlier about
> > sorting out probed modes for which the DC cannot generate an
> > appropriate frequency. That won't work currently because the
> > reparenting is done later than the mode probing. If using pll_d and
> > pll_d2 on Tegra20 and Tegra30 respectively is a safe default we
> > could actually set up the clock at module probe time so it will be
> > properly configured when the modes are probed and a call to
> > clk_round_rate() should be enough to verify that the required
> > frequency can be set.
> >
> >> On T30, HDMI will work with pll_d, but I'm tipping my hand and
> >> telling you that if you always use pll_d2 you won't have to mess
> >> with this code in the next few chips. :)
> >
> > If we make this configurable in the device tree, we could provide
> > both the "hdmi" and "base-clock" clocks, where base-clock defaults
> > to pll_d or pll_d2 depending on the Tegra generation. If some board
> > requires a more exotic setup it can override the clocks in the
> > board DTS.
> 
> I think if the parenting is going to be static, lets not have tegradrm
> do it, but rather encode it into one of
> arch/arm/mach-tegra/{common,board-dt-tegra[23]0}.c. Eventually, we
> wish to replace the clock setup tables in those files with tables in
> device tree, so they can differ easily per-board, which would also
> handle the DSI situation well.
> 

Well it's not entirely static. The clock parent should depend on the output type. rather than on which display controller you are using. Each DC can be configured for various output types. The output types can be represented in device-tree if you wish to have them fixed (I'd interpret it as a default rather than a fixed setting).

T30 and later:
DSIA - pll_d
HDMI - pll_d2
DSIB - pll_d or pll_d2
RGB(lcd/lvds) - pll_p or pll_d

At one time I wanted to just fix pll_d to dc0 and pll_d2 dc1, and may have mentioned this to a few of you before. But now I've learned that fixing plls to DCs isn't the best approach.

Long term I'd like to see DCs that can be switch to different outputs dynamically. Our X driver has the hooks to do this, our hardware can do it and other DRM drivers support this capability.

> If tegradrm does later gain the ability to dynamically fiddle with the
> clock parenting, we can add entries in the (DT) list of clocks that
> the DCs or ORs needs; when just 1 is specified it's e.g. the "hdmi"
> clock and tegradrm never does any reparenting, whereas in the future
> we could allow 3 to be specified; "hdmi", parent-a, parent-b, and
> "unlock" any reparenting support.
> 
> >> for T20, you'll want to use pll_d for HDMI, unless you need DSI,
> >> then you'll have to use pll_c for HDMI. That might be where the
> >> configuration comes in handy. If you want, you can pretend that
> >> DSI-based T20 devices aren't important enough to support now.
> >
> > I think we can safely ignore DSI for now since the driver doesn't
> > support it yet anyway. The driver is just a dummy that doesn't do
> > anything useful right now. But as I mentioned above, it should be
> > straightforward to override the clock setup on a per-board basis.
> 
> We definitely don't want to preclude DSI from working, so we should
> consider it. We do support Whistler upstream, and that has a DSI
> panel, and there were some noises about getting it working sometime.
> 

DSI should work, just not at the same time as HDMI. Getting both to work at the same time is a real pain in the neck. I think it is fair to draw a line in the sand and say, at this time, you cannot operate both DSI and HDMI on a Tegra2 device with this new driver. 

If someone wants to work out some special cases where it does work for a particular product, then great. But I would leave that support out of an upstreamable tegradrm driver.

So a "feature" I would like is that we handle DSI xor HDMI operation. dynamically switching between the two. Either by changing the SOR outputs for an individual DC (and thus only present a single /dev/fbX device). Or have a mechanism that crosses multiple device instances and refuses to allow them to be enabled at the same time. (that's how the Android/L4T driver works, but I don't like that way so much)

> >>>>>> For example, what if the clock frequency HDMI needs can
> >>>>>> be generated by PLL_C, so we make PLL_C the parent of
> >>>>>> HDMI (so as to save power by not running PLL_D, or to use
> >>>>>> PLL_D for a second display head), then something else
> >>>>>> comes along and /has/ to use PLL_C. If the two childs'
> >>>>>> frequency requirements are the same, everything is fine.
> >>>>>> If the two child's frequency requirements are not
> >>>>>> compatible, we might need to spin up PLL_D and reparent
> >>>>>> HDMI onto it. More complex scenarios are possible where
> >>>>>> PLL_C needs to be reprogrammed, yet can be in a way that
> >>>>>> supports both HDMI and some other requirement, so we only
> >>>>>> need to temporarily move HDMI onto PLL_D, then reprogram
> >>>>>> PLL_C, then move HDMI back to it and spin PLL_D down. All
> >>>>>> these decisions should be made centrally in the Tegra
> >>>>>> clock driver, or at least some kind of centralized policy
> >>>>>> management point, and not by the HDMI driver:
> >>>>>>
> >>>>>
> >>>>> This assumes we can glitch free provide a new parent clock
> >>>>> to eg. HDMI, I doubt the hardware can do that.
> >>>>
> >>>> I think for starters it would be enough if the clock code
> >>>> was smart enough to choose an appropriate parent. We could
> >>>> start thinking about power optimizations later. I took a
> >>>> brief look at the current code and it seems like all the
> >>>> information is already there, so we just need an algorithm
> >>>> that figures out the proper parent that can provide the right
> >>>> frequency and select it as parent in clk_set_rate()
> >>>> automatically. Such an algorithm could be as simple as this:
> >>>>
> >>>> if (current parent supports frequency) { set parent frequency
> >>>> } else { foreach (parent clock) { if (clock supports
> >>>> frequency) select as new parent }
> >>>>
> >>
> >>>> if (no parent found) abort }
> >>>>
> >>>> Checking for frequency support of a new parent would also
> >>>> involve checking for conflicts with other children of that
> >>>> clock. For some rudimentary power saving it should be enough
> >>>> to prefer already enabled clocks when looking for a new
> >>>> parent, so that if a compatible match is found no new clocks
> >>>> need to be enabled.
> >>>>
> >>>> Does that sound reasonable?
> >>>
> >>> At a high level it sounds OK to me. I'm sure there are many
> >>> details. Jon Mayo (now CC'd) (and other NVIDIA people already
> >>> CC'd) should chime in with any comments or objections though.
> >>
> >> Why bother trying to do some kind of best fit formula for clock
> >> frequencies? T30 and beyond have two PLLs dedicated to two DCs.
> >>
> >> You will never get a general purpose setup working on T20 if you
> >> have DSI. You can only make DSI and HDMI work together if you
> >> carefully choose what pixelclocks you wish to support on HDMI, as
> >> you will be sharing a clock with other unrelated peripherials. If
> >> you have LVDS panel on your T20 device (harmony and ventana do),
> >> then using pll_p for the LCD and pll_d for HDMI give you the most
> >> flexibility. But you may have to underclock the LCD panel if
> >> pll_p doesn't divide to something optiminal for that panel.
> >
> > Okay, that sounds like a fixed mapping would be a better
> > alternative. It obviously would preclude any of the power-saving
> > that Stephen proposed. However given that it keeps us from trouble
> > further down the road perhaps it really is preferable.
> 
> Considering a system with dual HDMI outputs and no built-in panel, and
> with mode timings on both outputs that could both be derived from a
> single clock, wouldn't using the same PLL for both outputs save power
> since the second PLL can be shut down? I'm not sure how much power
> this would save though; perhaps the power savings of a PLL aren't that
> important vs. the power savings from clock-gating or power-gating the
> logic that's clocked by the PLL.
> 

HDMI/CEA modes can be done by setting one pll to 297MHz. Then each DC can div-by-2 for 148.5MHz (1080p50/60, 720p60-3d, 1080p30-3d-fp, 1080p60-3d-sbs, ..) or div-by-4 for 74.25MHz (720p50/60, ...). Thus allowing you to connect any two HDMI TVs and use only one pll. 

But most systems with two HDMI outputs will be newer tegras with lots of PLLs, so it might not be too worthwhile to consider.

If you want to do VESA modes, you need dedicated PLLs. as they have lots of different unrelated clock rates.

> > Stephen, what do you think?
> 
> Overall though, a static clock setup, at least initially, sounds
> reasonable to me.

Great, if needed, we can extend it later. I'm satisfied this will cover some good use cases, and we can avoid some messy complications.

Re: Tegra DRM with HDMI support (\o/)

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 11319 bytes --]

On Wed, Oct 17, 2012 at 01:08:24PM -0700, Jon Mayo wrote:
> > -----Original Message-----
> > From: Stephen Warren [mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org]
> > Sent: Wednesday, October 17, 2012 12:42 PM
> > To: Thierry Reding
> > Cc: Jon Mayo; Peter De Schrijver; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Prashant
> > Gaikwad; Joseph Lo
> > Subject: Re: Tegra DRM with HDMI support (\o/)
> > 
> > On 10/17/2012 01:12 PM, Thierry Reding wrote:
> > > On Wed, Oct 17, 2012 at 11:04:05AM -0700, Jon Mayo wrote:
> > >>> -----Original Message----- From: Stephen Warren
> > >>> [mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org] Sent: Wednesday, October 17,
> > >>> 2012 8:21 AM To: Thierry Reding; Jon Mayo Cc: Peter De
> > >>> Schrijver; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Prashant Gaikwad;
> > >>> Joseph Lo Subject: Re: Tegra DRM with HDMI support (\o/)
> > >>>
> > >>> On 10/17/2012 12:55 AM, Thierry Reding wrote:
> > >>>> On Tue, Oct 16, 2012 at 11:28:27AM +0300, Peter De Schrijver
> > >>>> wrote:
> > >>>>>>
> > >>>>>> There are two ways this could work:
> > >>>>>>
> > >>>>>> 1)
> > >>>>>>
> > >>>>>> All clocks needed could be represented in the node of
> > >>>>>> that device (or perhaps in the DC node?) For example,
> > >>>>>> perhaps hdmi might contain:
> > >>>>>>
> > >>>>>> clocks = <&car TEGRA_CLK_PLL_D> <&car
> > >>>>>> TEGRA_CLK_PLL_D_OUT_0> <&car TEGRA_CLK_HDMI> ...;
> > >>>>>> clock-names = "pll_d", "pll_d_out_0", "hdmi", ...;
> > >>>>>>
> > >>>>>> That should work (I think) with pretty much no
> > >>>>>> modification to the current code, other than calling
> > >>>>>> clk_get(dev, "hdmi") rather than clk_get_sys(NULL,
> > >>>>>> "hdmi").
> > >>>>>>
> > >>>>>> However, I'm not sure this is the best way or not; this
> > >>>>>> still requires the HDMI driver to implement all the
> > >>>>>> clocking policy (e.g. is the HDMI clock a child of PLL_D,
> > >>>>>> PLL_C, PLL_P, ... and what rate should the parent PLL be
> > >>>>>> set to, etc.)
> > >>>>>>
> > >>
> > >> the output setup for HDMI should do setparent on hdmi and dc to
> > >> attach the proper pll and setup the divider (the divider is in
> > >> the DC HW and not in the clock, one of the unusual cases). A
> > >> default policy of always using pll_d2 / pll_d2_out_0 on T30 and
> > >> later for HDMI is going to cover every case we have today. Then
> > >> we can defer making it a configurable thing until someone
> > >> actually needs to do the esoteric configuration.
> > >
> > > I had been thinking about what Stephen mentioned earlier about
> > > sorting out probed modes for which the DC cannot generate an
> > > appropriate frequency. That won't work currently because the
> > > reparenting is done later than the mode probing. If using pll_d and
> > > pll_d2 on Tegra20 and Tegra30 respectively is a safe default we
> > > could actually set up the clock at module probe time so it will be
> > > properly configured when the modes are probed and a call to
> > > clk_round_rate() should be enough to verify that the required
> > > frequency can be set.
> > >
> > >> On T30, HDMI will work with pll_d, but I'm tipping my hand and
> > >> telling you that if you always use pll_d2 you won't have to mess
> > >> with this code in the next few chips. :)
> > >
> > > If we make this configurable in the device tree, we could provide
> > > both the "hdmi" and "base-clock" clocks, where base-clock defaults
> > > to pll_d or pll_d2 depending on the Tegra generation. If some board
> > > requires a more exotic setup it can override the clocks in the
> > > board DTS.
> > 
> > I think if the parenting is going to be static, lets not have tegradrm
> > do it, but rather encode it into one of
> > arch/arm/mach-tegra/{common,board-dt-tegra[23]0}.c. Eventually, we
> > wish to replace the clock setup tables in those files with tables in
> > device tree, so they can differ easily per-board, which would also
> > handle the DSI situation well.
> > 
> 
> Well it's not entirely static. The clock parent should depend on the output
> type. rather than on which display controller you are using. Each DC can be
> configured for various output types. The output types can be represented in
> device-tree if you wish to have them fixed (I'd interpret it as a default
> rather than a fixed setting).
> 
> T30 and later:
> DSIA - pll_d
> HDMI - pll_d2
> DSIB - pll_d or pll_d2
> RGB(lcd/lvds) - pll_p or pll_d
> 
> At one time I wanted to just fix pll_d to dc0 and pll_d2 dc1, and may have
> mentioned this to a few of you before. But now I've learned that fixing plls
> to DCs isn't the best approach.
> 
> Long term I'd like to see DCs that can be switch to different outputs
> dynamically. Our X driver has the hooks to do this, our hardware can do it
> and other DRM drivers support this capability.

I specifically wrote the driver to support this kind of setup. Though it
is actually a builtin feature of the DRM framework, which allows each
output to be configured with a mask of compatible CRTCs (DCs). While
this should work I have never tested this kind of setup before.

As for the clock setup, what you said above confirms that we can do the
reparenting at output module load time. That would allow us to use the
defaults unless otherwise specified by the device tree.

> > If tegradrm does later gain the ability to dynamically fiddle with the
> > clock parenting, we can add entries in the (DT) list of clocks that
> > the DCs or ORs needs; when just 1 is specified it's e.g. the "hdmi"
> > clock and tegradrm never does any reparenting, whereas in the future
> > we could allow 3 to be specified; "hdmi", parent-a, parent-b, and
> > "unlock" any reparenting support.
> > 
> > >> for T20, you'll want to use pll_d for HDMI, unless you need DSI,
> > >> then you'll have to use pll_c for HDMI. That might be where the
> > >> configuration comes in handy. If you want, you can pretend that
> > >> DSI-based T20 devices aren't important enough to support now.
> > >
> > > I think we can safely ignore DSI for now since the driver doesn't
> > > support it yet anyway. The driver is just a dummy that doesn't do
> > > anything useful right now. But as I mentioned above, it should be
> > > straightforward to override the clock setup on a per-board basis.
> > 
> > We definitely don't want to preclude DSI from working, so we should
> > consider it. We do support Whistler upstream, and that has a DSI
> > panel, and there were some noises about getting it working sometime.
> > 
> 
> DSI should work, just not at the same time as HDMI. Getting both to work at
> the same time is a real pain in the neck. I think it is fair to draw a line
> in the sand and say, at this time, you cannot operate both DSI and HDMI on
> a Tegra2 device with this new driver.

I wasn't saying that we shouldn't support DSI, all I was saying is that
we currently don't have any code to make it work. That's mainly because
I don't have any hardware to test it on and therefore wasn't very
motivated to port the 3000+ lines of code from the downstream driver.

> If someone wants to work out some special cases where it does work for a
> particular product, then great. But I would leave that support out of an
> upstreamable tegradrm driver.

Agreed.

> So a "feature" I would like is that we handle DSI xor HDMI operation.
> dynamically switching between the two. Either by changing the SOR outputs
> for an individual DC (and thus only present a single /dev/fbX device).

We should be able to make this work with the standard Xrandr interface.
If you use the xf86-video-modesetting driver on top of tegra-drm it,
then xrandr will be able to list all the configured outputs and allow
them to be enabled or disabled at will. Again I haven't tested any
setups with two concurrent outputs yet, but I have at least seen xrandr
successfully switch modes for the HDMI output or turn it off altogether.

Also with modern DRM the /dev/fbX interface is really just for legacy
purposes. Everything else can be done using the new DRM interfaces,
which basically allow you to allocate a buffer of a given size and make
it the framebuffer of a given output. So while there is still a /dev/fb0
that can be accessed for legacy purposes, the actual framebuffers aren't
visible as device files any longer but rather as buffer objects to the
owning application (and with the appropriate driver support shareable
using DMA-BUF).

> Or have a mechanism that crosses multiple device instances and refuses to
> allow them to be enabled at the same time. (that's how the Android/L4T
> driver works, but I don't like that way so much)

What DRM/xf86-video-modesetting implements is a mix of both of the
above. You can dynamically enable or disable any of the outputs. I'm not
certain how errors are propagated in case a certain combination cannot
be handled by the DRM, but I don't think that would be a major blocker.

> > Considering a system with dual HDMI outputs and no built-in panel, and
> > with mode timings on both outputs that could both be derived from a
> > single clock, wouldn't using the same PLL for both outputs save power
> > since the second PLL can be shut down? I'm not sure how much power
> > this would save though; perhaps the power savings of a PLL aren't that
> > important vs. the power savings from clock-gating or power-gating the
> > logic that's clocked by the PLL.
> > 
> 
> HDMI/CEA modes can be done by setting one pll to 297MHz. Then each DC can
> div-by-2 for 148.5MHz (1080p50/60, 720p60-3d, 1080p30-3d-fp, 1080p60-3d-sbs,
> ..) or div-by-4 for 74.25MHz (720p50/60, ...). Thus allowing you to connect
> any two HDMI TVs and use only one pll.

I think the majority of use-cases will be either LVDS only, HDMI only or
LVDS with HDMI, all of which can be covered using the clock setups that
we've been discussing.

> But most systems with two HDMI outputs will be newer tegras with lots of
> PLLs, so it might not be too worthwhile to consider.

According to the TRM, both the Tegra20 and the Tegra30 have only a
single HDMI output. Is that information wrong or are you talking about
newer generations?

> If you want to do VESA modes, you need dedicated PLLs. as they have lots
> of different unrelated clock rates.

Some of the TV sets that I've been testing on claim to support various
exotic VESA modes. One set even has a preferred mode of 1360x768. But it
doesn't even work properly. You have to switch it to 1366x768 to make it
look acceptable. Again I think if we support all of the HDMI/CEA modes a
lot of ground is covered.

> > > Stephen, what do you think?
> > 
> > Overall though, a static clock setup, at least initially, sounds
> > reasonable to me.
> 
> Great, if needed, we can extend it later. I'm satisfied this will cover
> some good use cases, and we can avoid some messy complications.

I suspect that once the driver has made it into mainline and more people
start using it, we'll likely get many bug reports about various modes
not working properly and whatnot. But I think it would be unrealistic to
expect the first version to be perfect.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

RE: Tegra DRM with HDMI support (\o/)

[Jon Mayo]

> -----Original Message-----
> From: Thierry Reding [mailto:thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org]
> Sent: Wednesday, October 17, 2012 11:29 PM
> To: Jon Mayo
> Cc: 'Stephen Warren'; Peter De Schrijver; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> Prashant Gaikwad; Joseph Lo
> Subject: Re: Tegra DRM with HDMI support (\o/)
> 
> * PGP Signed by an unknown key
> 
> On Wed, Oct 17, 2012 at 01:08:24PM -0700, Jon Mayo wrote:
> > > -----Original Message-----
> > > From: Stephen Warren [mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org]
> > > Sent: Wednesday, October 17, 2012 12:42 PM
> > > To: Thierry Reding
> > > Cc: Jon Mayo; Peter De Schrijver; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> Prashant
> > > Gaikwad; Joseph Lo
> > > Subject: Re: Tegra DRM with HDMI support (\o/)
> > >
> > > On 10/17/2012 01:12 PM, Thierry Reding wrote:
> > > > On Wed, Oct 17, 2012 at 11:04:05AM -0700, Jon Mayo wrote:
> > > >>> -----Original Message----- From: Stephen Warren
> > > >>> [mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org] Sent: Wednesday, October 17,
> > > >>> 2012 8:21 AM To: Thierry Reding; Jon Mayo Cc: Peter De
> > > >>> Schrijver; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Prashant Gaikwad;
> > > >>> Joseph Lo Subject: Re: Tegra DRM with HDMI support (\o/)
> > > >>>
> > > >>> On 10/17/2012 12:55 AM, Thierry Reding wrote:
> > > >>>> On Tue, Oct 16, 2012 at 11:28:27AM +0300, Peter De Schrijver
> > > >>>> wrote:
> > > >>>>>>
> > > >>>>>> There are two ways this could work:
> > > >>>>>>
> > > >>>>>> 1)
> > > >>>>>>
> > > >>>>>> All clocks needed could be represented in the node of
> > > >>>>>> that device (or perhaps in the DC node?) For example,
> > > >>>>>> perhaps hdmi might contain:
> > > >>>>>>
> > > >>>>>> clocks = <&car TEGRA_CLK_PLL_D> <&car
> > > >>>>>> TEGRA_CLK_PLL_D_OUT_0> <&car TEGRA_CLK_HDMI> ...;
> > > >>>>>> clock-names = "pll_d", "pll_d_out_0", "hdmi", ...;
> > > >>>>>>
> > > >>>>>> That should work (I think) with pretty much no
> > > >>>>>> modification to the current code, other than calling
> > > >>>>>> clk_get(dev, "hdmi") rather than clk_get_sys(NULL,
> > > >>>>>> "hdmi").
> > > >>>>>>
> > > >>>>>> However, I'm not sure this is the best way or not; this
> > > >>>>>> still requires the HDMI driver to implement all the
> > > >>>>>> clocking policy (e.g. is the HDMI clock a child of PLL_D,
> > > >>>>>> PLL_C, PLL_P, ... and what rate should the parent PLL be
> > > >>>>>> set to, etc.)
> > > >>>>>>
> > > >>
> > > >> the output setup for HDMI should do setparent on hdmi and dc to
> > > >> attach the proper pll and setup the divider (the divider is in
> > > >> the DC HW and not in the clock, one of the unusual cases). A
> > > >> default policy of always using pll_d2 / pll_d2_out_0 on T30 and
> > > >> later for HDMI is going to cover every case we have today. Then
> > > >> we can defer making it a configurable thing until someone
> > > >> actually needs to do the esoteric configuration.
> > > >
> > > > I had been thinking about what Stephen mentioned earlier about
> > > > sorting out probed modes for which the DC cannot generate an
> > > > appropriate frequency. That won't work currently because the
> > > > reparenting is done later than the mode probing. If using pll_d
> and
> > > > pll_d2 on Tegra20 and Tegra30 respectively is a safe default we
> > > > could actually set up the clock at module probe time so it will
> be
> > > > properly configured when the modes are probed and a call to
> > > > clk_round_rate() should be enough to verify that the required
> > > > frequency can be set.
> > > >
> > > >> On T30, HDMI will work with pll_d, but I'm tipping my hand and
> > > >> telling you that if you always use pll_d2 you won't have to mess
> > > >> with this code in the next few chips. :)
> > > >
> > > > If we make this configurable in the device tree, we could provide
> > > > both the "hdmi" and "base-clock" clocks, where base-clock
> defaults
> > > > to pll_d or pll_d2 depending on the Tegra generation. If some
> board
> > > > requires a more exotic setup it can override the clocks in the
> > > > board DTS.
> > >
> > > I think if the parenting is going to be static, lets not have
> tegradrm
> > > do it, but rather encode it into one of
> > > arch/arm/mach-tegra/{common,board-dt-tegra[23]0}.c. Eventually, we
> > > wish to replace the clock setup tables in those files with tables
> in
> > > device tree, so they can differ easily per-board, which would also
> > > handle the DSI situation well.
> > >
> >
> > Well it's not entirely static. The clock parent should depend on the
> output
> > type. rather than on which display controller you are using. Each DC
> can be
> > configured for various output types. The output types can be
> represented in
> > device-tree if you wish to have them fixed (I'd interpret it as a
> default
> > rather than a fixed setting).
> >
> > T30 and later:
> > DSIA - pll_d
> > HDMI - pll_d2
> > DSIB - pll_d or pll_d2
> > RGB(lcd/lvds) - pll_p or pll_d
> >
> > At one time I wanted to just fix pll_d to dc0 and pll_d2 dc1, and may
> have
> > mentioned this to a few of you before. But now I've learned that
> fixing plls
> > to DCs isn't the best approach.
> >
> > Long term I'd like to see DCs that can be switch to different outputs
> > dynamically. Our X driver has the hooks to do this, our hardware can
> do it
> > and other DRM drivers support this capability.
> 
> I specifically wrote the driver to support this kind of setup. Though
> it
> is actually a builtin feature of the DRM framework, which allows each
> output to be configured with a mask of compatible CRTCs (DCs). While
> this should work I have never tested this kind of setup before.
> 
> As for the clock setup, what you said above confirms that we can do the
> reparenting at output module load time. That would allow us to use the
> defaults unless otherwise specified by the device tree.
> 
> > > If tegradrm does later gain the ability to dynamically fiddle with
> the
> > > clock parenting, we can add entries in the (DT) list of clocks that
> > > the DCs or ORs needs; when just 1 is specified it's e.g. the "hdmi"
> > > clock and tegradrm never does any reparenting, whereas in the
> future
> > > we could allow 3 to be specified; "hdmi", parent-a, parent-b, and
> > > "unlock" any reparenting support.
> > >
> > > >> for T20, you'll want to use pll_d for HDMI, unless you need DSI,
> > > >> then you'll have to use pll_c for HDMI. That might be where the
> > > >> configuration comes in handy. If you want, you can pretend that
> > > >> DSI-based T20 devices aren't important enough to support now.
> > > >
> > > > I think we can safely ignore DSI for now since the driver doesn't
> > > > support it yet anyway. The driver is just a dummy that doesn't do
> > > > anything useful right now. But as I mentioned above, it should be
> > > > straightforward to override the clock setup on a per-board basis.
> > >
> > > We definitely don't want to preclude DSI from working, so we should
> > > consider it. We do support Whistler upstream, and that has a DSI
> > > panel, and there were some noises about getting it working
> sometime.
> > >
> >
> > DSI should work, just not at the same time as HDMI. Getting both to
> work at
> > the same time is a real pain in the neck. I think it is fair to draw
> a line
> > in the sand and say, at this time, you cannot operate both DSI and
> HDMI on
> > a Tegra2 device with this new driver.
> 
> I wasn't saying that we shouldn't support DSI, all I was saying is that
> we currently don't have any code to make it work. That's mainly because
> I don't have any hardware to test it on and therefore wasn't very
> motivated to port the 3000+ lines of code from the downstream driver.
> 

A continuous mode only support for DSI should be much easier to add. It's the setup to talk to smart panels and to switch between HS and LP modes that bloats the code. There is always a bit of DSI init that is unique per panel, people sometimes tried to put it into the core driver. but long term a blob of immutable programming values could be put into device tree, and then the driver init would be much simpler.

I'll ask around here to see who wants to do it. We have numerous DSI based devices here, both smart and dumb.

> > If someone wants to work out some special cases where it does work
> for a
> > particular product, then great. But I would leave that support out of
> an
> > upstreamable tegradrm driver.
> 
> Agreed.
> 
> > So a "feature" I would like is that we handle DSI xor HDMI operation.
> > dynamically switching between the two. Either by changing the SOR
> outputs
> > for an individual DC (and thus only present a single /dev/fbX
> device).
> 
> We should be able to make this work with the standard Xrandr interface.
> If you use the xf86-video-modesetting driver on top of tegra-drm it,
> then xrandr will be able to list all the configured outputs and allow
> them to be enabled or disabled at will. Again I haven't tested any
> setups with two concurrent outputs yet, but I have at least seen xrandr
> successfully switch modes for the HDMI output or turn it off
> altogether.
> 

Your design should work for this. there might be some details to work out after one of us actually tries it out, but architecturally it seems sound.

> Also with modern DRM the /dev/fbX interface is really just for legacy
> purposes. Everything else can be done using the new DRM interfaces,
> which basically allow you to allocate a buffer of a given size and make
> it the framebuffer of a given output. So while there is still a
> /dev/fb0
> that can be accessed for legacy purposes, the actual framebuffers
> aren't
> visible as device files any longer but rather as buffer objects to the
> owning application (and with the appropriate driver support shareable
> using DMA-BUF).
> 

On Android the OTA/recovery uses fbmem.c's interface, and uses ypan to do crude double buffering. And the fb console is mostly using the fb_set_par to scroll. So while it is legacy, I feel that fb is still going to be around for a while.

> > Or have a mechanism that crosses multiple device instances and
> refuses to
> > allow them to be enabled at the same time. (that's how the
> Android/L4T
> > driver works, but I don't like that way so much)
> 
> What DRM/xf86-video-modesetting implements is a mix of both of the
> above. You can dynamically enable or disable any of the outputs. I'm
> not
> certain how errors are propagated in case a certain combination cannot
> be handled by the DRM, but I don't think that would be a major blocker.
> 
> > > Considering a system with dual HDMI outputs and no built-in panel,
> and
> > > with mode timings on both outputs that could both be derived from a
> > > single clock, wouldn't using the same PLL for both outputs save
> power
> > > since the second PLL can be shut down? I'm not sure how much power
> > > this would save though; perhaps the power savings of a PLL aren't
> that
> > > important vs. the power savings from clock-gating or power-gating
> the
> > > logic that's clocked by the PLL.
> > >
> >
> > HDMI/CEA modes can be done by setting one pll to 297MHz. Then each DC
> can
> > div-by-2 for 148.5MHz (1080p50/60, 720p60-3d, 1080p30-3d-fp, 1080p60-
> 3d-sbs,
> > ..) or div-by-4 for 74.25MHz (720p50/60, ...). Thus allowing you to
> connect
> > any two HDMI TVs and use only one pll.
> 
> I think the majority of use-cases will be either LVDS only, HDMI only
> or
> LVDS with HDMI, all of which can be covered using the clock setups that
> we've been discussing.
> 
> > But most systems with two HDMI outputs will be newer tegras with lots
> of
> > PLLs, so it might not be too worthwhile to consider.
> 
> According to the TRM, both the Tegra20 and the Tegra30 have only a
> single HDMI output. Is that information wrong or are you talking about
> newer generations?
>

LVDS(digital rgb actually)->HDMI or DSI->HDMI transceivers. I even did some drivers for a crazy board for a MWC demo that used the 2 pixels / clock mode of RGB and fed it into a displayPort transceivers then into a DP to DVI and finally to a 2560x1600 monitor.

Occasionally we get customers that want to do multiple HDMI outputs because you can run the cables further than LVDS or DSI. And they generally have a specific panel in mind, and don't need HDCP. 

> > If you want to do VESA modes, you need dedicated PLLs. as they have
> lots
> > of different unrelated clock rates.
> 
> Some of the TV sets that I've been testing on claim to support various
> exotic VESA modes. One set even has a preferred mode of 1360x768. But
> it
> doesn't even work properly. You have to switch it to 1366x768 to make
> it
> look acceptable. Again I think if we support all of the HDMI/CEA modes
> a
> lot of ground is covered.
> 
> > > > Stephen, what do you think?
> > >
> > > Overall though, a static clock setup, at least initially, sounds
> > > reasonable to me.
> >
> > Great, if needed, we can extend it later. I'm satisfied this will
> cover
> > some good use cases, and we can avoid some messy complications.
> 
> I suspect that once the driver has made it into mainline and more
> people
> start using it, we'll likely get many bug reports about various modes
> not working properly and whatnot. But I think it would be unrealistic
> to
> expect the first version to be perfect.
> 
> Thierry
> 
> * Unknown Key
> * 0x7F3EB3A1

Agreed! Let's get lots of eyeballs on this driver, and it can evolve into something even better. 
I'm pretty excited about the direction of tegradrm. :)

- Jon

Re: Tegra DRM with HDMI support (\o/)

[Stephen Warren]

On 10/18/2012 12:29 AM, Thierry Reding wrote:
...
> According to the TRM, both the Tegra20 and the Tegra30 have only a 
> single HDMI output. Is that information wrong or are you talking
> about newer generations?

This isn't exactly what you asked, but:

TrimSlice pretends to have two HDMI outputs by using the real HDMI for
one connector, and converting the LVDS output to DVI signalling using
an external encoder/PHY, and routing the signals to an HDMI
form-factor connector.

Re: Tegra DRM with HDMI support (\o/)

[Stephen Warren]

On 10/17/2012 02:08 PM, Jon Mayo wrote:
> Stephen Warren wrote at Wednesday, October 17, 2012 12:42 PM:
>> On 10/17/2012 01:12 PM, Thierry Reding wrote:
>>> On Wed, Oct 17, 2012 at 11:04:05AM -0700, Jon Mayo wrote:
...
>>>> On T30, HDMI will work with pll_d, but I'm tipping my hand and
>>>> telling you that if you always use pll_d2 you won't have to mess
>>>> with this code in the next few chips. :)
>>>
>>> If we make this configurable in the device tree, we could provide
>>> both the "hdmi" and "base-clock" clocks, where base-clock defaults
>>> to pll_d or pll_d2 depending on the Tegra generation. If some board
>>> requires a more exotic setup it can override the clocks in the
>>> board DTS.
>>
>> I think if the parenting is going to be static, lets not have tegradrm
>> do it, but rather encode it into one of
>> arch/arm/mach-tegra/{common,board-dt-tegra[23]0}.c. Eventually, we
>> wish to replace the clock setup tables in those files with tables in
>> device tree, so they can differ easily per-board, which would also
>> handle the DSI situation well.
> 
> Well it's not entirely static. The clock parent should depend on the
> output type. rather than on which display controller you are using.
> Each DC can be configured for various output types. The output types
> can be represented in device-tree if you wish to have them fixed (I'd
> interpret it as a default rather than a fixed setting).

(Jon, the lack of line-wrapping makes your email a little hard to read)

Yes, that makes sense. We should leave the clock definitions up to the
OR driver rather than statically set them up based on the DC then.

Once clocks come from DT, that probably requires each OR to define two
clocks; the peripheral clock and the parent PLL, e.g.:

clocks = <&car CLK_HDMI> <&car CLK_PLL_D>;
clock-names = "output", "parent";

... and then do a clk_set_parent() when being activated.

Re: Tegra DRM with HDMI support (\o/)

[Mark Zhang]

Thank you for your hard work, Thierry!

We're looking forward to your final merged drm driver and hope we can
get it into 3.8.

Mark
On Fri, 2012-10-12 at 04:07 +0800, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> Hi,
> 
> I've finally managed to get HDMI working on Tegra20. Unfortunately the
> xf86-video-modesetting driver doesn't work on top of it yet, but I think
> that's a different error somewhere else and I'm still trying to figure
> out what exactly is going wrong. However, a framebuffer console can be
> run on top of it, as does a small test program that I've written for
> testing.
> 
> The branch that contains all of this is the tegra/next branch on my
> personal Linux kernel tree on gitorious[0]. Some other things are
> included as well, such as the latest PCIe driver, an experimental draft
> of a NAND driver (loosely based on the U-Boot driver by Jim Lin and
> Simon Glass) as well as various fixes and hacks. Some of it is probably
> also junk that I'll throw away at some point. The branch isn't very
> clean, but it should work well enough for anybody who wants to give it a
> try.
> 
> My plan was to get LVDS and HDMI to work on Tegra20 with CMA for GEM
> object allocations before submitting for mainline inclusion. Since all
> of these are now working I will clean up the branch, provide a branch
> with only the DRM bits into a separate branch and submit the patches for
> review, with the hope of getting them included in 3.8. A lot of the HDMI
> work has been contributed by NVIDIA's Mark Zhang, who is also busy (and
> apparently very close to) getting Tegra30 support working. If he can
> make it work, Tegra30 support can be included as well.
> 
> All of that said, I invite everyone with access to Tegra hardware to
> give the DRM driver a spin and report back if you encounter any
> problems. Feel free to also drop a note on success.
> 
> Thierry
> 
> [0]: git://gitorious.org/thierryreding/linux.git
> 
> * Unknown Key
> * 0x7F3EB3A1

Re: Tegra DRM with HDMI support (\o/)

[Mark Zhang]

Hi, 

I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
But the code that Thierry
hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
directly. If you build and run it, you'll get a white blank screen on
Tegra 3. I didn't find the root cause but I think this is not caused by
drm driver itself(maybe the PCIE codes?). The main symptom is: some
regulators can't be inited all the time. I tried to build drm driver as
kernel module, after kernel booted, modprobe the drm driver, and you'll
find there are a lot of error prints like this:

=========================================================
[   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
[   21.390299] reg-fixed-voltage 1.regulator: Failed to register
regulator: -517
[   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
probe deferral
[   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
probe deferral
=========================================================

So what I did is, fetched a copy of 1015 upstream codes, copy the drm
driver into it, then it worked on Tegra 3. One thing need to be noticed
is, to make the HDMI working, we also need to fix a minor problem:
change the type of variable "output_rate" in function
"tegra30_pll_round_rate" from "unsigned long" to "u64". Otherwise the
variable overflows which clock rate calculating. I'll send a patch about
this next. 

Mark
On Fri, 2012-10-12 at 04:07 +0800, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> Hi,
> 
> I've finally managed to get HDMI working on Tegra20. Unfortunately the
> xf86-video-modesetting driver doesn't work on top of it yet, but I think
> that's a different error somewhere else and I'm still trying to figure
> out what exactly is going wrong. However, a framebuffer console can be
> run on top of it, as does a small test program that I've written for
> testing.
> 
> The branch that contains all of this is the tegra/next branch on my
> personal Linux kernel tree on gitorious[0]. Some other things are
> included as well, such as the latest PCIe driver, an experimental draft
> of a NAND driver (loosely based on the U-Boot driver by Jim Lin and
> Simon Glass) as well as various fixes and hacks. Some of it is probably
> also junk that I'll throw away at some point. The branch isn't very
> clean, but it should work well enough for anybody who wants to give it a
> try.
> 
> My plan was to get LVDS and HDMI to work on Tegra20 with CMA for GEM
> object allocations before submitting for mainline inclusion. Since all
> of these are now working I will clean up the branch, provide a branch
> with only the DRM bits into a separate branch and submit the patches for
> review, with the hope of getting them included in 3.8. A lot of the HDMI
> work has been contributed by NVIDIA's Mark Zhang, who is also busy (and
> apparently very close to) getting Tegra30 support working. If he can
> make it work, Tegra30 support can be included as well.
> 
> All of that said, I invite everyone with access to Tegra hardware to
> give the DRM driver a spin and report back if you encounter any
> problems. Feel free to also drop a note on success.
> 
> Thierry
> 
> [0]: git://gitorious.org/thierryreding/linux.git
> 
> * Unknown Key
> * 0x7F3EB3A1

Re: Tegra DRM with HDMI support (\o/)

[Stephen Warren]

On 10/16/2012 02:18 AM, Mark Zhang wrote:
> Hi, 
> 
> I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
> But the code that Thierry
> hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
> directly. If you build and run it, you'll get a white blank screen on
> Tegra 3. I didn't find the root cause but I think this is not caused by
> drm driver itself(maybe the PCIE codes?). The main symptom is: some
> regulators can't be inited all the time. I tried to build drm driver as
> kernel module, after kernel booted, modprobe the drm driver, and you'll
> find there are a lot of error prints like this:
> 
> =========================================================
> [   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
> [   21.390299] reg-fixed-voltage 1.regulator: Failed to register
> regulator: -517
> [   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
> probe deferral
> [   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
> probe deferral
> =========================================================

This is normal; it's probe deferral. You'll find that those regulators
initialize correctly later. Hopefully tegra-drm handles this correctly,
and passes that error code through as the result of its probe.

Re: Tegra DRM with HDMI support (\o/)

[Mark Zhang]

On 10/17/2012 12:03 AM, Stephen Warren wrote:
> On 10/16/2012 02:18 AM, Mark Zhang wrote:
>> Hi,
>>
>> I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
>> But the code that Thierry
>> hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
>> directly. If you build and run it, you'll get a white blank screen on
>> Tegra 3. I didn't find the root cause but I think this is not caused by
>> drm driver itself(maybe the PCIE codes?). The main symptom is: some
>> regulators can't be inited all the time. I tried to build drm driver as
>> kernel module, after kernel booted, modprobe the drm driver, and you'll
>> find there are a lot of error prints like this:
>>
>> =========================================================
>> [   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
>> [   21.390299] reg-fixed-voltage 1.regulator: Failed to register
>> regulator: -517
>> [   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
>> probe deferral
>> [   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
>> probe deferral
>> =========================================================
>
> This is normal; it's probe deferral. You'll find that those regulators
> initialize correctly later. Hopefully tegra-drm handles this correctly,
> and passes that error code through as the result of its probe.
>

I don't think so. In normal scenario, the regulator will be inited 
correctly finally. But in the scenario I described above, these 
regulators will not be inited correctly all the time. Just as I 
mentioned above, I tried to build drm driver as kernel module, that 
means I will load drm driver after kernel booting finished. Besides, 
during the loading of drm driver, these regulator probe error were still 
be printed out. And I'm sure this is not the normal case.

Re: Tegra DRM with HDMI support (\o/)

[Stephen Warren]

On 10/16/2012 07:32 PM, Mark Zhang wrote:
> On 10/17/2012 12:03 AM, Stephen Warren wrote:
>> On 10/16/2012 02:18 AM, Mark Zhang wrote:
>>> Hi,
>>>
>>> I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
>>> But the code that Thierry
>>> hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
>>> directly. If you build and run it, you'll get a white blank screen on
>>> Tegra 3. I didn't find the root cause but I think this is not caused by
>>> drm driver itself(maybe the PCIE codes?). The main symptom is: some
>>> regulators can't be inited all the time. I tried to build drm driver as
>>> kernel module, after kernel booted, modprobe the drm driver, and you'll
>>> find there are a lot of error prints like this:
>>>
>>> =========================================================
>>> [   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
>>> [   21.390299] reg-fixed-voltage 1.regulator: Failed to register
>>> regulator: -517
>>> [   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
>>> probe deferral
>>> [   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
>>> probe deferral
>>> =========================================================
>>
>> This is normal; it's probe deferral. You'll find that those regulators
>> initialize correctly later. Hopefully tegra-drm handles this correctly,
>> and passes that error code through as the result of its probe.
>>
> 
> I don't think so. In normal scenario, the regulator will be inited
> correctly finally. But in the scenario I described above, these
> regulators will not be inited correctly all the time. Just as I
> mentioned above, I tried to build drm driver as kernel module, that
> means I will load drm driver after kernel booting finished. Besides,
> during the loading of drm driver, these regulator probe error were still
> be printed out. And I'm sure this is not the normal case.

The Cardhu PMIC driver isn't enabled in tegra_defconfig in Thierry's
branch. Do this:

> diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
> index c2a2f6e..90f4d5c 100644
> --- a/arch/arm/configs/tegra_defconfig
> +++ b/arch/arm/configs/tegra_defconfig
> @@ -124,16 +124,19 @@ CONFIG_SPI_OCORES=m
>  CONFIG_GPIO_SYSFS=y
>  CONFIG_GPIO_ADNP=y
>  CONFIG_GPIO_TPS6586X=y
> +CONFIG_GPIO_TPS65910=y
>  CONFIG_POWER_SUPPLY=y
>  CONFIG_BATTERY_SBS=y
>  CONFIG_SENSORS_LM90=y
> -CONFIG_SENSORS_ADC081C=m
>  CONFIG_MFD_TPS6586X=y
> +CONFIG_MFD_TPS65910=y
>  CONFIG_REGULATOR=y
>  CONFIG_REGULATOR_FIXED_VOLTAGE=y
>  CONFIG_REGULATOR_VIRTUAL_CONSUMER=y
>  CONFIG_REGULATOR_GPIO=y
> +CONFIG_REGULATOR_TPS62360=y
>  CONFIG_REGULATOR_TPS6586X=y
> +CONFIG_REGULATOR_TPS65910=y
>  CONFIG_DRM=m
>  CONFIG_DRM_TEGRA=m
>  CONFIG_DRM_TEGRA_DEBUG=y
> @@ -172,7 +175,7 @@ CONFIG_RTC_CLASS=y
>  CONFIG_RTC_INTF_SYSFS=y
>  CONFIG_RTC_INTF_PROC=y
>  CONFIG_RTC_INTF_DEV=y
> -CONFIG_RTC_DRV_PCF8523=y
> +CONFIG_RTC_DRV_TPS65910=y
>  CONFIG_RTC_DRV_TEGRA=y
>  CONFIG_DMADEVICES=y
>  CONFIG_TEGRA20_APB_DMA=y

(That still doesn't make tegradrm work for me on Cardhu, but it does
solve the regulator issue)

Re: Tegra DRM with HDMI support (\o/)

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4356 bytes --]

On Wed, Oct 17, 2012 at 02:38:50PM -0600, Stephen Warren wrote:
> On 10/16/2012 07:32 PM, Mark Zhang wrote:
> > On 10/17/2012 12:03 AM, Stephen Warren wrote:
> >> On 10/16/2012 02:18 AM, Mark Zhang wrote:
> >>> Hi,
> >>>
> >>> I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
> >>> But the code that Thierry
> >>> hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
> >>> directly. If you build and run it, you'll get a white blank screen on
> >>> Tegra 3. I didn't find the root cause but I think this is not caused by
> >>> drm driver itself(maybe the PCIE codes?). The main symptom is: some
> >>> regulators can't be inited all the time. I tried to build drm driver as
> >>> kernel module, after kernel booted, modprobe the drm driver, and you'll
> >>> find there are a lot of error prints like this:
> >>>
> >>> =========================================================
> >>> [   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
> >>> [   21.390299] reg-fixed-voltage 1.regulator: Failed to register
> >>> regulator: -517
> >>> [   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
> >>> probe deferral
> >>> [   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
> >>> probe deferral
> >>> =========================================================
> >>
> >> This is normal; it's probe deferral. You'll find that those regulators
> >> initialize correctly later. Hopefully tegra-drm handles this correctly,
> >> and passes that error code through as the result of its probe.
> >>
> > 
> > I don't think so. In normal scenario, the regulator will be inited
> > correctly finally. But in the scenario I described above, these
> > regulators will not be inited correctly all the time. Just as I
> > mentioned above, I tried to build drm driver as kernel module, that
> > means I will load drm driver after kernel booting finished. Besides,
> > during the loading of drm driver, these regulator probe error were still
> > be printed out. And I'm sure this is not the normal case.
> 
> The Cardhu PMIC driver isn't enabled in tegra_defconfig in Thierry's
> branch. Do this:

Yeah, I think my default configuration is pretty messed up by now. I'm
in the process of merging down patches and along the way I planned to
drop all the defconfig updates and do the from scratch so that these
issues will hopefully be gone.

> > diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
> > index c2a2f6e..90f4d5c 100644
> > --- a/arch/arm/configs/tegra_defconfig
> > +++ b/arch/arm/configs/tegra_defconfig
> > @@ -124,16 +124,19 @@ CONFIG_SPI_OCORES=m
> >  CONFIG_GPIO_SYSFS=y
> >  CONFIG_GPIO_ADNP=y
> >  CONFIG_GPIO_TPS6586X=y
> > +CONFIG_GPIO_TPS65910=y
> >  CONFIG_POWER_SUPPLY=y
> >  CONFIG_BATTERY_SBS=y
> >  CONFIG_SENSORS_LM90=y
> > -CONFIG_SENSORS_ADC081C=m
> >  CONFIG_MFD_TPS6586X=y
> > +CONFIG_MFD_TPS65910=y
> >  CONFIG_REGULATOR=y
> >  CONFIG_REGULATOR_FIXED_VOLTAGE=y
> >  CONFIG_REGULATOR_VIRTUAL_CONSUMER=y
> >  CONFIG_REGULATOR_GPIO=y
> > +CONFIG_REGULATOR_TPS62360=y
> >  CONFIG_REGULATOR_TPS6586X=y
> > +CONFIG_REGULATOR_TPS65910=y
> >  CONFIG_DRM=m
> >  CONFIG_DRM_TEGRA=m
> >  CONFIG_DRM_TEGRA_DEBUG=y
> > @@ -172,7 +175,7 @@ CONFIG_RTC_CLASS=y
> >  CONFIG_RTC_INTF_SYSFS=y
> >  CONFIG_RTC_INTF_PROC=y
> >  CONFIG_RTC_INTF_DEV=y
> > -CONFIG_RTC_DRV_PCF8523=y
> > +CONFIG_RTC_DRV_TPS65910=y
> >  CONFIG_RTC_DRV_TEGRA=y
> >  CONFIG_DMADEVICES=y
> >  CONFIG_TEGRA20_APB_DMA=y
> 
> (That still doesn't make tegradrm work for me on Cardhu, but it does
> solve the regulator issue)

I think Mark had some other modifications that were required to get HDMI
working on CardHu. Something related to the clocks IIRC. Does LVDS work?

Another thing I'm going to work into the cleaned up patches is support
for the video mode helpers by Steffen. I actually like the approach
better than the binary EDID blob, so I will probably replace the blob on
Medcom Wide in favour of a display mode entry in DT.

By the way, it would be nice if we could start to collect the various DT
snippets for the different boards. For most of the NVIDIA designs these
are still missing, just like for AC100. As for the TrimSlice I'm not
sure if it works properly.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Tegra DRM with HDMI support (\o/)

[Mark Zhang]

On 10/18/2012 01:55 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Wed, Oct 17, 2012 at 02:38:50PM -0600, Stephen Warren wrote:
>> On 10/16/2012 07:32 PM, Mark Zhang wrote:
>>> On 10/17/2012 12:03 AM, Stephen Warren wrote:
>>>> On 10/16/2012 02:18 AM, Mark Zhang wrote:
>>>>> Hi,
>>>>>
>>>>> I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
>>>>> But the code that Thierry
>>>>> hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
>>>>> directly. If you build and run it, you'll get a white blank screen on
>>>>> Tegra 3. I didn't find the root cause but I think this is not caused by
>>>>> drm driver itself(maybe the PCIE codes?). The main symptom is: some
>>>>> regulators can't be inited all the time. I tried to build drm driver as
>>>>> kernel module, after kernel booted, modprobe the drm driver, and you'll
>>>>> find there are a lot of error prints like this:
>>>>>
>>>>> =========================================================
>>>>> [   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
>>>>> [   21.390299] reg-fixed-voltage 1.regulator: Failed to register
>>>>> regulator: -517
>>>>> [   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
>>>>> probe deferral
>>>>> [   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
>>>>> probe deferral
>>>>> =========================================================
>>>>
>>>> This is normal; it's probe deferral. You'll find that those regulators
>>>> initialize correctly later. Hopefully tegra-drm handles this correctly,
>>>> and passes that error code through as the result of its probe.
>>>>
>>>
>>> I don't think so. In normal scenario, the regulator will be inited
>>> correctly finally. But in the scenario I described above, these
>>> regulators will not be inited correctly all the time. Just as I
>>> mentioned above, I tried to build drm driver as kernel module, that
>>> means I will load drm driver after kernel booting finished. Besides,
>>> during the loading of drm driver, these regulator probe error were still
>>> be printed out. And I'm sure this is not the normal case.
>>
>> The Cardhu PMIC driver isn't enabled in tegra_defconfig in Thierry's
>> branch. Do this:
>
> Yeah, I think my default configuration is pretty messed up by now. I'm
> in the process of merging down patches and along the way I planned to
> drop all the defconfig updates and do the from scratch so that these
> issues will hopefully be gone.
>
>>> diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
>>> index c2a2f6e..90f4d5c 100644
>>> --- a/arch/arm/configs/tegra_defconfig
>>> +++ b/arch/arm/configs/tegra_defconfig
>>> @@ -124,16 +124,19 @@ CONFIG_SPI_OCORES=m
>>>   CONFIG_GPIO_SYSFS=y
>>>   CONFIG_GPIO_ADNP=y
>>>   CONFIG_GPIO_TPS6586X=y
>>> +CONFIG_GPIO_TPS65910=y
>>>   CONFIG_POWER_SUPPLY=y
>>>   CONFIG_BATTERY_SBS=y
>>>   CONFIG_SENSORS_LM90=y
>>> -CONFIG_SENSORS_ADC081C=m
>>>   CONFIG_MFD_TPS6586X=y
>>> +CONFIG_MFD_TPS65910=y
>>>   CONFIG_REGULATOR=y
>>>   CONFIG_REGULATOR_FIXED_VOLTAGE=y
>>>   CONFIG_REGULATOR_VIRTUAL_CONSUMER=y
>>>   CONFIG_REGULATOR_GPIO=y
>>> +CONFIG_REGULATOR_TPS62360=y
>>>   CONFIG_REGULATOR_TPS6586X=y
>>> +CONFIG_REGULATOR_TPS65910=y
>>>   CONFIG_DRM=m
>>>   CONFIG_DRM_TEGRA=m
>>>   CONFIG_DRM_TEGRA_DEBUG=y
>>> @@ -172,7 +175,7 @@ CONFIG_RTC_CLASS=y
>>>   CONFIG_RTC_INTF_SYSFS=y
>>>   CONFIG_RTC_INTF_PROC=y
>>>   CONFIG_RTC_INTF_DEV=y
>>> -CONFIG_RTC_DRV_PCF8523=y
>>> +CONFIG_RTC_DRV_TPS65910=y
>>>   CONFIG_RTC_DRV_TEGRA=y
>>>   CONFIG_DMADEVICES=y
>>>   CONFIG_TEGRA20_APB_DMA=y
>>
>> (That still doesn't make tegradrm work for me on Cardhu, but it does
>> solve the regulator issue)
>
> I think Mark had some other modifications that were required to get HDMI
> working on CardHu. Something related to the clocks IIRC. Does LVDS work?
>
> Another thing I'm going to work into the cleaned up patches is support
> for the video mode helpers by Steffen. I actually like the approach
> better than the binary EDID blob, so I will probably replace the blob on
> Medcom Wide in favour of a display mode entry in DT.
>
> By the way, it would be nice if we could start to collect the various DT
> snippets for the different boards. For most of the NVIDIA designs these
> are still missing, just like for AC100. As for the TrimSlice I'm not
> sure if it works properly.
>

Yes, I have some DT snippets for Tegra 2 Ventana board. I'll send it to 
you later.

Mark
> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

Re: Tegra DRM with HDMI support (\o/)

[Mark Zhang]

On 10/18/2012 04:38 AM, Stephen Warren wrote:
> On 10/16/2012 07:32 PM, Mark Zhang wrote:
>> On 10/17/2012 12:03 AM, Stephen Warren wrote:
>>> On 10/16/2012 02:18 AM, Mark Zhang wrote:
>>>> Hi,
>>>>
>>>> I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
>>>> But the code that Thierry
>>>> hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
>>>> directly. If you build and run it, you'll get a white blank screen on
>>>> Tegra 3. I didn't find the root cause but I think this is not caused by
>>>> drm driver itself(maybe the PCIE codes?). The main symptom is: some
>>>> regulators can't be inited all the time. I tried to build drm driver as
>>>> kernel module, after kernel booted, modprobe the drm driver, and you'll
>>>> find there are a lot of error prints like this:
>>>>
>>>> =========================================================
>>>> [   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
>>>> [   21.390299] reg-fixed-voltage 1.regulator: Failed to register
>>>> regulator: -517
>>>> [   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
>>>> probe deferral
>>>> [   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
>>>> probe deferral
>>>> =========================================================
>>>
>>> This is normal; it's probe deferral. You'll find that those regulators
>>> initialize correctly later. Hopefully tegra-drm handles this correctly,
>>> and passes that error code through as the result of its probe.
>>>
>>
>> I don't think so. In normal scenario, the regulator will be inited
>> correctly finally. But in the scenario I described above, these
>> regulators will not be inited correctly all the time. Just as I
>> mentioned above, I tried to build drm driver as kernel module, that
>> means I will load drm driver after kernel booting finished. Besides,
>> during the loading of drm driver, these regulator probe error were still
>> be printed out. And I'm sure this is not the normal case.
>
> The Cardhu PMIC driver isn't enabled in tegra_defconfig in Thierry's
> branch. Do this:
>
>> diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
>> index c2a2f6e..90f4d5c 100644
>> --- a/arch/arm/configs/tegra_defconfig
>> +++ b/arch/arm/configs/tegra_defconfig
>> @@ -124,16 +124,19 @@ CONFIG_SPI_OCORES=m
>>   CONFIG_GPIO_SYSFS=y
>>   CONFIG_GPIO_ADNP=y
>>   CONFIG_GPIO_TPS6586X=y
>> +CONFIG_GPIO_TPS65910=y
>>   CONFIG_POWER_SUPPLY=y
>>   CONFIG_BATTERY_SBS=y
>>   CONFIG_SENSORS_LM90=y
>> -CONFIG_SENSORS_ADC081C=m
>>   CONFIG_MFD_TPS6586X=y
>> +CONFIG_MFD_TPS65910=y
>>   CONFIG_REGULATOR=y
>>   CONFIG_REGULATOR_FIXED_VOLTAGE=y
>>   CONFIG_REGULATOR_VIRTUAL_CONSUMER=y
>>   CONFIG_REGULATOR_GPIO=y
>> +CONFIG_REGULATOR_TPS62360=y
>>   CONFIG_REGULATOR_TPS6586X=y
>> +CONFIG_REGULATOR_TPS65910=y
>>   CONFIG_DRM=m
>>   CONFIG_DRM_TEGRA=m
>>   CONFIG_DRM_TEGRA_DEBUG=y
>> @@ -172,7 +175,7 @@ CONFIG_RTC_CLASS=y
>>   CONFIG_RTC_INTF_SYSFS=y
>>   CONFIG_RTC_INTF_PROC=y
>>   CONFIG_RTC_INTF_DEV=y
>> -CONFIG_RTC_DRV_PCF8523=y
>> +CONFIG_RTC_DRV_TPS65910=y
>>   CONFIG_RTC_DRV_TEGRA=y
>>   CONFIG_DMADEVICES=y
>>   CONFIG_TEGRA20_APB_DMA=y
>
> (That still doesn't make tegradrm work for me on Cardhu, but it does
> solve the regulator issue)

Yes, it works. Thank you.
To make the cardhu working(LVDS and HDMI), you need this patch(check 
below). Sorry I forget to send this patch to Thierry and I'll do that now.

diff --git a/arch/arm/mach-tegra/board-dt-tegra30.c 
b/arch/arm/mach-tegra/board-dt-tegra30.c
index 03d2932..9f83b35 100644
--- a/arch/arm/mach-tegra/board-dt-tegra30.c
+++ b/arch/arm/mach-tegra/board-dt-tegra30.c
@@ -73,6 +73,10 @@ static __initdata struct tegra_clk_init_table 
tegra_dt_clk_init_table[] = {
         { "i2s2",       "pll_a_out0",   11289600,       false},
         { "i2s3",       "pll_a_out0",   11289600,       false},
         { "i2s4",       "pll_a_out0",   11289600,       false},
+        { "host1x",     "pll_c",        144000000,      true },
+        { "disp1",      "pll_p",        74000000,       false },
+        { "disp2",      "pll_d2_out0",  148500000,      false },
+        { "hdmi",       "pll_d2_out0",  148500000,      false },
         { NULL,         NULL,           0,              0},
  };

Mark
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: Tegra DRM with HDMI support (\o/)

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]

On Tue, Oct 16, 2012 at 10:03:56AM -0600, Stephen Warren wrote:
> On 10/16/2012 02:18 AM, Mark Zhang wrote:
> > Hi, 
> > 
> > I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
> > But the code that Thierry
> > hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
> > directly. If you build and run it, you'll get a white blank screen on
> > Tegra 3. I didn't find the root cause but I think this is not caused by
> > drm driver itself(maybe the PCIE codes?). The main symptom is: some
> > regulators can't be inited all the time. I tried to build drm driver as
> > kernel module, after kernel booted, modprobe the drm driver, and you'll
> > find there are a lot of error prints like this:
> > 
> > =========================================================
> > [   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
> > [   21.390299] reg-fixed-voltage 1.regulator: Failed to register
> > regulator: -517
> > [   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
> > probe deferral
> > [   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
> > probe deferral
> > =========================================================
> 
> This is normal; it's probe deferral. You'll find that those regulators
> initialize correctly later. Hopefully tegra-drm handles this correctly,
> and passes that error code through as the result of its probe.

I presume those regulators are for the vdd and pll supplies of the HDMI
output? Those should be properly handled by the tegra_hdmi_probe()
function. Also the way tegra-drm works right now, unless all of the
outputs are registered properly the DRM portion will never even be
initialized.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Tegra DRM with HDMI support (\o/)

[Mark Zhang]

On 10/17/2012 01:42 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Tue, Oct 16, 2012 at 10:03:56AM -0600, Stephen Warren wrote:
>> On 10/16/2012 02:18 AM, Mark Zhang wrote:
>>> Hi,
>>>
>>> I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
>>> But the code that Thierry
>>> hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
>>> directly. If you build and run it, you'll get a white blank screen on
>>> Tegra 3. I didn't find the root cause but I think this is not caused by
>>> drm driver itself(maybe the PCIE codes?). The main symptom is: some
>>> regulators can't be inited all the time. I tried to build drm driver as
>>> kernel module, after kernel booted, modprobe the drm driver, and you'll
>>> find there are a lot of error prints like this:
>>>
>>> =========================================================
>>> [   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
>>> [   21.390299] reg-fixed-voltage 1.regulator: Failed to register
>>> regulator: -517
>>> [   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
>>> probe deferral
>>> [   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
>>> probe deferral
>>> =========================================================
>>
>> This is normal; it's probe deferral. You'll find that those regulators
>> initialize correctly later. Hopefully tegra-drm handles this correctly,
>> and passes that error code through as the result of its probe.
>
> I presume those regulators are for the vdd and pll supplies of the HDMI
> output? Those should be properly handled by the tegra_hdmi_probe()
> function. Also the way tegra-drm works right now, unless all of the
> outputs are registered properly the DRM portion will never even be
> initialized.
>

Not exactly. In hdmi driver, if I try to get/enable vdd & pll regulators 
which HDMI needs, error occurs. So currently to make HDMI working on 
Tegra 3, we need to temporarily remove these codes.
In fact, there are a lot of this kind of errors, I just pasted some of 
them. So there is something in your changes which blocks these 
regulators correctly inited cause this issue doesn't exist in upstream 
codes.

> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

Re: Tegra DRM with HDMI support (\o/)

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2870 bytes --]

On Thu, Oct 18, 2012 at 02:04:37PM +0800, Mark Zhang wrote:
> On 10/17/2012 01:42 PM, Thierry Reding wrote:
> >* PGP Signed by an unknown key
> >
> >On Tue, Oct 16, 2012 at 10:03:56AM -0600, Stephen Warren wrote:
> >>On 10/16/2012 02:18 AM, Mark Zhang wrote:
> >>>Hi,
> >>>
> >>>I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
> >>>But the code that Thierry
> >>>hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
> >>>directly. If you build and run it, you'll get a white blank screen on
> >>>Tegra 3. I didn't find the root cause but I think this is not caused by
> >>>drm driver itself(maybe the PCIE codes?). The main symptom is: some
> >>>regulators can't be inited all the time. I tried to build drm driver as
> >>>kernel module, after kernel booted, modprobe the drm driver, and you'll
> >>>find there are a lot of error prints like this:
> >>>
> >>>=========================================================
> >>>[   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
> >>>[   21.390299] reg-fixed-voltage 1.regulator: Failed to register
> >>>regulator: -517
> >>>[   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
> >>>probe deferral
> >>>[   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
> >>>probe deferral
> >>>=========================================================
> >>
> >>This is normal; it's probe deferral. You'll find that those regulators
> >>initialize correctly later. Hopefully tegra-drm handles this correctly,
> >>and passes that error code through as the result of its probe.
> >
> >I presume those regulators are for the vdd and pll supplies of the HDMI
> >output? Those should be properly handled by the tegra_hdmi_probe()
> >function. Also the way tegra-drm works right now, unless all of the
> >outputs are registered properly the DRM portion will never even be
> >initialized.
> >
> 
> Not exactly. In hdmi driver, if I try to get/enable vdd & pll
> regulators which HDMI needs, error occurs. So currently to make HDMI
> working on Tegra 3, we need to temporarily remove these codes.
> In fact, there are a lot of this kind of errors, I just pasted some
> of them. So there is something in your changes which blocks these
> regulators correctly inited cause this issue doesn't exist in
> upstream codes.

I don't understand. If you remove the code that enables the regulators,
how can HDMI work at all?

The code in tegra_hdmi_probe() tries to acquire the regulators and
should return an error if it can't. But if HDMI is enabled and the
driver can't access the resources for HDMI, then nothing will be
registered with the DRM core in the first place. That's actually what
the drm_clients list is used for. Basically this defers the DRM
registration until all clients are available.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Tegra DRM with HDMI support (\o/)

[Mark Zhang]

On 10/18/2012 02:34 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Thu, Oct 18, 2012 at 02:04:37PM +0800, Mark Zhang wrote:
>> On 10/17/2012 01:42 PM, Thierry Reding wrote:
>>>> Old Signed by an unknown key
>>>
>>> On Tue, Oct 16, 2012 at 10:03:56AM -0600, Stephen Warren wrote:
>>>> On 10/16/2012 02:18 AM, Mark Zhang wrote:
>>>>> Hi,
>>>>>
>>>>> I've made this drm driver worked on Tegra 3(LVDS and HDMI both).
>>>>> But the code that Thierry
>>>>> hosts(git://gitorious.org/thierryreding/linux.git) doesn't work
>>>>> directly. If you build and run it, you'll get a white blank screen on
>>>>> Tegra 3. I didn't find the root cause but I think this is not caused by
>>>>> drm driver itself(maybe the PCIE codes?). The main symptom is: some
>>>>> regulators can't be inited all the time. I tried to build drm driver as
>>>>> kernel module, after kernel booted, modprobe the drm driver, and you'll
>>>>> find there are a lot of error prints like this:
>>>>>
>>>>> =========================================================
>>>>> [   21.383529] reg-fixed-voltage 1.regulator: Failed to find supply vin
>>>>> [   21.390299] reg-fixed-voltage 1.regulator: Failed to register
>>>>> regulator: -517
>>>>> [   21.397645] platform 1.regulator: Driver reg-fixed-voltage requests
>>>>> probe deferral
>>>>> [   21.405604] platform 2.regulator: Driver reg-fixed-voltage requests
>>>>> probe deferral
>>>>> =========================================================
>>>>
>>>> This is normal; it's probe deferral. You'll find that those regulators
>>>> initialize correctly later. Hopefully tegra-drm handles this correctly,
>>>> and passes that error code through as the result of its probe.
>>>
>>> I presume those regulators are for the vdd and pll supplies of the HDMI
>>> output? Those should be properly handled by the tegra_hdmi_probe()
>>> function. Also the way tegra-drm works right now, unless all of the
>>> outputs are registered properly the DRM portion will never even be
>>> initialized.
>>>
>>
>> Not exactly. In hdmi driver, if I try to get/enable vdd & pll
>> regulators which HDMI needs, error occurs. So currently to make HDMI
>> working on Tegra 3, we need to temporarily remove these codes.
>> In fact, there are a lot of this kind of errors, I just pasted some
>> of them. So there is something in your changes which blocks these
>> regulators correctly inited cause this issue doesn't exist in
>> upstream codes.
>
> I don't understand. If you remove the code that enables the regulators,
> how can HDMI work at all?
>
> The code in tegra_hdmi_probe() tries to acquire the regulators and
> should return an error if it can't. But if HDMI is enabled and the
> driver can't access the resources for HDMI, then nothing will be
> registered with the DRM core in the first place. That's actually what
> the drm_clients list is used for. Basically this defers the DRM
> registration until all clients are available.
>

Stephen gave the answer. We need to enable TI TPS65910 and other related 
regulator/gpio settings in kernel config. And I tested just now that it 
worked and the HDMI regulator get/enble was OK as well.
Sorry for the confusion.

> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>