On 13/10/16 12:39 AM, StDenis, Tom wrote:
> It comes from amdgpu_query_gpu_info_init()
>
>
> for (i = 0; i < (int)dev->info.num_shader_engines; i++) {
> unsigned instance = (i << AMDGPU_INFO_MMR_SE_INDEX_SHIFT) |
> (*AMDGPU_INFO_MMR_SH_INDEX_MASK*<<
> AMDGPU_INFO_MMR_SH_INDEX_SHIFT);
>
> r = amdgpu_read_mm_registers(dev, 0x263d, 1, instance, 0,
> &dev->info.backend_disable[i]);
>
> This effectively reads from 0/* where the kernel adds the instance of *
> so it's 0/*/*. That line was last changed by Alex
>
> *0936139536380* (Alex Deucher 2015-04-20 12:04:22 -0400 174)
> (AMDGPU_INFO_MMR_SH_INDEX_MASK <<
As a side note, following that to the end in the kernel code, I noticed
an interesting minor difference between the AMDGPU_INFO_READ_MMR_REG
functionality used by this code and the debugfs interface:
With AMDGPU_INFO_READ_MMR_REG, the effect is that
amdgpu_asic_read_register() doesn't call amdgpu_gfx_select_se_sh() at
all before reading the register, so the read is performed from whichever
SH instance is currently selected.
Whereas with this patch, amdgpu_debugfs_regs_read() calls
amdgpu_gfx_select_se_sh(adev, se_bank, 0xFFFFFFFF, instance_bank) before
the register read, which translates to only the SH_BROADCAST_WRITES bit
being set for the SH instance index.
The end result should be the same though, since
amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff) is
normally called after every register read.
> I still don't get why this is a reason to hit pause on the patch(es)
> though.
At the very least, it should be documented in an appropriate place
(commit log and/or code, or any other place where the debugfs interface
semantics are documented) what actually happens when passing all ones
for the SE/SH index. Does the hardware ignore the *_BROADCAST_WRITES bit
for reads, so they're performed from instance 0, or does it combine the
values from all instances with logical and/or?
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
