Op 12-02-18 om 16:19 schreef Chris Wilson:
> Quoting Maarten Lankhorst (2018-02-09 09:54:02)
>> This requires being able to read the vblank counter with the
>> uncore.lock already held. This is also a preparation for
>> being able to run the entire vblank update sequence with
>> the uncore lock held.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_irq.c | 66 ++++++++++++++++++++++++++++++-------
>> drivers/gpu/drm/i915/i915_trace.h | 5 ++-
>> drivers/gpu/drm/i915/intel_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_sprite.c | 3 +-
>> 4 files changed, 60 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index eda9543a0199..6c491e63e07c 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -736,13 +736,12 @@ static void i915_enable_asle_pipestat(struct drm_i915_private *dev_priv)
>> /* Called from drm generic code, passed a 'crtc', which
>> * we use as a pipe index
>> */
>> -static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
>> +static u32 __i915_get_vblank_counter(struct intel_crtc *crtc)
>> {
>> - struct drm_i915_private *dev_priv = to_i915(dev);
>> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> i915_reg_t high_frame, low_frame;
>> u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
>> - const struct drm_display_mode *mode = &dev->vblank[pipe].hwmode;
>> - unsigned long irqflags;
>> + const struct drm_display_mode *mode = &crtc->base.dev->vblank[crtc->pipe].hwmode;
> lockdep_assert_held(&dev_priv->uncore.lock);
>
>>
>> htotal = mode->crtc_htotal;
>> hsync_start = mode->crtc_hsync_start;
>> @@ -756,10 +755,8 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
>> /* Start of vblank event occurs at start of hsync */
>> vbl_start -= htotal - hsync_start;
>>
>> - high_frame = PIPEFRAME(pipe);
>> - low_frame = PIPEFRAMEPIXEL(pipe);
>> -
>> - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>> + high_frame = PIPEFRAME(crtc->pipe);
>> + low_frame = PIPEFRAMEPIXEL(crtc->pipe);
>>
>> /*
>> * High & low register fields aren't synchronized, so make sure
>> @@ -772,8 +769,6 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
>> high2 = I915_READ_FW(high_frame) & PIPE_FRAME_HIGH_MASK;
>> } while (high1 != high2);
>>
>> - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>> -
>> high1 >>= PIPE_FRAME_HIGH_SHIFT;
>> pixel = low & PIPE_PIXEL_MASK;
>> low >>= PIPE_FRAME_LOW_SHIFT;
>> @@ -786,11 +781,60 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
>> return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xffffff;
>> }
>>
>> +static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
>> +{
>> + struct drm_i915_private *dev_priv = to_i915(dev);
>> + unsigned long irqflags;
>> + u32 ret;
>> +
>> + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>> + ret = i915_get_vblank_counter(dev, pipe);
>> + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>> +
>> + return ret;
>> +}
>> +
>> +static u32 __g4x_get_vblank_counter(struct intel_crtc *crtc)
>> +{
>> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> lockdep_assert_held(&dev_priv->uncore.lock); ?
>
> Ok, why do we need uncore.lock held here at all? Serialisation of mmio
> access to the same cacheline is the age old reason, can we guarantee
> that doesn't happen anyway? (Probably not, but really most callers do
> not need the mmio w/a.)
Is the serialization only needed for writing?
The only thing that can race with nonblocking atomic commits are legacy
cursor updates, but those can only happen if the cursor plane are not part
of the previous atomic commit. Those are also protected by plane->mutex,
so in theory same cache lines on the same pipes probably can't race..
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
