Commit 44958161 authored by Zhanjun Dong's avatar Zhanjun Dong Committed by Thomas Hellström
Browse files

drm/xe/guc: Fix missing init value and add register order check 



Fix missing initial value for last_value.
For GuC capture register definition, it is required to define 64bit
register in a pair of 2 consecutive 32bit register entries, low first,
then hi. Add code to check this order.

Changes from prior revs:
 v5:- Correct cross-line comment format
 v4:- Fix warn on condition and remove skipping
 v3:- Move break inside brace
 v2:- Correct the fix tag pointed commit
      Add examples in comments for warning
      Add 1 missing hi condition check

Fixes: ecb63364 ("drm/xe/guc: Plumb GuC-capture into dev coredump")
Signed-off-by: default avatarZhanjun Dong <zhanjun.dong@intel.com>
Reviewed-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241126201052.1937079-1-zhanjun.dong@intel.com


(cherry picked from commit 6f59fbcf)
Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
parent 40384c84

drivers/gpu/drm/xe/xe_guc_capture.c

+62 −15
Original line number Diff line number Diff line
@@ -102,6 +102,7 @@ struct __guc_capture_parsed_output { 
 *                   A 64 bit register define requires 2 consecutive entries,

 *                   with low dword first and hi dword the second.

 *     2. Register name: null for incompleted define

 *     3. Incorrect order will trigger XE_WARN.

 */

#define COMMON_XELP_BASE_GLOBAL \

	{ FORCEWAKE_GT,			REG_32BIT,	0,	0,	"FORCEWAKE_GT"}
@@ -1675,10 +1676,10 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_ 
	struct xe_devcoredump *devcoredump = &xe->devcoredump;

	struct xe_devcoredump_snapshot *devcore_snapshot = &devcoredump->snapshot;

	struct gcap_reg_list_info *reginfo = NULL;

	u32 last_value, i;

	bool is_ext;

	u32 i, last_value = 0;

	bool is_ext, low32_ready = false;



	if (!list || list->num_regs == 0)

	if (!list || !list->list || list->num_regs == 0)

		return;

	XE_WARN_ON(!devcore_snapshot->matched_node);
@@ -1701,17 +1702,53 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_ 
			continue;



		value = reg->value;

		if (reg_desc->data_type == REG_64BIT_LOW_DW) {

		switch (reg_desc->data_type) {

		case REG_64BIT_LOW_DW:

			last_value = value;



			/*

			 * A 64 bit register define requires 2 consecutive

			 * entries in register list, with low dword first

			 * and hi dword the second, like:

			 *  { XXX_REG_LO(0), REG_64BIT_LOW_DW, 0, 0, NULL},

			 *  { XXX_REG_HI(0), REG_64BIT_HI_DW,  0, 0, "XXX_REG"},

			 *

			 * Incorrect order will trigger XE_WARN.

			 *

			 * Possible double low here, for example:

			 *  { XXX_REG_LO(0), REG_64BIT_LOW_DW, 0, 0, NULL},

			 *  { XXX_REG_LO(0), REG_64BIT_LOW_DW, 0, 0, NULL},

			 */

			XE_WARN_ON(low32_ready);

			low32_ready = true;

			/* Low 32 bit dword saved, continue for high 32 bit */

			continue;

		} else if (reg_desc->data_type == REG_64BIT_HI_DW) {

			break;



		case REG_64BIT_HI_DW: {

			u64 value_qw = ((u64)value << 32) | last_value;



			/*

			 * Incorrect 64bit register order. Possible missing low.

			 * for example:

			 *  { XXX_REG(0), REG_32BIT, 0, 0, NULL},

			 *  { XXX_REG_HI(0), REG_64BIT_HI_DW, 0, 0, NULL},

			 */

			XE_WARN_ON(!low32_ready);

			low32_ready = false;



			drm_printf(p, "\t%s: 0x%016llx\n", reg_desc->regname, value_qw);

			continue;

			break;

		}



		case REG_32BIT:

			/*

			 * Incorrect 64bit register order. Possible missing high.

			 * for example:

			 *  { XXX_REG_LO(0), REG_64BIT_LOW_DW, 0, 0, NULL},

			 *  { XXX_REG(0), REG_32BIT, 0, 0, "XXX_REG"},

			 */

			XE_WARN_ON(low32_ready);



			if (is_ext) {

				int dss, group, instance;
@@ -1723,7 +1760,17 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_ 
			} else {

				drm_printf(p, "\t%s: 0x%08x\n", reg_desc->regname, value);

			}

			break;

		}

	}



	/*

	 * Incorrect 64bit register order. Possible missing high.

	 * for example:

	 *  { XXX_REG_LO(0), REG_64BIT_LOW_DW, 0, 0, NULL},

	 *  } // <- Register list end

	 */

	XE_WARN_ON(low32_ready);

}



/**