From: Jani Nikula <jani.nikula@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT
Date: Wed, 26 Feb 2020 11:27:25 +0200 [thread overview]
Message-ID: <87tv3dsnpe.fsf@intel.com> (raw)
In-Reply-To: <20200225143604.500731-1-chris@chris-wilson.co.uk>
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;
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-02-26 9:27 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-25 14:36 [Intel-gfx] [PATCH 1/3] drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT Chris Wilson
2020-02-25 14:36 ` [Intel-gfx] [PATCH 2/3] drm/i915: Drop I915_RESET_TIMEOUT and friends Chris Wilson
2020-02-26 9:27 ` Jani Nikula
2020-02-25 14:36 ` [Intel-gfx] [PATCH 3/3] drm/i915: Drop I915_IDLE_ENGINES_TIMEOUT Chris Wilson
2020-02-26 9:19 ` [Intel-gfx] [PATCH 1/3] drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT Jani Nikula
2020-02-26 9:27 ` Jani Nikula [this message]
2020-02-26 16:37 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] " Patchwork
2020-02-26 17:55 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-02-27 7:31 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tv3dsnpe.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.