[Intel-xe] [PATCH v3 5/9] drm/xe/wa: Track gt/engine/lrc active workarounds

Lucas De Marchi lucas.demarchi at intel.com
Wed May 17 20:23:19 UTC 2023


On Wed, May 17, 2023 at 10:50:56AM -0700, Matt Roper wrote:
>On Tue, May 16, 2023 at 03:19:46PM -0700, Lucas De Marchi wrote:
>> Allocate the data to track workarounds on the device, and pass that to
>> RTP so the active workarounds are enabled. For later reporting purposes,
>> there's no need to differentiate the engine or gt in which the
>> workaround got enabled, so just use a per-device bitmap.
>
>Going forward are we going to want to track these on a per-device or a
>per-GT basis?  The workaround database has switched to associating
>workarounds with the IP block rather than the platform since sometimes
>they're needed by both the graphics and media IP, other times they're
>only needed on one.
>
>For example, consider a couple different MTL workarounds:
> * Wa_14018575942 -- applies to both graphics and media GT
> * Wa_14015795083 -- applies to graphics GT but not media
> * Wa_14016790560 -- applies to media GT but not graphics
>
>As we get more post-MTL platforms where these chips may get mixed and
>matched into other platforms, I think it will be important to recognize
>not only whether the workaround is active somewhere on the platform, but
>exactly which GT(s) it's active on.

My idea was that for tracking purposes the device-level is sufficient as
I understand this would be more useful for knowing "is WA X
implemented and active?" rather than if the action it takes, applying
only to media or graphics GT, is correct. For the latter, we should
really be checking the register-save-restore in debugfs to know if the
bits are being applied correctly where they need to be applied
(and for OOB workarounds  to check whatever the thing the code is doing
in that code path)


I may be wrong here though... need to think a little bit more or more
time to be convinced otherwise. The change would be relatively easy, so
may be worth doing:

1) allocate bitmap per gt/engine besides the global device one
2) run the rules together with the gt creation
3) when reporting in debugfs optionally apply a bitmap_or() for the
    summary view, producing the same result we have today in addition to
    the drill down

Lucas De Marchi

>
>
>Matt
>
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_device.c       |  5 ++++
>>  drivers/gpu/drm/xe/xe_device_types.h | 10 ++++++++
>>  drivers/gpu/drm/xe/xe_rtp_types.h    |  2 ++
>>  drivers/gpu/drm/xe/xe_wa.c           | 36 ++++++++++++++++++++++++++++
>>  drivers/gpu/drm/xe/xe_wa.h           |  2 ++
>>  5 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>> index f7f6a6a97757..ac6898a28411 100644
>> --- a/drivers/gpu/drm/xe/xe_device.c
>> +++ b/drivers/gpu/drm/xe/xe_device.c
>> @@ -31,6 +31,7 @@
>>  #include "xe_ttm_sys_mgr.h"
>>  #include "xe_vm.h"
>>  #include "xe_vm_madvise.h"
>> +#include "xe_wa.h"
>>  #include "xe_wait_user_fence.h"
>>
>>  static int xe_file_open(struct drm_device *dev, struct drm_file *file)
>> @@ -251,6 +252,10 @@ int xe_device_probe(struct xe_device *xe)
>>  			return err;
>>  	}
>>
>> +	err = xe_wa_init(xe);
>> +	if (err)
>> +		return err;
>> +
>>  	err = xe_mmio_init(xe);
>>  	if (err)
>>  		return err;
>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>> index 6490a04614ce..b8d7864950c4 100644
>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> @@ -265,6 +265,16 @@ struct xe_device {
>>  	/** @d3cold_allowed: Indicates if d3cold is a valid device state */
>>  	bool d3cold_allowed;
>>
>> +	/** @wa_active: keep track of active workarounds */
>> +	struct {
>> +		/** @gt: bitmap with active GT workarounds */
>> +		unsigned long *gt;
>> +		/** @engine: bitmap with active engine workarounds */
>> +		unsigned long *engine;
>> +		/** @lrc: bitmap with active LRC workarounds */
>> +		unsigned long *lrc;
>> +	} wa_active;
>> +
>>  	/* private: */
>>
>>  #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
>> diff --git a/drivers/gpu/drm/xe/xe_rtp_types.h b/drivers/gpu/drm/xe/xe_rtp_types.h
>> index a9e0153ff87d..b1e77055f9cd 100644
>> --- a/drivers/gpu/drm/xe/xe_rtp_types.h
>> +++ b/drivers/gpu/drm/xe/xe_rtp_types.h
>> @@ -108,6 +108,8 @@ struct xe_rtp_process_ctx {
>>  		struct xe_hw_engine *hwe;
>>  	};
>>  	enum xe_rtp_process_type type;
>> +	unsigned long *active_entries;
>> +	size_t n_entries;
>>  };
>>
>>  #endif
>> diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
>> index 557e90d79f0b..fa37b1c0425a 100644
>> --- a/drivers/gpu/drm/xe/xe_wa.c
>> +++ b/drivers/gpu/drm/xe/xe_wa.c
>> @@ -5,6 +5,7 @@
>>
>>  #include "xe_wa.h"
>>
>> +#include <drm/drm_managed.h>
>>  #include <kunit/visibility.h>
>>  #include <linux/compiler_types.h>
>>
>> @@ -580,7 +581,10 @@ __diag_pop();
>>  void xe_wa_process_gt(struct xe_gt *gt)
>>  {
>>  	struct xe_rtp_process_ctx ctx = XE_RTP_PROCESS_CTX_INITIALIZER(gt);
>> +	struct xe_device *xe = gt_to_xe(gt);
>>
>> +	xe_rtp_process_ctx_enable_active_tracking(&ctx, xe->wa_active.gt,
>> +						  ARRAY_SIZE(gt_was));
>>  	xe_rtp_process_to_sr(&ctx, gt_was, &gt->reg_sr);
>>  }
>>  EXPORT_SYMBOL_IF_KUNIT(xe_wa_process_gt);
>> @@ -596,7 +600,10 @@ EXPORT_SYMBOL_IF_KUNIT(xe_wa_process_gt);
>>  void xe_wa_process_engine(struct xe_hw_engine *hwe)
>>  {
>>  	struct xe_rtp_process_ctx ctx = XE_RTP_PROCESS_CTX_INITIALIZER(hwe);
>> +	struct xe_device *xe = gt_to_xe(hwe->gt);
>>
>> +	xe_rtp_process_ctx_enable_active_tracking(&ctx, xe->wa_active.engine,
>> +						  ARRAY_SIZE(engine_was));
>>  	xe_rtp_process_to_sr(&ctx, engine_was, &hwe->reg_sr);
>>  }
>>
>> @@ -611,6 +618,35 @@ void xe_wa_process_engine(struct xe_hw_engine *hwe)
>>  void xe_wa_process_lrc(struct xe_hw_engine *hwe)
>>  {
>>  	struct xe_rtp_process_ctx ctx = XE_RTP_PROCESS_CTX_INITIALIZER(hwe);
>> +	struct xe_device *xe = gt_to_xe(hwe->gt);
>>
>> +	xe_rtp_process_ctx_enable_active_tracking(&ctx, xe->wa_active.lrc,
>> +						  ARRAY_SIZE(lrc_was));
>>  	xe_rtp_process_to_sr(&ctx, lrc_was, &hwe->reg_lrc);
>>  }
>> +
>> +/**
>> + * xe_wa_init - initialize xe with workaround bookkeeping
>> + * @xe: xe device instance
>> + *
>> + * Returns 0 for success, negative error code otherwise.
>> + */
>> +int xe_wa_init(struct xe_device *xe)
>> +{
>> +	size_t n_lrc, n_engine, n_gt, total;
>> +
>> +	n_gt = BITS_TO_LONGS(ARRAY_SIZE(gt_was));
>> +	n_engine = BITS_TO_LONGS(ARRAY_SIZE(engine_was));
>> +	n_lrc = BITS_TO_LONGS(ARRAY_SIZE(lrc_was));
>> +	total = n_gt + n_engine + n_lrc;
>> +
>> +	xe->wa_active.gt = drmm_kzalloc(&xe->drm, sizeof(long) * total,
>> +					GFP_KERNEL);
>> +	if (!xe->wa_active.gt)
>> +		return -ENOMEM;
>> +
>> +	xe->wa_active.engine = xe->wa_active.gt + n_gt;
>> +	xe->wa_active.lrc = xe->wa_active.engine + n_engine;
>> +
>> +	return 0;
>> +}
>> diff --git a/drivers/gpu/drm/xe/xe_wa.h b/drivers/gpu/drm/xe/xe_wa.h
>> index cd2307d58795..8edad4f92800 100644
>> --- a/drivers/gpu/drm/xe/xe_wa.h
>> +++ b/drivers/gpu/drm/xe/xe_wa.h
>> @@ -6,9 +6,11 @@
>>  #ifndef _XE_WA_
>>  #define _XE_WA_
>>
>> +struct xe_device;
>>  struct xe_gt;
>>  struct xe_hw_engine;
>>
>> +int xe_wa_init(struct xe_device *xe);
>>  void xe_wa_process_gt(struct xe_gt *gt);
>>  void xe_wa_process_engine(struct xe_hw_engine *hwe);
>>  void xe_wa_process_lrc(struct xe_hw_engine *hwe);
>> --
>> 2.40.1
>>
>
>-- 
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation


More information about the Intel-xe mailing list