[PATCH] drm/amd/amdgpu: Allow broadcast on debugfs read (v2)

[Michel Dänzer]

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