Both patches should make the communicating with the PMU more stable. Karol Herbst (2): pmu: fix queued messages while getting no IRQ pmu: be more strict about locking drm/nouveau/nvkm/subdev/pmu/base.c | 49 ++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 7 deletions(-) -- 2.7.2
Karol Herbst
2016-Mar-01 10:10 UTC
[Nouveau] [PATCH 1/2] pmu: fix queued messages while getting no IRQ
I encountered while stresstesting the reclocking code, that rarely (1 out of
20.000+ requests) we don't get any IRQ in nvkm_pmu_intr. Though this issue
was
fixed now, there can be other reasons to not get any reply from the pmu.
This means nouveau still waits infinitely in nvkm_pmu_send:
if (reply) {
wait_event(pmu->recv.wait, (pmu->recv.process == 0));
therefore let us use wait_event_timeout with a 1s timeout frame and just check
whether there is a message queued and handle it if there is one.
Return -ETIMEDOUT whenever we timed out and there is no message queued or when
we hit another timeout while trying to read the message without getting any IRQ
The benefit of not using wait_event is, that we don't have a kworker waiting
on an event, which makes it easier to reload the module at runtime, which helps
me developing on nouveau on my laptop a lot, because I don't need to reboot
anymore
Nethertheless, we shouldn't use wait_event here, because we can't
guarantee any
answere at all, can we?
v2: moved it into a new function
Signed-off-by: Karol Herbst <nouveau at karolherbst.de>
---
drm/nouveau/nvkm/subdev/pmu/base.c | 41 ++++++++++++++++++++++++++++++++++----
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/drm/nouveau/nvkm/subdev/pmu/base.c
b/drm/nouveau/nvkm/subdev/pmu/base.c
index d95eb86..fa3cc5b 100644
--- a/drm/nouveau/nvkm/subdev/pmu/base.c
+++ b/drm/nouveau/nvkm/subdev/pmu/base.c
@@ -32,6 +32,36 @@ nvkm_pmu_pgob(struct nvkm_pmu *pmu, bool enable)
pmu->func->pgob(pmu, enable);
}
+static int
+wait_for_pmu_reply(struct nvkm_pmu *pmu, u32 reply[2])
+{
+ struct nvkm_subdev *subdev = &pmu->subdev;
+ struct nvkm_device *device = subdev->device;
+ unsigned long js = msecs_to_jiffies(1000);
+
+ if (!wait_event_timeout(pmu->recv.wait, pmu->recv.process == 0, js)) {
+ u32 addr = nvkm_rd32(device, 0x10a4cc);
+ nvkm_error(subdev, "wait on reply timed out\n");
+
+ if (addr == nvkm_rd32(device, 0x10a4c8))
+ return -ETIMEDOUT;
+
+ nvkm_error(subdev, "found queued message without getting an"
+ "interrupt\n");
+ schedule_work(&pmu->recv.work);
+
+ if (!wait_event_timeout(pmu->recv.wait, pmu->recv.process == 0,
+ js)) {
+ nvkm_error(subdev, "failed to repair PMU state\n");
+ return -ETIMEDOUT;
+ }
+ }
+
+ reply[0] = pmu->recv.data[0];
+ reply[1] = pmu->recv.data[1];
+ return 0;
+}
+
int
nvkm_pmu_send(struct nvkm_pmu *pmu, u32 reply[2],
u32 process, u32 message, u32 data0, u32 data1)
@@ -39,6 +69,7 @@ nvkm_pmu_send(struct nvkm_pmu *pmu, u32 reply[2],
struct nvkm_subdev *subdev = &pmu->subdev;
struct nvkm_device *device = subdev->device;
u32 addr;
+ int ret = 0;
/* wait for a free slot in the fifo */
addr = nvkm_rd32(device, 0x10a4a0);
@@ -78,13 +109,15 @@ nvkm_pmu_send(struct nvkm_pmu *pmu, u32 reply[2],
/* wait for reply, if requested */
if (reply) {
- wait_event(pmu->recv.wait, (pmu->recv.process == 0));
- reply[0] = pmu->recv.data[0];
- reply[1] = pmu->recv.data[1];
+ ret = wait_for_pmu_reply(pmu, reply);
+ if (ret < 0) {
+ reply[0] = 0;
+ reply[1] = 0;
+ }
mutex_unlock(&subdev->mutex);
}
- return 0;
+ return ret;
}
static void
--
2.7.2
Karol Herbst
2016-Mar-01 10:10 UTC
[Nouveau] [PATCH 2/2] pmu: be more strict about locking
when we start communicating with the pmu a bit more, the current code is a real
issue. I encountered a dead lock here, while testing my dynamic reclocking code
Signed-off-by: Karol Herbst <nouveau at karolherbst.de>
---
drm/nouveau/nvkm/subdev/pmu/base.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drm/nouveau/nvkm/subdev/pmu/base.c
b/drm/nouveau/nvkm/subdev/pmu/base.c
index fa3cc5b..67d319d 100644
--- a/drm/nouveau/nvkm/subdev/pmu/base.c
+++ b/drm/nouveau/nvkm/subdev/pmu/base.c
@@ -71,21 +71,23 @@ nvkm_pmu_send(struct nvkm_pmu *pmu, u32 reply[2],
u32 addr;
int ret = 0;
+ mutex_lock(&subdev->mutex);
/* wait for a free slot in the fifo */
addr = nvkm_rd32(device, 0x10a4a0);
if (nvkm_msec(device, 2000,
u32 tmp = nvkm_rd32(device, 0x10a4b0);
if (tmp != (addr ^ 8))
break;
- ) < 0)
+ ) < 0) {
+ mutex_unlock(&subdev->mutex);
return -EBUSY;
+ }
/* we currently only support a single process at a time waiting
* on a synchronous reply, take the PMU mutex and tell the
* receive handler what we're waiting for
*/
if (reply) {
- mutex_lock(&subdev->mutex);
pmu->recv.message = message;
pmu->recv.process = process;
}
@@ -114,9 +116,9 @@ nvkm_pmu_send(struct nvkm_pmu *pmu, u32 reply[2],
reply[0] = 0;
reply[1] = 0;
}
- mutex_unlock(&subdev->mutex);
}
+ mutex_unlock(&subdev->mutex);
return ret;
}
--
2.7.2