[Intel-gfx] [PATCH 1/3] drm/i915: Create a blitter context for GGTT updates

Matt Roper matthew.d.roper at intel.com
Thu Aug 24 15:51:46 UTC 2023


On Tue, Aug 22, 2023 at 11:28:57AM -0400, Oak Zeng wrote:
> From: Nirmoy Das <nirmoy.das at intel.com>
> 
> Create a separate blitter context if a platform requires
> GGTT updates using MI_UPDATE_GTT blitter command.
> 
> Subsequent patch will introduce methods to update
> GGTT using this blitter context and MI_UPDATE_GTT blitter
> command.
> 
> v2: Fix a typo in comment. (Oak)
> 
> Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
> Signed-off-by: Oak Zeng <oak.zeng at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine.h       |  4 ++
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 44 +++++++++++++++++++-
>  drivers/gpu/drm/i915/gt/intel_engine_types.h |  3 ++
>  drivers/gpu/drm/i915/gt/intel_gtt.c          |  4 ++
>  drivers/gpu/drm/i915/gt/intel_gtt.h          |  2 +
>  5 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index b58c30ac8ef0..ee36db2fdaa7 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -170,6 +170,8 @@ intel_write_status_page(struct intel_engine_cs *engine, int reg, u32 value)
>  #define I915_GEM_HWS_SEQNO		0x40
>  #define I915_GEM_HWS_SEQNO_ADDR		(I915_GEM_HWS_SEQNO * sizeof(u32))
>  #define I915_GEM_HWS_MIGRATE		(0x42 * sizeof(u32))
> +#define I915_GEM_HWS_GGTT_BLIT		0x46
> +#define I915_GEM_HWS_GGTT_BLIT_ADDR	(I915_GEM_HWS_GGTT_BLIT * sizeof(u32))
>  #define I915_GEM_HWS_PXP		0x60
>  #define I915_GEM_HWS_PXP_ADDR		(I915_GEM_HWS_PXP * sizeof(u32))
>  #define I915_GEM_HWS_GSC		0x62
> @@ -356,4 +358,6 @@ u64 intel_clamp_preempt_timeout_ms(struct intel_engine_cs *engine, u64 value);
>  u64 intel_clamp_stop_timeout_ms(struct intel_engine_cs *engine, u64 value);
>  u64 intel_clamp_timeslice_duration_ms(struct intel_engine_cs *engine, u64 value);
>  
> +void intel_engine_blitter_context_set_ready(struct intel_gt *gt, bool ready);
> +bool intel_engine_blitter_context_ready(struct intel_gt *gt);
>  #endif /* _INTEL_RINGBUFFER_H_ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index dfb69fc977a0..d8c492a507a4 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -27,6 +27,7 @@
>  #include "intel_gt_mcr.h"
>  #include "intel_gt_pm.h"
>  #include "intel_gt_requests.h"
> +#include "intel_gtt.h"
>  #include "intel_lrc.h"
>  #include "intel_lrc_reg.h"
>  #include "intel_reset.h"
> @@ -1419,6 +1420,34 @@ void intel_engine_destroy_pinned_context(struct intel_context *ce)
>  	intel_context_put(ce);
>  }
>  
> +void intel_engine_blitter_context_set_ready(struct intel_gt *gt, bool ready)
> +{
> +	struct intel_engine_cs *engine = gt->engine[BCS0];
> +
> +	if (engine && engine->blitter_context)
> +		atomic_set(&engine->blitter_context_ready, ready ? 1 : 0);
> +}
> +
> +bool intel_engine_blitter_context_ready(struct intel_gt *gt)
> +{
> +	struct intel_engine_cs *engine = gt->engine[BCS0];
> +
> +	if (engine)
> +		return atomic_read(&engine->blitter_context_ready) == 1;
> +
> +	return false;
> +}
> +
> +static struct intel_context *
> +create_ggtt_blitter_context(struct intel_engine_cs *engine)
> +{
> +	static struct lock_class_key kernel;
> +
> +	/* MI_UPDATE_GTT can insert up to 512 PTE entries so get a bigger ring */
> +	return intel_engine_create_pinned_context(engine, engine->gt->vm, SZ_512K,
> +						  I915_GEM_HWS_GGTT_BLIT_ADDR,
> +						  &kernel, "ggtt_blitter_context");
> +}
>  static struct intel_context *
>  create_kernel_context(struct intel_engine_cs *engine)
>  {
> @@ -1442,7 +1471,7 @@ create_kernel_context(struct intel_engine_cs *engine)
>   */
>  static int engine_init_common(struct intel_engine_cs *engine)
>  {
> -	struct intel_context *ce;
> +	struct intel_context *ce, *bce = NULL;
>  	int ret;
>  
>  	engine->set_default_submission(engine);
> @@ -1458,6 +1487,15 @@ static int engine_init_common(struct intel_engine_cs *engine)
>  	ce = create_kernel_context(engine);
>  	if (IS_ERR(ce))
>  		return PTR_ERR(ce);
> +	/*
> +	 * Create a separate pinned context for GGTT update using blitter
> +	 * if a platform require such service.
> +	 */
> +	if (i915_ggtt_require_blitter(engine->i915) && engine->id == BCS0) {
> +		bce = create_ggtt_blitter_context(engine);
> +		if (IS_ERR(bce))
> +			return PTR_ERR(bce);
> +	}
>  
>  	ret = measure_breadcrumb_dw(ce);
>  	if (ret < 0)
> @@ -1465,6 +1503,7 @@ static int engine_init_common(struct intel_engine_cs *engine)
>  
>  	engine->emit_fini_breadcrumb_dw = ret;
>  	engine->kernel_context = ce;
> +	engine->blitter_context = bce;
>  
>  	return 0;
>  
> @@ -1537,6 +1576,9 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
>  
>  	if (engine->kernel_context)
>  		intel_engine_destroy_pinned_context(engine->kernel_context);
> +	if (engine->blitter_context)
> +		intel_engine_destroy_pinned_context(engine->blitter_context);
> +
>  
>  	GEM_BUG_ON(!llist_empty(&engine->barrier_tasks));
>  	cleanup_status_page(engine);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index e99a6fa03d45..62095c0d8783 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -415,6 +415,9 @@ struct intel_engine_cs {
>  	struct llist_head barrier_tasks;
>  
>  	struct intel_context *kernel_context; /* pinned */
> +	struct intel_context *blitter_context; /* pinned, only for BCS0 */
> +	/* mark the blitter engine's availability status */
> +	atomic_t blitter_context_ready;
>  
>  	/**
>  	 * pinned_contexts_list: List of pinned contexts. This list is only
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
> index 13944a14ea2d..9c77c97670fe 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> @@ -21,6 +21,10 @@
>  #include "intel_gt_regs.h"
>  #include "intel_gtt.h"
>  
> +bool i915_ggtt_require_blitter(struct drm_i915_private *i915)
> +{
> +	return IS_METEORLAKE(i915);

Drive-by comment:  this workaround is tied to the Xe_LPM+ media IP, not
to the MTL platform.  Other platforms that re-use Xe_LPM+ IP will also
be affected, whereas MTL platforms that lack media, or integrate a
different media chiplet will not be affected.  So the condition here
should be:

        /* Wa_13010847436 */
        return MEDIA_VER_FULL(i915) == IP_VER(13, 0);

But does this even belong in this patch?  It sounds like patch #3 of the
series is where you intended to hook up this programming to the specific
workaround.


Matt

> +}
>  
>  static bool intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
>  {
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
> index 4d6296cdbcfd..9710eb031fb2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> @@ -688,4 +688,6 @@ static inline struct sgt_dma {
>  	return (struct sgt_dma){ sg, addr, addr + sg_dma_len(sg) };
>  }
>  
> +bool i915_ggtt_require_blitter(struct drm_i915_private *i915);
> +
>  #endif
> -- 
> 2.26.3
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation


More information about the Intel-gfx mailing list