[PATCH 2/3] drm/xe/guc: Use the steering flag when printing registers

[John Harrison]

On 4/17/2025 1:17 PM, Matt Roper wrote:
> On Thu, Apr 17, 2025 at 12:52:13PM -0700, John.C.Harrison at Intel.com wrote:
>> From: John Harrison <John.C.Harrison at Intel.com>
>>
>> The printing code was doing a test on which list a register was in to
>> decide whether it is steered or not. That might be valid at this
>> moment but there may be other reasons for extended lists in the
>> future. Plus, there is a flag specifically for identifying steered
>> registers. So, just use that instead - it is simpler and safer.
>>
>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_guc_capture.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c b/drivers/gpu/drm/xe/xe_guc_capture.c
>> index 9095618648bc..f4b08136e472 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_capture.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_capture.c
>> @@ -1672,18 +1672,16 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_
>>   {
>>   	struct xe_gt *gt = snapshot->hwe->gt;
>>   	struct xe_device *xe = gt_to_xe(gt);
>> -	struct xe_guc *guc = &gt->uc.guc;
>>   	struct xe_devcoredump *devcoredump = &xe->devcoredump;
>>   	struct xe_devcoredump_snapshot *devcore_snapshot = &devcoredump->snapshot;
>>   	struct gcap_reg_list_info *reginfo = NULL;
>>   	u32 i, last_value = 0;
>> -	bool is_ext, low32_ready = false;
>> +	bool low32_ready = false;
>>   
>>   	if (!list || !list->list || list->num_regs == 0)
>>   		return;
>>   	XE_WARN_ON(!devcore_snapshot->matched_node);
>>   
>> -	is_ext = list == guc->capture->extlists;
>>   	reginfo = &devcore_snapshot->matched_node->reginfo[type];
>>   
>>   	/*
>> @@ -1749,7 +1747,7 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_
>>   			 */
>>   			XE_WARN_ON(low32_ready);
>>   
>> -			if (is_ext) {
>> +			if (FIELD_GET(GUC_REGSET_STEERING_NEEDED, reg_desc->flags)) {
> This is a single-bit flag, so can't we just do a simple '&' rather than
> using FIELD_GET?
I was just being consistent with the other usages. Could convert them 
all to bit ops instead, but having a mix seems wrong.

John.

>
>
> Matt
>
>>   				int dss, group, instance;
>>   
>>   				group = FIELD_GET(GUC_REGSET_STEERING_GROUP, reg_desc->flags);
>> -- 
>> 2.49.0
>>
>>