[PATCH umr] Add ability to read/write SMC registers directly

[Tom St Denis]

On 15/06/17 02:09 PM, Deucher, Alexander wrote:
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
>> Of Tom St Denis
>> Sent: Thursday, June 15, 2017 12:54 PM
>> To: amd-gfx at lists.freedesktop.org
>> Cc: StDenis, Tom
>> Subject: [PATCH umr] Add ability to read/write SMC registers directly
>> 
>> On SI..VI platforms this allows access to SMC registers without kernel access.
>> 
>> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
>> ---
>>  src/app/main.c        |   4 +-
>>  src/app/scan.c        |   4 +-
>>  src/lib/mmio.c        | 112 +++++++++++++++++++++++++++++++++++++++-
>> ----------
>>  src/lib/read_sgpr.c   |   4 +-
>>  src/lib/read_vram.c   |   6 +--
>>  src/lib/wave_status.c |   8 ++--
>>  src/umr.h             |   4 +-
>>  7 files changed, 104 insertions(+), 38 deletions(-)
>> 
>> diff --git a/src/app/main.c b/src/app/main.c
>> index 60bf20480fd3..bcca76225727 100644
>> --- a/src/app/main.c
>> +++ b/src/app/main.c
>> @@ -224,7 +224,7 @@ int main(int argc, char **argv)
>>                                if (!asic)
>>                                        asic = get_asic();
>>                                if (!memcmp(argv[i+1], "0x", 2) &&
>> sscanf(argv[i+1], "%"SCNx32, &reg) == 1 && sscanf(argv[i+2], "%"SCNx32,
>> &val) == 1)
>> -                                     umr_write_reg(asic, reg, val);
>> +                                     umr_write_reg(asic, reg, val,
>> REG_MMIO);
>>                                else
>>                                        umr_set_register(asic, argv[i+1],
>> argv[i+2]);
>>                                i += 2;
>> @@ -271,7 +271,7 @@ int main(int argc, char **argv)
>>                                        asic = get_asic();
>> 
>>                                if (!memcmp(argv[i+1], "0x", 2) &&
>> sscanf(argv[i+1], "%"SCNx32, &reg) == 1) {
>> -                                     reg = umr_read_reg(asic, reg);
>> +                                     reg = umr_read_reg(asic, reg,
>> REG_MMIO);
>>                                        printf("0x%08lx\n", (unsigned
>> long)reg);
>>                                } else {
>>                                        str = strstr(argv[i+1], ".");
>> diff --git a/src/app/scan.c b/src/app/scan.c
>> index 0e1f9e3f94b5..29a3e46ba3f7 100644
>> --- a/src/app/scan.c
>> +++ b/src/app/scan.c
>> @@ -87,10 +87,10 @@ int umr_scan_asic(struct umr_asic *asic, char
>> *asicname, char *ipname, char *reg
>>                                                                r = -1;
>>                                                                goto error;
>>                                                        }
>> -                                             } else if (asic->blocks[i]-
>> >regs[j].type == REG_MMIO) {
>> +                                             } else if (asic->blocks[i]-
>> >regs[j].type == REG_MMIO || asic->blocks[i]->regs[j].type == REG_SMC) {
>>                                                        if (options.use_bank
>> && options.no_kernel)
>> 
>>        umr_grbm_select_index(asic, options.se_bank, options.sh_bank,
>> options.instance_bank);
>> -                                                     asic->blocks[i]-
>> >regs[j].value = umr_read_reg(asic, asic->blocks[i]->regs[j].addr * 4);
>> +                                                     asic->blocks[i]-
>> >regs[j].value = umr_read_reg(asic, asic->blocks[i]->regs[j].addr * (asic-
>> >blocks[i]->regs[j].type == REG_MMIO ? 4 : 1), asic->blocks[i]->regs[j].type);
>>                                                        if (options.use_bank
>> && options.no_kernel)
>> 
>>        umr_grbm_select_index(asic, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF);
>>                                                }
>> diff --git a/src/lib/mmio.c b/src/lib/mmio.c
>> index 22110e09a9b4..58f69fc56bd5 100644
>> --- a/src/lib/mmio.c
>> +++ b/src/lib/mmio.c
>> @@ -24,51 +24,117 @@
>>   */
>>  #include "umr.h"
>> 
>> -uint32_t umr_read_reg(struct umr_asic *asic, uint64_t addr)
>> +static uint32_t umr_smc_read(struct umr_asic *asic, uint64_t addr)
>> +{
>> +     switch (asic->config.gfx.family) {
>> +             case 120: // CIK
>> +             case 110: // SI
>> +                     umr_write_reg_by_name(asic,
>> "mmSMC_IND_INDEX_0", addr);
>> +                     return umr_read_reg_by_name(asic,
>> "mmSMC_IND_DATA_0");
>> +             case 130: // VI
>> +                     umr_write_reg_by_name(asic,
>> "mmSMC_IND_INDEX_11", addr);
>> +                     return umr_read_reg_by_name(asic,
>> "mmSMC_IND_DATA_11");
>> +             case 135: // CZ
>> +                     umr_write_reg_by_name(asic,
>> "mmMP0PUB_IND_INDEX", addr);
>> +                     return umr_read_reg_by_name(asic,
>> "mmMP0PUB_IND_DATA");
> 
> You could use another instance of these accessors to avoid clashing with 
> the driver.
> 
> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

I sent out a v2 before I saw your RB line... :-)

Thanks,
Tom