* [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL
@ 2018-01-31 9:47 Chris Wilson
2018-01-31 9:47 ` [PATCH v2 2/3] drm/i915: Move the scheduler feature bits into the purview of the engines Chris Wilson
` (8 more replies)
0 siblings, 9 replies; 23+ messages in thread
From: Chris Wilson @ 2018-01-31 9:47 UTC (permalink / raw)
To: intel-gfx
In the next patch, we may only conditionally allocate the preempt-client
if there is a global preempt context and so we need to be prepared in
case the preempt-client itself is NULL.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/intel_guc_submission.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 1f3a8786bbdc..4ea65df05e02 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -832,10 +832,12 @@ static int guc_clients_doorbell_init(struct intel_guc *guc)
if (ret)
return ret;
- ret = create_doorbell(guc->preempt_client);
- if (ret) {
- destroy_doorbell(guc->execbuf_client);
- return ret;
+ if (guc->preempt_client) {
+ ret = create_doorbell(guc->preempt_client);
+ if (ret) {
+ destroy_doorbell(guc->execbuf_client);
+ return ret;
+ }
}
return 0;
@@ -848,8 +850,11 @@ static void guc_clients_doorbell_fini(struct intel_guc *guc)
* Instead of trying (in vain) to communicate with it, let's just
* cleanup the doorbell HW and our internal state.
*/
- __destroy_doorbell(guc->preempt_client);
- __update_doorbell_desc(guc->preempt_client, GUC_DOORBELL_INVALID);
+ if (guc->preempt_client) {
+ __destroy_doorbell(guc->preempt_client);
+ __update_doorbell_desc(guc->preempt_client,
+ GUC_DOORBELL_INVALID);
+ }
__destroy_doorbell(guc->execbuf_client);
__update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID);
}
@@ -998,10 +1003,11 @@ static void guc_clients_destroy(struct intel_guc *guc)
{
struct intel_guc_client *client;
- client = fetch_and_zero(&guc->execbuf_client);
- guc_client_free(client);
-
client = fetch_and_zero(&guc->preempt_client);
+ if (client)
+ guc_client_free(client);
+
+ client = fetch_and_zero(&guc->execbuf_client);
guc_client_free(client);
}
@@ -1160,7 +1166,8 @@ int intel_guc_submission_enable(struct intel_guc *guc)
GEM_BUG_ON(!guc->execbuf_client);
guc_reset_wq(guc->execbuf_client);
- guc_reset_wq(guc->preempt_client);
+ if (guc->preempt_client)
+ guc_reset_wq(guc->preempt_client);
err = intel_guc_sample_forcewake(guc);
if (err)
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 2/3] drm/i915: Move the scheduler feature bits into the purview of the engines
2018-01-31 9:47 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
@ 2018-01-31 9:47 ` Chris Wilson
2018-01-31 13:58 ` Mika Kuoppala
2018-01-31 9:47 ` [PATCH v2 3/3] drm/i915: Only allocate preempt context when required Chris Wilson
` (7 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2018-01-31 9:47 UTC (permalink / raw)
To: intel-gfx
Rather than having the high level ioctl interface guess the underlying
implementation details, having the implementation declare what
capabilities it exports. We define an intel_driver_caps, similar to the
intel_device_info, which instead of trying to describe the HW gives
details on what the driver itself supports. This is then populated by
the engine backend for the new scheduler capability field for use
elsewhere.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tomasz Lis <tomasz.lis@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 8 +-------
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_gem.c | 3 +++
drivers/gpu/drm/i915/i915_gpu_error.c | 7 +++++--
drivers/gpu/drm/i915/intel_device_info.c | 6 ++++++
drivers/gpu/drm/i915/intel_device_info.h | 7 +++++++
drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++
7 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1ec12add34b2..733f71637914 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -372,13 +372,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
value = i915_gem_mmap_gtt_version();
break;
case I915_PARAM_HAS_SCHEDULER:
- value = 0;
- if (dev_priv->engine[RCS] && dev_priv->engine[RCS]->schedule) {
- value |= I915_SCHEDULER_CAP_ENABLED;
- value |= I915_SCHEDULER_CAP_PRIORITY;
- if (HAS_LOGICAL_RING_PREEMPTION(dev_priv))
- value |= I915_SCHEDULER_CAP_PREEMPTION;
- }
+ value = dev_priv->caps.scheduler;
break;
case I915_PARAM_MMAP_VERSION:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 388b72cccde4..ae434c9d2b4f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -468,6 +468,7 @@ struct i915_gpu_state {
u32 reset_count;
u32 suspend_count;
struct intel_device_info device_info;
+ struct intel_driver_caps driver_caps;
struct i915_params params;
struct i915_error_uc {
@@ -1808,6 +1809,7 @@ struct drm_i915_private {
struct kmem_cache *priorities;
const struct intel_device_info info;
+ struct intel_driver_caps caps;
/**
* Data Stolen Memory - aka "i915 stolen memory" gives us the start and
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 062b21408698..ad7bd0dc57ee 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3223,8 +3223,11 @@ void i915_gem_set_wedged(struct drm_i915_private *i915)
* start to complete all requests.
*/
engine->submit_request = nop_complete_submit_request;
+ engine->schedule = NULL;
}
+ i915->caps.scheduler = 0;
+
/*
* Make sure no request can slip through without getting completed by
* either this call here to intel_engine_init_global_seqno, or the one
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index a81351d9e3a6..a92b0c0377c7 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -573,11 +573,13 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
}
static void err_print_capabilities(struct drm_i915_error_state_buf *m,
- const struct intel_device_info *info)
+ const struct intel_device_info *info,
+ const struct intel_driver_caps *caps)
{
struct drm_printer p = i915_error_printer(m);
intel_device_info_dump_flags(info, &p);
+ intel_driver_caps_print(caps, &p);
}
static void err_print_params(struct drm_i915_error_state_buf *m,
@@ -800,7 +802,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
if (error->display)
intel_display_print_error_state(m, error->display);
- err_print_capabilities(m, &error->device_info);
+ err_print_capabilities(m, &error->device_info, &error->driver_caps);
err_print_params(m, &error->params);
err_print_uc(m, &error->uc);
@@ -1731,6 +1733,7 @@ static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
memcpy(&error->device_info,
INTEL_INFO(dev_priv),
sizeof(error->device_info));
+ error->driver_caps = dev_priv->caps;
}
static __always_inline void dup_param(const char *type, void *x)
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index a2c16140169f..298f8996cc54 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -586,3 +586,9 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
/* Initialize command stream timestamp frequency */
info->cs_timestamp_frequency_khz = read_timestamp_frequency(dev_priv);
}
+
+void intel_driver_caps_print(const struct intel_driver_caps *caps,
+ struct drm_printer *p)
+{
+ drm_printf(p, "scheduler: %x\n", caps->scheduler);
+}
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 9542018d11d0..71fdfb0451ef 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -167,6 +167,10 @@ struct intel_device_info {
} color;
};
+struct intel_driver_caps {
+ unsigned int scheduler;
+};
+
static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
{
return hweight8(sseu->slice_mask) * hweight8(sseu->subslice_mask);
@@ -182,4 +186,7 @@ void intel_device_info_dump_flags(const struct intel_device_info *info,
void intel_device_info_dump_runtime(const struct intel_device_info *info,
struct drm_printer *p);
+void intel_driver_caps_print(const struct intel_driver_caps *caps,
+ struct drm_printer *p);
+
#endif
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2fa328d512fc..5390894001f0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1906,6 +1906,12 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
engine->unpark = NULL;
engine->flags |= I915_ENGINE_SUPPORTS_STATS;
+
+ engine->i915->caps.scheduler =
+ I915_SCHEDULER_CAP_ENABLED |
+ I915_SCHEDULER_CAP_PRIORITY;
+ if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
+ engine->i915->caps.scheduler |= I915_SCHEDULER_CAP_PREEMPTION;
}
static void
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 3/3] drm/i915: Only allocate preempt context when required
2018-01-31 9:47 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
2018-01-31 9:47 ` [PATCH v2 2/3] drm/i915: Move the scheduler feature bits into the purview of the engines Chris Wilson
@ 2018-01-31 9:47 ` Chris Wilson
2018-01-31 14:38 ` Mika Kuoppala
2018-02-03 10:40 ` [PATCH v2] " Chris Wilson
2018-01-31 11:01 ` ✗ Fi.CI.BAT: warning for series starting with [v2,1/3] drm/i915/guc: Allow preempt-client to be NULL Patchwork
` (6 subsequent siblings)
8 siblings, 2 replies; 23+ messages in thread
From: Chris Wilson @ 2018-01-31 9:47 UTC (permalink / raw)
To: intel-gfx; +Cc: Mika Kuoppala
If we remove some hardcoded assumptions about the preempt context having
a fixed id, reserved from use by normal user contexts, we may only
allocate the i915_gem_context when required. Then the subsequent
decisions on using preemption reduce to having the preempt context
available.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/i915_gem_context.c | 29 ++++++++++++------------
drivers/gpu/drm/i915/intel_engine_cs.c | 6 ++---
drivers/gpu/drm/i915/intel_guc_submission.c | 24 +++++++++++---------
drivers/gpu/drm/i915/intel_lrc.c | 17 +++++++++-----
drivers/gpu/drm/i915/intel_ringbuffer.h | 5 ++++
drivers/gpu/drm/i915/selftests/mock_gem_device.c | 6 -----
6 files changed, 46 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 648e7536ff51..d69c8f1a4e78 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -449,10 +449,14 @@ destroy_kernel_context(struct i915_gem_context **ctxp)
i915_gem_context_free(ctx);
}
+static bool needs_preempt_context(struct drm_i915_private *i915)
+{
+ return HAS_LOGICAL_RING_PREEMPTION(i915);
+}
+
int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
{
struct i915_gem_context *ctx;
- int err;
GEM_BUG_ON(dev_priv->kernel_context);
@@ -468,8 +472,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
ctx = i915_gem_context_create_kernel(dev_priv, I915_PRIORITY_MIN);
if (IS_ERR(ctx)) {
DRM_ERROR("Failed to create default global context\n");
- err = PTR_ERR(ctx);
- goto err;
+ return PTR_ERR(ctx);
}
/*
* For easy recognisablity, we want the kernel context to be 0 and then
@@ -479,23 +482,18 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
dev_priv->kernel_context = ctx;
/* highest priority; preempting task */
- ctx = i915_gem_context_create_kernel(dev_priv, INT_MAX);
- if (IS_ERR(ctx)) {
- DRM_ERROR("Failed to create default preempt context\n");
- err = PTR_ERR(ctx);
- goto err_kernel_context;
+ if (needs_preempt_context(dev_priv)) {
+ ctx = i915_gem_context_create_kernel(dev_priv, INT_MAX);
+ if (!IS_ERR(ctx))
+ dev_priv->preempt_context = ctx;
+ else
+ DRM_ERROR("Failed to create preempt context; disabling preemption\n");
}
- dev_priv->preempt_context = ctx;
DRM_DEBUG_DRIVER("%s context support initialized\n",
dev_priv->engine[RCS]->context_size ? "logical" :
"fake");
return 0;
-
-err_kernel_context:
- destroy_kernel_context(&dev_priv->kernel_context);
-err:
- return err;
}
void i915_gem_contexts_lost(struct drm_i915_private *dev_priv)
@@ -521,7 +519,8 @@ void i915_gem_contexts_fini(struct drm_i915_private *i915)
{
lockdep_assert_held(&i915->drm.struct_mutex);
- destroy_kernel_context(&i915->preempt_context);
+ if (i915->preempt_context)
+ destroy_kernel_context(&i915->preempt_context);
destroy_kernel_context(&i915->kernel_context);
/* Must free all deferred contexts (via flush_workqueue) first */
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 7eebfbb95e89..bf634432c9c6 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -631,7 +631,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
* Similarly the preempt context must always be available so that
* we can interrupt the engine at any time.
*/
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915)) {
+ if (engine->i915->preempt_context) {
ring = engine->context_pin(engine,
engine->i915->preempt_context);
if (IS_ERR(ring)) {
@@ -656,7 +656,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
err_breadcrumbs:
intel_engine_fini_breadcrumbs(engine);
err_unpin_preempt:
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
+ if (engine->i915->preempt_context)
engine->context_unpin(engine, engine->i915->preempt_context);
err_unpin_kernel:
engine->context_unpin(engine, engine->i915->kernel_context);
@@ -686,7 +686,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
if (engine->default_state)
i915_gem_object_put(engine->default_state);
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
+ if (engine->i915->preempt_context)
engine->context_unpin(engine, engine->i915->preempt_context);
engine->context_unpin(engine, engine->i915->kernel_context);
}
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 4ea65df05e02..b43b58cc599b 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -688,7 +688,7 @@ static void guc_dequeue(struct intel_engine_cs *engine)
goto unlock;
if (port_isset(port)) {
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915)) {
+ if (engine->i915->preempt_context) {
struct guc_preempt_work *preempt_work =
&engine->i915->guc.preempt_work[engine->id];
@@ -984,17 +984,19 @@ static int guc_clients_create(struct intel_guc *guc)
}
guc->execbuf_client = client;
- client = guc_client_alloc(dev_priv,
- INTEL_INFO(dev_priv)->ring_mask,
- GUC_CLIENT_PRIORITY_KMD_HIGH,
- dev_priv->preempt_context);
- if (IS_ERR(client)) {
- DRM_ERROR("Failed to create GuC client for preemption!\n");
- guc_client_free(guc->execbuf_client);
- guc->execbuf_client = NULL;
- return PTR_ERR(client);
+ if (dev_priv->preempt_context) {
+ client = guc_client_alloc(dev_priv,
+ INTEL_INFO(dev_priv)->ring_mask,
+ GUC_CLIENT_PRIORITY_KMD_HIGH,
+ dev_priv->preempt_context);
+ if (IS_ERR(client)) {
+ DRM_ERROR("Failed to create GuC client for preemption!\n");
+ guc_client_free(guc->execbuf_client);
+ guc->execbuf_client = NULL;
+ return PTR_ERR(client);
+ }
+ guc->preempt_client = client;
}
- guc->preempt_client = client;
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 5390894001f0..221b62d72c72 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -161,7 +161,6 @@
#define EXECLISTS_REQUEST_SIZE 64 /* bytes */
#define WA_TAIL_DWORDS 2
#define WA_TAIL_BYTES (sizeof(u32) * WA_TAIL_DWORDS)
-#define PREEMPT_ID 0x1
static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
@@ -448,7 +447,8 @@ static void inject_preempt_context(struct intel_engine_cs *engine)
&engine->i915->preempt_context->engine[engine->id];
unsigned int n;
- GEM_BUG_ON(engine->i915->preempt_context->hw_id != PREEMPT_ID);
+ GEM_BUG_ON(engine->execlists.preempt_complete_status !=
+ upper_32_bits(ce->lrc_desc));
GEM_BUG_ON(!IS_ALIGNED(ce->ring->size, WA_TAIL_BYTES));
memset(ce->ring->vaddr + ce->ring->tail, 0, WA_TAIL_BYTES);
@@ -528,7 +528,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_HWACK))
goto unlock;
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915) &&
+ if (engine->i915->preempt_context &&
rb_entry(rb, struct i915_priolist, node)->priority >
max(last->priotree.priority, 0)) {
/*
@@ -844,7 +844,7 @@ static void execlists_submission_tasklet(unsigned long data)
GEM_BUG_ON(status & GEN8_CTX_STATUS_IDLE_ACTIVE);
if (status & GEN8_CTX_STATUS_COMPLETE &&
- buf[2*head + 1] == PREEMPT_ID) {
+ buf[2*head + 1] == execlists->preempt_complete_status) {
GEM_TRACE("%s preempt-idle\n", engine->name);
execlists_cancel_port_requests(execlists);
@@ -1910,7 +1910,7 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
engine->i915->caps.scheduler =
I915_SCHEDULER_CAP_ENABLED |
I915_SCHEDULER_CAP_PRIORITY;
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
+ if (engine->i915->preempt_context)
engine->i915->caps.scheduler |= I915_SCHEDULER_CAP_PREEMPTION;
}
@@ -1988,6 +1988,11 @@ static int logical_ring_init(struct intel_engine_cs *engine)
engine->execlists.elsp =
engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine));
+ engine->execlists.preempt_complete_status = ~0u;
+ if (engine->i915->preempt_context)
+ engine->execlists.preempt_complete_status =
+ upper_32_bits(engine->i915->preempt_context->engine[engine->id].lrc_desc);
+
return 0;
error:
@@ -2250,7 +2255,7 @@ populate_lr_context(struct i915_gem_context *ctx,
if (!engine->default_state)
regs[CTX_CONTEXT_CONTROL + 1] |=
_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
- if (ctx->hw_id == PREEMPT_ID)
+ if (ctx == ctx->i915->preempt_context)
regs[CTX_CONTEXT_CONTROL + 1] |=
_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c5ff203e42d6..03be8995f415 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -279,6 +279,11 @@ struct intel_engine_execlists {
* @csb_use_mmio: access csb through mmio, instead of hwsp
*/
bool csb_use_mmio;
+
+ /**
+ * @preempt_complete_status: expected CSB upon completing preemption
+ */
+ u32 preempt_complete_status;
};
#define INTEL_ENGINE_CS_MAX_NAME 8
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 1bc61f3f76fc..3175db70cc6e 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -243,16 +243,10 @@ struct drm_i915_private *mock_gem_device(void)
if (!i915->kernel_context)
goto err_engine;
- i915->preempt_context = mock_context(i915, NULL);
- if (!i915->preempt_context)
- goto err_kernel_context;
-
WARN_ON(i915_gemfs_init(i915));
return i915;
-err_kernel_context:
- i915_gem_context_put(i915->kernel_context);
err_engine:
for_each_engine(engine, i915, id)
mock_engine_free(engine);
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [v2,1/3] drm/i915/guc: Allow preempt-client to be NULL
2018-01-31 9:47 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
2018-01-31 9:47 ` [PATCH v2 2/3] drm/i915: Move the scheduler feature bits into the purview of the engines Chris Wilson
2018-01-31 9:47 ` [PATCH v2 3/3] drm/i915: Only allocate preempt context when required Chris Wilson
@ 2018-01-31 11:01 ` Patchwork
2018-01-31 11:31 ` [PATCH v2 1/3] " Mika Kuoppala
` (5 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-01-31 11:01 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,1/3] drm/i915/guc: Allow preempt-client to be NULL
URL : https://patchwork.freedesktop.org/series/37395/
State : warning
== Summary ==
Series 37395v1 series starting with [v2,1/3] drm/i915/guc: Allow preempt-client to be NULL
https://patchwork.freedesktop.org/api/1.0/series/37395/revisions/1/mbox/
Test debugfs_test:
Subgroup read_all_entries:
dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989 +6
Test gem_ringfill:
Subgroup basic-default-hang:
pass -> DMESG-WARN (fi-elk-e7500) fdo#104058
Test gem_sync:
Subgroup basic-all:
pass -> SKIP (fi-elk-e7500)
Subgroup basic-each:
pass -> SKIP (fi-elk-e7500)
Subgroup basic-many-each:
pass -> SKIP (fi-elk-e7500)
Subgroup basic-store-all:
pass -> SKIP (fi-elk-e7500)
Subgroup basic-store-each:
pass -> SKIP (fi-elk-e7500)
Test gem_tiled_blits:
Subgroup basic:
pass -> SKIP (fi-elk-e7500)
Test gem_tiled_fence_blits:
Subgroup basic:
pass -> SKIP (fi-elk-e7500)
Test gem_wait:
Subgroup basic-busy-all:
pass -> SKIP (fi-elk-e7500)
Subgroup basic-wait-all:
pass -> SKIP (fi-elk-e7500)
Subgroup basic-await-all:
pass -> SKIP (fi-elk-e7500)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> INCOMPLETE (fi-snb-2520m) fdo#103713
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:414s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:426s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:371s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:486s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:282s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:480s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:486s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:466s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:452s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:575s
fi-elk-e7500 total:229 pass:161 dwarn:7 dfail:1 fail:0 skip:59
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:278s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:505s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:388s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:396s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:408s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:444s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:411s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:460s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:494s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:450s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:496s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:574s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:424s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:515s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:527s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:489s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:483s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:416s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:428s
fi-snb-2520m total:245 pass:211 dwarn:0 dfail:0 fail:0 skip:33
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:392s
Blacklisted hosts:
fi-glk-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:468s
5d84aeda7cd418fe9d81419a42d1e7a46264e209 drm-tip: 2018y-01m-31d-10h-16m-14s UTC integration manifest
d7ab3f9a2164 drm/i915: Only allocate preempt context when required
b7a9c5fa91e5 drm/i915: Move the scheduler feature bits into the purview of the engines
8852942ea5bd drm/i915/guc: Allow preempt-client to be NULL
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7836/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL
2018-01-31 9:47 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
` (2 preceding siblings ...)
2018-01-31 11:01 ` ✗ Fi.CI.BAT: warning for series starting with [v2,1/3] drm/i915/guc: Allow preempt-client to be NULL Patchwork
@ 2018-01-31 11:31 ` Mika Kuoppala
2018-01-31 11:34 ` Chris Wilson
2018-01-31 12:03 ` [PATCH v2] " Chris Wilson
2018-01-31 13:00 ` ✓ Fi.CI.BAT: success for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev2) Patchwork
` (4 subsequent siblings)
8 siblings, 2 replies; 23+ messages in thread
From: Mika Kuoppala @ 2018-01-31 11:31 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> In the next patch, we may only conditionally allocate the preempt-client
> if there is a global preempt context and so we need to be prepared in
> case the preempt-client itself is NULL.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
> drivers/gpu/drm/i915/intel_guc_submission.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
You need to prepare the debugfs to handle the null also.
Does our simple igt/debugfs test include reading through all the nodes
it finds?
-Mika
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 1f3a8786bbdc..4ea65df05e02 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -832,10 +832,12 @@ static int guc_clients_doorbell_init(struct intel_guc *guc)
> if (ret)
> return ret;
>
> - ret = create_doorbell(guc->preempt_client);
> - if (ret) {
> - destroy_doorbell(guc->execbuf_client);
> - return ret;
> + if (guc->preempt_client) {
> + ret = create_doorbell(guc->preempt_client);
> + if (ret) {
> + destroy_doorbell(guc->execbuf_client);
> + return ret;
> + }
> }
>
> return 0;
> @@ -848,8 +850,11 @@ static void guc_clients_doorbell_fini(struct intel_guc *guc)
> * Instead of trying (in vain) to communicate with it, let's just
> * cleanup the doorbell HW and our internal state.
> */
> - __destroy_doorbell(guc->preempt_client);
> - __update_doorbell_desc(guc->preempt_client, GUC_DOORBELL_INVALID);
> + if (guc->preempt_client) {
> + __destroy_doorbell(guc->preempt_client);
> + __update_doorbell_desc(guc->preempt_client,
> + GUC_DOORBELL_INVALID);
> + }
> __destroy_doorbell(guc->execbuf_client);
> __update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID);
> }
> @@ -998,10 +1003,11 @@ static void guc_clients_destroy(struct intel_guc *guc)
> {
> struct intel_guc_client *client;
>
> - client = fetch_and_zero(&guc->execbuf_client);
> - guc_client_free(client);
> -
> client = fetch_and_zero(&guc->preempt_client);
> + if (client)
> + guc_client_free(client);
> +
> + client = fetch_and_zero(&guc->execbuf_client);
> guc_client_free(client);
> }
>
> @@ -1160,7 +1166,8 @@ int intel_guc_submission_enable(struct intel_guc *guc)
> GEM_BUG_ON(!guc->execbuf_client);
>
> guc_reset_wq(guc->execbuf_client);
> - guc_reset_wq(guc->preempt_client);
> + if (guc->preempt_client)
> + guc_reset_wq(guc->preempt_client);
>
> err = intel_guc_sample_forcewake(guc);
> if (err)
> --
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL
2018-01-31 11:31 ` [PATCH v2 1/3] " Mika Kuoppala
@ 2018-01-31 11:34 ` Chris Wilson
2018-01-31 12:42 ` Mika Kuoppala
2018-01-31 12:03 ` [PATCH v2] " Chris Wilson
1 sibling, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2018-01-31 11:34 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2018-01-31 11:31:09)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > In the next patch, we may only conditionally allocate the preempt-client
> > if there is a global preempt context and so we need to be prepared in
> > case the preempt-client itself is NULL.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michal Winiarski <michal.winiarski@intel.com>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Michel Thierry <michel.thierry@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_guc_submission.c | 27 +++++++++++++++++----------
> > 1 file changed, 17 insertions(+), 10 deletions(-)
> >
>
> You need to prepare the debugfs to handle the null also.
>
> Does our simple igt/debugfs test include reading through all the nodes
> it finds?
It does, but we don't actually end up with NULLs.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2] drm/i915/guc: Allow preempt-client to be NULL
2018-01-31 11:31 ` [PATCH v2 1/3] " Mika Kuoppala
2018-01-31 11:34 ` Chris Wilson
@ 2018-01-31 12:03 ` Chris Wilson
2018-01-31 13:14 ` Mika Kuoppala
1 sibling, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2018-01-31 12:03 UTC (permalink / raw)
To: intel-gfx
In the next patch, we may only conditionally allocate the preempt-client
if there is a global preempt context and so we need to be prepared in
case the preempt-client itself is NULL.
v2: Grep for more preempt_client.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 8 +++++---
drivers/gpu/drm/i915/intel_guc_submission.c | 27 +++++++++++++++++----------
drivers/gpu/drm/i915/selftests/intel_guc.c | 20 +++++++++++---------
3 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3849ded354e3..9e44adef30f0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2338,7 +2338,6 @@ static int i915_guc_info(struct seq_file *m, void *data)
return -ENODEV;
GEM_BUG_ON(!guc->execbuf_client);
- GEM_BUG_ON(!guc->preempt_client);
seq_printf(m, "Doorbell map:\n");
seq_printf(m, "\t%*pb\n", GUC_NUM_DOORBELLS, guc->doorbell_bitmap);
@@ -2346,8 +2345,11 @@ static int i915_guc_info(struct seq_file *m, void *data)
seq_printf(m, "\nGuC execbuf client @ %p:\n", guc->execbuf_client);
i915_guc_client_info(m, dev_priv, guc->execbuf_client);
- seq_printf(m, "\nGuC preempt client @ %p:\n", guc->preempt_client);
- i915_guc_client_info(m, dev_priv, guc->preempt_client);
+ if (guc->preempt_client) {
+ seq_printf(m, "\nGuC preempt client @ %p:\n",
+ guc->preempt_client);
+ i915_guc_client_info(m, dev_priv, guc->preempt_client);
+ }
i915_guc_log_info(m, dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 1f3a8786bbdc..4ea65df05e02 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -832,10 +832,12 @@ static int guc_clients_doorbell_init(struct intel_guc *guc)
if (ret)
return ret;
- ret = create_doorbell(guc->preempt_client);
- if (ret) {
- destroy_doorbell(guc->execbuf_client);
- return ret;
+ if (guc->preempt_client) {
+ ret = create_doorbell(guc->preempt_client);
+ if (ret) {
+ destroy_doorbell(guc->execbuf_client);
+ return ret;
+ }
}
return 0;
@@ -848,8 +850,11 @@ static void guc_clients_doorbell_fini(struct intel_guc *guc)
* Instead of trying (in vain) to communicate with it, let's just
* cleanup the doorbell HW and our internal state.
*/
- __destroy_doorbell(guc->preempt_client);
- __update_doorbell_desc(guc->preempt_client, GUC_DOORBELL_INVALID);
+ if (guc->preempt_client) {
+ __destroy_doorbell(guc->preempt_client);
+ __update_doorbell_desc(guc->preempt_client,
+ GUC_DOORBELL_INVALID);
+ }
__destroy_doorbell(guc->execbuf_client);
__update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID);
}
@@ -998,10 +1003,11 @@ static void guc_clients_destroy(struct intel_guc *guc)
{
struct intel_guc_client *client;
- client = fetch_and_zero(&guc->execbuf_client);
- guc_client_free(client);
-
client = fetch_and_zero(&guc->preempt_client);
+ if (client)
+ guc_client_free(client);
+
+ client = fetch_and_zero(&guc->execbuf_client);
guc_client_free(client);
}
@@ -1160,7 +1166,8 @@ int intel_guc_submission_enable(struct intel_guc *guc)
GEM_BUG_ON(!guc->execbuf_client);
guc_reset_wq(guc->execbuf_client);
- guc_reset_wq(guc->preempt_client);
+ if (guc->preempt_client)
+ guc_reset_wq(guc->preempt_client);
err = intel_guc_sample_forcewake(guc);
if (err)
diff --git a/drivers/gpu/drm/i915/selftests/intel_guc.c b/drivers/gpu/drm/i915/selftests/intel_guc.c
index 3f9016466dea..fb74e2cf8a0a 100644
--- a/drivers/gpu/drm/i915/selftests/intel_guc.c
+++ b/drivers/gpu/drm/i915/selftests/intel_guc.c
@@ -87,7 +87,7 @@ static int validate_client(struct intel_guc_client *client,
static bool client_doorbell_in_sync(struct intel_guc_client *client)
{
- return doorbell_ok(client->guc, client->doorbell_id);
+ return !client || doorbell_ok(client->guc, client->doorbell_id);
}
/*
@@ -137,7 +137,6 @@ static int igt_guc_clients(void *args)
goto unlock;
}
GEM_BUG_ON(!guc->execbuf_client);
- GEM_BUG_ON(!guc->preempt_client);
err = validate_client(guc->execbuf_client,
GUC_CLIENT_PRIORITY_KMD_NORMAL, false);
@@ -146,16 +145,18 @@ static int igt_guc_clients(void *args)
goto out;
}
- err = validate_client(guc->preempt_client,
- GUC_CLIENT_PRIORITY_KMD_HIGH, true);
- if (err) {
- pr_err("preempt client validation failed\n");
- goto out;
+ if (guc->preempt_client) {
+ err = validate_client(guc->preempt_client,
+ GUC_CLIENT_PRIORITY_KMD_HIGH, true);
+ if (err) {
+ pr_err("preempt client validation failed\n");
+ goto out;
+ }
}
/* each client should now have reserved a doorbell */
if (!has_doorbell(guc->execbuf_client) ||
- !has_doorbell(guc->preempt_client)) {
+ (guc->preempt_client && !has_doorbell(guc->preempt_client))) {
pr_err("guc_clients_create didn't reserve doorbells\n");
err = -EINVAL;
goto out;
@@ -224,7 +225,8 @@ static int igt_guc_clients(void *args)
* clients during unload.
*/
destroy_doorbell(guc->execbuf_client);
- destroy_doorbell(guc->preempt_client);
+ if (guc->preempt_client)
+ destroy_doorbell(guc->preempt_client);
guc_clients_destroy(guc);
guc_clients_create(guc);
guc_clients_doorbell_init(guc);
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL
2018-01-31 11:34 ` Chris Wilson
@ 2018-01-31 12:42 ` Mika Kuoppala
0 siblings, 0 replies; 23+ messages in thread
From: Mika Kuoppala @ 2018-01-31 12:42 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Quoting Mika Kuoppala (2018-01-31 11:31:09)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>>
>> > In the next patch, we may only conditionally allocate the preempt-client
>> > if there is a global preempt context and so we need to be prepared in
>> > case the preempt-client itself is NULL.
>> >
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Michal Winiarski <michal.winiarski@intel.com>
>> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> > Cc: Michel Thierry <michel.thierry@intel.com>
>> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/intel_guc_submission.c | 27 +++++++++++++++++----------
>> > 1 file changed, 17 insertions(+), 10 deletions(-)
>> >
>>
>> You need to prepare the debugfs to handle the null also.
>>
>> Does our simple igt/debugfs test include reading through all the nodes
>> it finds?
>
> It does, but we don't actually end up with NULLs.
Not with this patch alone but the commit message promises
that we prepare for it :)
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev2)
2018-01-31 9:47 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
` (3 preceding siblings ...)
2018-01-31 11:31 ` [PATCH v2 1/3] " Mika Kuoppala
@ 2018-01-31 13:00 ` Patchwork
2018-01-31 16:44 ` ✓ Fi.CI.IGT: " Patchwork
` (3 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-01-31 13:00 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev2)
URL : https://patchwork.freedesktop.org/series/37395/
State : success
== Summary ==
Series 37395v2 series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL
https://patchwork.freedesktop.org/api/1.0/series/37395/revisions/2/mbox/
Test debugfs_test:
Subgroup read_all_entries:
dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:415s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:425s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:370s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:485s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:281s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:482s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:485s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:465s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:452s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:565s
fi-elk-e7500 total:224 pass:168 dwarn:9 dfail:1 fail:0 skip:45
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:280s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:511s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:389s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:398s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:413s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:462s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:412s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:457s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:493s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:448s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:502s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:572s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:429s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:506s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:526s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:486s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:489s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:414s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:429s
fi-snb-2520m total:245 pass:211 dwarn:0 dfail:0 fail:0 skip:33
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:398s
Blacklisted hosts:
fi-glk-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:469s
badc0398a06018a07f6f50a1ff5a5fd8a30b9944 drm-tip: 2018y-01m-31d-12h-17m-12s UTC integration manifest
b566d4c192e0 drm/i915: Only allocate preempt context when required
9d317ed56342 drm/i915: Move the scheduler feature bits into the purview of the engines
c74489d84b5e drm/i915/guc: Allow preempt-client to be NULL
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7838/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] drm/i915/guc: Allow preempt-client to be NULL
2018-01-31 12:03 ` [PATCH v2] " Chris Wilson
@ 2018-01-31 13:14 ` Mika Kuoppala
0 siblings, 0 replies; 23+ messages in thread
From: Mika Kuoppala @ 2018-01-31 13:14 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> In the next patch, we may only conditionally allocate the preempt-client
> if there is a global preempt context and so we need to be prepared in
> case the preempt-client itself is NULL.
>
> v2: Grep for more preempt_client.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 8 +++++---
> drivers/gpu/drm/i915/intel_guc_submission.c | 27 +++++++++++++++++----------
> drivers/gpu/drm/i915/selftests/intel_guc.c | 20 +++++++++++---------
> 3 files changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3849ded354e3..9e44adef30f0 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2338,7 +2338,6 @@ static int i915_guc_info(struct seq_file *m, void *data)
> return -ENODEV;
>
> GEM_BUG_ON(!guc->execbuf_client);
> - GEM_BUG_ON(!guc->preempt_client);
>
> seq_printf(m, "Doorbell map:\n");
> seq_printf(m, "\t%*pb\n", GUC_NUM_DOORBELLS, guc->doorbell_bitmap);
> @@ -2346,8 +2345,11 @@ static int i915_guc_info(struct seq_file *m, void *data)
>
> seq_printf(m, "\nGuC execbuf client @ %p:\n", guc->execbuf_client);
> i915_guc_client_info(m, dev_priv, guc->execbuf_client);
> - seq_printf(m, "\nGuC preempt client @ %p:\n", guc->preempt_client);
> - i915_guc_client_info(m, dev_priv, guc->preempt_client);
> + if (guc->preempt_client) {
> + seq_printf(m, "\nGuC preempt client @ %p:\n",
> + guc->preempt_client);
> + i915_guc_client_info(m, dev_priv, guc->preempt_client);
> + }
>
> i915_guc_log_info(m, dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 1f3a8786bbdc..4ea65df05e02 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -832,10 +832,12 @@ static int guc_clients_doorbell_init(struct intel_guc *guc)
> if (ret)
> return ret;
>
> - ret = create_doorbell(guc->preempt_client);
> - if (ret) {
> - destroy_doorbell(guc->execbuf_client);
> - return ret;
> + if (guc->preempt_client) {
> + ret = create_doorbell(guc->preempt_client);
> + if (ret) {
> + destroy_doorbell(guc->execbuf_client);
> + return ret;
> + }
> }
>
> return 0;
> @@ -848,8 +850,11 @@ static void guc_clients_doorbell_fini(struct intel_guc *guc)
> * Instead of trying (in vain) to communicate with it, let's just
> * cleanup the doorbell HW and our internal state.
> */
> - __destroy_doorbell(guc->preempt_client);
> - __update_doorbell_desc(guc->preempt_client, GUC_DOORBELL_INVALID);
> + if (guc->preempt_client) {
> + __destroy_doorbell(guc->preempt_client);
> + __update_doorbell_desc(guc->preempt_client,
> + GUC_DOORBELL_INVALID);
> + }
> __destroy_doorbell(guc->execbuf_client);
> __update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID);
> }
> @@ -998,10 +1003,11 @@ static void guc_clients_destroy(struct intel_guc *guc)
> {
> struct intel_guc_client *client;
>
> - client = fetch_and_zero(&guc->execbuf_client);
> - guc_client_free(client);
> -
> client = fetch_and_zero(&guc->preempt_client);
> + if (client)
> + guc_client_free(client);
> +
> + client = fetch_and_zero(&guc->execbuf_client);
> guc_client_free(client);
> }
>
> @@ -1160,7 +1166,8 @@ int intel_guc_submission_enable(struct intel_guc *guc)
> GEM_BUG_ON(!guc->execbuf_client);
>
> guc_reset_wq(guc->execbuf_client);
> - guc_reset_wq(guc->preempt_client);
> + if (guc->preempt_client)
> + guc_reset_wq(guc->preempt_client);
>
> err = intel_guc_sample_forcewake(guc);
> if (err)
> diff --git a/drivers/gpu/drm/i915/selftests/intel_guc.c b/drivers/gpu/drm/i915/selftests/intel_guc.c
> index 3f9016466dea..fb74e2cf8a0a 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_guc.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_guc.c
> @@ -87,7 +87,7 @@ static int validate_client(struct intel_guc_client *client,
>
> static bool client_doorbell_in_sync(struct intel_guc_client *client)
> {
> - return doorbell_ok(client->guc, client->doorbell_id);
> + return !client || doorbell_ok(client->guc, client->doorbell_id);
> }
>
> /*
> @@ -137,7 +137,6 @@ static int igt_guc_clients(void *args)
> goto unlock;
> }
> GEM_BUG_ON(!guc->execbuf_client);
> - GEM_BUG_ON(!guc->preempt_client);
>
> err = validate_client(guc->execbuf_client,
> GUC_CLIENT_PRIORITY_KMD_NORMAL, false);
> @@ -146,16 +145,18 @@ static int igt_guc_clients(void *args)
> goto out;
> }
>
> - err = validate_client(guc->preempt_client,
> - GUC_CLIENT_PRIORITY_KMD_HIGH, true);
> - if (err) {
> - pr_err("preempt client validation failed\n");
> - goto out;
> + if (guc->preempt_client) {
> + err = validate_client(guc->preempt_client,
> + GUC_CLIENT_PRIORITY_KMD_HIGH, true);
> + if (err) {
> + pr_err("preempt client validation failed\n");
> + goto out;
> + }
> }
>
> /* each client should now have reserved a doorbell */
> if (!has_doorbell(guc->execbuf_client) ||
> - !has_doorbell(guc->preempt_client)) {
> + (guc->preempt_client && !has_doorbell(guc->preempt_client))) {
> pr_err("guc_clients_create didn't reserve doorbells\n");
> err = -EINVAL;
> goto out;
> @@ -224,7 +225,8 @@ static int igt_guc_clients(void *args)
> * clients during unload.
> */
> destroy_doorbell(guc->execbuf_client);
> - destroy_doorbell(guc->preempt_client);
> + if (guc->preempt_client)
> + destroy_doorbell(guc->preempt_client);
> guc_clients_destroy(guc);
> guc_clients_create(guc);
> guc_clients_doorbell_init(guc);
> --
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/3] drm/i915: Move the scheduler feature bits into the purview of the engines
2018-01-31 9:47 ` [PATCH v2 2/3] drm/i915: Move the scheduler feature bits into the purview of the engines Chris Wilson
@ 2018-01-31 13:58 ` Mika Kuoppala
2018-02-01 18:54 ` Chris Wilson
2018-02-01 19:02 ` [PATCH v2] " Chris Wilson
0 siblings, 2 replies; 23+ messages in thread
From: Mika Kuoppala @ 2018-01-31 13:58 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Rather than having the high level ioctl interface guess the underlying
> implementation details, having the implementation declare what
> capabilities it exports. We define an intel_driver_caps, similar to the
> intel_device_info, which instead of trying to describe the HW gives
> details on what the driver itself supports. This is then populated by
> the engine backend for the new scheduler capability field for use
> elsewhere.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tomasz Lis <tomasz.lis@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 8 +-------
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_gem.c | 3 +++
> drivers/gpu/drm/i915/i915_gpu_error.c | 7 +++++--
> drivers/gpu/drm/i915/intel_device_info.c | 6 ++++++
> drivers/gpu/drm/i915/intel_device_info.h | 7 +++++++
> drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++
> 7 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ec12add34b2..733f71637914 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -372,13 +372,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
> value = i915_gem_mmap_gtt_version();
> break;
> case I915_PARAM_HAS_SCHEDULER:
> - value = 0;
> - if (dev_priv->engine[RCS] && dev_priv->engine[RCS]->schedule) {
> - value |= I915_SCHEDULER_CAP_ENABLED;
> - value |= I915_SCHEDULER_CAP_PRIORITY;
> - if (HAS_LOGICAL_RING_PREEMPTION(dev_priv))
> - value |= I915_SCHEDULER_CAP_PREEMPTION;
> - }
> + value = dev_priv->caps.scheduler;
Use the shiny CAP_PRIORITY instead of rcs->schedule on the
I915_CONTEXT_PARAM_PRIORITY validation?
-Mika
> break;
>
> case I915_PARAM_MMAP_VERSION:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 388b72cccde4..ae434c9d2b4f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -468,6 +468,7 @@ struct i915_gpu_state {
> u32 reset_count;
> u32 suspend_count;
> struct intel_device_info device_info;
> + struct intel_driver_caps driver_caps;
> struct i915_params params;
>
> struct i915_error_uc {
> @@ -1808,6 +1809,7 @@ struct drm_i915_private {
> struct kmem_cache *priorities;
>
> const struct intel_device_info info;
> + struct intel_driver_caps caps;
>
> /**
> * Data Stolen Memory - aka "i915 stolen memory" gives us the start and
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 062b21408698..ad7bd0dc57ee 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3223,8 +3223,11 @@ void i915_gem_set_wedged(struct drm_i915_private *i915)
> * start to complete all requests.
> */
> engine->submit_request = nop_complete_submit_request;
> + engine->schedule = NULL;
> }
>
> + i915->caps.scheduler = 0;
> +
> /*
> * Make sure no request can slip through without getting completed by
> * either this call here to intel_engine_init_global_seqno, or the one
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index a81351d9e3a6..a92b0c0377c7 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -573,11 +573,13 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
> }
>
> static void err_print_capabilities(struct drm_i915_error_state_buf *m,
> - const struct intel_device_info *info)
> + const struct intel_device_info *info,
> + const struct intel_driver_caps *caps)
> {
> struct drm_printer p = i915_error_printer(m);
>
> intel_device_info_dump_flags(info, &p);
> + intel_driver_caps_print(caps, &p);
> }
>
> static void err_print_params(struct drm_i915_error_state_buf *m,
> @@ -800,7 +802,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> if (error->display)
> intel_display_print_error_state(m, error->display);
>
> - err_print_capabilities(m, &error->device_info);
> + err_print_capabilities(m, &error->device_info, &error->driver_caps);
> err_print_params(m, &error->params);
> err_print_uc(m, &error->uc);
>
> @@ -1731,6 +1733,7 @@ static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
> memcpy(&error->device_info,
> INTEL_INFO(dev_priv),
> sizeof(error->device_info));
> + error->driver_caps = dev_priv->caps;
> }
>
> static __always_inline void dup_param(const char *type, void *x)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index a2c16140169f..298f8996cc54 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -586,3 +586,9 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
> /* Initialize command stream timestamp frequency */
> info->cs_timestamp_frequency_khz = read_timestamp_frequency(dev_priv);
> }
> +
> +void intel_driver_caps_print(const struct intel_driver_caps *caps,
> + struct drm_printer *p)
> +{
> + drm_printf(p, "scheduler: %x\n", caps->scheduler);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 9542018d11d0..71fdfb0451ef 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -167,6 +167,10 @@ struct intel_device_info {
> } color;
> };
>
> +struct intel_driver_caps {
> + unsigned int scheduler;
> +};
> +
> static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
> {
> return hweight8(sseu->slice_mask) * hweight8(sseu->subslice_mask);
> @@ -182,4 +186,7 @@ void intel_device_info_dump_flags(const struct intel_device_info *info,
> void intel_device_info_dump_runtime(const struct intel_device_info *info,
> struct drm_printer *p);
>
> +void intel_driver_caps_print(const struct intel_driver_caps *caps,
> + struct drm_printer *p);
> +
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 2fa328d512fc..5390894001f0 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1906,6 +1906,12 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
> engine->unpark = NULL;
>
> engine->flags |= I915_ENGINE_SUPPORTS_STATS;
> +
> + engine->i915->caps.scheduler =
> + I915_SCHEDULER_CAP_ENABLED |
> + I915_SCHEDULER_CAP_PRIORITY;
> + if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
> + engine->i915->caps.scheduler |= I915_SCHEDULER_CAP_PREEMPTION;
> }
>
> static void
> --
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/i915: Only allocate preempt context when required
2018-01-31 9:47 ` [PATCH v2 3/3] drm/i915: Only allocate preempt context when required Chris Wilson
@ 2018-01-31 14:38 ` Mika Kuoppala
2018-01-31 15:25 ` Chris Wilson
2018-02-03 10:40 ` [PATCH v2] " Chris Wilson
1 sibling, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2018-01-31 14:38 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> If we remove some hardcoded assumptions about the preempt context having
> a fixed id, reserved from use by normal user contexts, we may only
> allocate the i915_gem_context when required. Then the subsequent
> decisions on using preemption reduce to having the preempt context
> available.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 29 ++++++++++++------------
> drivers/gpu/drm/i915/intel_engine_cs.c | 6 ++---
> drivers/gpu/drm/i915/intel_guc_submission.c | 24 +++++++++++---------
> drivers/gpu/drm/i915/intel_lrc.c | 17 +++++++++-----
> drivers/gpu/drm/i915/intel_ringbuffer.h | 5 ++++
> drivers/gpu/drm/i915/selftests/mock_gem_device.c | 6 -----
> 6 files changed, 46 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 648e7536ff51..d69c8f1a4e78 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -449,10 +449,14 @@ destroy_kernel_context(struct i915_gem_context **ctxp)
> i915_gem_context_free(ctx);
> }
>
> +static bool needs_preempt_context(struct drm_i915_private *i915)
> +{
> + return HAS_LOGICAL_RING_PREEMPTION(i915);
> +}
> +
> int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
> {
> struct i915_gem_context *ctx;
> - int err;
>
> GEM_BUG_ON(dev_priv->kernel_context);
Please consider adding
GEM_BUG_ON(dev_priv->preempt_context);
here even tho the kernel_context should act as a master guard for init ordering
errors. Even if no other than documenting that we expect a clean slate
in this regard also.
>
> @@ -468,8 +472,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
> ctx = i915_gem_context_create_kernel(dev_priv, I915_PRIORITY_MIN);
> if (IS_ERR(ctx)) {
> DRM_ERROR("Failed to create default global context\n");
> - err = PTR_ERR(ctx);
> - goto err;
> + return PTR_ERR(ctx);
> }
> /*
> * For easy recognisablity, we want the kernel context to be 0 and then
> @@ -479,23 +482,18 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
> dev_priv->kernel_context = ctx;
>
> /* highest priority; preempting task */
> - ctx = i915_gem_context_create_kernel(dev_priv, INT_MAX);
> - if (IS_ERR(ctx)) {
> - DRM_ERROR("Failed to create default preempt context\n");
> - err = PTR_ERR(ctx);
> - goto err_kernel_context;
It seems this error path has been wrong all along.
> + if (needs_preempt_context(dev_priv)) {
> + ctx = i915_gem_context_create_kernel(dev_priv, INT_MAX);
> + if (!IS_ERR(ctx))
> + dev_priv->preempt_context = ctx;
> + else
> + DRM_ERROR("Failed to create preempt context; disabling preemption\n");
> }
> - dev_priv->preempt_context = ctx;
>
> DRM_DEBUG_DRIVER("%s context support initialized\n",
> dev_priv->engine[RCS]->context_size ? "logical" :
> "fake");
> return 0;
> -
> -err_kernel_context:
> - destroy_kernel_context(&dev_priv->kernel_context);
> -err:
> - return err;
> }
>
> void i915_gem_contexts_lost(struct drm_i915_private *dev_priv)
> @@ -521,7 +519,8 @@ void i915_gem_contexts_fini(struct drm_i915_private *i915)
> {
> lockdep_assert_held(&i915->drm.struct_mutex);
>
> - destroy_kernel_context(&i915->preempt_context);
> + if (i915->preempt_context)
> + destroy_kernel_context(&i915->preempt_context);
> destroy_kernel_context(&i915->kernel_context);
>
> /* Must free all deferred contexts (via flush_workqueue) first */
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 7eebfbb95e89..bf634432c9c6 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -631,7 +631,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
> * Similarly the preempt context must always be available so that
> * we can interrupt the engine at any time.
> */
> - if (HAS_LOGICAL_RING_PREEMPTION(engine->i915)) {
> + if (engine->i915->preempt_context) {
> ring = engine->context_pin(engine,
> engine->i915->preempt_context);
> if (IS_ERR(ring)) {
> @@ -656,7 +656,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
> err_breadcrumbs:
> intel_engine_fini_breadcrumbs(engine);
> err_unpin_preempt:
> - if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
> + if (engine->i915->preempt_context)
> engine->context_unpin(engine, engine->i915->preempt_context);
> err_unpin_kernel:
> engine->context_unpin(engine, engine->i915->kernel_context);
> @@ -686,7 +686,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
> if (engine->default_state)
> i915_gem_object_put(engine->default_state);
>
> - if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
> + if (engine->i915->preempt_context)
> engine->context_unpin(engine, engine->i915->preempt_context);
> engine->context_unpin(engine, engine->i915->kernel_context);
> }
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 4ea65df05e02..b43b58cc599b 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -688,7 +688,7 @@ static void guc_dequeue(struct intel_engine_cs *engine)
> goto unlock;
>
> if (port_isset(port)) {
> - if (HAS_LOGICAL_RING_PREEMPTION(engine->i915)) {
> + if (engine->i915->preempt_context) {
> struct guc_preempt_work *preempt_work =
> &engine->i915->guc.preempt_work[engine->id];
>
> @@ -984,17 +984,19 @@ static int guc_clients_create(struct intel_guc *guc)
> }
> guc->execbuf_client = client;
>
> - client = guc_client_alloc(dev_priv,
> - INTEL_INFO(dev_priv)->ring_mask,
> - GUC_CLIENT_PRIORITY_KMD_HIGH,
> - dev_priv->preempt_context);
> - if (IS_ERR(client)) {
> - DRM_ERROR("Failed to create GuC client for preemption!\n");
> - guc_client_free(guc->execbuf_client);
> - guc->execbuf_client = NULL;
> - return PTR_ERR(client);
> + if (dev_priv->preempt_context) {
> + client = guc_client_alloc(dev_priv,
> + INTEL_INFO(dev_priv)->ring_mask,
> + GUC_CLIENT_PRIORITY_KMD_HIGH,
> + dev_priv->preempt_context);
> + if (IS_ERR(client)) {
> + DRM_ERROR("Failed to create GuC client for preemption!\n");
> + guc_client_free(guc->execbuf_client);
> + guc->execbuf_client = NULL;
> + return PTR_ERR(client);
> + }
> + guc->preempt_client = client;
> }
> - guc->preempt_client = client;
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 5390894001f0..221b62d72c72 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -161,7 +161,6 @@
> #define EXECLISTS_REQUEST_SIZE 64 /* bytes */
> #define WA_TAIL_DWORDS 2
> #define WA_TAIL_BYTES (sizeof(u32) * WA_TAIL_DWORDS)
> -#define PREEMPT_ID 0x1
>
> static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
> struct intel_engine_cs *engine);
> @@ -448,7 +447,8 @@ static void inject_preempt_context(struct intel_engine_cs *engine)
> &engine->i915->preempt_context->engine[engine->id];
> unsigned int n;
>
> - GEM_BUG_ON(engine->i915->preempt_context->hw_id != PREEMPT_ID);
> + GEM_BUG_ON(engine->execlists.preempt_complete_status !=
> + upper_32_bits(ce->lrc_desc));
> GEM_BUG_ON(!IS_ALIGNED(ce->ring->size, WA_TAIL_BYTES));
>
> memset(ce->ring->vaddr + ce->ring->tail, 0, WA_TAIL_BYTES);
> @@ -528,7 +528,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_HWACK))
> goto unlock;
>
> - if (HAS_LOGICAL_RING_PREEMPTION(engine->i915) &&
> + if (engine->i915->preempt_context &&
> rb_entry(rb, struct i915_priolist, node)->priority >
> max(last->priotree.priority, 0)) {
> /*
> @@ -844,7 +844,7 @@ static void execlists_submission_tasklet(unsigned long data)
> GEM_BUG_ON(status & GEN8_CTX_STATUS_IDLE_ACTIVE);
>
> if (status & GEN8_CTX_STATUS_COMPLETE &&
> - buf[2*head + 1] == PREEMPT_ID) {
> + buf[2*head + 1] == execlists->preempt_complete_status) {
> GEM_TRACE("%s preempt-idle\n", engine->name);
>
> execlists_cancel_port_requests(execlists);
> @@ -1910,7 +1910,7 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
> engine->i915->caps.scheduler =
> I915_SCHEDULER_CAP_ENABLED |
> I915_SCHEDULER_CAP_PRIORITY;
> - if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
> + if (engine->i915->preempt_context)
> engine->i915->caps.scheduler |= I915_SCHEDULER_CAP_PREEMPTION;
> }
>
> @@ -1988,6 +1988,11 @@ static int logical_ring_init(struct intel_engine_cs *engine)
> engine->execlists.elsp =
> engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine));
>
> + engine->execlists.preempt_complete_status = ~0u;
This is to ensure that we catch a rogue status? Atleast we will
bug on of preempting with this status as the EXECLIST_ACTIVE_PREEMPT
will be false.
> + if (engine->i915->preempt_context)
> + engine->execlists.preempt_complete_status =
> + upper_32_bits(engine->i915->preempt_context->engine[engine->id].lrc_desc);
We have not upgraded the descriptor yet, so just use preempt_context->hw_id;
I would also like duplicate, from i915_gem_context.c, the assertion that
hw_id needs to be <= INT_MAX but didn't find a good spot.
-Mika
> +
> return 0;
>
> error:
> @@ -2250,7 +2255,7 @@ populate_lr_context(struct i915_gem_context *ctx,
> if (!engine->default_state)
> regs[CTX_CONTEXT_CONTROL + 1] |=
> _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
> - if (ctx->hw_id == PREEMPT_ID)
> + if (ctx == ctx->i915->preempt_context)
> regs[CTX_CONTEXT_CONTROL + 1] |=
> _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
> CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index c5ff203e42d6..03be8995f415 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -279,6 +279,11 @@ struct intel_engine_execlists {
> * @csb_use_mmio: access csb through mmio, instead of hwsp
> */
> bool csb_use_mmio;
> +
> + /**
> + * @preempt_complete_status: expected CSB upon completing preemption
> + */
> + u32 preempt_complete_status;
> };
>
> #define INTEL_ENGINE_CS_MAX_NAME 8
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> index 1bc61f3f76fc..3175db70cc6e 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> @@ -243,16 +243,10 @@ struct drm_i915_private *mock_gem_device(void)
> if (!i915->kernel_context)
> goto err_engine;
>
> - i915->preempt_context = mock_context(i915, NULL);
> - if (!i915->preempt_context)
> - goto err_kernel_context;
> -
> WARN_ON(i915_gemfs_init(i915));
>
> return i915;
>
> -err_kernel_context:
> - i915_gem_context_put(i915->kernel_context);
> err_engine:
> for_each_engine(engine, i915, id)
> mock_engine_free(engine);
> --
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/i915: Only allocate preempt context when required
2018-01-31 14:38 ` Mika Kuoppala
@ 2018-01-31 15:25 ` Chris Wilson
0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2018-01-31 15:25 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2018-01-31 14:38:02)
> > @@ -1988,6 +1988,11 @@ static int logical_ring_init(struct intel_engine_cs *engine)
> > engine->execlists.elsp =
> > engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine));
> >
> > + engine->execlists.preempt_complete_status = ~0u;
>
> This is to ensure that we catch a rogue status? Atleast we will
> bug on of preempting with this status as the EXECLIST_ACTIVE_PREEMPT
> will be false.
0 is the value for the i915->kernel_context. ~0u is currently invalid
and that looks to be the case (that at least it will never match an
active context) for the near future.
> > + if (engine->i915->preempt_context)
> > + engine->execlists.preempt_complete_status =
> > + upper_32_bits(engine->i915->preempt_context->engine[engine->id].lrc_desc);
>
>
> We have not upgraded the descriptor yet, so just use preempt_context->hw_id;
The upper_32_bits should be set; and I think it is important that the
usage is consistent. If we change the upper_32_bits later, we should try
to remember to update the preempt_complete_status. I.e. I feel it is
vital to remember that the complete_status is the
upper_32_bits(lrc_desc) that just happens to be hw_id due to our limited
definition today.
> I would also like duplicate, from i915_gem_context.c, the assertion that
> hw_id needs to be <= INT_MAX but didn't find a good spot.
For what purpose? When defining lrc_desc checking that hw_id fits within
the field width would be sensible.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev2)
2018-01-31 9:47 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
` (4 preceding siblings ...)
2018-01-31 13:00 ` ✓ Fi.CI.BAT: success for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev2) Patchwork
@ 2018-01-31 16:44 ` Patchwork
2018-02-01 19:22 ` ✗ Fi.CI.BAT: failure for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev3) Patchwork
` (2 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-01-31 16:44 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev2)
URL : https://patchwork.freedesktop.org/series/37395/
State : success
== Summary ==
Test perf_pmu:
Subgroup semaphore-wait-bcs0:
fail -> PASS (shard-apl) fdo#104829
Test gem_eio:
Subgroup in-flight-suspend:
pass -> FAIL (shard-hsw) fdo#104676
fdo#104829 https://bugs.freedesktop.org/show_bug.cgi?id=104829
fdo#104676 https://bugs.freedesktop.org/show_bug.cgi?id=104676
shard-apl total:1638 pass:1017 dwarn:1 dfail:0 fail:16 skip:604 time:7460s
shard-hsw total:2838 pass:1735 dwarn:1 dfail:0 fail:11 skip:1090 time:11932s
shard-snb total:2838 pass:1330 dwarn:1 dfail:0 fail:10 skip:1497 time:6594s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7838/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/3] drm/i915: Move the scheduler feature bits into the purview of the engines
2018-01-31 13:58 ` Mika Kuoppala
@ 2018-02-01 18:54 ` Chris Wilson
2018-02-01 19:02 ` [PATCH v2] " Chris Wilson
1 sibling, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2018-02-01 18:54 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2018-01-31 13:58:17)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > Rather than having the high level ioctl interface guess the underlying
> > implementation details, having the implementation declare what
> > capabilities it exports. We define an intel_driver_caps, similar to the
> > intel_device_info, which instead of trying to describe the HW gives
> > details on what the driver itself supports. This is then populated by
> > the engine backend for the new scheduler capability field for use
> > elsewhere.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tomasz Lis <tomasz.lis@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 8 +-------
> > drivers/gpu/drm/i915/i915_drv.h | 2 ++
> > drivers/gpu/drm/i915/i915_gem.c | 3 +++
> > drivers/gpu/drm/i915/i915_gpu_error.c | 7 +++++--
> > drivers/gpu/drm/i915/intel_device_info.c | 6 ++++++
> > drivers/gpu/drm/i915/intel_device_info.h | 7 +++++++
> > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++
> > 7 files changed, 30 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 1ec12add34b2..733f71637914 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -372,13 +372,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
> > value = i915_gem_mmap_gtt_version();
> > break;
> > case I915_PARAM_HAS_SCHEDULER:
> > - value = 0;
> > - if (dev_priv->engine[RCS] && dev_priv->engine[RCS]->schedule) {
> > - value |= I915_SCHEDULER_CAP_ENABLED;
> > - value |= I915_SCHEDULER_CAP_PRIORITY;
> > - if (HAS_LOGICAL_RING_PREEMPTION(dev_priv))
> > - value |= I915_SCHEDULER_CAP_PREEMPTION;
> > - }
> > + value = dev_priv->caps.scheduler;
>
> Use the shiny CAP_PRIORITY instead of rcs->schedule on the
> I915_CONTEXT_PARAM_PRIORITY validation?
Yeah, that makes sense.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2] drm/i915: Move the scheduler feature bits into the purview of the engines
2018-01-31 13:58 ` Mika Kuoppala
2018-02-01 18:54 ` Chris Wilson
@ 2018-02-01 19:02 ` Chris Wilson
2018-02-02 13:55 ` Mika Kuoppala
2018-02-02 14:06 ` Lis, Tomasz
1 sibling, 2 replies; 23+ messages in thread
From: Chris Wilson @ 2018-02-01 19:02 UTC (permalink / raw)
To: intel-gfx
Rather than having the high level ioctl interface guess the underlying
implementation details, having the implementation declare what
capabilities it exports. We define an intel_driver_caps, similar to the
intel_device_info, which instead of trying to describe the HW gives
details on what the driver itself supports. This is then populated by
the engine backend for the new scheduler capability field for use
elsewhere.
v2: Use caps.scheduler for validating CONTEXT_PARAM_SET_PRIORITY (Mika)
One less assumption of engine[RCS] \o/
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tomasz Lis <tomasz.lis@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 1 +
drivers/gpu/drm/i915/i915_drv.c | 8 +-------
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_gem.c | 3 +++
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 7 +++++--
drivers/gpu/drm/i915/intel_device_info.c | 6 ++++++
drivers/gpu/drm/i915/intel_device_info.h | 7 +++++++
drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++
9 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9e44adef30f0..2bdce9fea671 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -49,6 +49,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
intel_device_info_dump_flags(info, &p);
intel_device_info_dump_runtime(info, &p);
+ intel_driver_caps_print(&dev_priv->caps, &p);
kernel_param_lock(THIS_MODULE);
i915_params_dump(&i915_modparams, &p);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1ec12add34b2..733f71637914 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -372,13 +372,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
value = i915_gem_mmap_gtt_version();
break;
case I915_PARAM_HAS_SCHEDULER:
- value = 0;
- if (dev_priv->engine[RCS] && dev_priv->engine[RCS]->schedule) {
- value |= I915_SCHEDULER_CAP_ENABLED;
- value |= I915_SCHEDULER_CAP_PRIORITY;
- if (HAS_LOGICAL_RING_PREEMPTION(dev_priv))
- value |= I915_SCHEDULER_CAP_PREEMPTION;
- }
+ value = dev_priv->caps.scheduler;
break;
case I915_PARAM_MMAP_VERSION:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c676269ed843..24333042e1e9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -468,6 +468,7 @@ struct i915_gpu_state {
u32 reset_count;
u32 suspend_count;
struct intel_device_info device_info;
+ struct intel_driver_caps driver_caps;
struct i915_params params;
struct i915_error_uc {
@@ -1809,6 +1810,7 @@ struct drm_i915_private {
struct kmem_cache *priorities;
const struct intel_device_info info;
+ struct intel_driver_caps caps;
/**
* Data Stolen Memory - aka "i915 stolen memory" gives us the start and
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c049496e8757..8d732f97e23e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3222,8 +3222,11 @@ void i915_gem_set_wedged(struct drm_i915_private *i915)
* start to complete all requests.
*/
engine->submit_request = nop_complete_submit_request;
+ engine->schedule = NULL;
}
+ i915->caps.scheduler = 0;
+
/*
* Make sure no request can slip through without getting completed by
* either this call here to intel_engine_init_global_seqno, or the one
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 648e7536ff51..8337d15bb0e5 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -807,7 +807,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
if (args->size)
ret = -EINVAL;
- else if (!to_i915(dev)->engine[RCS]->schedule)
+ else if (!(to_i915(dev)->caps.scheduler & I915_SCHEDULER_CAP_PRIORITY))
ret = -ENODEV;
else if (priority > I915_CONTEXT_MAX_USER_PRIORITY ||
priority < I915_CONTEXT_MIN_USER_PRIORITY)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index a81351d9e3a6..a92b0c0377c7 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -573,11 +573,13 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
}
static void err_print_capabilities(struct drm_i915_error_state_buf *m,
- const struct intel_device_info *info)
+ const struct intel_device_info *info,
+ const struct intel_driver_caps *caps)
{
struct drm_printer p = i915_error_printer(m);
intel_device_info_dump_flags(info, &p);
+ intel_driver_caps_print(caps, &p);
}
static void err_print_params(struct drm_i915_error_state_buf *m,
@@ -800,7 +802,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
if (error->display)
intel_display_print_error_state(m, error->display);
- err_print_capabilities(m, &error->device_info);
+ err_print_capabilities(m, &error->device_info, &error->driver_caps);
err_print_params(m, &error->params);
err_print_uc(m, &error->uc);
@@ -1731,6 +1733,7 @@ static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
memcpy(&error->device_info,
INTEL_INFO(dev_priv),
sizeof(error->device_info));
+ error->driver_caps = dev_priv->caps;
}
static __always_inline void dup_param(const char *type, void *x)
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index a2c16140169f..298f8996cc54 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -586,3 +586,9 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
/* Initialize command stream timestamp frequency */
info->cs_timestamp_frequency_khz = read_timestamp_frequency(dev_priv);
}
+
+void intel_driver_caps_print(const struct intel_driver_caps *caps,
+ struct drm_printer *p)
+{
+ drm_printf(p, "scheduler: %x\n", caps->scheduler);
+}
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 9542018d11d0..71fdfb0451ef 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -167,6 +167,10 @@ struct intel_device_info {
} color;
};
+struct intel_driver_caps {
+ unsigned int scheduler;
+};
+
static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
{
return hweight8(sseu->slice_mask) * hweight8(sseu->subslice_mask);
@@ -182,4 +186,7 @@ void intel_device_info_dump_flags(const struct intel_device_info *info,
void intel_device_info_dump_runtime(const struct intel_device_info *info,
struct drm_printer *p);
+void intel_driver_caps_print(const struct intel_driver_caps *caps,
+ struct drm_printer *p);
+
#endif
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 41419f249fc2..a52bd2525398 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1922,6 +1922,12 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
engine->unpark = NULL;
engine->flags |= I915_ENGINE_SUPPORTS_STATS;
+
+ engine->i915->caps.scheduler =
+ I915_SCHEDULER_CAP_ENABLED |
+ I915_SCHEDULER_CAP_PRIORITY;
+ if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
+ engine->i915->caps.scheduler |= I915_SCHEDULER_CAP_PREEMPTION;
}
static void
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev3)
2018-01-31 9:47 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
` (5 preceding siblings ...)
2018-01-31 16:44 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-02-01 19:22 ` Patchwork
2018-02-03 11:17 ` ✓ Fi.CI.BAT: success for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev4) Patchwork
2018-02-04 6:51 ` ✗ Fi.CI.IGT: failure " Patchwork
8 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-02-01 19:22 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev3)
URL : https://patchwork.freedesktop.org/series/37395/
State : failure
== Summary ==
Series 37395v3 series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL
https://patchwork.freedesktop.org/api/1.0/series/37395/revisions/3/mbox/
Test gem_mmap_gtt:
Subgroup basic-small-bo-tiledx:
pass -> FAIL (fi-gdg-551) fdo#102575
Test gem_sync:
Subgroup basic-many-each:
pass -> DMESG-FAIL (fi-bsw-n3050)
Subgroup basic-store-all:
pass -> SKIP (fi-bsw-n3050)
Subgroup basic-store-each:
pass -> SKIP (fi-bsw-n3050)
Test gem_tiled_blits:
Subgroup basic:
pass -> SKIP (fi-bsw-n3050)
Test gem_tiled_fence_blits:
Subgroup basic:
pass -> SKIP (fi-bsw-n3050)
Test gem_wait:
Subgroup basic-busy-all:
pass -> SKIP (fi-bsw-n3050)
Subgroup basic-wait-all:
pass -> SKIP (fi-bsw-n3050)
Subgroup basic-await-all:
pass -> SKIP (fi-bsw-n3050)
Test gem_workarounds:
Subgroup basic-read:
pass -> SKIP (fi-bsw-n3050)
Test kms_busy:
Subgroup basic-flip-c:
pass -> SKIP (fi-bsw-n3050)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
pass -> SKIP (fi-bsw-n3050)
Subgroup basic-busy-flip-before-cursor-legacy:
pass -> SKIP (fi-bsw-n3050)
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> SKIP (fi-bsw-n3050) fdo#104571
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-c:
pass -> DMESG-FAIL (fi-bsw-n3050)
Subgroup suspend-read-crc-pipe-c:
pass -> DMESG-WARN (fi-bsw-n3050)
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104571 https://bugs.freedesktop.org/show_bug.cgi?id=104571
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:417s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:425s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:374s
fi-bsw-n3050 total:288 pass:227 dwarn:1 dfail:2 fail:0 skip:58 time:479s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:282s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:480s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:490s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:474s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:460s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:566s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:413s
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:277s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:510s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:390s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:404s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:412s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:452s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:413s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:457s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:498s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:453s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:498s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:576s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:431s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:510s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:531s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:492s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:496s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:415s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:432s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:526s
Blacklisted hosts:
fi-glk-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:467s
cc6c1df8bef5c07d1b05bebf56e66a9f10a4702b drm-tip: 2018y-02m-01d-16h-46m-26s UTC integration manifest
8a388728ae1f drm/i915: Only allocate preempt context when required
e2891884a12d drm/i915: Move the scheduler feature bits into the purview of the engines
5f390ed3a415 drm/i915/guc: Allow preempt-client to be NULL
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7855/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] drm/i915: Move the scheduler feature bits into the purview of the engines
2018-02-01 19:02 ` [PATCH v2] " Chris Wilson
@ 2018-02-02 13:55 ` Mika Kuoppala
2018-02-02 14:06 ` Lis, Tomasz
1 sibling, 0 replies; 23+ messages in thread
From: Mika Kuoppala @ 2018-02-02 13:55 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Rather than having the high level ioctl interface guess the underlying
> implementation details, having the implementation declare what
> capabilities it exports. We define an intel_driver_caps, similar to the
> intel_device_info, which instead of trying to describe the HW gives
> details on what the driver itself supports. This is then populated by
> the engine backend for the new scheduler capability field for use
> elsewhere.
>
> v2: Use caps.scheduler for validating CONTEXT_PARAM_SET_PRIORITY (Mika)
> One less assumption of engine[RCS] \o/
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tomasz Lis <tomasz.lis@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 1 +
> drivers/gpu/drm/i915/i915_drv.c | 8 +-------
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_gem.c | 3 +++
> drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
> drivers/gpu/drm/i915/i915_gpu_error.c | 7 +++++--
> drivers/gpu/drm/i915/intel_device_info.c | 6 ++++++
> drivers/gpu/drm/i915/intel_device_info.h | 7 +++++++
> drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++
> 9 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9e44adef30f0..2bdce9fea671 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -49,6 +49,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
>
> intel_device_info_dump_flags(info, &p);
> intel_device_info_dump_runtime(info, &p);
> + intel_driver_caps_print(&dev_priv->caps, &p);
>
> kernel_param_lock(THIS_MODULE);
> i915_params_dump(&i915_modparams, &p);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ec12add34b2..733f71637914 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -372,13 +372,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
> value = i915_gem_mmap_gtt_version();
> break;
> case I915_PARAM_HAS_SCHEDULER:
> - value = 0;
> - if (dev_priv->engine[RCS] && dev_priv->engine[RCS]->schedule) {
> - value |= I915_SCHEDULER_CAP_ENABLED;
> - value |= I915_SCHEDULER_CAP_PRIORITY;
> - if (HAS_LOGICAL_RING_PREEMPTION(dev_priv))
> - value |= I915_SCHEDULER_CAP_PREEMPTION;
> - }
> + value = dev_priv->caps.scheduler;
> break;
>
> case I915_PARAM_MMAP_VERSION:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c676269ed843..24333042e1e9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -468,6 +468,7 @@ struct i915_gpu_state {
> u32 reset_count;
> u32 suspend_count;
> struct intel_device_info device_info;
> + struct intel_driver_caps driver_caps;
> struct i915_params params;
>
> struct i915_error_uc {
> @@ -1809,6 +1810,7 @@ struct drm_i915_private {
> struct kmem_cache *priorities;
>
> const struct intel_device_info info;
> + struct intel_driver_caps caps;
>
> /**
> * Data Stolen Memory - aka "i915 stolen memory" gives us the start and
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c049496e8757..8d732f97e23e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3222,8 +3222,11 @@ void i915_gem_set_wedged(struct drm_i915_private *i915)
> * start to complete all requests.
> */
> engine->submit_request = nop_complete_submit_request;
> + engine->schedule = NULL;
> }
>
> + i915->caps.scheduler = 0;
> +
> /*
> * Make sure no request can slip through without getting completed by
> * either this call here to intel_engine_init_global_seqno, or the one
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 648e7536ff51..8337d15bb0e5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -807,7 +807,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
>
> if (args->size)
> ret = -EINVAL;
> - else if (!to_i915(dev)->engine[RCS]->schedule)
> + else if (!(to_i915(dev)->caps.scheduler & I915_SCHEDULER_CAP_PRIORITY))
> ret = -ENODEV;
> else if (priority > I915_CONTEXT_MAX_USER_PRIORITY ||
> priority < I915_CONTEXT_MIN_USER_PRIORITY)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index a81351d9e3a6..a92b0c0377c7 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -573,11 +573,13 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
> }
>
> static void err_print_capabilities(struct drm_i915_error_state_buf *m,
> - const struct intel_device_info *info)
> + const struct intel_device_info *info,
> + const struct intel_driver_caps *caps)
> {
> struct drm_printer p = i915_error_printer(m);
>
> intel_device_info_dump_flags(info, &p);
> + intel_driver_caps_print(caps, &p);
> }
>
> static void err_print_params(struct drm_i915_error_state_buf *m,
> @@ -800,7 +802,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> if (error->display)
> intel_display_print_error_state(m, error->display);
>
> - err_print_capabilities(m, &error->device_info);
> + err_print_capabilities(m, &error->device_info, &error->driver_caps);
> err_print_params(m, &error->params);
> err_print_uc(m, &error->uc);
>
> @@ -1731,6 +1733,7 @@ static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
> memcpy(&error->device_info,
> INTEL_INFO(dev_priv),
> sizeof(error->device_info));
> + error->driver_caps = dev_priv->caps;
> }
>
> static __always_inline void dup_param(const char *type, void *x)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index a2c16140169f..298f8996cc54 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -586,3 +586,9 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
> /* Initialize command stream timestamp frequency */
> info->cs_timestamp_frequency_khz = read_timestamp_frequency(dev_priv);
> }
> +
> +void intel_driver_caps_print(const struct intel_driver_caps *caps,
> + struct drm_printer *p)
> +{
> + drm_printf(p, "scheduler: %x\n", caps->scheduler);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 9542018d11d0..71fdfb0451ef 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -167,6 +167,10 @@ struct intel_device_info {
> } color;
> };
>
> +struct intel_driver_caps {
> + unsigned int scheduler;
> +};
> +
> static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
> {
> return hweight8(sseu->slice_mask) * hweight8(sseu->subslice_mask);
> @@ -182,4 +186,7 @@ void intel_device_info_dump_flags(const struct intel_device_info *info,
> void intel_device_info_dump_runtime(const struct intel_device_info *info,
> struct drm_printer *p);
>
> +void intel_driver_caps_print(const struct intel_driver_caps *caps,
> + struct drm_printer *p);
> +
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 41419f249fc2..a52bd2525398 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1922,6 +1922,12 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
> engine->unpark = NULL;
>
> engine->flags |= I915_ENGINE_SUPPORTS_STATS;
> +
> + engine->i915->caps.scheduler =
> + I915_SCHEDULER_CAP_ENABLED |
> + I915_SCHEDULER_CAP_PRIORITY;
> + if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
> + engine->i915->caps.scheduler |= I915_SCHEDULER_CAP_PREEMPTION;
> }
>
> static void
> --
> 2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] drm/i915: Move the scheduler feature bits into the purview of the engines
2018-02-01 19:02 ` [PATCH v2] " Chris Wilson
2018-02-02 13:55 ` Mika Kuoppala
@ 2018-02-02 14:06 ` Lis, Tomasz
1 sibling, 0 replies; 23+ messages in thread
From: Lis, Tomasz @ 2018-02-02 14:06 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
So the functional purpose of this patch is to provide capabilities
(including preemption status) within error information. I agree this is
required.
On 2018-02-01 20:02, Chris Wilson wrote:
> Rather than having the high level ioctl interface guess the underlying
> implementation details, having the implementation declare what
> capabilities it exports. We define an intel_driver_caps, similar to the
> intel_device_info, which instead of trying to describe the HW gives
> details on what the driver itself supports. This is then populated by
> the engine backend for the new scheduler capability field for use
> elsewhere.
>
> v2: Use caps.scheduler for validating CONTEXT_PARAM_SET_PRIORITY (Mika)
> One less assumption of engine[RCS] \o/
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tomasz Lis <tomasz.lis@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Tomasz Lis <tomasz.lis@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 1 +
> drivers/gpu/drm/i915/i915_drv.c | 8 +-------
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_gem.c | 3 +++
> drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
> drivers/gpu/drm/i915/i915_gpu_error.c | 7 +++++--
> drivers/gpu/drm/i915/intel_device_info.c | 6 ++++++
> drivers/gpu/drm/i915/intel_device_info.h | 7 +++++++
> drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++
> 9 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9e44adef30f0..2bdce9fea671 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -49,6 +49,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
>
> intel_device_info_dump_flags(info, &p);
> intel_device_info_dump_runtime(info, &p);
> + intel_driver_caps_print(&dev_priv->caps, &p);
>
> kernel_param_lock(THIS_MODULE);
> i915_params_dump(&i915_modparams, &p);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ec12add34b2..733f71637914 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -372,13 +372,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
> value = i915_gem_mmap_gtt_version();
> break;
> case I915_PARAM_HAS_SCHEDULER:
> - value = 0;
> - if (dev_priv->engine[RCS] && dev_priv->engine[RCS]->schedule) {
> - value |= I915_SCHEDULER_CAP_ENABLED;
> - value |= I915_SCHEDULER_CAP_PRIORITY;
> - if (HAS_LOGICAL_RING_PREEMPTION(dev_priv))
> - value |= I915_SCHEDULER_CAP_PREEMPTION;
> - }
> + value = dev_priv->caps.scheduler;
> break;
>
> case I915_PARAM_MMAP_VERSION:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c676269ed843..24333042e1e9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -468,6 +468,7 @@ struct i915_gpu_state {
> u32 reset_count;
> u32 suspend_count;
> struct intel_device_info device_info;
> + struct intel_driver_caps driver_caps;
> struct i915_params params;
>
> struct i915_error_uc {
> @@ -1809,6 +1810,7 @@ struct drm_i915_private {
> struct kmem_cache *priorities;
>
> const struct intel_device_info info;
> + struct intel_driver_caps caps;
>
> /**
> * Data Stolen Memory - aka "i915 stolen memory" gives us the start and
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c049496e8757..8d732f97e23e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3222,8 +3222,11 @@ void i915_gem_set_wedged(struct drm_i915_private *i915)
> * start to complete all requests.
> */
> engine->submit_request = nop_complete_submit_request;
> + engine->schedule = NULL;
> }
>
> + i915->caps.scheduler = 0;
> +
> /*
> * Make sure no request can slip through without getting completed by
> * either this call here to intel_engine_init_global_seqno, or the one
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 648e7536ff51..8337d15bb0e5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -807,7 +807,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
>
> if (args->size)
> ret = -EINVAL;
> - else if (!to_i915(dev)->engine[RCS]->schedule)
> + else if (!(to_i915(dev)->caps.scheduler & I915_SCHEDULER_CAP_PRIORITY))
> ret = -ENODEV;
> else if (priority > I915_CONTEXT_MAX_USER_PRIORITY ||
> priority < I915_CONTEXT_MIN_USER_PRIORITY)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index a81351d9e3a6..a92b0c0377c7 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -573,11 +573,13 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
> }
>
> static void err_print_capabilities(struct drm_i915_error_state_buf *m,
> - const struct intel_device_info *info)
> + const struct intel_device_info *info,
> + const struct intel_driver_caps *caps)
> {
> struct drm_printer p = i915_error_printer(m);
>
> intel_device_info_dump_flags(info, &p);
> + intel_driver_caps_print(caps, &p);
> }
>
> static void err_print_params(struct drm_i915_error_state_buf *m,
> @@ -800,7 +802,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> if (error->display)
> intel_display_print_error_state(m, error->display);
>
> - err_print_capabilities(m, &error->device_info);
> + err_print_capabilities(m, &error->device_info, &error->driver_caps);
> err_print_params(m, &error->params);
> err_print_uc(m, &error->uc);
>
> @@ -1731,6 +1733,7 @@ static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
> memcpy(&error->device_info,
> INTEL_INFO(dev_priv),
> sizeof(error->device_info));
> + error->driver_caps = dev_priv->caps;
> }
>
> static __always_inline void dup_param(const char *type, void *x)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index a2c16140169f..298f8996cc54 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -586,3 +586,9 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
> /* Initialize command stream timestamp frequency */
> info->cs_timestamp_frequency_khz = read_timestamp_frequency(dev_priv);
> }
> +
> +void intel_driver_caps_print(const struct intel_driver_caps *caps,
> + struct drm_printer *p)
> +{
> + drm_printf(p, "scheduler: %x\n", caps->scheduler);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 9542018d11d0..71fdfb0451ef 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -167,6 +167,10 @@ struct intel_device_info {
> } color;
> };
>
> +struct intel_driver_caps {
> + unsigned int scheduler;
> +};
> +
> static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
> {
> return hweight8(sseu->slice_mask) * hweight8(sseu->subslice_mask);
> @@ -182,4 +186,7 @@ void intel_device_info_dump_flags(const struct intel_device_info *info,
> void intel_device_info_dump_runtime(const struct intel_device_info *info,
> struct drm_printer *p);
>
> +void intel_driver_caps_print(const struct intel_driver_caps *caps,
> + struct drm_printer *p);
> +
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 41419f249fc2..a52bd2525398 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1922,6 +1922,12 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
> engine->unpark = NULL;
>
> engine->flags |= I915_ENGINE_SUPPORTS_STATS;
> +
> + engine->i915->caps.scheduler =
> + I915_SCHEDULER_CAP_ENABLED |
> + I915_SCHEDULER_CAP_PRIORITY;
> + if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
> + engine->i915->caps.scheduler |= I915_SCHEDULER_CAP_PREEMPTION;
> }
>
> static void
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2] drm/i915: Only allocate preempt context when required
2018-01-31 9:47 ` [PATCH v2 3/3] drm/i915: Only allocate preempt context when required Chris Wilson
2018-01-31 14:38 ` Mika Kuoppala
@ 2018-02-03 10:40 ` Chris Wilson
1 sibling, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2018-02-03 10:40 UTC (permalink / raw)
To: intel-gfx; +Cc: Mika Kuoppala
If we remove some hardcoded assumptions about the preempt context having
a fixed id, reserved from use by normal user contexts, we may only
allocate the i915_gem_context when required. Then the subsequent
decisions on using preemption reduce to having the preempt context
available.
v2: Include an assert that we don't allocate the preempt context twice.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/i915_gem_context.c | 31 ++++++++++++------------
drivers/gpu/drm/i915/intel_engine_cs.c | 6 ++---
drivers/gpu/drm/i915/intel_guc_submission.c | 24 +++++++++---------
drivers/gpu/drm/i915/intel_lrc.c | 17 ++++++++-----
drivers/gpu/drm/i915/intel_ringbuffer.h | 5 ++++
drivers/gpu/drm/i915/selftests/mock_gem_device.c | 6 -----
6 files changed, 48 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 8337d15bb0e5..dd9efb9d0e0b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -449,12 +449,18 @@ destroy_kernel_context(struct i915_gem_context **ctxp)
i915_gem_context_free(ctx);
}
+static bool needs_preempt_context(struct drm_i915_private *i915)
+{
+ return HAS_LOGICAL_RING_PREEMPTION(i915);
+}
+
int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
{
struct i915_gem_context *ctx;
- int err;
+ /* Reassure ourselves we are only called once */
GEM_BUG_ON(dev_priv->kernel_context);
+ GEM_BUG_ON(dev_priv->preempt_context);
INIT_LIST_HEAD(&dev_priv->contexts.list);
INIT_WORK(&dev_priv->contexts.free_work, contexts_free_worker);
@@ -468,8 +474,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
ctx = i915_gem_context_create_kernel(dev_priv, I915_PRIORITY_MIN);
if (IS_ERR(ctx)) {
DRM_ERROR("Failed to create default global context\n");
- err = PTR_ERR(ctx);
- goto err;
+ return PTR_ERR(ctx);
}
/*
* For easy recognisablity, we want the kernel context to be 0 and then
@@ -479,23 +484,18 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
dev_priv->kernel_context = ctx;
/* highest priority; preempting task */
- ctx = i915_gem_context_create_kernel(dev_priv, INT_MAX);
- if (IS_ERR(ctx)) {
- DRM_ERROR("Failed to create default preempt context\n");
- err = PTR_ERR(ctx);
- goto err_kernel_context;
+ if (needs_preempt_context(dev_priv)) {
+ ctx = i915_gem_context_create_kernel(dev_priv, INT_MAX);
+ if (!IS_ERR(ctx))
+ dev_priv->preempt_context = ctx;
+ else
+ DRM_ERROR("Failed to create preempt context; disabling preemption\n");
}
- dev_priv->preempt_context = ctx;
DRM_DEBUG_DRIVER("%s context support initialized\n",
dev_priv->engine[RCS]->context_size ? "logical" :
"fake");
return 0;
-
-err_kernel_context:
- destroy_kernel_context(&dev_priv->kernel_context);
-err:
- return err;
}
void i915_gem_contexts_lost(struct drm_i915_private *dev_priv)
@@ -521,7 +521,8 @@ void i915_gem_contexts_fini(struct drm_i915_private *i915)
{
lockdep_assert_held(&i915->drm.struct_mutex);
- destroy_kernel_context(&i915->preempt_context);
+ if (i915->preempt_context)
+ destroy_kernel_context(&i915->preempt_context);
destroy_kernel_context(&i915->kernel_context);
/* Must free all deferred contexts (via flush_workqueue) first */
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 7eebfbb95e89..bf634432c9c6 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -631,7 +631,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
* Similarly the preempt context must always be available so that
* we can interrupt the engine at any time.
*/
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915)) {
+ if (engine->i915->preempt_context) {
ring = engine->context_pin(engine,
engine->i915->preempt_context);
if (IS_ERR(ring)) {
@@ -656,7 +656,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
err_breadcrumbs:
intel_engine_fini_breadcrumbs(engine);
err_unpin_preempt:
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
+ if (engine->i915->preempt_context)
engine->context_unpin(engine, engine->i915->preempt_context);
err_unpin_kernel:
engine->context_unpin(engine, engine->i915->kernel_context);
@@ -686,7 +686,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
if (engine->default_state)
i915_gem_object_put(engine->default_state);
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
+ if (engine->i915->preempt_context)
engine->context_unpin(engine, engine->i915->preempt_context);
engine->context_unpin(engine, engine->i915->kernel_context);
}
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 4ea65df05e02..b43b58cc599b 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -688,7 +688,7 @@ static void guc_dequeue(struct intel_engine_cs *engine)
goto unlock;
if (port_isset(port)) {
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915)) {
+ if (engine->i915->preempt_context) {
struct guc_preempt_work *preempt_work =
&engine->i915->guc.preempt_work[engine->id];
@@ -984,17 +984,19 @@ static int guc_clients_create(struct intel_guc *guc)
}
guc->execbuf_client = client;
- client = guc_client_alloc(dev_priv,
- INTEL_INFO(dev_priv)->ring_mask,
- GUC_CLIENT_PRIORITY_KMD_HIGH,
- dev_priv->preempt_context);
- if (IS_ERR(client)) {
- DRM_ERROR("Failed to create GuC client for preemption!\n");
- guc_client_free(guc->execbuf_client);
- guc->execbuf_client = NULL;
- return PTR_ERR(client);
+ if (dev_priv->preempt_context) {
+ client = guc_client_alloc(dev_priv,
+ INTEL_INFO(dev_priv)->ring_mask,
+ GUC_CLIENT_PRIORITY_KMD_HIGH,
+ dev_priv->preempt_context);
+ if (IS_ERR(client)) {
+ DRM_ERROR("Failed to create GuC client for preemption!\n");
+ guc_client_free(guc->execbuf_client);
+ guc->execbuf_client = NULL;
+ return PTR_ERR(client);
+ }
+ guc->preempt_client = client;
}
- guc->preempt_client = client;
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index fb2dd2f3bc8d..ee7390f69e93 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -161,7 +161,6 @@
#define EXECLISTS_REQUEST_SIZE 64 /* bytes */
#define WA_TAIL_DWORDS 2
#define WA_TAIL_BYTES (sizeof(u32) * WA_TAIL_DWORDS)
-#define PREEMPT_ID 0x1
static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
@@ -448,7 +447,8 @@ static void inject_preempt_context(struct intel_engine_cs *engine)
&engine->i915->preempt_context->engine[engine->id];
unsigned int n;
- GEM_BUG_ON(engine->i915->preempt_context->hw_id != PREEMPT_ID);
+ GEM_BUG_ON(engine->execlists.preempt_complete_status !=
+ upper_32_bits(ce->lrc_desc));
GEM_BUG_ON(!IS_ALIGNED(ce->ring->size, WA_TAIL_BYTES));
memset(ce->ring->vaddr + ce->ring->tail, 0, WA_TAIL_BYTES);
@@ -528,7 +528,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_HWACK))
goto unlock;
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915) &&
+ if (engine->i915->preempt_context &&
rb_entry(rb, struct i915_priolist, node)->priority >
max(last->priotree.priority, 0)) {
/*
@@ -844,7 +844,7 @@ static void execlists_submission_tasklet(unsigned long data)
GEM_BUG_ON(status & GEN8_CTX_STATUS_IDLE_ACTIVE);
if (status & GEN8_CTX_STATUS_COMPLETE &&
- buf[2*head + 1] == PREEMPT_ID) {
+ buf[2*head + 1] == execlists->preempt_complete_status) {
GEM_TRACE("%s preempt-idle\n", engine->name);
execlists_cancel_port_requests(execlists);
@@ -1931,7 +1931,7 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
engine->i915->caps.scheduler =
I915_SCHEDULER_CAP_ENABLED |
I915_SCHEDULER_CAP_PRIORITY;
- if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
+ if (engine->i915->preempt_context)
engine->i915->caps.scheduler |= I915_SCHEDULER_CAP_PREEMPTION;
}
@@ -2009,6 +2009,11 @@ static int logical_ring_init(struct intel_engine_cs *engine)
engine->execlists.elsp =
engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine));
+ engine->execlists.preempt_complete_status = ~0u;
+ if (engine->i915->preempt_context)
+ engine->execlists.preempt_complete_status =
+ upper_32_bits(engine->i915->preempt_context->engine[engine->id].lrc_desc);
+
return 0;
error:
@@ -2271,7 +2276,7 @@ populate_lr_context(struct i915_gem_context *ctx,
if (!engine->default_state)
regs[CTX_CONTEXT_CONTROL + 1] |=
_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
- if (ctx->hw_id == PREEMPT_ID)
+ if (ctx == ctx->i915->preempt_context)
regs[CTX_CONTEXT_CONTROL + 1] |=
_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c5ff203e42d6..03be8995f415 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -279,6 +279,11 @@ struct intel_engine_execlists {
* @csb_use_mmio: access csb through mmio, instead of hwsp
*/
bool csb_use_mmio;
+
+ /**
+ * @preempt_complete_status: expected CSB upon completing preemption
+ */
+ u32 preempt_complete_status;
};
#define INTEL_ENGINE_CS_MAX_NAME 8
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 1bc61f3f76fc..3175db70cc6e 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -243,16 +243,10 @@ struct drm_i915_private *mock_gem_device(void)
if (!i915->kernel_context)
goto err_engine;
- i915->preempt_context = mock_context(i915, NULL);
- if (!i915->preempt_context)
- goto err_kernel_context;
-
WARN_ON(i915_gemfs_init(i915));
return i915;
-err_kernel_context:
- i915_gem_context_put(i915->kernel_context);
err_engine:
for_each_engine(engine, i915, id)
mock_engine_free(engine);
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev4)
2018-01-31 9:47 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
` (6 preceding siblings ...)
2018-02-01 19:22 ` ✗ Fi.CI.BAT: failure for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev3) Patchwork
@ 2018-02-03 11:17 ` Patchwork
2018-02-04 6:51 ` ✗ Fi.CI.IGT: failure " Patchwork
8 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-02-03 11:17 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev4)
URL : https://patchwork.freedesktop.org/series/37395/
State : success
== Summary ==
Series 37395v4 series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL
https://patchwork.freedesktop.org/api/1.0/series/37395/revisions/4/mbox/
Test debugfs_test:
Subgroup read_all_entries:
pass -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test gem_mmap_gtt:
Subgroup basic-small-bo-tiledx:
pass -> FAIL (fi-gdg-551) fdo#102575
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:420s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:423s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:372s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:485s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:283s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:480s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:483s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:469s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:454s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:562s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:412s
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:278s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:511s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:392s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:401s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:414s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:451s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:412s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:456s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:496s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:454s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:502s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:581s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:431s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:508s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:530s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:488s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:483s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:414s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:431s
fi-snb-2520m total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:400s
Blacklisted hosts:
fi-glk-dsi total:288 pass:152 dwarn:1 dfail:4 fail:0 skip:131 time:263s
2e76a2952923eba64c4f9baf461613bc42ee997a drm-tip: 2018y-02m-02d-20h-33m-12s UTC integration manifest
d28ada97885a drm/i915: Only allocate preempt context when required
6166a85814ef drm/i915: Move the scheduler feature bits into the purview of the engines
3c85087dcfae drm/i915/guc: Allow preempt-client to be NULL
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7879/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✗ Fi.CI.IGT: failure for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev4)
2018-01-31 9:47 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
` (7 preceding siblings ...)
2018-02-03 11:17 ` ✓ Fi.CI.BAT: success for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev4) Patchwork
@ 2018-02-04 6:51 ` Patchwork
8 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-02-04 6:51 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev4)
URL : https://patchwork.freedesktop.org/series/37395/
State : failure
== Summary ==
Test gem_eio:
Subgroup in-flight-contexts:
dmesg-warn -> PASS (shard-snb) fdo#104058
Test perf:
Subgroup oa-exponents:
pass -> FAIL (shard-apl) fdo#102254
Test kms_flip:
Subgroup 2x-plain-flip-ts-check:
fail -> PASS (shard-hsw)
Subgroup 2x-flip-vs-expired-vblank-interruptible:
fail -> PASS (shard-hsw) fdo#102887
Test perf_pmu:
Subgroup busy-double-start-vcs0:
pass -> INCOMPLETE (shard-apl)
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-a-frame-sequence:
pass -> FAIL (shard-apl) fdo#103481
Test kms_cursor_legacy:
Subgroup 2x-long-flip-vs-cursor-legacy:
pass -> FAIL (shard-hsw) fdo#104873
Subgroup flip-vs-cursor-toggle:
fail -> PASS (shard-hsw) fdo#102670 +1
Test kms_setmode:
Subgroup basic:
fail -> PASS (shard-hsw) fdo#99912
fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
shard-apl total:2795 pass:1723 dwarn:1 dfail:0 fail:21 skip:1049 time:12048s
shard-hsw total:2836 pass:1733 dwarn:1 dfail:0 fail:11 skip:1090 time:11710s
shard-snb total:2836 pass:1328 dwarn:1 dfail:0 fail:10 skip:1497 time:6382s
Blacklisted hosts:
shard-kbl total:2836 pass:1872 dwarn:1 dfail:0 fail:21 skip:942 time:9340s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7879/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL
@ 2018-02-07 21:05 Chris Wilson
0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2018-02-07 21:05 UTC (permalink / raw)
To: intel-gfx
In the next patch, we may only conditionally allocate the preempt-client
if there is a global preempt context and so we need to be prepared in
case the preempt-client itself is NULL.
v2: Grep for more preempt_client.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 8 +++++---
drivers/gpu/drm/i915/intel_guc_submission.c | 27 +++++++++++++++++----------
drivers/gpu/drm/i915/selftests/intel_guc.c | 20 +++++++++++---------
3 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3849ded354e3..9e44adef30f0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2338,7 +2338,6 @@ static int i915_guc_info(struct seq_file *m, void *data)
return -ENODEV;
GEM_BUG_ON(!guc->execbuf_client);
- GEM_BUG_ON(!guc->preempt_client);
seq_printf(m, "Doorbell map:\n");
seq_printf(m, "\t%*pb\n", GUC_NUM_DOORBELLS, guc->doorbell_bitmap);
@@ -2346,8 +2345,11 @@ static int i915_guc_info(struct seq_file *m, void *data)
seq_printf(m, "\nGuC execbuf client @ %p:\n", guc->execbuf_client);
i915_guc_client_info(m, dev_priv, guc->execbuf_client);
- seq_printf(m, "\nGuC preempt client @ %p:\n", guc->preempt_client);
- i915_guc_client_info(m, dev_priv, guc->preempt_client);
+ if (guc->preempt_client) {
+ seq_printf(m, "\nGuC preempt client @ %p:\n",
+ guc->preempt_client);
+ i915_guc_client_info(m, dev_priv, guc->preempt_client);
+ }
i915_guc_log_info(m, dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 1f3a8786bbdc..4ea65df05e02 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -832,10 +832,12 @@ static int guc_clients_doorbell_init(struct intel_guc *guc)
if (ret)
return ret;
- ret = create_doorbell(guc->preempt_client);
- if (ret) {
- destroy_doorbell(guc->execbuf_client);
- return ret;
+ if (guc->preempt_client) {
+ ret = create_doorbell(guc->preempt_client);
+ if (ret) {
+ destroy_doorbell(guc->execbuf_client);
+ return ret;
+ }
}
return 0;
@@ -848,8 +850,11 @@ static void guc_clients_doorbell_fini(struct intel_guc *guc)
* Instead of trying (in vain) to communicate with it, let's just
* cleanup the doorbell HW and our internal state.
*/
- __destroy_doorbell(guc->preempt_client);
- __update_doorbell_desc(guc->preempt_client, GUC_DOORBELL_INVALID);
+ if (guc->preempt_client) {
+ __destroy_doorbell(guc->preempt_client);
+ __update_doorbell_desc(guc->preempt_client,
+ GUC_DOORBELL_INVALID);
+ }
__destroy_doorbell(guc->execbuf_client);
__update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID);
}
@@ -998,10 +1003,11 @@ static void guc_clients_destroy(struct intel_guc *guc)
{
struct intel_guc_client *client;
- client = fetch_and_zero(&guc->execbuf_client);
- guc_client_free(client);
-
client = fetch_and_zero(&guc->preempt_client);
+ if (client)
+ guc_client_free(client);
+
+ client = fetch_and_zero(&guc->execbuf_client);
guc_client_free(client);
}
@@ -1160,7 +1166,8 @@ int intel_guc_submission_enable(struct intel_guc *guc)
GEM_BUG_ON(!guc->execbuf_client);
guc_reset_wq(guc->execbuf_client);
- guc_reset_wq(guc->preempt_client);
+ if (guc->preempt_client)
+ guc_reset_wq(guc->preempt_client);
err = intel_guc_sample_forcewake(guc);
if (err)
diff --git a/drivers/gpu/drm/i915/selftests/intel_guc.c b/drivers/gpu/drm/i915/selftests/intel_guc.c
index 3f9016466dea..fb74e2cf8a0a 100644
--- a/drivers/gpu/drm/i915/selftests/intel_guc.c
+++ b/drivers/gpu/drm/i915/selftests/intel_guc.c
@@ -87,7 +87,7 @@ static int validate_client(struct intel_guc_client *client,
static bool client_doorbell_in_sync(struct intel_guc_client *client)
{
- return doorbell_ok(client->guc, client->doorbell_id);
+ return !client || doorbell_ok(client->guc, client->doorbell_id);
}
/*
@@ -137,7 +137,6 @@ static int igt_guc_clients(void *args)
goto unlock;
}
GEM_BUG_ON(!guc->execbuf_client);
- GEM_BUG_ON(!guc->preempt_client);
err = validate_client(guc->execbuf_client,
GUC_CLIENT_PRIORITY_KMD_NORMAL, false);
@@ -146,16 +145,18 @@ static int igt_guc_clients(void *args)
goto out;
}
- err = validate_client(guc->preempt_client,
- GUC_CLIENT_PRIORITY_KMD_HIGH, true);
- if (err) {
- pr_err("preempt client validation failed\n");
- goto out;
+ if (guc->preempt_client) {
+ err = validate_client(guc->preempt_client,
+ GUC_CLIENT_PRIORITY_KMD_HIGH, true);
+ if (err) {
+ pr_err("preempt client validation failed\n");
+ goto out;
+ }
}
/* each client should now have reserved a doorbell */
if (!has_doorbell(guc->execbuf_client) ||
- !has_doorbell(guc->preempt_client)) {
+ (guc->preempt_client && !has_doorbell(guc->preempt_client))) {
pr_err("guc_clients_create didn't reserve doorbells\n");
err = -EINVAL;
goto out;
@@ -224,7 +225,8 @@ static int igt_guc_clients(void *args)
* clients during unload.
*/
destroy_doorbell(guc->execbuf_client);
- destroy_doorbell(guc->preempt_client);
+ if (guc->preempt_client)
+ destroy_doorbell(guc->preempt_client);
guc_clients_destroy(guc);
guc_clients_create(guc);
guc_clients_doorbell_init(guc);
--
2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
end of thread, other threads:[~2018-02-07 21:05 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-31 9:47 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
2018-01-31 9:47 ` [PATCH v2 2/3] drm/i915: Move the scheduler feature bits into the purview of the engines Chris Wilson
2018-01-31 13:58 ` Mika Kuoppala
2018-02-01 18:54 ` Chris Wilson
2018-02-01 19:02 ` [PATCH v2] " Chris Wilson
2018-02-02 13:55 ` Mika Kuoppala
2018-02-02 14:06 ` Lis, Tomasz
2018-01-31 9:47 ` [PATCH v2 3/3] drm/i915: Only allocate preempt context when required Chris Wilson
2018-01-31 14:38 ` Mika Kuoppala
2018-01-31 15:25 ` Chris Wilson
2018-02-03 10:40 ` [PATCH v2] " Chris Wilson
2018-01-31 11:01 ` ✗ Fi.CI.BAT: warning for series starting with [v2,1/3] drm/i915/guc: Allow preempt-client to be NULL Patchwork
2018-01-31 11:31 ` [PATCH v2 1/3] " Mika Kuoppala
2018-01-31 11:34 ` Chris Wilson
2018-01-31 12:42 ` Mika Kuoppala
2018-01-31 12:03 ` [PATCH v2] " Chris Wilson
2018-01-31 13:14 ` Mika Kuoppala
2018-01-31 13:00 ` ✓ Fi.CI.BAT: success for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev2) Patchwork
2018-01-31 16:44 ` ✓ Fi.CI.IGT: " Patchwork
2018-02-01 19:22 ` ✗ Fi.CI.BAT: failure for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev3) Patchwork
2018-02-03 11:17 ` ✓ Fi.CI.BAT: success for series starting with [v2] drm/i915/guc: Allow preempt-client to be NULL (rev4) Patchwork
2018-02-04 6:51 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-02-07 21:05 [PATCH v2 1/3] drm/i915/guc: Allow preempt-client to be NULL Chris Wilson
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.