Re: [PATCH v5 7/9] drm/xe: Move lrc snapshot capturing to xe_lrc.c

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Hey,

On 2024-02-21 18:56, Souza, Jose wrote:
> On Wed, 2024-02-21 at 14:30 +0100, Maarten Lankhorst wrote:
>> This allows the dumping of HWSP and HW Context without exporting more
>> functions.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_guc_submit.c       | 33 ++++++-----------
>>   drivers/gpu/drm/xe/xe_guc_submit_types.h | 13 +------
>>   drivers/gpu/drm/xe/xe_lrc.c              | 45 ++++++++++++++++++++++++
>>   drivers/gpu/drm/xe/xe_lrc.h              |  4 +++
>>   drivers/gpu/drm/xe/xe_lrc_types.h        |  2 ++
>>   5 files changed, 63 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
>> index ff77bc8da1b27..7348689ec5aef 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
>> @@ -1814,21 +1814,14 @@ xe_guc_exec_queue_snapshot_capture(struct xe_sched_job *job)
>>   	snapshot->sched_props.preempt_timeout_us =
>>   		q->sched_props.preempt_timeout_us;
>>   
>> -	snapshot->lrc = kmalloc_array(q->width, sizeof(struct lrc_snapshot),
>> +	snapshot->lrc = kmalloc_array(q->width, sizeof(struct xe_lrc_snapshot *),
>>   				      GFP_ATOMIC);
>>   
>>   	if (snapshot->lrc) {
>>   		for (i = 0; i < q->width; ++i) {
>>   			struct xe_lrc *lrc = q->lrc + i;
>>   
>> -			snapshot->lrc[i].context_desc =
>> -				lower_32_bits(xe_lrc_ggtt_addr(lrc));
>> -			snapshot->lrc[i].head = xe_lrc_ring_head(lrc);
>> -			snapshot->lrc[i].tail.internal = lrc->ring.tail;
>> -			snapshot->lrc[i].tail.memory =
>> -				xe_lrc_read_ctx_reg(lrc, CTX_RING_TAIL);
>> -			snapshot->lrc[i].start_seqno = xe_lrc_start_seqno(lrc);
>> -			snapshot->lrc[i].seqno = xe_lrc_seqno(lrc);
>> +			snapshot->lrc[i] = xe_lrc_snapshot_capture(lrc);
>>   		}
>>   	}
>>   
>> @@ -1894,18 +1887,9 @@ xe_guc_exec_queue_snapshot_print(struct xe_guc_submit_exec_queue_snapshot *snaps
>>   	drm_printf(p, "\tPreempt timeout: %u (us)\n",
>>   		   snapshot->sched_props.preempt_timeout_us);
>>   
>> -	for (i = 0; snapshot->lrc && i < snapshot->width; ++i) {
>> -		drm_printf(p, "\tHW Context Desc: 0x%08x\n",
>> -			   snapshot->lrc[i].context_desc);
>> -		drm_printf(p, "\tLRC Head: (memory) %u\n",
>> -			   snapshot->lrc[i].head);
>> -		drm_printf(p, "\tLRC Tail: (internal) %u, (memory) %u\n",
>> -			   snapshot->lrc[i].tail.internal,
>> -			   snapshot->lrc[i].tail.memory);
>> -		drm_printf(p, "\tStart seqno: (memory) %d\n",
>> -			   snapshot->lrc[i].start_seqno);
>> -		drm_printf(p, "\tSeqno: (memory) %d\n", snapshot->lrc[i].seqno);
>> -	}
>> +	for (i = 0; snapshot->lrc && i < snapshot->width; ++i)
>> +		xe_lrc_snapshot_print(snapshot->lrc[i], p);
>> +
>>   	drm_printf(p, "\tSchedule State: 0x%x\n", snapshot->schedule_state);
>>   	drm_printf(p, "\tFlags: 0x%lx\n", snapshot->exec_queue_flags);
> 
> can you please print this before xe_lrc_snapshot_print()? to make the file easier to read.
Yeah, will do so.

I've pushed patches 1-6, that should at least be in time for merge 
window closing now.

Cheers,
~Maarten