[Nouveau] [PATCH 06/12] drm/nouveau/ibus: add GK20A support
On Wed, Apr 2, 2014 at 11:18 PM, Ilia Mirkin wrote:
> On Wed, Apr 2, 2014 at 9:52 AM, Alexandre Courbot wrote:
>> On Tue, Mar 25, 2014 at 7:34 AM, Thierry Reding
>> wrote:
>>> On Mon, Mar 24, 2014 at 05:42:28PM +0900, Alexandre Courbot wrote:
>>> [...]
diff --git a/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
b/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
>>> [...]
+#include
+
+struct nvea_ibus_priv {
+ struct nouveau_ibus base;
+};
+
+static void
+nvea_ibus_init_priv_ring(struct nvea_ibus_priv *priv)
+{
+ nv_mask(priv, 0x137250, 0x3f, 0);
+
+ nv_mask(priv, 0x000200, 0x20, 0);
+ udelay(20);
>>>
>>> usleep_range()?
>>
>> Sure.
>>
>>>
+static void
+nvea_ibus_intr(struct nouveau_subdev *subdev)
+{
>>> [...]
+ /* Acknowledge interrupt */
+ nv_mask(priv, 0x12004c, 0x2, 0x2);
+
+ while (--retry >= 0) {
+ command = nv_rd32(priv, 0x12004c) & 0x3f;
+ if (command == 0)
+ break;
+ }
+
+ if (retry < 0)
+ nv_warn(priv, "timeout waiting for ringmaster ack\n");
+}
>>>
>>> Perhaps I'm being paranoid, but this loop now depends on the frequency
>>> of the various clocks involved and therefore might break at some point
>>> if the frequencies get sufficiently high.
>>>
>>> So a slightly safer implementation would use a proper timeout using a
>>> combination of msecs_to_jiffies(), time_before() and usleep_range(),
>>> like so:
>>>
>>> timeout = jiffies + msecs_to_jiffies(...);
>>>
>>> while (time_before(jiffies, timeout)) {
>>> command = nv_rd32(...) & 0x3f;
>>> if (command == 0)
>>> break;
>>>
>>> usleep_range(...);
>>> }
>>>
>>> if (time_after(jiffies, timeout))
>>> nv_warn(...);
>>
>> Right, now that I look at this code again I don't even understand why
>> I left it this way. Maybe I left some early test code slip into the
>> final patch, sorry about that.
>
> I just remembered about this, but there's also the nv_wait() macro,
> which you could use, e.g.
>
> if (!nv_wait(subdev, 0x12004c, 0x3f, 0x00))
> nv_warn(...)
>
> It has built-in timeout logic/etc (although no sleeps in the middle).
> It does use the timer subdev, so if that's not operational at this
> point, you can't use it.
IBUS comes after TIMER in the nv_subdev_type enum, so I guess that
should work. Will try this solution, thanks!
[PATCH 06/12] drm/nouveau/ibus: add GK20A support
On Tue, Mar 25, 2014 at 7:34 AM, Thierry Reding
wrote:
> On Mon, Mar 24, 2014 at 05:42:28PM +0900, Alexandre Courbot wrote:
> [...]
>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
>> b/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
> [...]
>> +#include
>> +
>> +struct nvea_ibus_priv {
>> + struct nouveau_ibus base;
>> +};
>> +
>> +static void
>> +nvea_ibus_init_priv_ring(struct nvea_ibus_priv *priv)
>> +{
>> + nv_mask(priv, 0x137250, 0x3f, 0);
>> +
>> + nv_mask(priv, 0x000200, 0x20, 0);
>> + udelay(20);
>
> usleep_range()?
Sure.
>
>> +static void
>> +nvea_ibus_intr(struct nouveau_subdev *subdev)
>> +{
> [...]
>> + /* Acknowledge interrupt */
>> + nv_mask(priv, 0x12004c, 0x2, 0x2);
>> +
>> + while (--retry >= 0) {
>> + command = nv_rd32(priv, 0x12004c) & 0x3f;
>> + if (command == 0)
>> + break;
>> + }
>> +
>> + if (retry < 0)
>> + nv_warn(priv, "timeout waiting for ringmaster ack\n");
>> +}
>
> Perhaps I'm being paranoid, but this loop now depends on the frequency
> of the various clocks involved and therefore might break at some point
> if the frequencies get sufficiently high.
>
> So a slightly safer implementation would use a proper timeout using a
> combination of msecs_to_jiffies(), time_before() and usleep_range(),
> like so:
>
> timeout = jiffies + msecs_to_jiffies(...);
>
> while (time_before(jiffies, timeout)) {
> command = nv_rd32(...) & 0x3f;
> if (command == 0)
> break;
>
> usleep_range(...);
> }
>
> if (time_after(jiffies, timeout))
> nv_warn(...);
Right, now that I look at this code again I don't even understand why
I left it this way. Maybe I left some early test code slip into the
final patch, sorry about that.
> This assumes that there's some known timeout after which the ringmaster
> is expected to have acked the interrupt. On that note, I wonder if the
> warning is accurate here: it's my understanding that writing 0x2 to the
> register does acknowledge the interrupt, so the ringmaster does in fact
> "clear" it rather than "acknowledge" it, doesn't it?
>
> Although now that I mention it I seem to remember that this write is
> actually sending a command to the ring master and perhaps waiting for
> the register to return to 0 is indeed waiting for an ACK of sorts. Maybe
> adding a comment or so describing what this sequence does would be
> appropriate here?
Can we from an IP point of view? AFAIK this sequence has never been
publicly documented.
[Nouveau] [PATCH 06/12] drm/nouveau/ibus: add GK20A support
On Wed, Apr 2, 2014 at 9:52 AM, Alexandre Courbot wrote:
> On Tue, Mar 25, 2014 at 7:34 AM, Thierry Reding
> wrote:
>> On Mon, Mar 24, 2014 at 05:42:28PM +0900, Alexandre Courbot wrote:
>> [...]
>>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
>>> b/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
>> [...]
>>> +#include
>>> +
>>> +struct nvea_ibus_priv {
>>> + struct nouveau_ibus base;
>>> +};
>>> +
>>> +static void
>>> +nvea_ibus_init_priv_ring(struct nvea_ibus_priv *priv)
>>> +{
>>> + nv_mask(priv, 0x137250, 0x3f, 0);
>>> +
>>> + nv_mask(priv, 0x000200, 0x20, 0);
>>> + udelay(20);
>>
>> usleep_range()?
>
> Sure.
>
>>
>>> +static void
>>> +nvea_ibus_intr(struct nouveau_subdev *subdev)
>>> +{
>> [...]
>>> + /* Acknowledge interrupt */
>>> + nv_mask(priv, 0x12004c, 0x2, 0x2);
>>> +
>>> + while (--retry >= 0) {
>>> + command = nv_rd32(priv, 0x12004c) & 0x3f;
>>> + if (command == 0)
>>> + break;
>>> + }
>>> +
>>> + if (retry < 0)
>>> + nv_warn(priv, "timeout waiting for ringmaster ack\n");
>>> +}
>>
>> Perhaps I'm being paranoid, but this loop now depends on the frequency
>> of the various clocks involved and therefore might break at some point
>> if the frequencies get sufficiently high.
>>
>> So a slightly safer implementation would use a proper timeout using a
>> combination of msecs_to_jiffies(), time_before() and usleep_range(),
>> like so:
>>
>> timeout = jiffies + msecs_to_jiffies(...);
>>
>> while (time_before(jiffies, timeout)) {
>> command = nv_rd32(...) & 0x3f;
>> if (command == 0)
>> break;
>>
>> usleep_range(...);
>> }
>>
>> if (time_after(jiffies, timeout))
>> nv_warn(...);
>
> Right, now that I look at this code again I don't even understand why
> I left it this way. Maybe I left some early test code slip into the
> final patch, sorry about that.
I just remembered about this, but there's also the nv_wait() macro,
which you could use, e.g.
if (!nv_wait(subdev, 0x12004c, 0x3f, 0x00))
nv_warn(...)
It has built-in timeout logic/etc (although no sleeps in the middle).
It does use the timer subdev, so if that's not operational at this
point, you can't use it.
>
>> This assumes that there's some known timeout after which the ringmaster
>> is expected to have acked the interrupt. On that note, I wonder if the
>> warning is accurate here: it's my understanding that writing 0x2 to the
>> register does acknowledge the interrupt, so the ringmaster does in fact
>> "clear" it rather than "acknowledge" it, doesn't it?
>>
>> Although now that I mention it I seem to remember that this write is
>> actually sending a command to the ring master and perhaps waiting for
>> the register to return to 0 is indeed waiting for an ACK of sorts. Maybe
>> adding a comment or so describing what this sequence does would be
>> appropriate here?
>
> Can we from an IP point of view? AFAIK this sequence has never been
> publicly documented.
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
[PATCH 06/12] drm/nouveau/ibus: add GK20A support
On Mon, Mar 24, 2014 at 05:42:28PM +0900, Alexandre Courbot wrote:
[...]
> diff --git a/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
> b/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
[...]
> +#include
> +
> +struct nvea_ibus_priv {
> + struct nouveau_ibus base;
> +};
> +
> +static void
> +nvea_ibus_init_priv_ring(struct nvea_ibus_priv *priv)
> +{
> + nv_mask(priv, 0x137250, 0x3f, 0);
> +
> + nv_mask(priv, 0x000200, 0x20, 0);
> + udelay(20);
usleep_range()?
> +static void
> +nvea_ibus_intr(struct nouveau_subdev *subdev)
> +{
[...]
> + /* Acknowledge interrupt */
> + nv_mask(priv, 0x12004c, 0x2, 0x2);
> +
> + while (--retry >= 0) {
> + command = nv_rd32(priv, 0x12004c) & 0x3f;
> + if (command == 0)
> + break;
> + }
> +
> + if (retry < 0)
> + nv_warn(priv, "timeout waiting for ringmaster ack\n");
> +}
Perhaps I'm being paranoid, but this loop now depends on the frequency
of the various clocks involved and therefore might break at some point
if the frequencies get sufficiently high.
So a slightly safer implementation would use a proper timeout using a
combination of msecs_to_jiffies(), time_before() and usleep_range(),
like so:
timeout = jiffies + msecs_to_jiffies(...);
while (time_before(jiffies, timeout)) {
command = nv_rd32(...) & 0x3f;
if (command == 0)
break;
usleep_range(...);
}
if (time_after(jiffies, timeout))
nv_warn(...);
This assumes that there's some known timeout after which the ringmaster
is expected to have acked the interrupt. On that note, I wonder if the
warning is accurate here: it's my understanding that writing 0x2 to the
register does acknowledge the interrupt, so the ringmaster does in fact
"clear" it rather than "acknowledge" it, doesn't it?
Although now that I mention it I seem to remember that this write is
actually sending a command to the ring master and perhaps waiting for
the register to return to 0 is indeed waiting for an ACK of sorts. Maybe
adding a comment or so describing what this sequence does would be
appropriate here?
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
[PATCH 06/12] drm/nouveau/ibus: add GK20A support
Add support for initializing the priv ring of GK20A. This is done by the
BIOS on desktop GPUs, but needs to be done by hand on Tegra.
Signed-off-by: Alexandre Courbot
---
drivers/gpu/drm/nouveau/Makefile | 1 +
drivers/gpu/drm/nouveau/core/include/subdev/ibus.h | 1 +
drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c| 110 +
3 files changed, 112 insertions(+)
create mode 100644 drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
diff --git a/drivers/gpu/drm/nouveau/Makefile b/drivers/gpu/drm/nouveau/Makefile
index a90087bbdf88..592141e62dda 100644
--- a/drivers/gpu/drm/nouveau/Makefile
+++ b/drivers/gpu/drm/nouveau/Makefile
@@ -132,6 +132,7 @@ nouveau-y += core/subdev/i2c/nv94.o
nouveau-y += core/subdev/i2c/nvd0.o
nouveau-y += core/subdev/ibus/nvc0.o
nouveau-y += core/subdev/ibus/nve0.o
+nouveau-y += core/subdev/ibus/nvea.o
nouveau-y += core/subdev/instmem/base.o
nouveau-y += core/subdev/instmem/nv04.o
nouveau-y += core/subdev/instmem/nv40.o
diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/ibus.h
b/drivers/gpu/drm/nouveau/core/include/subdev/ibus.h
index 88814f159d89..056a42f92705 100644
--- a/drivers/gpu/drm/nouveau/core/include/subdev/ibus.h
+++ b/drivers/gpu/drm/nouveau/core/include/subdev/ibus.h
@@ -30,5 +30,6 @@ nouveau_ibus(void *obj)
extern struct nouveau_oclass nvc0_ibus_oclass;
extern struct nouveau_oclass nve0_ibus_oclass;
+extern struct nouveau_oclass nvea_ibus_oclass;
#endif
diff --git a/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
b/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
new file mode 100644
index ..151851286e99
--- /dev/null
+++ b/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2014, NVIDIA CORPORATION. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include
+
+struct nvea_ibus_priv {
+ struct nouveau_ibus base;
+};
+
+static void
+nvea_ibus_init_priv_ring(struct nvea_ibus_priv *priv)
+{
+ nv_mask(priv, 0x137250, 0x3f, 0);
+
+ nv_mask(priv, 0x000200, 0x20, 0);
+ udelay(20);
+ nv_mask(priv, 0x000200, 0x20, 0x20);
+
+ nv_wr32(priv, 0x12004c, 0x4);
+ nv_wr32(priv, 0x122204, 0x2);
+ nv_rd32(priv, 0x122204);
+}
+
+static void
+nvea_ibus_intr(struct nouveau_subdev *subdev)
+{
+ struct nvea_ibus_priv *priv = (void *)subdev;
+ u32 status0 = nv_rd32(priv, 0x120058);
+ s32 retry = 100;
+ u32 command;
+
+ if (status0 & 0x7) {
+ nv_debug(priv, "resetting priv ring\n");
+ nvea_ibus_init_priv_ring(priv);
+ }
+
+ /* Acknowledge interrupt */
+ nv_mask(priv, 0x12004c, 0x2, 0x2);
+
+ while (--retry >= 0) {
+ command = nv_rd32(priv, 0x12004c) & 0x3f;
+ if (command == 0)
+ break;
+ }
+
+ if (retry < 0)
+ nv_warn(priv, "timeout waiting for ringmaster ack\n");
+}
+
+static int
+nvea_ibus_init(struct nouveau_object *object)
+{
+ struct nvea_ibus_priv *priv = (void *)object;
+ int ret;
+
+ ret = _nouveau_ibus_init(object);
+ if (ret)
+ return ret;
+
+ nvea_ibus_init_priv_ring(priv);
+
+ return 0;
+}
+
+static int
+nvea_ibus_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
+ struct nouveau_oclass *oclass, void *data, u32 size,
+ struct nouveau_object **pobject)
+{
+ struct nvea_ibus_priv *priv;
+ int ret;
+
+ ret = nouveau_ibus_create(parent, engine, oclass, );
+ *pobject = nv_object(priv);
+ if (ret)
+ return ret;
+
+ nv_subdev(priv)->intr = nvea_ibus_intr;
+ return 0;
+}
+
+struct nouveau_oclass
+nvea_ibus_oclass = {
+ .handle = NV_SUBDEV(IBUS, 0xea),
+ .ofuncs = &(struct nouveau_ofuncs) {
+ .ctor = nvea_ibus_ctor,
+ .dtor =
