Hello all,
For a while now, I've been working to fix tearing with PRIME. This is my third version of the solution, revised according to Chris Wilson and Maarten Lankhorst's suggestions to remove the unnecessary lock on object_name_lock and to move fence waiting from intel_atomic_commit() to intel_prepare_plane_fb().
Repeat of overview below:
I have two patches, one that implements fencing for i915's legacy mmio_flip path, and one for atomic modesetting for futureproofing. Currently the mmio_flip path is the one ultimately used by the X patches, due to the lack of asynchronous atomic modesetting support in i915.
With my synchronization patches to X, it is possible to export two shared buffers per crtc instead of just one. The sink driver uses the legacy drmModePageFlip() to flip between the buffers, as the rest of the driver has yet to be ported to atomics. In the pageflip/vblank event handler, the sink driver requests a present from the source using the new X ABI function pScreen->PresentTrackedFlippingPixmap(). If the call returns successfully, it uses drmModePageFlip() to flip to the updated buffer, otherwise it waits until the next vblank and tries again.
When the source driver presents on a given buffer, it first attaches a fence. The source driver is responsible for either using software signaling or hardware semaphore-backed fences to ensure the fence is signaled when the present is finished. If the sink's DRM driver implements fencing in the flipping path, it will guarantee that that flip won't occur until the present has finished.
This means that DRM drivers that don't implement fencing in their flipping paths won't be able to guarantee 100% tear-free PRIME with my X patches. However, the good news is that even without fencing, tearing is rare. Generally presenting finishes before the next vblank, so there is no need to wait on the fence. The X patches are a drastic improvement with or without fencing, but the fencing is nonetheless important to guarantee tear-free under all conditions.
To give some greater context, I've uploaded my branches for DRM and the X server to Github. I'll move forward with upstreaming the X changes if and when these DRM patches go in.
DRM Tree: https://github.com/GoinsWithTheWind/drm-prime-sync X Tree: https://github.com/GoinsWithTheWind/xserver-prime-sync
(branch agoins-prime-v3)
Thanks, Alex @ NVIDIA Linux Driver Team
Alex Goins (2): i915: wait for fences in mmio_flip() i915: wait for fence in prepare_plane_fb
drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
If a buffer is backed by dmabuf, wait on its reservation object's fences before flipping.
Signed-off-by: Alex Goins agoins@nvidia.com --- drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b2270d5..acec108a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -44,6 +44,8 @@ #include <drm/drm_plane_helper.h> #include <drm/drm_rect.h> #include <linux/dma_remapping.h> +#include <linux/reservation.h> +#include <linux/dma-buf.h>
/* Primary plane formats for gen <= 3 */ static const uint32_t i8xx_primary_formats[] = { @@ -11170,10 +11172,19 @@ static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc) static void intel_do_mmio_flip(struct intel_crtc *intel_crtc) { struct drm_device *dev = intel_crtc->base.dev; + struct drm_i915_gem_object *pending_flip_obj = + intel_crtc->unpin_work->pending_flip_obj; u32 start_vbl_count;
intel_mark_page_flip_active(intel_crtc);
+ /* For framebuffer backed by dmabuf, wait for fence */ + if (pending_flip_obj->base.dma_buf) { + reservation_object_wait_timeout_rcu( + pending_flip_obj->base.dma_buf->resv, + true, false, msecs_to_jiffies(96)); + } + intel_pipe_update_start(intel_crtc, &start_vbl_count);
if (INTEL_INFO(dev)->gen >= 9)
On Thu, Nov 12, 2015 at 05:49:28PM -0800, Alex Goins wrote:
This wait should be prior to marking the flip as waiting for the flip-completion interrupt. My personal preference (aside from putting this next to the other wait) would to have been to use crtc->primary->fb to match the do_mmip_flips funcs (I expect that we will eliminate the pending_flip_obj in the near future).
Also we are missing the addition of if (obj->base.dma_buf && obj->base.dma_buf->resv->fence_excl) return true; to use_mmio_flip(). -Chris
Hi,
On 13 November 2015 at 10:08, Chris Wilson chris@chris-wilson.co.uk wrote:
No, don't use crtc->primary->fb for anything.
Cheers, Daniel
On Fri, Nov 13, 2015 at 10:38:07AM +0000, Daniel Stone wrote:
s/crtc->primary->fb/whatever is actually used by the ilk_do_mmio_flip and co which is certainly not pending_flip_obj/ -Chris
Sorry; needless to say I'm not super familiar with the Intel driver. ilk_do_mmio_flip() uses crtc->primary->fb to fetch the gem object:
struct intel_framebuffer *intel_fb = to_intel_framebuffer(intel_crtc->base.primary->fb); struct drm_i915_gem_object *obj = intel_fb->obj;
Given that, would it be okay for me to do the same?
Thanks, Alex
-----Original Message----- From: Chris Wilson [mailto:chris@chris-wilson.co.uk] Sent: Friday, November 13, 2015 2:46 AM To: Daniel Stone Cc: Alexander Goins; dri-devel; Daniel Vetter; Maarten Lankhorst Subject: Re: [PATCH i915 v3 1/2] i915: wait for fences in mmio_flip()
On Fri, Nov 13, 2015 at 10:38:07AM +0000, Daniel Stone wrote:
s/crtc->primary->fb/whatever is actually used by the ilk_do_mmio_flip and co which is certainly not pending_flip_obj/ -Chris
-- Chris Wilson, Intel Open Source Technology Centre
On Fri, Nov 13, 2015 at 07:38:21PM +0000, Alexander Goins wrote:
I think so, since this is the legacy page_flip. Which will hopefully disappear real soon now ... -Daniel
Okay, implemented these suggestions. Will send out v4 patch set once the crtc->primary->fb thing is cleared up.
-----Original Message----- From: Chris Wilson [mailto:chris@chris-wilson.co.uk] Sent: Friday, November 13, 2015 2:08 AM To: Alexander Goins Cc: dri-devel@lists.freedesktop.org; daniel@fooishbar.org; daniel@ffwll.ch; maarten.lankhorst@linux.intel.com Subject: Re: [PATCH i915 v3 1/2] i915: wait for fences in mmio_flip()
On Thu, Nov 12, 2015 at 05:49:28PM -0800, Alex Goins wrote:
This wait should be prior to marking the flip as waiting for the flip-completion interrupt. My personal preference (aside from putting this next to the other wait) would to have been to use crtc->primary->fb to match the do_mmip_flips funcs (I expect that we will eliminate the pending_flip_obj in the near future).
Also we are missing the addition of if (obj->base.dma_buf && obj->base.dma_buf->resv->fence_excl) return true; to use_mmio_flip(). -Chris
-- Chris Wilson, Intel Open Source Technology Centre
In intel_prepare_plane_fb, if fb is backed by dma-buf, wait for fence.
Signed-off-by: Alex Goins agoins@nvidia.com --- drivers/gpu/drm/i915/intel_display.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index acec108a..36c558a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13345,6 +13345,13 @@ intel_prepare_plane_fb(struct drm_plane *plane, if (!obj) return 0;
+ /* For framebuffer backed by dmabuf, wait for fence */ + if (obj->base.dma_buf) { + reservation_object_wait_timeout_rcu( + obj->base.dma_buf->resv, + true, false, msecs_to_jiffies(96)); + } + mutex_lock(&dev->struct_mutex);
if (plane->type == DRM_PLANE_TYPE_CURSOR &&
dri-devel@lists.freedesktop.org
-
Alex Goins -
Alexander Goins
-
Chris Wilson -
Daniel Stone -
Daniel Vetter