[Intel-xe] [PATCH 11/19] drm/xe/reg_sr: Tweak verbosity for register printing

[Lucas De Marchi]

On Wed, Mar 08, 2023 at 02:37:45PM -0800, Matt Roper wrote:
>On Tue, Mar 07, 2023 at 01:24:38AM -0800, Lucas De Marchi wrote:
>> If there is no register to save-restore or whitelist, just return. This
>> drops some noise from the log, particurlarly for platforms with several
>> engines like PVC:
>>
>> 	[drm:xe_reg_sr_apply_mmio [xe]] Applying bcs0 save-restore MMIOs
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] Whitelisting bcs0 registers
>> 	[drm:xe_reg_sr_apply_mmio [xe]] Applying bcs1 save-restore MMIOs
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] Whitelisting bcs1 registers
>> 	[drm:xe_reg_sr_apply_mmio [xe]] Applying bcs2 save-restore MMIOs
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] Whitelisting bcs2 registers
>> 	[drm:xe_reg_sr_apply_mmio [xe]] Applying bcs5 save-restore MMIOs
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] Whitelisting bcs5 registers
>> 	[drm:xe_reg_sr_apply_mmio [xe]] Applying bcs6 save-restore MMIOs
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] Whitelisting bcs6 registers
>> 	[drm:xe_reg_sr_apply_mmio [xe]] Applying bcs7 save-restore MMIOs
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] Whitelisting bcs7 registers
>> 	[drm:xe_reg_sr_apply_mmio [xe]] Applying bcs8 save-restore MMIOs
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] Whitelisting bcs8 registers
>> 	[drm:xe_reg_sr_apply_mmio [xe]] Applying ccs0 save-restore MMIOs
>> 	[drm:xe_reg_sr_apply_mmio [xe]] REG[0x20e4] = 0x00008000
>> 	[drm:xe_reg_sr_apply_mmio [xe]] REG[0xb01c] = 0x00000001
>> 	[drm:xe_reg_sr_apply_mmio [xe]] REG[0xe48c] = 0x00000800
>> 	[drm:xe_reg_sr_apply_mmio [xe]] REG[0xe7c8] = 0x40000000
>> 	...
>>
>> On the other hand, contrary to the save-restore, it was not printing the
>> registers being whitelisted, so add them now. On a PVC system it should
>> show something like:
>>
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] Whitelisting bcs7 registers
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] (slot=0) REG[0x4400] = 0x40000003
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] (slot=1) REG[0x4500] = 0x40000003
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] (slot=2) REG[0x3ec3a8] = 0x10000000
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] Whitelisting bcs8 registers
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] (slot=0) REG[0x4400] = 0x40000003
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] (slot=1) REG[0x4500] = 0x40000003
>> 	[drm:xe_reg_sr_apply_whitelist [xe]] (slot=2) REG[0x3ee3a8] = 0x10000000
>> 	[drm:xe_reg_sr_apply_mmio [xe]] Applying ccs0 save-restore MMIOs
>> 	[drm:xe_reg_sr_apply_mmio [xe]] REG[0x20e4] = 0x00008000
>> 	[drm:xe_reg_sr_apply_mmio [xe]] REG[0xb01c] = 0x00000001
>> 	[drm:xe_reg_sr_apply_mmio [xe]] REG[0xe48c] = 0x00000800
>> 	[drm:xe_reg_sr_apply_mmio [xe]] REG[0xe7c8] = 0x40000000
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_reg_sr.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c
>> index 855d86a6c43a..8ae803393ff0 100644
>> --- a/drivers/gpu/drm/xe/xe_reg_sr.c
>> +++ b/drivers/gpu/drm/xe/xe_reg_sr.c
>> @@ -169,6 +169,9 @@ void xe_reg_sr_apply_mmio(struct xe_reg_sr *sr, struct xe_gt *gt)
>>  	unsigned long reg;
>>  	int err;
>>
>> +	if (xa_empty(&sr->xa))
>> +		return;
>> +
>>  	drm_dbg(&xe->drm, "Applying %s save-restore MMIOs\n", sr->name);
>>
>>  	err = xe_force_wake_get(&gt->mmio.fw, XE_FORCEWAKE_ALL);
>> @@ -196,6 +199,9 @@ void xe_reg_sr_apply_whitelist(struct xe_reg_sr *sr, u32 mmio_base,
>>  	unsigned int slot = 0;
>>  	int err;
>>
>> +	if (xa_empty(&sr->xa))
>> +		return;
>> +
>>  	drm_dbg(&xe->drm, "Whitelisting %s registers\n", sr->name);
>>
>>  	err = xe_force_wake_get(&gt->mmio.fw, XE_FORCEWAKE_ALL);
>> @@ -203,6 +209,8 @@ void xe_reg_sr_apply_whitelist(struct xe_reg_sr *sr, u32 mmio_base,
>>  		goto err_force_wake;
>>
>>  	xa_for_each(&sr->xa, reg, entry) {
>> +		drm_dbg(&xe->drm, "(slot=%d) REG[0x%lx] = 0x%08x", slot, reg,
>> +			entry->set_bits);
>
>Printing the flags in hex form seems kind of hard to work with.  Should
>we actually interpret them so that we print a more readable message with
>ranges(when appropriate), read vs read-write listing, and allow vs deny?

ok. And then I can reuse the same logic for printing the debugfs.

I will respin this one.

thanks
Lucas De Marchi

>
>
>Matt
>
>>  		xe_mmio_write32(gt, RING_FORCE_TO_NONPRIV(mmio_base, slot).reg,
>>  				reg | entry->set_bits);
>>  		slot++;
>> --
>> 2.39.0
>>
>
>-- 
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation