lkml.org 
[lkml]   [2014]   [Mar]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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 <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()?

    > +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
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2014-03-24 23:41    [W:4.065 / U:0.064 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site