amdgpu support for GRBM readings

trek00 commented 4 years ago

New kernels can be configured with secure boot lockdown on /dev/mem, thus radeontop is no more running on the amdgpu driver. This patch set adds the support for readings GRBM via drm on amdgpu.

It needs some refinement, but it's seems to me a good starting point. It is tested on KAVERI with full opensource stack with drm and /dev/mem access. No tests was made with closed source drivers nor radeon drm driver.

Comments and suggestions are appreciated :)

clbr commented 4 years ago

The last commit breaks VRAM reporting on radeon, looking into it. Otherwise this looks fine.

trek00 commented 4 years ago

On Wed, 31 Jul 2019 22:29:30 -0700 clbr notifications@github.com wrote:

Merged #84 into master.

thank you for fast reply, review & commit

sorry for the bug, but I submitted patches mostly for review and to know if it would be acceptable before to do further work, but I was tired and I missed to write it

I want to add some remarks to these patches:

I haven't tested the mclk/sclk reporting as it doesn't report anything on my chipset, but it does on gallium_hud, so I should dig further
using amdgpu_query_sensor_info() it needs libdrm 2.4.79, whereas querying AMDGPU_INFO_SENSOR only 2.4.77, so with my commit we need newer library, but it was released only four days later so I don't think it will make too much trouble to the users
I never used closed source drivers, but reading radeontop code it seems to me they are compatible with drm radeon interface: is this correct and so this code path needs to be preserved? (on the amdgpu-pro side it could not differ as it should use amdgpu drm)

thanks again and ciao!

clbr commented 4 years ago

The closed drivers are only compatible with the /dev/mem interface (speaking about fglrx, not the user-side-only amdgpu-pro-whatever).

trek00 commented 4 years ago

thank you for reply, but then I don't understand this commit https://github.com/clbr/radeontop/commit/2499679fda60c3f6239886296fd2a74155f45f77

I'm asking only to not throw out nor break some supported feature, as I'm refactoring device.c code, to split driver specific code into separate files and then add some new feature

ciao!

clbr commented 4 years ago

That was a contribution, I've never used fglrx myself. Presumably he intended to add support for fglrx-specific queries.

clbr / radeontop

amdgpu support for GRBM readings #84