Re: [PATCH v5 0/5] Asynchronous flip implementation for i915

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em seg, 2020-07-20 às 17:01 +0530, Karthik B S escreveu:
> Without async flip support in the kernel, fullscreen apps where game
> resolution is equal to the screen resolution, must perform an extra blit
> per frame prior to flipping.
> 
> Asynchronous page flips will also boost the FPS of Mesa benchmarks.

We had a discussion in patch 1 of v3 regarding the semantics of
asynchronous flips from the point of view of the user space: how we
handle our vblank counters, how/when we increment the sequence events
and how we handle timestamps, how/when we deliver vblank events. Since
apparently AMD has already enabled this feature, our job would be to
implement their current behavior so KMS clients can continue to work
regardless of the driver. 

From reading this series it's not super clear to me what exactly is the
behavior that we're trying to follow. Can you please document somewhere
what are these rules and expectations? This way, people writing user
space code (or people improving the other drivers) will have an easier
time. In addition to text documentation, I believe all our assumptions
and rules should be coded in IGT: we want to be confident a driver
implements async page flips correctly when we can verify it passes the
IGT.

Also, in the other patches I raise some additional questions regarding
mixing async with non-async vblanks: IMHO this should also be
documented as text and as IGT.

> 
> v2: -Few patches have been squashed and patches have been shuffled as
>      per the reviews on the previous version.
> 
> v3: -Few patches have been squashed and patches have been shuffled as
>      per the reviews on the previous version.
> 
> v4: -Made changes to fix the sequence and time stamp issue as per the
>      comments received on the previous version.
>     -Timestamps are calculated using the flip done time stamp and current
>      timestamp. Here I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP flag is used
>      for timestamp calculations.
>     -Event is sent from the interrupt handler immediately using this
>      updated timestamps and sequence.
>     -Added more state checks as async flip should only allow change in plane
>      surface address and nothing else should be allowed to change.
>     -Added a separate plane hook for async flip.
>     -Need to find a way to reject fbc enabling if it comes as part of this
>      flip as bspec states that changes to FBC are not allowed.
> 
> v5: -Fixed the Checkpatch and sparse warnings.
> 
> Karthik B S (5):
>   drm/i915: Add enable/disable flip done and flip done handler
>   drm/i915: Add support for async flips in I915
>   drm/i915: Add checks specific to async flips
>   drm/i915: Do not call drm_crtc_arm_vblank_event in async flips
>   drm/i915: Enable async flips in i915
> 
>  drivers/gpu/drm/i915/display/intel_display.c | 123 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_sprite.c  |  33 ++++-
>  drivers/gpu/drm/i915/i915_irq.c              |  83 +++++++++++--
>  drivers/gpu/drm/i915/i915_irq.h              |   2 +
>  drivers/gpu/drm/i915/i915_reg.h              |   5 +-
>  5 files changed, 237 insertions(+), 9 deletions(-)
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel