[PATCH 0/2] Module parameter to enable execlists

[PATCH 0/2] Module parameter to enable execlists

[Damien Lespiau]

I feel like it'd be wise, when we merge execlists support, to provide a way to
disable them, just in case.

Actually, it may be even wiser, would we want to merge them early, to merge
execlists disabled by default.

-- 
Damien

Damien Lespiau (2):
  drm/i915: Factor out a enable_execlists() function
  drm/i915: Add a module parameter to enable execlists

 drivers/gpu/drm/i915/i915_drv.h    |  1 +
 drivers/gpu/drm/i915/i915_gem.c    | 10 +++++++++-
 drivers/gpu/drm/i915/i915_params.c |  6 ++++++
 3 files changed, 16 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[PATCH 1/2] drm/i915: Factor out a enable_execlists() function

[Damien Lespiau]

So it's clear what the condition is. We'll add a bit more code shortly.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1a58e6e..998f647 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4460,6 +4460,11 @@ err_out:
 	return ret;
 }
 
+static bool enable_execlists(struct drm_device *dev)
+{
+	return HAS_LOGICAL_RING_CONTEXTS(dev) && USES_PPGTT(dev);
+}
+
 int i915_gem_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -4478,7 +4483,7 @@ int i915_gem_init(struct drm_device *dev)
 
 	intel_init_rings_early(dev);
 
-	if (HAS_LOGICAL_RING_CONTEXTS(dev) && USES_PPGTT(dev))
+	if (enable_execlists(dev))
 		ret = gen8_gem_context_init(dev);
 
 	if (ret) {
-- 
1.8.3.1

[PATCH 2/2] drm/i915: Add a module parameter to enable execlists

[Damien Lespiau]

Execlist are relatively new, and so it'd be wise to be able to merge
that support disabled by default while still allowing a module parameter
to enable that feature.

Even if we end up enabling execlists by default, it'll be handy to be
able to switch back to ring submission to debug subtle problems that
will inevitably arise.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    | 1 +
 drivers/gpu/drm/i915/i915_gem.c    | 3 +++
 drivers/gpu/drm/i915/i915_params.c | 6 ++++++
 3 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 34a8fe5..8f0d27a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2013,6 +2013,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 998f647..57ef2a7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4462,6 +4462,9 @@ err_out:
 
 static bool enable_execlists(struct drm_device *dev)
 {
+	if (!i915.enable_execlists)
+		return false;
+
 	return HAS_LOGICAL_RING_CONTEXTS(dev) && USES_PPGTT(dev);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index d1d7980..32112f2 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -37,6 +37,7 @@ struct i915_params i915 __read_mostly = {
 	.enable_fbc = -1,
 	.enable_hangcheck = true,
 	.enable_ppgtt = -1,
+	.enable_execlists = 0,
 	.enable_psr = 0,
 	.preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
 	.disable_power_well = 1,
@@ -115,6 +116,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, 0=disabled [default], 1=enabled)");
+
 module_param_named(enable_psr, i915.enable_psr, int, 0600);
 MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)");
 
-- 
1.8.3.1

Re: [PATCH 2/2] drm/i915: Add a module parameter to enable execlists

[Damien Lespiau]

On Mon, Apr 07, 2014 at 03:05:39PM +0100, Damien Lespiau wrote:
> Execlist are relatively new, and so it'd be wise to be able to merge
> that support disabled by default while still allowing a module parameter
> to enable that feature.
> 
> Even if we end up enabling execlists by default, it'll be handy to be
> able to switch back to ring submission to debug subtle problems that
> will inevitably arise.

After a quick chat with Daniel on IRC, we actually want that new code to
be enabled by default to get as much testing as possible, while
retaining the module parameter to quickly identify if a bug we're seeing
can be reproduced with the legacy ring submission or not.

> @@ -37,6 +37,7 @@ struct i915_params i915 __read_mostly = {
>  	.enable_fbc = -1,
>  	.enable_hangcheck = true,
>  	.enable_ppgtt = -1,
> +	.enable_execlists = 0,

So, this should be -1.

-- 
Damien

Re: [PATCH 2/2] drm/i915: Add a module parameter to enable execlists

[Mateo Lozano, Oscar]

> On Mon, Apr 07, 2014 at 03:05:39PM +0100, Damien Lespiau wrote:
> > Execlist are relatively new, and so it'd be wise to be able to merge
> > that support disabled by default while still allowing a module
> > parameter to enable that feature.
> >
> > Even if we end up enabling execlists by default, it'll be handy to be
> > able to switch back to ring submission to debug subtle problems that
> > will inevitably arise.
> 
> After a quick chat with Daniel on IRC, we actually want that new code to be
> enabled by default to get as much testing as possible, while retaining the
> module parameter to quickly identify if a bug we're seeing can be reproduced
> with the legacy ring submission or not.
> 
> > @@ -37,6 +37,7 @@ struct i915_params i915 __read_mostly = {
> >  	.enable_fbc = -1,
> >  	.enable_hangcheck = true,
> >  	.enable_ppgtt = -1,
> > +	.enable_execlists = 0,
> 
> So, this should be -1.

I´ll add these two patches to the series and resubmit with default = -1

Thanks!
Oscar