Re: [Intel-gfx] [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl
On Tue, 2017-12-05 at 10:41 +, Rogovin, Kevin wrote: > Thanks, I will make a v2 and post it shortly to correct for my > terribly embarrassing omission caused by even more terribly > embarrassing ignorance. To avoid v3, do concept the code around suggested existing I915_GEM_CONTEXT_GETPARAM and I915_GEM_CONTEXT_SETPARAM ioctls. Just add a new parameter I915_CONTEXT_PARAM_SCRATCH_PAGE. The interface should become pretty bullet-proof that way. Regards, Joonas > > -Kevin > > -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Tuesday, December 5, 2017 12:39 PM > To: Rogovin, Kevin ; intel-gfx@lists.freedes > ktop.org > Subject: RE: [Intel-gfx] [PATCH 3/3] i965: check scratch page in a > locked fashion on each ioctl > > Quoting Rogovin, Kevin (2017-12-05 10:30:04) > > Hi, > > > > > Per context, then you can remove the locking. It's fitting since > > > the scratch page is per-context anyway. > > > > The scratch page is per context? This I did not know, I thought it > > was per PPGTT. If that is the case, then my proposed interface to > > get/set the scratch page contents is wrong because it does not pass > > the HW context id. > > Yes, it per-vm, which is per-ctx on everything you want to > investigate on. gen4-7 it is a global GTT with a global scratch, and > just a mutex inside one process is not going to give you atomicity. > -Chris > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl
Thanks, I will make a v2 and post it shortly to correct for my terribly embarrassing omission caused by even more terribly embarrassing ignorance. -Kevin -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Tuesday, December 5, 2017 12:39 PM To: Rogovin, Kevin ; intel-gfx@lists.freedesktop.org Subject: RE: [Intel-gfx] [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl Quoting Rogovin, Kevin (2017-12-05 10:30:04) > Hi, > > > Per context, then you can remove the locking. It's fitting since the > > scratch page is per-context anyway. > > The scratch page is per context? This I did not know, I thought it was per > PPGTT. If that is the case, then my proposed interface to get/set the scratch > page contents is wrong because it does not pass the HW context id. Yes, it per-vm, which is per-ctx on everything you want to investigate on. gen4-7 it is a global GTT with a global scratch, and just a mutex inside one process is not going to give you atomicity. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl
Quoting Rogovin, Kevin (2017-12-05 10:30:04) > Hi, > > > Per context, then you can remove the locking. It's fitting since the > > scratch page is per-context anyway. > > The scratch page is per context? This I did not know, I thought it was per > PPGTT. If that is the case, then my proposed interface to get/set the scratch > page contents is wrong because it does not pass the HW context id. Yes, it per-vm, which is per-ctx on everything you want to investigate on. gen4-7 it is a global GTT with a global scratch, and just a mutex inside one process is not going to give you atomicity. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl
Hi, > Per context, then you can remove the locking. It's fitting since the scratch > page is per-context anyway. The scratch page is per context? This I did not know, I thought it was per PPGTT. If that is the case, then my proposed interface to get/set the scratch page contents is wrong because it does not pass the HW context id. -Kevin ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl
Quoting kevin.rogo...@intel.com (2017-12-05 07:48:14)
> From: Kevin Rogovin
>
> ---
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 27
> ++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 216073129b..53b3eaf49b 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -804,7 +804,8 @@ static int
> submit_batch(struct brw_context *brw, int in_fence_fd, int *out_fence_fd)
> {
> const struct gen_device_info *devinfo = &brw->screen->devinfo;
> - __DRIscreen *dri_screen = brw->screen->driScrnPriv;
> + struct intel_screen *screen = brw->screen;
> + __DRIscreen *dri_screen = screen->driScrnPriv;
> struct intel_batchbuffer *batch = &brw->batch;
> int ret = 0;
>
> @@ -875,10 +876,34 @@ submit_batch(struct brw_context *brw, int in_fence_fd,
> int *out_fence_fd)
> batch->validation_list[index] = tmp;
>}
>
> + if (unlikely(screen->debug_batchbuffer.enabled)) {
> + simple_mtx_lock(&screen->debug_batchbuffer.mutex);
> + }
Per context, then you can remove the locking. It's fitting since the
scratch page is per-context anyway.
> +
>ret = execbuffer(dri_screen->fd, batch, hw_ctx,
> 4 * USED_BATCH(*batch),
> in_fence_fd, out_fence_fd, flags);
>
> + if (unlikely(screen->debug_batchbuffer.enabled)) {
> + struct drm_i915_scratch_page sc;
> + int ret;
Tie this into INTEL_DEBUG & SYNC, then you can do all the synchronous
operations in one place.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl
From: Kevin Rogovin
---
src/mesa/drivers/dri/i965/intel_batchbuffer.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 216073129b..53b3eaf49b 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -804,7 +804,8 @@ static int
submit_batch(struct brw_context *brw, int in_fence_fd, int *out_fence_fd)
{
const struct gen_device_info *devinfo = &brw->screen->devinfo;
- __DRIscreen *dri_screen = brw->screen->driScrnPriv;
+ struct intel_screen *screen = brw->screen;
+ __DRIscreen *dri_screen = screen->driScrnPriv;
struct intel_batchbuffer *batch = &brw->batch;
int ret = 0;
@@ -875,10 +876,34 @@ submit_batch(struct brw_context *brw, int in_fence_fd,
int *out_fence_fd)
batch->validation_list[index] = tmp;
}
+ if (unlikely(screen->debug_batchbuffer.enabled)) {
+ simple_mtx_lock(&screen->debug_batchbuffer.mutex);
+ }
+
ret = execbuffer(dri_screen->fd, batch, hw_ctx,
4 * USED_BATCH(*batch),
in_fence_fd, out_fence_fd, flags);
+ if (unlikely(screen->debug_batchbuffer.enabled)) {
+ struct drm_i915_scratch_page sc;
+ int ret;
+
+ while (brw_bo_busy(batch->bo)) {
+usleep(10);
+ }
+
+ sc.buffer_size = screen->debug_batchbuffer.buffer_size;
+ sc.buffer_ptr = (__u64)(uintptr_t) screen->debug_batchbuffer.tmp;
+
+ ret = drmIoctl(dri_screen->fd, DRM_IOCTL_I915_READ_SCRATCH_PAGE, &sc);
+ assert(ret == 0);
+ assert(sc.buffer_size == screen->debug_batchbuffer.buffer_size);
+ assert(memcmp(screen->debug_batchbuffer.tmp,
+ screen->debug_batchbuffer.noise_values,
+ screen->debug_batchbuffer.buffer_size) == 0);
+ simple_mtx_unlock(&screen->debug_batchbuffer.mutex);
+ }
+
throttle(brw);
}
--
2.15.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
