Re: [Intel-gfx] [PATCH v2] drm/i915: Move the scheduler feature bits into the purview of the engines
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
Cc: Tomasz Lis
Cc: Joonas Lahtinen
Cc: Tvrtko Ursulin
Cc: Michal Wajdeczko
Cc: Mika Kuoppala
Reviewed-by: Tomasz Lis
---
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/
Re: [Intel-gfx] [PATCH v2] drm/i915: Move the scheduler feature bits into the purview of the engines
Chris Wilson 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
> Cc: Tomasz Lis
> Cc: Joonas Lahtinen
> Cc: Tvrtko Ursulin
> Cc: Michal Wajdeczko
> Cc: Mika Kuoppala
Reviewed-by: Mika Kuoppala
> ---
> 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/d
[Intel-gfx] [PATCH v2] drm/i915: Move the scheduler feature bits into the purview of the engines
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
Cc: Tomasz Lis
Cc: Joonas Lahtinen
Cc: Tvrtko Ursulin
Cc: Michal Wajdeczko
Cc: Mika Kuoppala
---
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_erro
