> -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel > Vetter > Sent: Wednesday, June 18, 2014 9:19 PM > To: Mateo Lozano, Oscar > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH 08/53] drm/i915/bdw: Macro for LRCs and > module option for Execlists > > On Fri, Jun 13, 2014 at 04:37:26PM +0100, oscar.mateo@xxxxxxxxx wrote: > > From: Oscar Mateo <oscar.mateo@xxxxxxxxx> > > > > 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. In dev_priv, lrc_enabled will reflect the state of whether or > > not we've actually properly initialized these new contexts. This helps > > the transition in the code but is a candidate for removal at some point. > > > > 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. > > > > Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx> (v1) > > Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx> (v3) > > Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx> (v2 & v4) > > --- > > drivers/gpu/drm/i915/i915_drv.h | 6 ++++++ > > drivers/gpu/drm/i915/i915_params.c | 6 ++++++ > > drivers/gpu/drm/i915/intel_lrc.c | 8 ++++++++ > > 3 files changed, 20 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h index ccc1ba6..dac0db1 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1519,6 +1519,7 @@ struct drm_i915_private { > > > > uint32_t hw_context_size; > > struct list_head context_list; > > + bool lrc_enabled; > > > > u32 fdi_rx_config; > > > > @@ -1944,6 +1945,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) > > @@ -2029,6 +2031,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; > > @@ -2420,6 +2423,9 @@ struct intel_context * > > i915_gem_context_validate(struct drm_device *dev, struct drm_file *file, > > struct intel_engine_cs *ring, const u32 ctx_id); > > > > +/* intel_lrc.c */ > > +bool intel_enable_execlists(struct drm_device *dev); > > + > > /* i915_gem_render_state.c */ > > int i915_gem_render_state_init(struct intel_engine_cs *ring); > > /* i915_gem_evict.c */ > > diff --git a/drivers/gpu/drm/i915/i915_params.c > > b/drivers/gpu/drm/i915/i915_params.c > > index d05a2af..b7455f8 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 = -1, > > .enable_psr = 0, > > .preliminary_hw_support = > IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > > .disable_power_well = 1, > > @@ -116,6 +117,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: false)"); > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index 49bb6fc..58cead1 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -40,3 +40,11 @@ > > #include <drm/drmP.h> > > #include <drm/i915_drm.h> > > #include "i915_drv.h" > > + > > +bool intel_enable_execlists(struct drm_device *dev) { > > + if (!i915.enable_execlists) > > + return false; > > + > > + return HAS_LOGICAL_RING_CONTEXTS(dev) && USES_PPGTT(dev); } > > Nitpick: Best practice nowadays for options with complicated details is to > have a sanitized function called early in init. Code then just uses i915.foo > without calling anything. And the parameter needs to be read-only, but that's > already the case. See e.g. ppgtt handling. > > Of course if your code only uses this once then this is moot - I didn't read > ahead. > -Daniel Yes, I haven´t missed the 20+ True PPGTT early sanitize patches in the list :) Ok, will do! -- Oscar _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
