[PATCH] drm/amd/amdgpu: Fix amdgpu_debugfs_gpr_read debugfs entry
Tom St Denis
tstdenis at amd.com
Wed Apr 11 15:03:21 UTC 2018
On 04/11/2018 08:17 AM, Christian König wrote:
> Am 11.04.2018 um 14:03 schrieb Tom St Denis:
>> On 04/11/2018 07:58 AM, Christian König wrote:
>>>> - if (size & 3 || *pos & 3)
>>>> + if (size & 3 || size > (4 * AMDGPU_DEBUGFS_MAX_SGPR_READ))
>>> I think checking the position alignment here is still necessary,
>>> cause we can't read from not dw boundaries don't we?
>> The index is a dword index as fed into SQ_IND_INDEX (offset => start
>> => regno as you trace from the debugfs entry to
>> gfx_v8_0_read_wave_sgprs to wave_read_regs (or analogues...)).
>> SQ_IND_INDEX doesn't take a byte offset but a dword offset, for instance:
>> include/asic_reg/gc/gc_9_0_offset.h:#define ixSQ_WAVE_TTMP0 0x026c
>> include/asic_reg/gc/gc_9_0_offset.h:#define ixSQ_WAVE_TTMP1 0x026d
>> include/asic_reg/gc/gc_9_0_offset.h:#define ixSQ_WAVE_TTMP2 0x026e
>> The current way for instance would prohibit reading (directly)
>> I agree it's not really how a typical file device works but it's not a
>> typical file device :-). It's assumed every read would be preceded by
>> a seek to set the higher order bits anyways.
> So pos=0 is one dw and pos=1 is another one? If that's the case then
> that would be a bug since pos is by definition a byte offset.
> I suggest to keep the limitation and instead fix how pos is interpreted
It would break copies of umr out there already (in ways that won't be
obvious and therefore result in lost time/etc). Since umr is probably
the only user of this it's not really a big deal. Ideally, everyone
uses the latest umr each day :-) but that's not realistically the case.
I agree that had I written this today I would keep the file offset
consistent with the byte offset into the register file (shift down by 4
before calling the callback).
Since you need to seek to a very specific address to use this device
it's not the sort of thing that someone would cat and then expect
sensible output anyways.
More information about the amd-gfx