[Nouveau] [RESEND PATCH nouveau 3/3] volt: add support for GK20A
Alexandre Courbot
gnurou at gmail.com
Sat Nov 29 00:44:57 PST 2014
Hi Roy,
On Fri, Nov 28, 2014 at 9:09 PM, Roy Spliet <seven at nimrod-online.com> wrote:
> Hello Vince,
>
> Op 28-11-14 om 12:57 schreef Vince Hsu:
>
>> Hi Roy,
>>
>> On 11/28/2014 07:25 PM, Roy Spliet wrote:
>>>
>>> Hello Vince,
>>>
>>> One minor question inline.
>>>
>>> Op 28-11-14 om 12:13 schreef Vince Hsu:
>>>>
>>>> The voltage value are calculated by the hardware characterized
>>>> result.
>>>>
>>>> Signed-off-by: Vince Hsu <vinceh at nvidia.com>
>>>> ---
>>>>
>>>> Resend this patch with the fuse change and proper patch prefix
>>>> per Thierry's request.
>>>>
>>>> drm/Kbuild | 1 +
>>>> drm/core/subdev/volt/gk20a.c | 1 +
>>>> nvkm/engine/device/nve0.c | 1 +
>>>> nvkm/include/subdev/volt.h | 1 +
>>>> nvkm/subdev/clock/gk20a.c | 15 ++++
>>>> nvkm/subdev/volt/gk20a.c | 202
>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>> 6 files changed, 221 insertions(+)
>>>> create mode 120000 drm/core/subdev/volt/gk20a.c
>>>> create mode 100644 nvkm/subdev/volt/gk20a.c
>>>>
>>>> diff --git a/drm/Kbuild b/drm/Kbuild
>>>> index 728bc5b66b29..7c49e6655066 100644
>>>> --- a/drm/Kbuild
>>>> +++ b/drm/Kbuild
>>>> @@ -225,6 +225,7 @@ nouveau-y += core/subdev/vm/nvc0.o
>>>> nouveau-y += core/subdev/volt/base.o
>>>> nouveau-y += core/subdev/volt/gpio.o
>>>> nouveau-y += core/subdev/volt/nv40.o
>>>> +nouveau-y += core/subdev/volt/gk20a.o
>>>> nouveau-y += core/engine/falcon.o
>>>> nouveau-y += core/engine/xtensa.o
>>>> diff --git a/drm/core/subdev/volt/gk20a.c b/drm/core/subdev/volt/gk20a.c
>>>> new file mode 120000
>>>> index 000000000000..2894eb1ede13
>>>> --- /dev/null
>>>> +++ b/drm/core/subdev/volt/gk20a.c
>>>> @@ -0,0 +1 @@
>>>> +../../../../nvkm/subdev/volt/gk20a.c
>>>> \ No newline at end of file
>>>> diff --git a/nvkm/engine/device/nve0.c b/nvkm/engine/device/nve0.c
>>>> index b1b2e484ecfa..674da1f095b2 100644
>>>> --- a/nvkm/engine/device/nve0.c
>>>> +++ b/nvkm/engine/device/nve0.c
>>>> @@ -179,6 +179,7 @@ nve0_identify(struct nouveau_device *device)
>>>> device->oclass[NVDEV_ENGINE_GR ] = gk20a_graph_oclass;
>>>> device->oclass[NVDEV_ENGINE_COPY2 ] = &nve0_copy2_oclass;
>>>> device->oclass[NVDEV_ENGINE_PERFMON] = &nve0_perfmon_oclass;
>>>> + device->oclass[NVDEV_SUBDEV_VOLT ] = &gk20a_volt_oclass;
>>>> break;
>>>> case 0xf0:
>>>> device->cname = "GK110";
>>>> diff --git a/nvkm/include/subdev/volt.h b/nvkm/include/subdev/volt.h
>>>> index 820b62ffd75b..67db5e58880d 100644
>>>> --- a/nvkm/include/subdev/volt.h
>>>> +++ b/nvkm/include/subdev/volt.h
>>>> @@ -52,6 +52,7 @@ int _nouveau_volt_init(struct nouveau_object *);
>>>> #define _nouveau_volt_fini _nouveau_subdev_fini
>>>> extern struct nouveau_oclass nv40_volt_oclass;
>>>> +extern struct nouveau_oclass gk20a_volt_oclass;
>>>> int nouveau_voltgpio_init(struct nouveau_volt *);
>>>> int nouveau_voltgpio_get(struct nouveau_volt *);
>>>> diff --git a/nvkm/subdev/clock/gk20a.c b/nvkm/subdev/clock/gk20a.c
>>>> index 82abbea2be12..fb4fad374bdd 100644
>>>> --- a/nvkm/subdev/clock/gk20a.c
>>>> +++ b/nvkm/subdev/clock/gk20a.c
>>>> @@ -470,76 +470,91 @@ gk20a_pstates[] = {
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 72000,
>>>> + .voltage = 0,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 108000,
>>>> + .voltage = 1,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 180000,
>>>> + .voltage = 2,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 252000,
>>>> + .voltage = 3,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 324000,
>>>> + .voltage = 4,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 396000,
>>>> + .voltage = 5,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 468000,
>>>> + .voltage = 6,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 540000,
>>>> + .voltage = 7,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 612000,
>>>> + .voltage = 8,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 648000,
>>>> + .voltage = 9,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 684000,
>>>> + .voltage = 10,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 708000,
>>>> + .voltage = 11,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 756000,
>>>> + .voltage = 12,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 804000,
>>>> + .voltage = 13,
>>>> },
>>>> },
>>>> {
>>>> .base = {
>>>> .domain[nv_clk_src_gpc] = 852000,
>>>> + .voltage = 14,
>>>> },
>>>> },
>>>> };
>>>
>>>
>>> Is there a particular reason why this table is hard-coded rather than
>>> stored in the device tree? It doesn't seem to differ much between different
>>> gk20a's (but this might change with the denver-core version?), but I do
>>> anticipate a lot of "code" duplication when post-K1 cores are released and
>>> supported.
>>
>> Hmmm.. That's probably because I just realized we have some other example
>> like the bios subdev is using the device tress stuff. The table is chip
>> specific, not board specific. Should we put it in device tree?
>> And the table should be the same between all the gk20a's although there is
>> no guarantee. If the post-K1 cores also integrate the gk20a, they should use
>> the same driver including this file. If not, I believe we will have some
>> clever way to handle that. That's unlikely to happen though. :)
>>
>> Thanks,
>> Vince
>
>
> I'm not sure if I completely understand your reply, so forgive me if I am
> stating some obvious things:
> The reason why I brought this up is because, the way I see it, DTS is the
> replacement for (V)BIOS on ARM platforms, giving a set of parameters that
> drivers (nouveau) can use for that particular instance (the Tegra K1 SoC) of
> some more generic IP (gk20a). All the other devices nouveau supports have a
> VBIOS to describe this kind of information to us, hence we haven't seen this
> before. For CPUs there are plenty of examples though of such params defined
> in DT: in arch/arm/boot/dts/ : imx6qdl.dtsi documents the min and max volt
> for regulators, while the CPUs have a little freq<->volt mapping in
> imx6q.dtsi. GPUs are new in a sense that NVIDIA is the first to actively
> support upstream development (thanks!)
> Secondly, we should keep in mind that DT is not tied to Linux; I believe
> Linaro's long term goal is to take the DT from the Linux tree and maintain
> it as a separate tree, to be used with U-boot, *BSD, maybe even Windows.
> These kind of parameters are not very platform-dependent and although they
> seem like a little detail that's easy to reproduce on every platform,
> looking at the sheer size of the VBIOS data that could mean a *lot* of
> duplication.
> I bring this up not because I think I know better, but rather because I
> believe it's a good discussion to have now that there still is little legacy
> on ARM SoCs in nouveau. DTS is still a work-in-progress, and at this moment
> we have the opportunity to consider what needs to be documented in there and
> what doesn't. I would actually love to hear other, more experienced
> developers chime in as well (Rob Clark? Ben Skeggs? Linaro folks?) and see
> how they feel.
Thanks for raising this point. I agree with your interpretation that
DT is comparable to the VBIOS in desktop GPUs. The question then
becomes whether this data can vary between different GK20A-using
boards (and in this case this should probably be part of DT) or not
(in which case I would advocate having this static information in the
driver itself). Since I don't expect different GK20A-using chips to
require different voltage for given frequencies, my gut feeling for
the moment is that having this information in the driver is fine. I
have added a few other NVIDIA people to gather thoughts.
IIUC this question also applies to other tables in this patch that
depend on frequency, like gk20a_cvb_coef. Huge tables of data are
generally frowned-upon by the DT folks, but there are already examples
of this (see for instance
http://article.gmane.org/gmane.linux.kernel/1831719 ).
More information about the Nouveau
mailing list