Re: [Intel-gfx] [PATCH 03/43] drm/i915/bdw: Macro for LRCs and module option for Execlists

2014-08-11 Thread Daniel Vetter
On Thu, Jul 24, 2014 at 05:04:11PM +0100, Thomas Daniel wrote:
> From: Oscar Mateo 
> 
> GEN8 brings an expansion of the HW contexts: "Logical Ring Contexts".
> These expanded contexts enable a number of new abilities, especially
> "Execlists".
> 
> The macro is defined to off until we have things in place to hope to
> work.
> 
> v2: Rename "advanced contexts" to the more correct "logical ring
> contexts".
> 
> v3: Add a module parameter to enable execlists. Execlist are relatively
> new, and so it'd be wise to be able to switch back to ring submission
> to debug subtle problems that will inevitably arise.
> 
> v4: Add an intel_enable_execlists function.
> 
> v5: Sanitize early, as suggested by Daniel. Remove lrc_enabled.
> 
> Signed-off-by: Ben Widawsky  (v1)
> Signed-off-by: Damien Lespiau  (v3)
> Signed-off-by: Oscar Mateo  (v2, v4 & v5)
> ---
>  drivers/gpu/drm/i915/i915_drv.h|2 ++
>  drivers/gpu/drm/i915/i915_gem.c|3 +++
>  drivers/gpu/drm/i915/i915_params.c |6 ++
>  drivers/gpu/drm/i915/intel_lrc.c   |   11 +++
>  drivers/gpu/drm/i915/intel_lrc.h   |3 +++
>  5 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 54c2bd9..a793d6d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2037,6 +2037,7 @@ struct drm_i915_cmd_table {
>  #define I915_NEED_GFX_HWS(dev)   (INTEL_INFO(dev)->need_gfx_hws)
>  
>  #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
> +#define HAS_LOGICAL_RING_CONTEXTS(dev)   0
>  #define HAS_ALIASING_PPGTT(dev)  (INTEL_INFO(dev)->gen >= 6)
>  #define HAS_PPGTT(dev)   (INTEL_INFO(dev)->gen >= 7 && 
> !IS_GEN8(dev))
>  #define USES_PPGTT(dev)  intel_enable_ppgtt(dev, false)
> @@ -2122,6 +2123,7 @@ struct i915_params {
>   int enable_rc6;
>   int enable_fbc;
>   int enable_ppgtt;
> + int enable_execlists;
>   int enable_psr;
>   unsigned int preliminary_hw_support;
>   int disable_power_well;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e5d4d73..d8bf4fa 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4746,6 +4746,9 @@ int i915_gem_init(struct drm_device *dev)
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   int ret;
>  
> + i915.enable_execlists = intel_sanitize_enable_execlists(dev,
> + i915.enable_execlists);

The ordering constraint on i915.enabel_ppgtt sanitization is imo
super-fragile. You'll all get a big price (and grumpy maintainer) if this
blows up. I'll add a patch on top to WARN about this.
-Daniel

> +
>   mutex_lock(&dev->struct_mutex);
>  
>   if (IS_VALLEYVIEW(dev)) {
> diff --git a/drivers/gpu/drm/i915/i915_params.c 
> b/drivers/gpu/drm/i915/i915_params.c
> index bbdee21..7f0fb72 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -35,6 +35,7 @@ struct i915_params i915 __read_mostly = {
>   .vbt_sdvo_panel_type = -1,
>   .enable_rc6 = -1,
>   .enable_fbc = -1,
> + .enable_execlists = -1,
>   .enable_hangcheck = true,
>   .enable_ppgtt = -1,
>   .enable_psr = 1,
> @@ -117,6 +118,11 @@ MODULE_PARM_DESC(enable_ppgtt,
>   "Override PPGTT usage. "
>   "(-1=auto [default], 0=disabled, 1=aliasing, 2=full)");
>  
> +module_param_named(enable_execlists, i915.enable_execlists, int, 0400);
> +MODULE_PARM_DESC(enable_execlists,
> + "Override execlists usage. "
> + "(-1=auto [default], 0=disabled, 1=enabled)");
> +
>  module_param_named(enable_psr, i915.enable_psr, int, 0600);
>  MODULE_PARM_DESC(enable_psr, "Enable PSR (default: true)");
>  
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 49bb6fc..21f7f1c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -40,3 +40,14 @@
>  #include 
>  #include 
>  #include "i915_drv.h"
> +
> +int intel_sanitize_enable_execlists(struct drm_device *dev, int 
> enable_execlists)
> +{
> + if (enable_execlists == 0)
> + return 0;
> +
> + if (HAS_LOGICAL_RING_CONTEXTS(dev) && USES_PPGTT(dev))
> + return 1;
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h 
> b/drivers/gpu/drm/i915/intel_lrc.h
> index f6830a4..75ee9c3 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -24,4 +24,7 @@
>  #ifndef _INTEL_LRC_H_
>  #define _INTEL_LRC_H_
>  
> +/* Execlists */
> +int intel_sanitize_enable_execlists(struct drm_device *dev, int 
> enable_execlists);
> +
>  #endif /* _INTEL_LRC_H_ */
> -- 
> 1.7.9.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 

[Intel-gfx] [PATCH 03/43] drm/i915/bdw: Macro for LRCs and module option for Execlists

2014-07-24 Thread Thomas Daniel
From: Oscar Mateo 

GEN8 brings an expansion of the HW contexts: "Logical Ring Contexts".
These expanded contexts enable a number of new abilities, especially
"Execlists".

The macro is defined to off until we have things in place to hope to
work.

v2: Rename "advanced contexts" to the more correct "logical ring
contexts".

v3: Add a module parameter to enable execlists. Execlist are relatively
new, and so it'd be wise to be able to switch back to ring submission
to debug subtle problems that will inevitably arise.

v4: Add an intel_enable_execlists function.

v5: Sanitize early, as suggested by Daniel. Remove lrc_enabled.

Signed-off-by: Ben Widawsky  (v1)
Signed-off-by: Damien Lespiau  (v3)
Signed-off-by: Oscar Mateo  (v2, v4 & v5)
---
 drivers/gpu/drm/i915/i915_drv.h|2 ++
 drivers/gpu/drm/i915/i915_gem.c|3 +++
 drivers/gpu/drm/i915/i915_params.c |6 ++
 drivers/gpu/drm/i915/intel_lrc.c   |   11 +++
 drivers/gpu/drm/i915/intel_lrc.h   |3 +++
 5 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 54c2bd9..a793d6d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2037,6 +2037,7 @@ struct drm_i915_cmd_table {
 #define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)->need_gfx_hws)
 
 #define HAS_HW_CONTEXTS(dev)   (INTEL_INFO(dev)->gen >= 6)
+#define HAS_LOGICAL_RING_CONTEXTS(dev) 0
 #define HAS_ALIASING_PPGTT(dev)(INTEL_INFO(dev)->gen >= 6)
 #define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_GEN8(dev))
 #define USES_PPGTT(dev)intel_enable_ppgtt(dev, false)
@@ -2122,6 +2123,7 @@ struct i915_params {
int enable_rc6;
int enable_fbc;
int enable_ppgtt;
+   int enable_execlists;
int enable_psr;
unsigned int preliminary_hw_support;
int disable_power_well;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e5d4d73..d8bf4fa 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4746,6 +4746,9 @@ int i915_gem_init(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
 
+   i915.enable_execlists = intel_sanitize_enable_execlists(dev,
+   i915.enable_execlists);
+
mutex_lock(&dev->struct_mutex);
 
if (IS_VALLEYVIEW(dev)) {
diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index bbdee21..7f0fb72 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -35,6 +35,7 @@ struct i915_params i915 __read_mostly = {
.vbt_sdvo_panel_type = -1,
.enable_rc6 = -1,
.enable_fbc = -1,
+   .enable_execlists = -1,
.enable_hangcheck = true,
.enable_ppgtt = -1,
.enable_psr = 1,
@@ -117,6 +118,11 @@ MODULE_PARM_DESC(enable_ppgtt,
"Override PPGTT usage. "
"(-1=auto [default], 0=disabled, 1=aliasing, 2=full)");
 
+module_param_named(enable_execlists, i915.enable_execlists, int, 0400);
+MODULE_PARM_DESC(enable_execlists,
+   "Override execlists usage. "
+   "(-1=auto [default], 0=disabled, 1=enabled)");
+
 module_param_named(enable_psr, i915.enable_psr, int, 0600);
 MODULE_PARM_DESC(enable_psr, "Enable PSR (default: true)");
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 49bb6fc..21f7f1c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -40,3 +40,14 @@
 #include 
 #include 
 #include "i915_drv.h"
+
+int intel_sanitize_enable_execlists(struct drm_device *dev, int 
enable_execlists)
+{
+   if (enable_execlists == 0)
+   return 0;
+
+   if (HAS_LOGICAL_RING_CONTEXTS(dev) && USES_PPGTT(dev))
+   return 1;
+
+   return 0;
+}
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index f6830a4..75ee9c3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -24,4 +24,7 @@
 #ifndef _INTEL_LRC_H_
 #define _INTEL_LRC_H_
 
+/* Execlists */
+int intel_sanitize_enable_execlists(struct drm_device *dev, int 
enable_execlists);
+
 #endif /* _INTEL_LRC_H_ */
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx