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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
