On Wed, 25 Sep 2013 06:32:10 +0200
Mario Kleiner [off-list ref] 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