| Message ID | 20200225143604.500731-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT | expand |
On Tue, 25 Feb 2020, Chris Wilson <chris@chris-wilson.co.uk> wrote: > diff --git a/drivers/gpu/drm/i915/i915_config.c b/drivers/gpu/drm/i915/i915_config.c > new file mode 100644 > index 000000000000..c879d26369e1 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_config.c > @@ -0,0 +1,15 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2020 Intel Corporation > + */ > + > +#include "i915_drv.h" > + > +unsigned long i915_fence_timeout(const struct drm_i915_private *i915) i915_config_fence_timeout()? I guess you're planning on adding more functions, and I'd really like to name based on filenames. We all use cscope or GNU global or somesuch, but IMO the naming makes the codebase easier to navigate. BR, Jani. > +{ > +#if IS_ACTIVE(CONFIG_DRM_I915_FENCE_TIMEOUT) > + return msecs_to_jiffies_timeout(CONFIG_DRM_I915_FENCE_TIMEOUT); > +#else > + return 0; > +#endif > +}
On Tue, 25 Feb 2020, Chris Wilson <chris@chris-wilson.co.uk> wrote: > 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 <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > 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 | 2 +- > drivers/gpu/drm/i915/i915_request.c | 4 ++-- > 9 files changed, 38 insertions(+), 10 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 c280b6ae38eb..5d7b440b890d 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 10000 # milliseconds The documentation for the Kconfig could say milliseconds too. "Timeout (in milliseconds) ..." or something. > + 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 ff5c3983efff..51f923e5df47 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -37,6 +37,7 @@ subdir-ccflags-y += -I$(srctree)/$(src) > > # core driver code > i915-y += i915_drv.o \ > + i915_config.o \ c before d? > 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 3031e64ee518..54b8243ce986 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -15978,7 +15978,7 @@ intel_prepare_plane_fb(struct drm_plane *_plane, > if (new_plane_state->uapi.fence) { /* explicit fencing */ > ret = i915_sw_fence_await_dma_fence(&state->commit_ready, > new_plane_state->uapi.fence, > - I915_FENCE_TIMEOUT, > + i915_fence_timeout(dev_priv), > GFP_KERNEL); > if (ret < 0) > return ret; > @@ -16005,7 +16005,8 @@ intel_prepare_plane_fb(struct drm_plane *_plane, > > ret = i915_sw_fence_await_reservation(&state->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(&clflush->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, &clflush->base.dma); > dma_fence_work_commit(&clflush->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 81366aa4812b..5548e3be35c8 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c > @@ -289,8 +289,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(&work->wait, > - obj->base.resv, NULL, > - true, I915_FENCE_TIMEOUT, > + obj->base.resv, NULL, true, 0, Unmentioned functional change? I don't know if the change is okay, but if it is, please mention it in the commit message. Or make it a separate patch. > I915_FENCE_GFP); > if (err < 0) { > dma_fence_set_error(&work->dma, err); > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_fence.c b/drivers/gpu/drm/i915/gem/i915_gem_fence.c > index 2f6100ec2608..8ab842c80f99 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_fence.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_fence.c > @@ -72,8 +72,8 @@ i915_gem_object_lock_fence(struct drm_i915_gem_object *obj) > 0, 0); > > if (i915_sw_fence_await_reservation(&stub->chain, > - obj->base.resv, NULL, > - true, I915_FENCE_TIMEOUT, > + obj->base.resv, NULL, true, > + i915_fence_timeout(to_i915(obj->base.dev)), > I915_FENCE_GFP) < 0) > goto err; > > diff --git a/drivers/gpu/drm/i915/i915_config.c b/drivers/gpu/drm/i915/i915_config.c > new file mode 100644 > index 000000000000..c879d26369e1 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_config.c > @@ -0,0 +1,15 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2020 Intel Corporation > + */ > + > +#include "i915_drv.h" > + > +unsigned long i915_fence_timeout(const struct drm_i915_private *i915) > +{ > +#if IS_ACTIVE(CONFIG_DRM_I915_FENCE_TIMEOUT) > + return msecs_to_jiffies_timeout(CONFIG_DRM_I915_FENCE_TIMEOUT); > +#else > + return 0; > +#endif > +} > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8c2c69a5adb0..de1f1cbcc41d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -610,8 +610,8 @@ struct i915_gem_mm { > > #define I915_IDLE_ENGINES_TIMEOUT (200) /* in ms */ > > +unsigned long i915_fence_timeout(const struct drm_i915_private *i915); Oh, and in the interest of trimming down i915_drv.h, please add i915_config.h. Other than the nitpicks, Reviewed-by: Jani Nikula <jani.nikula@intel.com> BR, Jani. > #define I915_RESET_TIMEOUT (10 * HZ) /* 10s */ > -#define I915_FENCE_TIMEOUT (10 * HZ) /* 10s */ > > #define I915_ENGINE_DEAD_TIMEOUT (4 * HZ) /* Seqno, head and subunits dead */ > #define I915_SEQNO_DEAD_TIMEOUT (12 * HZ) /* Seqno dead with active head */ > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > index 51d2b3d4be67..4bcaf2b589a2 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -1017,7 +1017,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) > ret = i915_request_await_request(rq, to_request(fence)); > else > ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, > - fence->context ? I915_FENCE_TIMEOUT : 0, > + fence->context ? i915_fence_timeout(rq->i915) : 0, > I915_FENCE_GFP); > if (ret < 0) > return ret; > @@ -1122,7 +1122,7 @@ i915_request_await_execution(struct i915_request *rq, > hook); > else > ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, > - I915_FENCE_TIMEOUT, > + i915_fence_timeout(rq->i915), > GFP_KERNEL); > if (ret < 0) > return ret;
diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile index c280b6ae38eb..5d7b440b890d 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 10000 # 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 ff5c3983efff..51f923e5df47 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -37,6 +37,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 3031e64ee518..54b8243ce986 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -15978,7 +15978,7 @@ intel_prepare_plane_fb(struct drm_plane *_plane, if (new_plane_state->uapi.fence) { /* explicit fencing */ ret = i915_sw_fence_await_dma_fence(&state->commit_ready, new_plane_state->uapi.fence, - I915_FENCE_TIMEOUT, + i915_fence_timeout(dev_priv), GFP_KERNEL); if (ret < 0) return ret; @@ -16005,7 +16005,8 @@ intel_prepare_plane_fb(struct drm_plane *_plane, ret = i915_sw_fence_await_reservation(&state->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(&clflush->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, &clflush->base.dma); dma_fence_work_commit(&clflush->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 81366aa4812b..5548e3be35c8 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c @@ -289,8 +289,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(&work->wait, - obj->base.resv, NULL, - true, I915_FENCE_TIMEOUT, + obj->base.resv, NULL, true, 0, I915_FENCE_GFP); if (err < 0) { dma_fence_set_error(&work->dma, err); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_fence.c b/drivers/gpu/drm/i915/gem/i915_gem_fence.c index 2f6100ec2608..8ab842c80f99 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_fence.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_fence.c @@ -72,8 +72,8 @@ i915_gem_object_lock_fence(struct drm_i915_gem_object *obj) 0, 0); if (i915_sw_fence_await_reservation(&stub->chain, - obj->base.resv, NULL, - true, I915_FENCE_TIMEOUT, + obj->base.resv, NULL, true, + i915_fence_timeout(to_i915(obj->base.dev)), I915_FENCE_GFP) < 0) goto err; diff --git a/drivers/gpu/drm/i915/i915_config.c b/drivers/gpu/drm/i915/i915_config.c new file mode 100644 index 000000000000..c879d26369e1 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_config.c @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2020 Intel Corporation + */ + +#include "i915_drv.h" + +unsigned long i915_fence_timeout(const struct drm_i915_private *i915) +{ +#if IS_ACTIVE(CONFIG_DRM_I915_FENCE_TIMEOUT) + return msecs_to_jiffies_timeout(CONFIG_DRM_I915_FENCE_TIMEOUT); +#else + return 0; +#endif +} diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8c2c69a5adb0..de1f1cbcc41d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -610,8 +610,8 @@ struct i915_gem_mm { #define I915_IDLE_ENGINES_TIMEOUT (200) /* in ms */ +unsigned long i915_fence_timeout(const struct drm_i915_private *i915); #define I915_RESET_TIMEOUT (10 * HZ) /* 10s */ -#define I915_FENCE_TIMEOUT (10 * HZ) /* 10s */ #define I915_ENGINE_DEAD_TIMEOUT (4 * HZ) /* Seqno, head and subunits dead */ #define I915_SEQNO_DEAD_TIMEOUT (12 * HZ) /* Seqno dead with active head */ diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 51d2b3d4be67..4bcaf2b589a2 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1017,7 +1017,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) ret = i915_request_await_request(rq, to_request(fence)); else ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, - fence->context ? I915_FENCE_TIMEOUT : 0, + fence->context ? i915_fence_timeout(rq->i915) : 0, I915_FENCE_GFP); if (ret < 0) return ret; @@ -1122,7 +1122,7 @@ i915_request_await_execution(struct i915_request *rq, hook); else ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, - I915_FENCE_TIMEOUT, + i915_fence_timeout(rq->i915), GFP_KERNEL); if (ret < 0) return ret;
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 <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- 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 | 2 +- drivers/gpu/drm/i915/i915_request.c | 4 ++-- 9 files changed, 38 insertions(+), 10 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_config.c