[Intel-gfx] [PATCH v5 1/5] drm/i915: Add enable/disable flip done and flip done handler

[Karthik B S]

On 7/27/2020 5:57 PM, Michel Dänzer wrote:
> On 2020-07-25 1:26 a.m., Paulo Zanoni wrote:
>> Em seg, 2020-07-20 às 17:01 +0530, Karthik B S escreveu:
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>>> index 1fa67700d8f4..95953b393941 100644
>>> --- a/drivers/gpu/drm/i915/i915_irq.c
>>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>>> @@ -697,14 +697,24 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc)
>>>   	return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xffffff;
>>>   }
>>>   
>>> +static u32 g4x_get_flip_counter(struct drm_crtc *crtc)
>>> +{
>>> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>>> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>>> +
>>> +	return I915_READ(PIPE_FLIPCOUNT_G4X(pipe));
>>> +}
>>> +
>>>   u32 g4x_get_vblank_counter(struct drm_crtc *crtc)
>>>   {
>>>   	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>>>   	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>>>   
>>> +	if (crtc->state->async_flip)
>>> +		return g4x_get_flip_counter(crtc);
>>> +
>>>   	return I915_READ(PIPE_FRMCOUNT_G4X(pipe));
>>
>> I don't understand the intention behind this, can you please clarify?
>> This goes back to my reply of the cover letter. It seems that here
>> we're going to alternate between two different counters in our vblank
>> count. So if user space alternates between sometimes using async flips
>> and sometimes using normal flip it's going to get some very weird
>> deltas, isn't it? At least this is what I remember from when I played
>> with these registers: FLIPCOUNT drifts away from FRMCOUNT when we start
>> using async flips.
> 
> This definitely looks wrong. The counter value returned by the
> get_vblank_counter hook is supposed to increment when a vertical blank
> period occurs; page flips are not supposed to affect this in any way.
> 

Thanks for the review.
As per the feedback received, I will be removing this and will revert 
back to the original implementation in the next revision.

Thanks,
Karthik.B.S
>