On 25.09.13 16:13, Steven Rostedt wrote:
On Wed, 25 Sep 2013 06:32:10 +0200
Mario Kleiner <mario.klei...@tuebingen.mpg.de> wrote:


But given the new situation, your proposal is great! If we push the
clock readouts into the get_scanoutpos routine, we can make this robust
without causing grief for the rt people and without the need for a new
separate lock for display regs in intel-kms.

E.g., for intel-kms:

i915_get_crtc_scanoutpos(..., ktime_t *stime, ktime_t *etime)
{
    ...
    spin_lock_irqsave(...uncore.lock);
    preempt_disable();
    *stime = ktime_get();
    position = __raw_i915_read32(dev_priv, PIPEDSL(pipe));
    *etime = ktime_get();
    preempt_enable();
    spin_unlock_irqrestore(...uncore.lock)
    ...
}

With your patchset to reduce the amount of register reads needed in that
function, and given that forcewake handling isn't needed for these
registers, this should make it robust again and wouldn't need new locks.

Unless ktime_get is also a bad thing to do in a preempt disabled section?

ktime_get() works fine in preempt_disable sections, although it may add
some latencies, but you shouldn't need to worry about it.

I like this solution the best too, but if it does go in, I would ask to
send us the patch for adding the preempt_disable() and we can add the
preempt_disable_rt() to it. Why make mainline have a little more
overhead?

-- Steve

Good! I will do that. Thanks for clarifying the irq and constraints on raw locks in the other thread.

-mario

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to