On Tue, 25 Feb 2020, Chris Wilson <ch...@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 <ch...@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahti...@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.nik...@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;
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
