search

clbr / radeontop

GNU General Public License v3.0
804 stars 70 forks source link

Add support for Video encode/decode utilization and GTT usage reporting #45

Closed cnamburu closed 1 year ago

cnamburu commented 7 years ago

Fixes #29. Tested on CARRIZO, STONEY and KAVERI.

Added GTT memory usage details

clbr commented 7 years ago

I've applied the GTT patch. Some points on the other one:

cnamburu commented 7 years ago

Thank you for the comments.

there's no space for two more entries in a normal 24-line terminal. They should be hidden on cards that do not have them, and maybe other adjustments are needed too

How about control it using command line arguments? If user need those details window can be resized

one binary must support all radeons, this patch would limit it to only amdgpu or only r600

Buy enabling ENABLE_AMDGPU macro both r600 and amgpu will be supported by one binary. Please correct me if I am wrong.

I dislike the string comparisons at runtime ("radeon", "amdgpu"), I'd prefer an enum var

I have reused this from existing code. I will correct that.

clbr commented 7 years ago

They may be running on a VT, which can't be resized. Need to think about the best way.

Buy enabling ENABLE_AMDGPU macro both r600 and amgpu will be supported by one binary. Please correct me if I am wrong.

No, with this patch the enums in the ENABLE_AMDGPU case would be wrong on r600. It would try to access the GRBM register 0x2004 instead of 0x8010, etc.

I have reused this from existing code. I will correct that.

Those were during startup, at that time it was fine, but yours were at runtime.

cnamburu commented 7 years ago

They may be running on a VT, which can't be resized. Need to think about the best way.

As an other way, encoder and decoder status will not be populated when not active

No, with this patch the enums in the ENABLE_AMDGPU case would be wrong on r600. It would try to access the GRBM register 0x2004 instead of 0x8010, etc. Those were during startup, at that time it was fine, but yours were at runtime.

Will modify code accordingly. Thank you

clbr commented 7 years ago

Oh, the dump mode additions are missing spaces. With the current patch, the output would be ugly "cb 0.00uvd 0.00vce0 0.00" - all together.

cnamburu commented 7 years ago

I am working on the comments. will submit new patch with changes suggested.

kode54 commented 5 years ago

Any updates on this, now that it's been broken by over a year of commits and/or merges?

E: I see GTT is already in 1.1, and therefore also master, so this patch needs to be rebased, or remade.

userofryzen commented 4 years ago

any news about it? it is very useful for laptops with ryzen, dont you think ? I don't know if the code is incomplete or what

userofryzen commented 4 years ago

@cnamburu what happened to this idea?

cnamburu commented 4 years ago

@userofryzen yes this patch will be useful. Will start modifying the patch soon.

trek00 commented 4 years ago

@cnamburu I have ported your patches to the new structure, but it has some issues to be resolved. Anyway I will put online the parts without issues in the next hours. Thank you for your work!

trek00 commented 4 years ago

@cnamburu I have pushed your patch updated to the latest git master.

It was not tested on radeon drm, the screen overflows on 80x24 terminals and si/cik on amdgpu needs a patch pending for review https://lists.freedesktop.org/archives/dri-devel/2019-September/233886.html

please to tell me if I missed something or there are issues, but I can't work on this before the next week ciao!

cnamburu commented 4 years ago

@trek00 Thank you for for updating patch to latest git master. issue in adding the vce and uvd status is that it will cross normal 24-line terminal.

an0nfunc commented 4 years ago

Can't we just use a flag to enable vce and uvd status for now? My terminals for example are mostly bigger then 24 lines, so at least the option is there.

Although I agree that a proper solution should be found, like swapping some metrics every x seconds or use more columns for some less 'important' metrics.

userofryzen commented 4 years ago

Sinceriously it is not a problem for me. But I understand that some people has problems with that. The best solution is to be able to change the metrics with a key, for example "d" or other. And change uvd and vcn with other metrics. Don't you think ? I hope this patch will merge soon( sorry for not being able to help)

cnamburu commented 4 years ago

To solve the 24 line limitation, if we have multiple frames and toggle the screen between two frames on user request. New frame can accommodate all future requests. @clbr can this be supported ?

clbr commented 4 years ago

Yes, a button to switch "windows" should work.

trek00 commented 4 years ago

may be this patch is sufficient to fix the issue at least for now, but in the future i would like to make something like top, with up and down arrow keys to scroll the fields

trek00/radeontop@c706c98

edgecase14 commented 3 years ago

I'd like to get UVD/VCE stats working on Ryzen 5 2600G (Vega graphics)... I tried trek00/radeontop branches drm4 and drm5, as well as clbr/radeontop (doesn't seems to contain this code) Trek00's gives:

Failed to get UVD usage: error 14 (Bad address)
Failed to get VCE usage: error 14 (Bad address)
Collecting data, please wait....

I also have an RX570 I could try, if that is better supported? This is on Ubuntu Focal Fossa