Hi Chris,

The info ioctl code path seems already covered at libdrm initialization time:

amdgpu_device_initialize()
-> amdgpu_queury_gpu_info_init()
    -> amdgpu_read_mm_registers()
        -> drmCommandWrite(fd, DRM_AMDGPU_INFO, ...)

BR,
Xiaojie

________________________________________
From: Christian König <[email protected]>
Sent: Tuesday, November 19, 2019 12:18 AM
To: Yuan, Xiaojie; Alex Deucher
Cc: Deucher, Alexander; [email protected]
Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read 
interface

Hi Xiaojie,

could you add that test to the unit tests we have in libdrm?

Would be rather nice to have,
Christian.

Am 18.11.19 um 15:09 schrieb Yuan, Xiaojie:
> Hi Alex,
>
> I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test 
> program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG 
> via DRM_IOCTL_AMDGPU_INFO.
>
> All read out values are valid as below (in this read cycle, I saw gfx block 
> is first awakened and then powered off again automatically):
>
> drm version: 3.36.0
> grbm_status: 0x00003028
> cp_stat: 0x00000000
> gb_addr_config: 0x00000043
>
> At least for Navi, reading register when gfxoff seems not a problem...
>
> Test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <libdrm/drm.h>
> #include <libdrm/amdgpu_drm.h>
>
> int get_reg_val(int fd, int reg_offset, int *val)
> {
>          struct drm_amdgpu_info request;
>          int r;
>
>          memset(&request, 0, sizeof(request));
>          request.return_pointer = (uintptr_t)val;
>          request.return_size = sizeof(uint32_t);
>          request.query = AMDGPU_INFO_READ_MMR_REG;
>          request.read_mmr_reg.dword_offset = reg_offset;
>          request.read_mmr_reg.count = 1;
>          request.read_mmr_reg.instance = 0xffffffff;
>          request.read_mmr_reg.flags = 0;
>
>          r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
>          if (r < 0) {
>                  perror("failed to read register");
>                  return -errno;
>          }
>
>          return 0;
> }
>
> int main(int argc, const char *argv[])
> {
>          const char *device = "/dev/dri/card0";
>          int fd;
>          int r;
>          struct drm_version version;
>          struct drm_auth auth;
>          int cp_stat;
>          int grbm_status;
>          int gb_addr_config;
>
>          fd = open(device, O_RDWR | O_CLOEXEC);
>          if (fd < 0) {
>                  perror("failed to open device\n");
>                  return -errno;
>          }
>
>          memset(&version, 0, sizeof(version));
>
>          r = ioctl(fd, DRM_IOCTL_VERSION, &version);
>          if (r < 0) {
>                  perror("failed to get drm version");
>                  r = -errno;
>                  goto close;
>          }
>
>          printf("drm version: %d.%d.%d\n", version.version_major,
>                                            version.version_minor,
>                                            version.version_patchlevel);
>
>          r = get_reg_val(fd, 0x2004, &grbm_status);
>          if (r < 0)
>                  goto close;
>          printf("grbm_status: 0x%08x\n", grbm_status);
>
>          r = get_reg_val(fd, 0x21a0, &cp_stat);
>          if (r < 0)
>                  goto close;
>          printf("cp_stat: 0x%08x\n", cp_stat);
>
>          r = get_reg_val(fd, 0x263e, &gb_addr_config);
>          if (r < 0)
>                  goto close;
>          printf("gb_addr_config: 0x%08x\n", gb_addr_config);
>
> close:
>          close(fd);
>
>          return r;
> }
>
> BR,
> Xiaojie
>
> ________________________________________
> From: amd-gfx <[email protected]> on behalf of Yuan, 
> Xiaojie <[email protected]>
> Sent: Saturday, November 16, 2019 1:02 AM
> To: Alex Deucher
> Cc: Deucher, Alexander; [email protected]
> Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read 
> interface
>
> Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon 
> function returns, gfx block is guaranteed to be in power-up state). Anyway, 
> let me confirm about that soon.
>
> BR,
> Xiaojie
>
>> On Nov 16, 2019, at 12:52 AM, Alex Deucher <[email protected]> wrote:
>>
>>> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <[email protected]> wrote:
>>>
>>> Hi Alex,
>>>
>>> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block 
>>> will be immediately powered up, so I'm not sure MMIO register access will 
>>> be successful within this time window(maybe GRBM access will be pending 
>>> until gfx block is powered up?)
>>>
>>> If you are not in a hurry to commit this fix, I can verify on my Navi 
>>> boards next Monday.
>> That would be great.  Maybe we can add a delay in that function to
>> take that into account?
>>
>> Thanks!
>>
>> Alex
>>
>>> BR,
>>> Xiaojie
>>>
>>>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <[email protected]> wrote:
>>>>
>>>> When gfxoff is enabled, accessing gfx registers via MMIO
>>>> can lead to a hang.
>>>>
>>>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
>>>> Signed-off-by: Alex Deucher <[email protected]>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> index 6ddea7607ad0..5f3b3a705b29 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, 
>>>> void *data, struct drm_file
>>>>            return -ENOMEM;
>>>>        alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>>>>
>>>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
>>>> +        amdgpu_gfx_off_ctrl(adev, false);
>>>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>>>>            if (amdgpu_asic_read_register(adev, se_num, sh_num,
>>>>                              info->read_mmr_reg.dword_offset + i,
>>>>                              &regs[i])) {
>>>>                DRM_DEBUG_KMS("unallowed offset %#x\n",
>>>>                          info->read_mmr_reg.dword_offset + i);
>>>>                kfree(regs);
>>>> +                amdgpu_gfx_off_ctrl(adev, true);
>>>>                return -EFAULT;
>>>>            }
>>>> +        }
>>>> +        amdgpu_gfx_off_ctrl(adev, true);
>>>>        n = copy_to_user(out, regs, min(size, alloc_size));
>>>>        kfree(regs);
>>>>        return n ? -EFAULT : 0;
>>>> --
>>>> 2.23.0
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> [email protected]
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to