* [PATCH] dma-buf: Wait on the reservation object when sync'ing before CPU access
@ 2016-06-21 7:04 ` Chris Wilson
0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2016-06-21 7:04 UTC (permalink / raw)
To: dri-devel
Cc: Chris Wilson, Sumit Semwal, Daniel Vetter, linux-media,
linaro-mm-sig, linux-kernel
Rendering operations to the dma-buf are tracked implicitly via the
reservation_object (dmabuf->resv). This is used to allow poll() to
wait upon outstanding rendering (or just query the current status of
rendering). The dma-buf sync ioctl allows userspace to prepare the
dma-buf for CPU access, which should include waiting upon rendering.
(Some drivers may need to do more work to ensure that the dma-buf mmap
is coherent as well as complete.)
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
---
I'm wondering whether it makes sense just to always do the wait first.
It is one of the first operations every driver has to make. A driver
that wants to implement it differently (e.g. they can special case
native waits) will still require a wait on the reservation object to
finish external rendering.
-Chris
---
drivers/dma-buf/dma-buf.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ddaee60ae52a..123f14b8e882 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
}
EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
+static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
+ enum dma_data_direction direction)
+{
+ bool write = (direction == DMA_BIDIRECTIONAL ||
+ direction == DMA_TO_DEVICE);
+ struct reservation_object *resv = dma_buf->resv;
+ long ret;
+
+ /* Wait on any implicit rendering fences */
+ ret = reservation_object_wait_timeout_rcu(resv, write, true,
+ MAX_SCHEDULE_TIMEOUT);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
/**
* dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
@@ -607,6 +623,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
if (dmabuf->ops->begin_cpu_access)
ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
+ else
+ ret = __dma_buf_begin_cpu_access(dmabuf, direction);
return ret;
}
--
2.8.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] dma-buf: Wait on the reservation object when sync'ing before CPU access
@ 2016-06-21 7:04 ` Chris Wilson
0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2016-06-21 7:04 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, linux-kernel, linaro-mm-sig, linux-media
Rendering operations to the dma-buf are tracked implicitly via the
reservation_object (dmabuf->resv). This is used to allow poll() to
wait upon outstanding rendering (or just query the current status of
rendering). The dma-buf sync ioctl allows userspace to prepare the
dma-buf for CPU access, which should include waiting upon rendering.
(Some drivers may need to do more work to ensure that the dma-buf mmap
is coherent as well as complete.)
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
---
I'm wondering whether it makes sense just to always do the wait first.
It is one of the first operations every driver has to make. A driver
that wants to implement it differently (e.g. they can special case
native waits) will still require a wait on the reservation object to
finish external rendering.
-Chris
---
drivers/dma-buf/dma-buf.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ddaee60ae52a..123f14b8e882 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
}
EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
+static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
+ enum dma_data_direction direction)
+{
+ bool write = (direction == DMA_BIDIRECTIONAL ||
+ direction == DMA_TO_DEVICE);
+ struct reservation_object *resv = dma_buf->resv;
+ long ret;
+
+ /* Wait on any implicit rendering fences */
+ ret = reservation_object_wait_timeout_rcu(resv, write, true,
+ MAX_SCHEDULE_TIMEOUT);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
/**
* dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
@@ -607,6 +623,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
if (dmabuf->ops->begin_cpu_access)
ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
+ else
+ ret = __dma_buf_begin_cpu_access(dmabuf, direction);
return ret;
}
--
2.8.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] dma-buf: Wait on the reservation object when sync'ing before CPU access
2016-06-21 7:04 ` Chris Wilson
@ 2016-06-21 8:44 ` Daniel Vetter
-1 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2016-06-21 8:44 UTC (permalink / raw)
To: Chris Wilson
Cc: dri-devel, Sumit Semwal, Daniel Vetter, linux-media,
linaro-mm-sig, Linux Kernel Mailing List, Maarten Lankhorst,
Sean Paul, Zach Reizner
On Tue, Jun 21, 2016 at 08:04:00AM +0100, Chris Wilson wrote:
> Rendering operations to the dma-buf are tracked implicitly via the
> reservation_object (dmabuf->resv). This is used to allow poll() to
> wait upon outstanding rendering (or just query the current status of
> rendering). The dma-buf sync ioctl allows userspace to prepare the
> dma-buf for CPU access, which should include waiting upon rendering.
> (Some drivers may need to do more work to ensure that the dma-buf mmap
> is coherent as well as complete.)
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: linux-media@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linaro-mm-sig@lists.linaro.org
> Cc: linux-kernel@vger.kernel.org
> ---
>
> I'm wondering whether it makes sense just to always do the wait first.
> It is one of the first operations every driver has to make. A driver
> that wants to implement it differently (e.g. they can special case
> native waits) will still require a wait on the reservation object to
> finish external rendering.
Worst case (if the driver uses reservation objects also internally) we'll
end up calling this twice. It should be cheap enough to do that. I'll add
a few folks who might want to chip in with an opinion ...
-Daniel
> -Chris
>
> ---
> drivers/dma-buf/dma-buf.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index ddaee60ae52a..123f14b8e882 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
> }
> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
>
> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> + enum dma_data_direction direction)
> +{
> + bool write = (direction == DMA_BIDIRECTIONAL ||
> + direction == DMA_TO_DEVICE);
> + struct reservation_object *resv = dma_buf->resv;
> + long ret;
> +
> + /* Wait on any implicit rendering fences */
> + ret = reservation_object_wait_timeout_rcu(resv, write, true,
> + MAX_SCHEDULE_TIMEOUT);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
>
> /**
> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
> @@ -607,6 +623,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>
> if (dmabuf->ops->begin_cpu_access)
> ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
> + else
> + ret = __dma_buf_begin_cpu_access(dmabuf, direction);
>
> return ret;
> }
> --
> 2.8.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] dma-buf: Wait on the reservation object when sync'ing before CPU access
@ 2016-06-21 8:44 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2016-06-21 8:44 UTC (permalink / raw)
To: Chris Wilson
Cc: Sean Paul, Daniel Vetter, Linux Kernel Mailing List, dri-devel,
linaro-mm-sig, Zach Reizner, linux-media
On Tue, Jun 21, 2016 at 08:04:00AM +0100, Chris Wilson wrote:
> Rendering operations to the dma-buf are tracked implicitly via the
> reservation_object (dmabuf->resv). This is used to allow poll() to
> wait upon outstanding rendering (or just query the current status of
> rendering). The dma-buf sync ioctl allows userspace to prepare the
> dma-buf for CPU access, which should include waiting upon rendering.
> (Some drivers may need to do more work to ensure that the dma-buf mmap
> is coherent as well as complete.)
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: linux-media@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linaro-mm-sig@lists.linaro.org
> Cc: linux-kernel@vger.kernel.org
> ---
>
> I'm wondering whether it makes sense just to always do the wait first.
> It is one of the first operations every driver has to make. A driver
> that wants to implement it differently (e.g. they can special case
> native waits) will still require a wait on the reservation object to
> finish external rendering.
Worst case (if the driver uses reservation objects also internally) we'll
end up calling this twice. It should be cheap enough to do that. I'll add
a few folks who might want to chip in with an opinion ...
-Daniel
> -Chris
>
> ---
> drivers/dma-buf/dma-buf.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index ddaee60ae52a..123f14b8e882 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
> }
> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
>
> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> + enum dma_data_direction direction)
> +{
> + bool write = (direction == DMA_BIDIRECTIONAL ||
> + direction == DMA_TO_DEVICE);
> + struct reservation_object *resv = dma_buf->resv;
> + long ret;
> +
> + /* Wait on any implicit rendering fences */
> + ret = reservation_object_wait_timeout_rcu(resv, write, true,
> + MAX_SCHEDULE_TIMEOUT);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
>
> /**
> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
> @@ -607,6 +623,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>
> if (dmabuf->ops->begin_cpu_access)
> ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
> + else
> + ret = __dma_buf_begin_cpu_access(dmabuf, direction);
>
> return ret;
> }
> --
> 2.8.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
2016-06-21 7:04 ` Chris Wilson
@ 2016-08-15 15:37 ` Chris Wilson
-1 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2016-08-15 15:37 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, Chris Wilson, Sumit Semwal, Daniel Vetter,
Eric Anholt, linux-media, dri-devel, linaro-mm-sig, linux-kernel
Rendering operations to the dma-buf are tracked implicitly via the
reservation_object (dmabuf->resv). This is used to allow poll() to
wait upon outstanding rendering (or just query the current status of
rendering). The dma-buf sync ioctl allows userspace to prepare the
dma-buf for CPU access, which should include waiting upon rendering.
(Some drivers may need to do more work to ensure that the dma-buf mmap
is coherent as well as complete.)
v2: Always wait upon the reservation object implicitly. We choose to do
it after the native handler in case it can do so more efficiently.
Testcase: igt/prime_vgem
Testcase: igt/gem_concurrent_blit # *vgem*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Eric Anholt <eric@anholt.net>
Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
---
drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ddaee60ae52a..cf04d249a6a4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
}
EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
+static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
+ enum dma_data_direction direction)
+{
+ bool write = (direction == DMA_BIDIRECTIONAL ||
+ direction == DMA_TO_DEVICE);
+ struct reservation_object *resv = dmabuf->resv;
+ long ret;
+
+ /* Wait on any implicit rendering fences */
+ ret = reservation_object_wait_timeout_rcu(resv, write, true,
+ MAX_SCHEDULE_TIMEOUT);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
/**
* dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
@@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
if (dmabuf->ops->begin_cpu_access)
ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
+ /* Ensure that all fences are waited upon - but we first allow
+ * the native handler the chance to do so more efficiently if it
+ * chooses. A double invocation here will be reasonably cheap no-op.
+ */
+ if (ret == 0)
+ ret = __dma_buf_begin_cpu_access(dmabuf, direction);
+
return ret;
}
EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
--
2.8.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
@ 2016-08-15 15:37 ` Chris Wilson
0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2016-08-15 15:37 UTC (permalink / raw)
To: dri-devel
Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel, linaro-mm-sig,
Sumit Semwal, linux-media
Rendering operations to the dma-buf are tracked implicitly via the
reservation_object (dmabuf->resv). This is used to allow poll() to
wait upon outstanding rendering (or just query the current status of
rendering). The dma-buf sync ioctl allows userspace to prepare the
dma-buf for CPU access, which should include waiting upon rendering.
(Some drivers may need to do more work to ensure that the dma-buf mmap
is coherent as well as complete.)
v2: Always wait upon the reservation object implicitly. We choose to do
it after the native handler in case it can do so more efficiently.
Testcase: igt/prime_vgem
Testcase: igt/gem_concurrent_blit # *vgem*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Eric Anholt <eric@anholt.net>
Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
---
drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ddaee60ae52a..cf04d249a6a4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
}
EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
+static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
+ enum dma_data_direction direction)
+{
+ bool write = (direction == DMA_BIDIRECTIONAL ||
+ direction == DMA_TO_DEVICE);
+ struct reservation_object *resv = dmabuf->resv;
+ long ret;
+
+ /* Wait on any implicit rendering fences */
+ ret = reservation_object_wait_timeout_rcu(resv, write, true,
+ MAX_SCHEDULE_TIMEOUT);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
/**
* dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
@@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
if (dmabuf->ops->begin_cpu_access)
ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
+ /* Ensure that all fences are waited upon - but we first allow
+ * the native handler the chance to do so more efficiently if it
+ * chooses. A double invocation here will be reasonably cheap no-op.
+ */
+ if (ret == 0)
+ ret = __dma_buf_begin_cpu_access(dmabuf, direction);
+
return ret;
}
EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2] drm/i915: Use common RPS scheme for Cherryview
2016-06-21 7:04 ` Chris Wilson
` (2 preceding siblings ...)
(?)
@ 2016-08-15 15:37 ` Chris Wilson
2016-08-15 15:41 ` Chris Wilson
-1 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2016-08-15 15:37 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi
Cherryview uses a custom static definition of its RPS parameters (for
automatically driving GPU frequency selection) - yet still couples into
the waitboosting scheme of the common RPS setup.
The rationale given in commit 8fb55197e64d ("drm/i915: Agressive
downclocking on Baytrail") was that Cherryview might have to take the
common powerwell (unlike Baytrail it has multiple powerwells) to read the
RPS registers more often. However, we have reports that the current values
are not working well for kodi (the frequency stays too low). Lets use the
common values and see if we can tune them appropriately to benefit
everyone.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: fritsch@kodi.tv
Cc: Deepak S <deepak.s@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_pm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 20794804f3bb..a140fe075e1b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4837,8 +4837,7 @@ static void valleyview_set_rps(struct drm_i915_private *dev_priv, u8 val)
if (val != dev_priv->rps.cur_freq) {
vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
- if (!IS_CHERRYVIEW(dev_priv))
- gen6_set_rps_thresholds(dev_priv, val);
+ gen6_set_rps_thresholds(dev_priv, val);
}
dev_priv->rps.cur_freq = val;
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/i915: Use common RPS scheme for Cherryview
2016-08-15 15:37 ` [PATCH v2] drm/i915: Use common RPS scheme for Cherryview Chris Wilson
@ 2016-08-15 15:41 ` Chris Wilson
0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2016-08-15 15:41 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi
Apolgies, I changed trees between resending after the first git-send-email
bounced off the wrong address.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
2016-06-21 7:04 ` Chris Wilson
@ 2016-08-15 15:42 ` Chris Wilson
-1 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2016-08-15 15:42 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, Chris Wilson, Sumit Semwal, Daniel Vetter,
Eric Anholt, linux-media, linaro-mm-sig, linux-kernel
Rendering operations to the dma-buf are tracked implicitly via the
reservation_object (dmabuf->resv). This is used to allow poll() to
wait upon outstanding rendering (or just query the current status of
rendering). The dma-buf sync ioctl allows userspace to prepare the
dma-buf for CPU access, which should include waiting upon rendering.
(Some drivers may need to do more work to ensure that the dma-buf mmap
is coherent as well as complete.)
v2: Always wait upon the reservation object implicitly. We choose to do
it after the native handler in case it can do so more efficiently.
Testcase: igt/prime_vgem
Testcase: igt/gem_concurrent_blit # *vgem*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Eric Anholt <eric@anholt.net>
Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
---
drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ddaee60ae52a..cf04d249a6a4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
}
EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
+static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
+ enum dma_data_direction direction)
+{
+ bool write = (direction == DMA_BIDIRECTIONAL ||
+ direction == DMA_TO_DEVICE);
+ struct reservation_object *resv = dmabuf->resv;
+ long ret;
+
+ /* Wait on any implicit rendering fences */
+ ret = reservation_object_wait_timeout_rcu(resv, write, true,
+ MAX_SCHEDULE_TIMEOUT);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
/**
* dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
@@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
if (dmabuf->ops->begin_cpu_access)
ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
+ /* Ensure that all fences are waited upon - but we first allow
+ * the native handler the chance to do so more efficiently if it
+ * chooses. A double invocation here will be reasonably cheap no-op.
+ */
+ if (ret == 0)
+ ret = __dma_buf_begin_cpu_access(dmabuf, direction);
+
return ret;
}
EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
--
2.8.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
@ 2016-08-15 15:42 ` Chris Wilson
0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2016-08-15 15:42 UTC (permalink / raw)
To: dri-devel
Cc: Daniel Vetter, intel-gfx, linux-kernel, linaro-mm-sig, linux-media
Rendering operations to the dma-buf are tracked implicitly via the
reservation_object (dmabuf->resv). This is used to allow poll() to
wait upon outstanding rendering (or just query the current status of
rendering). The dma-buf sync ioctl allows userspace to prepare the
dma-buf for CPU access, which should include waiting upon rendering.
(Some drivers may need to do more work to ensure that the dma-buf mmap
is coherent as well as complete.)
v2: Always wait upon the reservation object implicitly. We choose to do
it after the native handler in case it can do so more efficiently.
Testcase: igt/prime_vgem
Testcase: igt/gem_concurrent_blit # *vgem*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Eric Anholt <eric@anholt.net>
Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
---
drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ddaee60ae52a..cf04d249a6a4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
}
EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
+static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
+ enum dma_data_direction direction)
+{
+ bool write = (direction == DMA_BIDIRECTIONAL ||
+ direction == DMA_TO_DEVICE);
+ struct reservation_object *resv = dmabuf->resv;
+ long ret;
+
+ /* Wait on any implicit rendering fences */
+ ret = reservation_object_wait_timeout_rcu(resv, write, true,
+ MAX_SCHEDULE_TIMEOUT);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
/**
* dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
@@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
if (dmabuf->ops->begin_cpu_access)
ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
+ /* Ensure that all fences are waited upon - but we first allow
+ * the native handler the chance to do so more efficiently if it
+ * chooses. A double invocation here will be reasonably cheap no-op.
+ */
+ if (ret == 0)
+ ret = __dma_buf_begin_cpu_access(dmabuf, direction);
+
return ret;
}
EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
--
2.8.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
2016-08-15 15:42 ` Chris Wilson
@ 2016-08-15 16:02 ` Daniel Vetter
-1 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2016-08-15 16:02 UTC (permalink / raw)
To: Chris Wilson
Cc: dri-devel, intel-gfx, Sumit Semwal, Daniel Vetter, Eric Anholt,
linux-media, linaro-mm-sig, linux-kernel
On Mon, Aug 15, 2016 at 04:42:18PM +0100, Chris Wilson wrote:
> Rendering operations to the dma-buf are tracked implicitly via the
> reservation_object (dmabuf->resv). This is used to allow poll() to
> wait upon outstanding rendering (or just query the current status of
> rendering). The dma-buf sync ioctl allows userspace to prepare the
> dma-buf for CPU access, which should include waiting upon rendering.
> (Some drivers may need to do more work to ensure that the dma-buf mmap
> is coherent as well as complete.)
>
> v2: Always wait upon the reservation object implicitly. We choose to do
> it after the native handler in case it can do so more efficiently.
>
> Testcase: igt/prime_vgem
> Testcase: igt/gem_concurrent_blit # *vgem*
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: linux-media@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linaro-mm-sig@lists.linaro.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index ddaee60ae52a..cf04d249a6a4 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
> }
> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
>
> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> + enum dma_data_direction direction)
> +{
> + bool write = (direction == DMA_BIDIRECTIONAL ||
> + direction == DMA_TO_DEVICE);
> + struct reservation_object *resv = dmabuf->resv;
> + long ret;
> +
> + /* Wait on any implicit rendering fences */
> + ret = reservation_object_wait_timeout_rcu(resv, write, true,
> + MAX_SCHEDULE_TIMEOUT);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
>
> /**
> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
> @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> if (dmabuf->ops->begin_cpu_access)
> ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>
> + /* Ensure that all fences are waited upon - but we first allow
> + * the native handler the chance to do so more efficiently if it
> + * chooses. A double invocation here will be reasonably cheap no-op.
> + */
> + if (ret == 0)
> + ret = __dma_buf_begin_cpu_access(dmabuf, direction);
Not sure we should wait first and the flush or the other way round. But I
don't think it'll matter for any current dma-buf exporter, so meh.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Sumits, can you pls pick this one up and put into drm-misc?
-Daniel
> +
> return ret;
> }
> EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
> --
> 2.8.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
@ 2016-08-15 16:02 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2016-08-15 16:02 UTC (permalink / raw)
To: Chris Wilson
Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel, linaro-mm-sig,
Sumit Semwal, linux-media
On Mon, Aug 15, 2016 at 04:42:18PM +0100, Chris Wilson wrote:
> Rendering operations to the dma-buf are tracked implicitly via the
> reservation_object (dmabuf->resv). This is used to allow poll() to
> wait upon outstanding rendering (or just query the current status of
> rendering). The dma-buf sync ioctl allows userspace to prepare the
> dma-buf for CPU access, which should include waiting upon rendering.
> (Some drivers may need to do more work to ensure that the dma-buf mmap
> is coherent as well as complete.)
>
> v2: Always wait upon the reservation object implicitly. We choose to do
> it after the native handler in case it can do so more efficiently.
>
> Testcase: igt/prime_vgem
> Testcase: igt/gem_concurrent_blit # *vgem*
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: linux-media@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linaro-mm-sig@lists.linaro.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index ddaee60ae52a..cf04d249a6a4 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
> }
> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
>
> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> + enum dma_data_direction direction)
> +{
> + bool write = (direction == DMA_BIDIRECTIONAL ||
> + direction == DMA_TO_DEVICE);
> + struct reservation_object *resv = dmabuf->resv;
> + long ret;
> +
> + /* Wait on any implicit rendering fences */
> + ret = reservation_object_wait_timeout_rcu(resv, write, true,
> + MAX_SCHEDULE_TIMEOUT);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
>
> /**
> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
> @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> if (dmabuf->ops->begin_cpu_access)
> ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>
> + /* Ensure that all fences are waited upon - but we first allow
> + * the native handler the chance to do so more efficiently if it
> + * chooses. A double invocation here will be reasonably cheap no-op.
> + */
> + if (ret == 0)
> + ret = __dma_buf_begin_cpu_access(dmabuf, direction);
Not sure we should wait first and the flush or the other way round. But I
don't think it'll matter for any current dma-buf exporter, so meh.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Sumits, can you pls pick this one up and put into drm-misc?
-Daniel
> +
> return ret;
> }
> EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
> --
> 2.8.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✗ Ro.CI.BAT: failure for dma-buf: Wait on the reservation object when sync'ing before CPU access
2016-06-21 7:04 ` Chris Wilson
` (4 preceding siblings ...)
(?)
@ 2016-08-15 16:35 ` Patchwork
-1 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2016-08-15 16:35 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: dma-buf: Wait on the reservation object when sync'ing before CPU access
URL : https://patchwork.freedesktop.org/series/11116/
State : failure
== Summary ==
Series 11116v1 dma-buf: Wait on the reservation object when sync'ing before CPU access
http://patchwork.freedesktop.org/api/1.0/series/11116/revisions/1/mbox
Test gem_exec_gttfill:
Subgroup basic:
skip -> PASS (fi-snb-i7-2600)
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> DMESG-WARN (ro-bdw-i7-5600u)
Test kms_cursor_legacy:
Subgroup basic-flip-vs-cursor-varying-size:
pass -> FAIL (ro-skl3-i5-6260u)
pass -> FAIL (ro-bdw-i5-5250u)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
pass -> FAIL (ro-bdw-i7-5600u)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
dmesg-warn -> PASS (ro-bdw-i7-5600u)
fi-kbl-qkkr total:244 pass:185 dwarn:29 dfail:0 fail:3 skip:27
fi-skl-i7-6700k total:244 pass:208 dwarn:4 dfail:2 fail:2 skip:28
fi-snb-i7-2600 total:244 pass:202 dwarn:0 dfail:0 fail:0 skip:42
ro-bdw-i5-5250u total:240 pass:218 dwarn:3 dfail:0 fail:2 skip:17
ro-bdw-i7-5600u total:240 pass:205 dwarn:1 dfail:0 fail:2 skip:32
ro-bsw-n3050 total:240 pass:195 dwarn:0 dfail:0 fail:3 skip:42
ro-byt-n2820 total:240 pass:197 dwarn:0 dfail:0 fail:3 skip:40
ro-hsw-i3-4010u total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26
ro-hsw-i7-4770r total:240 pass:185 dwarn:0 dfail:0 fail:0 skip:55
ro-ilk1-i5-650 total:235 pass:174 dwarn:0 dfail:0 fail:1 skip:60
ro-ivb-i7-3770 total:240 pass:205 dwarn:0 dfail:0 fail:0 skip:35
ro-ivb2-i7-3770 total:240 pass:209 dwarn:0 dfail:0 fail:0 skip:31
ro-skl3-i5-6260u total:240 pass:222 dwarn:0 dfail:0 fail:4 skip:14
Results at /archive/results/CI_IGT_test/RO_Patchwork_1878/
bc5705c drm-intel-nightly: 2016y-08m-15d-15h-16m-01s UTC integration manifest
a034e8e dma-buf: Wait on the reservation object when sync'ing before CPU access
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
2016-08-15 16:02 ` Daniel Vetter
@ 2016-12-19 1:40 ` Inki Dae
-1 siblings, 0 replies; 17+ messages in thread
From: Inki Dae @ 2016-12-19 1:40 UTC (permalink / raw)
To: Chris Wilson, dri-devel, intel-gfx, Sumit Semwal, Eric Anholt,
linux-media, linaro-mm-sig, linux-kernel
2016년 08월 16일 01:02에 Daniel Vetter 이(가) 쓴 글:
> On Mon, Aug 15, 2016 at 04:42:18PM +0100, Chris Wilson wrote:
>> Rendering operations to the dma-buf are tracked implicitly via the
>> reservation_object (dmabuf->resv). This is used to allow poll() to
>> wait upon outstanding rendering (or just query the current status of
>> rendering). The dma-buf sync ioctl allows userspace to prepare the
>> dma-buf for CPU access, which should include waiting upon rendering.
>> (Some drivers may need to do more work to ensure that the dma-buf mmap
>> is coherent as well as complete.)
>>
>> v2: Always wait upon the reservation object implicitly. We choose to do
>> it after the native handler in case it can do so more efficiently.
>>
>> Testcase: igt/prime_vgem
>> Testcase: igt/gem_concurrent_blit # *vgem*
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Sumit Semwal <sumit.semwal@linaro.org>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Eric Anholt <eric@anholt.net>
>> Cc: linux-media@vger.kernel.org
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: linaro-mm-sig@lists.linaro.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>> drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index ddaee60ae52a..cf04d249a6a4 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
>> }
>> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
>>
>> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>> + enum dma_data_direction direction)
>> +{
>> + bool write = (direction == DMA_BIDIRECTIONAL ||
>> + direction == DMA_TO_DEVICE);
>> + struct reservation_object *resv = dmabuf->resv;
>> + long ret;
>> +
>> + /* Wait on any implicit rendering fences */
>> + ret = reservation_object_wait_timeout_rcu(resv, write, true,
>> + MAX_SCHEDULE_TIMEOUT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return 0;
>> +}
>>
>> /**
>> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
>> @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>> if (dmabuf->ops->begin_cpu_access)
>> ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>>
>> + /* Ensure that all fences are waited upon - but we first allow
>> + * the native handler the chance to do so more efficiently if it
>> + * chooses. A double invocation here will be reasonably cheap no-op.
>> + */
>> + if (ret == 0)
>> + ret = __dma_buf_begin_cpu_access(dmabuf, direction);
>
> Not sure we should wait first and the flush or the other way round. But I
> don't think it'll matter for any current dma-buf exporter, so meh.
>
Sorry for late comment. I wonder there is no problem in case that GPU or other DMA device tries to access this dma buffer after dma_buf_begin_cpu_access call.
I think in this case, they - GPU or DMA devices - would make a mess of the dma buffer while CPU is accessing the buffer.
This patch is in mainline already so if this is real problem then I think we sould choose,
1. revert this patch from mainline
2. make sure to prevent other DMA devices to try to access the buffer while CPU is accessing the buffer.
Thanks.
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Sumits, can you pls pick this one up and put into drm-misc?
> -Daniel
>
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>> --
>> 2.8.1
>>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
@ 2016-12-19 1:40 ` Inki Dae
0 siblings, 0 replies; 17+ messages in thread
From: Inki Dae @ 2016-12-19 1:40 UTC (permalink / raw)
To: Chris Wilson, dri-devel, intel-gfx, Sumit Semwal, Eric Anholt,
linux-media, linaro-mm-sig, linux-kernel
2016년 08월 16일 01:02에 Daniel Vetter 이(가) 쓴 글:
> On Mon, Aug 15, 2016 at 04:42:18PM +0100, Chris Wilson wrote:
>> Rendering operations to the dma-buf are tracked implicitly via the
>> reservation_object (dmabuf->resv). This is used to allow poll() to
>> wait upon outstanding rendering (or just query the current status of
>> rendering). The dma-buf sync ioctl allows userspace to prepare the
>> dma-buf for CPU access, which should include waiting upon rendering.
>> (Some drivers may need to do more work to ensure that the dma-buf mmap
>> is coherent as well as complete.)
>>
>> v2: Always wait upon the reservation object implicitly. We choose to do
>> it after the native handler in case it can do so more efficiently.
>>
>> Testcase: igt/prime_vgem
>> Testcase: igt/gem_concurrent_blit # *vgem*
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Sumit Semwal <sumit.semwal@linaro.org>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Eric Anholt <eric@anholt.net>
>> Cc: linux-media@vger.kernel.org
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: linaro-mm-sig@lists.linaro.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>> drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index ddaee60ae52a..cf04d249a6a4 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
>> }
>> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
>>
>> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>> + enum dma_data_direction direction)
>> +{
>> + bool write = (direction == DMA_BIDIRECTIONAL ||
>> + direction == DMA_TO_DEVICE);
>> + struct reservation_object *resv = dmabuf->resv;
>> + long ret;
>> +
>> + /* Wait on any implicit rendering fences */
>> + ret = reservation_object_wait_timeout_rcu(resv, write, true,
>> + MAX_SCHEDULE_TIMEOUT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return 0;
>> +}
>>
>> /**
>> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
>> @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>> if (dmabuf->ops->begin_cpu_access)
>> ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>>
>> + /* Ensure that all fences are waited upon - but we first allow
>> + * the native handler the chance to do so more efficiently if it
>> + * chooses. A double invocation here will be reasonably cheap no-op.
>> + */
>> + if (ret == 0)
>> + ret = __dma_buf_begin_cpu_access(dmabuf, direction);
>
> Not sure we should wait first and the flush or the other way round. But I
> don't think it'll matter for any current dma-buf exporter, so meh.
>
Sorry for late comment. I wonder there is no problem in case that GPU or other DMA device tries to access this dma buffer after dma_buf_begin_cpu_access call.
I think in this case, they - GPU or DMA devices - would make a mess of the dma buffer while CPU is accessing the buffer.
This patch is in mainline already so if this is real problem then I think we sould choose,
1. revert this patch from mainline
2. make sure to prevent other DMA devices to try to access the buffer while CPU is accessing the buffer.
Thanks.
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Sumits, can you pls pick this one up and put into drm-misc?
> -Daniel
>
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>> --
>> 2.8.1
>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
2016-12-19 1:40 ` Inki Dae
@ 2016-12-19 10:23 ` Chris Wilson
-1 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2016-12-19 10:23 UTC (permalink / raw)
To: Inki Dae
Cc: dri-devel, intel-gfx, Sumit Semwal, Eric Anholt, linux-media,
linaro-mm-sig, linux-kernel
On Mon, Dec 19, 2016 at 10:40:41AM +0900, Inki Dae wrote:
>
>
> 2016년 08월 16일 01:02에 Daniel Vetter 이(가) 쓴 글:
> > On Mon, Aug 15, 2016 at 04:42:18PM +0100, Chris Wilson wrote:
> >> Rendering operations to the dma-buf are tracked implicitly via the
> >> reservation_object (dmabuf->resv). This is used to allow poll() to
> >> wait upon outstanding rendering (or just query the current status of
> >> rendering). The dma-buf sync ioctl allows userspace to prepare the
> >> dma-buf for CPU access, which should include waiting upon rendering.
> >> (Some drivers may need to do more work to ensure that the dma-buf mmap
> >> is coherent as well as complete.)
> >>
> >> v2: Always wait upon the reservation object implicitly. We choose to do
> >> it after the native handler in case it can do so more efficiently.
> >>
> >> Testcase: igt/prime_vgem
> >> Testcase: igt/gem_concurrent_blit # *vgem*
> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Cc: Eric Anholt <eric@anholt.net>
> >> Cc: linux-media@vger.kernel.org
> >> Cc: dri-devel@lists.freedesktop.org
> >> Cc: linaro-mm-sig@lists.linaro.org
> >> Cc: linux-kernel@vger.kernel.org
> >> ---
> >> drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++
> >> 1 file changed, 23 insertions(+)
> >>
> >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> >> index ddaee60ae52a..cf04d249a6a4 100644
> >> --- a/drivers/dma-buf/dma-buf.c
> >> +++ b/drivers/dma-buf/dma-buf.c
> >> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
> >> }
> >> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
> >>
> >> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >> + enum dma_data_direction direction)
> >> +{
> >> + bool write = (direction == DMA_BIDIRECTIONAL ||
> >> + direction == DMA_TO_DEVICE);
> >> + struct reservation_object *resv = dmabuf->resv;
> >> + long ret;
> >> +
> >> + /* Wait on any implicit rendering fences */
> >> + ret = reservation_object_wait_timeout_rcu(resv, write, true,
> >> + MAX_SCHEDULE_TIMEOUT);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + return 0;
> >> +}
> >>
> >> /**
> >> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
> >> @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >> if (dmabuf->ops->begin_cpu_access)
> >> ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
> >>
> >> + /* Ensure that all fences are waited upon - but we first allow
> >> + * the native handler the chance to do so more efficiently if it
> >> + * chooses. A double invocation here will be reasonably cheap no-op.
> >> + */
> >> + if (ret == 0)
> >> + ret = __dma_buf_begin_cpu_access(dmabuf, direction);
> >
> > Not sure we should wait first and the flush or the other way round. But I
> > don't think it'll matter for any current dma-buf exporter, so meh.
> >
>
> Sorry for late comment. I wonder there is no problem in case that GPU or other DMA device tries to access this dma buffer after dma_buf_begin_cpu_access call.
> I think in this case, they - GPU or DMA devices - would make a mess of the dma buffer while CPU is accessing the buffer.
>
> This patch is in mainline already so if this is real problem then I think we sould choose,
> 1. revert this patch from mainline
That scenario is irrespective of this patch. It just depends on there
being concurrent CPU access with destructive DMA access (or vice-versa).
> 2. make sure to prevent other DMA devices to try to access the buffer while CPU is accessing the buffer.
Is the safeguard you want, and the one employed elsewhere, which you could
accomplish by adding a fence to the reservation object for the CPU access
in begin_access and signaling from end_access. It would need to be an
autosignaled fence because userspace may forget to end its access
(or otherwise be terminated whilst holding the fence).
Everyone using the mmap without begin/end can of course still reek havoc
on the buffer.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
@ 2016-12-19 10:23 ` Chris Wilson
0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2016-12-19 10:23 UTC (permalink / raw)
To: Inki Dae; +Cc: intel-gfx, linux-kernel, dri-devel, linaro-mm-sig, linux-media
On Mon, Dec 19, 2016 at 10:40:41AM +0900, Inki Dae wrote:
>
>
> 2016년 08월 16일 01:02에 Daniel Vetter 이(가) 쓴 글:
> > On Mon, Aug 15, 2016 at 04:42:18PM +0100, Chris Wilson wrote:
> >> Rendering operations to the dma-buf are tracked implicitly via the
> >> reservation_object (dmabuf->resv). This is used to allow poll() to
> >> wait upon outstanding rendering (or just query the current status of
> >> rendering). The dma-buf sync ioctl allows userspace to prepare the
> >> dma-buf for CPU access, which should include waiting upon rendering.
> >> (Some drivers may need to do more work to ensure that the dma-buf mmap
> >> is coherent as well as complete.)
> >>
> >> v2: Always wait upon the reservation object implicitly. We choose to do
> >> it after the native handler in case it can do so more efficiently.
> >>
> >> Testcase: igt/prime_vgem
> >> Testcase: igt/gem_concurrent_blit # *vgem*
> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Cc: Eric Anholt <eric@anholt.net>
> >> Cc: linux-media@vger.kernel.org
> >> Cc: dri-devel@lists.freedesktop.org
> >> Cc: linaro-mm-sig@lists.linaro.org
> >> Cc: linux-kernel@vger.kernel.org
> >> ---
> >> drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++
> >> 1 file changed, 23 insertions(+)
> >>
> >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> >> index ddaee60ae52a..cf04d249a6a4 100644
> >> --- a/drivers/dma-buf/dma-buf.c
> >> +++ b/drivers/dma-buf/dma-buf.c
> >> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
> >> }
> >> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
> >>
> >> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >> + enum dma_data_direction direction)
> >> +{
> >> + bool write = (direction == DMA_BIDIRECTIONAL ||
> >> + direction == DMA_TO_DEVICE);
> >> + struct reservation_object *resv = dmabuf->resv;
> >> + long ret;
> >> +
> >> + /* Wait on any implicit rendering fences */
> >> + ret = reservation_object_wait_timeout_rcu(resv, write, true,
> >> + MAX_SCHEDULE_TIMEOUT);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + return 0;
> >> +}
> >>
> >> /**
> >> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
> >> @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >> if (dmabuf->ops->begin_cpu_access)
> >> ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
> >>
> >> + /* Ensure that all fences are waited upon - but we first allow
> >> + * the native handler the chance to do so more efficiently if it
> >> + * chooses. A double invocation here will be reasonably cheap no-op.
> >> + */
> >> + if (ret == 0)
> >> + ret = __dma_buf_begin_cpu_access(dmabuf, direction);
> >
> > Not sure we should wait first and the flush or the other way round. But I
> > don't think it'll matter for any current dma-buf exporter, so meh.
> >
>
> Sorry for late comment. I wonder there is no problem in case that GPU or other DMA device tries to access this dma buffer after dma_buf_begin_cpu_access call.
> I think in this case, they - GPU or DMA devices - would make a mess of the dma buffer while CPU is accessing the buffer.
>
> This patch is in mainline already so if this is real problem then I think we sould choose,
> 1. revert this patch from mainline
That scenario is irrespective of this patch. It just depends on there
being concurrent CPU access with destructive DMA access (or vice-versa).
> 2. make sure to prevent other DMA devices to try to access the buffer while CPU is accessing the buffer.
Is the safeguard you want, and the one employed elsewhere, which you could
accomplish by adding a fence to the reservation object for the CPU access
in begin_access and signaling from end_access. It would need to be an
autosignaled fence because userspace may forget to end its access
(or otherwise be terminated whilst holding the fence).
Everyone using the mmap without begin/end can of course still reek havoc
on the buffer.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-12-19 10:25 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-21 7:04 [PATCH] dma-buf: Wait on the reservation object when sync'ing before CPU access Chris Wilson
2016-06-21 7:04 ` Chris Wilson
2016-06-21 8:44 ` Daniel Vetter
2016-06-21 8:44 ` Daniel Vetter
2016-08-15 15:37 ` [PATCH v2] " Chris Wilson
2016-08-15 15:37 ` Chris Wilson
2016-08-15 15:37 ` [PATCH v2] drm/i915: Use common RPS scheme for Cherryview Chris Wilson
2016-08-15 15:41 ` Chris Wilson
2016-08-15 15:42 ` [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access Chris Wilson
2016-08-15 15:42 ` Chris Wilson
2016-08-15 16:02 ` Daniel Vetter
2016-08-15 16:02 ` Daniel Vetter
2016-12-19 1:40 ` Inki Dae
2016-12-19 1:40 ` Inki Dae
2016-12-19 10:23 ` Chris Wilson
2016-12-19 10:23 ` Chris Wilson
2016-08-15 16:35 ` ✗ Ro.CI.BAT: failure for " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.