On Wed, Apr 2, 2014 at 11:18 PM, Ilia Mirkin imirkin@alum.mit.edu wrote:
On Wed, Apr 2, 2014 at 9:52 AM, Alexandre Courbot gnurou@gmail.com wrote:
On Tue, Mar 25, 2014 at 7:34 AM, Thierry Reding thierry.reding@gmail.com 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 <subdev/ibus.h>
+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!