On 13/10/16 04:20 PM, Michel Dänzer wrote:
> 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?
I'm not sure how to interpret the fact that this patch has landed
without any changes or followups.
FWIW, I'm still interested in (pointers to) information about what the
libdrm_amdgpu code above expects and what the hardware does for reads
with the broadcast bit enabled, from anyone.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
