Re: [Intel-gfx] [PATCH 12/15] drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT
Quoting Ruhl, Michael J (2020-05-08 14:54:50) > >-Original Message- > >From: Chris Wilson > >Sent: Thursday, May 7, 2020 9:57 AM > >To: Ruhl, Michael J ; intel- > >g...@lists.freedesktop.org > >Subject: Re: [Intel-gfx] [PATCH 12/15] drm/i915: Replace the hardcoded > >I915_FENCE_TIMEOUT > > > >Quoting Ruhl, Michael J (2020-05-07 14:53:00) > >> > >> > >> >-Original Message- > >> >From: Intel-gfx On Behalf Of > >Chris > >> >Wilson > >> >diff --git a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c > >> >b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c > >> >index 3a146aa2593b..d3a86a4d5c04 100644 > >> >--- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c > >> >+++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c > >> >@@ -288,8 +288,7 @@ int i915_gem_schedule_fill_pages_blt(struct > >> >drm_i915_gem_object *obj, > >> > > >> > i915_gem_object_lock(obj); > >> > err = i915_sw_fence_await_reservation(>wait, > >> >-obj->base.resv, NULL, > >> >-true, I915_FENCE_TIMEOUT, > >> >+obj->base.resv, NULL, true, > >> >0, > >> > >> Did you miss this one, or is the '0' on purpose? > > > >It should be 0, as it should be only handling internal fences which may > >take as long as required and should not be timed out. > > > >Still this is a placeholder function and should not be taken too > >seriously. > > Assuming that the "placeholder function" is the _fill_pages_blt()... It is. I'm not fond of await_reservation either. That extra 'exclude' parameter belies a weakness of the one-size fits all approach. Just look at the trouble we go to with i915_request_await_dma_fence() to determine the "best" means of waiting on the fence, and then worry about how to fit that into a more generic api. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/15] drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT
>-Original Message- >From: Chris Wilson >Sent: Thursday, May 7, 2020 9:57 AM >To: Ruhl, Michael J ; intel- >g...@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH 12/15] drm/i915: Replace the hardcoded >I915_FENCE_TIMEOUT > >Quoting Ruhl, Michael J (2020-05-07 14:53:00) >> >> >> >-Original Message- >> >From: Intel-gfx On Behalf Of >Chris >> >Wilson >> >diff --git a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c >> >b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c >> >index 3a146aa2593b..d3a86a4d5c04 100644 >> >--- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c >> >+++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c >> >@@ -288,8 +288,7 @@ int i915_gem_schedule_fill_pages_blt(struct >> >drm_i915_gem_object *obj, >> > >> > i915_gem_object_lock(obj); >> > err = i915_sw_fence_await_reservation(>wait, >> >-obj->base.resv, NULL, >> >-true, I915_FENCE_TIMEOUT, >> >+obj->base.resv, NULL, true, 0, >> >> Did you miss this one, or is the '0' on purpose? > >It should be 0, as it should be only handling internal fences which may >take as long as required and should not be timed out. > >Still this is a placeholder function and should not be taken too >seriously. Assuming that the "placeholder function" is the _fill_pages_blt()... Acked-by: Michael J. Ruhl Mike >-Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/15] drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT
Quoting Ruhl, Michael J (2020-05-07 14:53:00) > > > >-Original Message- > >From: Intel-gfx On Behalf Of Chris > >Wilson > >diff --git a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c > >b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c > >index 3a146aa2593b..d3a86a4d5c04 100644 > >--- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c > >+++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c > >@@ -288,8 +288,7 @@ int i915_gem_schedule_fill_pages_blt(struct > >drm_i915_gem_object *obj, > > > > i915_gem_object_lock(obj); > > err = i915_sw_fence_await_reservation(>wait, > >-obj->base.resv, NULL, > >-true, I915_FENCE_TIMEOUT, > >+obj->base.resv, NULL, true, 0, > > Did you miss this one, or is the '0' on purpose? It should be 0, as it should be only handling internal fences which may take as long as required and should not be timed out. Still this is a placeholder function and should not be taken too seriously. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/15] drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT
>-Original Message-
>From: Intel-gfx On Behalf Of Chris
>Wilson
>Sent: Wednesday, May 6, 2020 4:59 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Chris Wilson
>Subject: [Intel-gfx] [PATCH 12/15] drm/i915: Replace the hardcoded
>I915_FENCE_TIMEOUT
>
>Expose the hardcoded timeout for unsignaled foreign fences as a Kconfig
>option, primarily to allow brave systems to disable the timeout and
>solely rely on correct signaling.
>
>Signed-off-by: Chris Wilson
>Cc: Joonas Lahtinen
>---
> drivers/gpu/drm/i915/Kconfig.profile | 12
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/display/intel_display.c | 5 +++--
> drivers/gpu/drm/i915/gem/i915_gem_clflush.c| 2 +-
> drivers/gpu/drm/i915/gem/i915_gem_client_blt.c | 3 +--
> drivers/gpu/drm/i915/gem/i915_gem_fence.c | 4 ++--
> drivers/gpu/drm/i915/i915_config.c | 15 +++
> drivers/gpu/drm/i915/i915_drv.h| 10 +-
> drivers/gpu/drm/i915/i915_request.c| 9 ++---
> 9 files changed, 50 insertions(+), 11 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/i915_config.c
>
>diff --git a/drivers/gpu/drm/i915/Kconfig.profile
>b/drivers/gpu/drm/i915/Kconfig.profile
>index 0bfd276c19fe..3925be65d314 100644
>--- a/drivers/gpu/drm/i915/Kconfig.profile
>+++ b/drivers/gpu/drm/i915/Kconfig.profile
>@@ -1,3 +1,15 @@
>+config DRM_I915_FENCE_TIMEOUT
>+ int "Timeout for unsignaled foreign fences"
>+ default 1 # milliseconds
>+ help
>+When listening to a foreign fence, we install a supplementary timer
>+to ensure that we are always signaled and our userspace is able to
>+make forward progress. This value specifies the timeout used for an
>+unsignaled foreign fence.
>+
>+May be 0 to disable the timeout, and rely on the foreign fence being
>+eventually signaled.
>+
> config DRM_I915_USERFAULT_AUTOSUSPEND
> int "Runtime autosuspend delay for userspace GGTT mmaps (ms)"
> default 250 # milliseconds
>diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>index 5359c736c789..b0da6ea6e3f1 100644
>--- a/drivers/gpu/drm/i915/Makefile
>+++ b/drivers/gpu/drm/i915/Makefile
>@@ -35,6 +35,7 @@ subdir-ccflags-y += -I$(srctree)/$(src)
>
> # core driver code
> i915-y += i915_drv.o \
>+i915_config.o \
> i915_irq.o \
> i915_getparam.o \
> i915_params.o \
>diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>b/drivers/gpu/drm/i915/display/intel_display.c
>index fd6d63b03489..432b4eeaf9f6 100644
>--- a/drivers/gpu/drm/i915/display/intel_display.c
>+++ b/drivers/gpu/drm/i915/display/intel_display.c
>@@ -15814,7 +15814,7 @@ intel_prepare_plane_fb(struct drm_plane
>*_plane,
> if (new_plane_state->uapi.fence) { /* explicit fencing */
> ret = i915_sw_fence_await_dma_fence(
>>commit_ready,
> new_plane_state-
>>uapi.fence,
>- I915_FENCE_TIMEOUT,
>+
>i915_fence_timeout(dev_priv),
> GFP_KERNEL);
> if (ret < 0)
> return ret;
>@@ -15841,7 +15841,8 @@ intel_prepare_plane_fb(struct drm_plane
>*_plane,
>
> ret = i915_sw_fence_await_reservation(
>>commit_ready,
> obj->base.resv, NULL,
>-false,
>I915_FENCE_TIMEOUT,
>+false,
>+
>i915_fence_timeout(dev_priv),
> GFP_KERNEL);
> if (ret < 0)
> goto unpin_fb;
>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
>b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
>index 34be4c0ee7c5..bc0223716906 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
>@@ -108,7 +108,7 @@ bool i915_gem_clflush_object(struct
>drm_i915_gem_object *obj,
> if (clflush) {
> i915_sw_fence_await_reservation(>base.chain,
> obj->base.resv, NULL, true,
>- I915_FENCE_TIMEOUT,
>+
> i915_fence_timeout(to_i915(obj->base.dev)),
> I915_FENCE_GFP);
> dma_resv_add_excl_fence(obj->base.resv,
>>base.dma);
> dma_fence_work_commit(>base);
>diff --
[Intel-gfx] [PATCH 12/15] drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT
Expose the hardcoded timeout for unsignaled foreign fences as a Kconfig
option, primarily to allow brave systems to disable the timeout and
solely rely on correct signaling.
Signed-off-by: Chris Wilson
Cc: Joonas Lahtinen
---
drivers/gpu/drm/i915/Kconfig.profile | 12
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/display/intel_display.c | 5 +++--
drivers/gpu/drm/i915/gem/i915_gem_clflush.c| 2 +-
drivers/gpu/drm/i915/gem/i915_gem_client_blt.c | 3 +--
drivers/gpu/drm/i915/gem/i915_gem_fence.c | 4 ++--
drivers/gpu/drm/i915/i915_config.c | 15 +++
drivers/gpu/drm/i915/i915_drv.h| 10 +-
drivers/gpu/drm/i915/i915_request.c| 9 ++---
9 files changed, 50 insertions(+), 11 deletions(-)
create mode 100644 drivers/gpu/drm/i915/i915_config.c
diff --git a/drivers/gpu/drm/i915/Kconfig.profile
b/drivers/gpu/drm/i915/Kconfig.profile
index 0bfd276c19fe..3925be65d314 100644
--- a/drivers/gpu/drm/i915/Kconfig.profile
+++ b/drivers/gpu/drm/i915/Kconfig.profile
@@ -1,3 +1,15 @@
+config DRM_I915_FENCE_TIMEOUT
+ int "Timeout for unsignaled foreign fences"
+ default 1 # milliseconds
+ help
+ When listening to a foreign fence, we install a supplementary timer
+ to ensure that we are always signaled and our userspace is able to
+ make forward progress. This value specifies the timeout used for an
+ unsignaled foreign fence.
+
+ May be 0 to disable the timeout, and rely on the foreign fence being
+ eventually signaled.
+
config DRM_I915_USERFAULT_AUTOSUSPEND
int "Runtime autosuspend delay for userspace GGTT mmaps (ms)"
default 250 # milliseconds
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 5359c736c789..b0da6ea6e3f1 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -35,6 +35,7 @@ subdir-ccflags-y += -I$(srctree)/$(src)
# core driver code
i915-y += i915_drv.o \
+ i915_config.o \
i915_irq.o \
i915_getparam.o \
i915_params.o \
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index fd6d63b03489..432b4eeaf9f6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15814,7 +15814,7 @@ intel_prepare_plane_fb(struct drm_plane *_plane,
if (new_plane_state->uapi.fence) { /* explicit fencing */
ret = i915_sw_fence_await_dma_fence(>commit_ready,
new_plane_state->uapi.fence,
- I915_FENCE_TIMEOUT,
+
i915_fence_timeout(dev_priv),
GFP_KERNEL);
if (ret < 0)
return ret;
@@ -15841,7 +15841,8 @@ intel_prepare_plane_fb(struct drm_plane *_plane,
ret = i915_sw_fence_await_reservation(>commit_ready,
obj->base.resv, NULL,
- false, I915_FENCE_TIMEOUT,
+ false,
+
i915_fence_timeout(dev_priv),
GFP_KERNEL);
if (ret < 0)
goto unpin_fb;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
index 34be4c0ee7c5..bc0223716906 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
@@ -108,7 +108,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object
*obj,
if (clflush) {
i915_sw_fence_await_reservation(>base.chain,
obj->base.resv, NULL, true,
- I915_FENCE_TIMEOUT,
+
i915_fence_timeout(to_i915(obj->base.dev)),
I915_FENCE_GFP);
dma_resv_add_excl_fence(obj->base.resv, >base.dma);
dma_fence_work_commit(>base);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
index 3a146aa2593b..d3a86a4d5c04 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
@@ -288,8 +288,7 @@ int i915_gem_schedule_fill_pages_blt(struct
drm_i915_gem_object *obj,
i915_gem_object_lock(obj);
err = i915_sw_fence_await_reservation(>wait,
- obj->base.resv, NULL,
- true, I915_FENCE_TIMEOUT,
+
