Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

${nvidiabar mtrfreq[cur]} is always at 100% #1177

Closed
z1atk0 opened this issue Feb 24, 2022 · 10 comments
Closed

${nvidiabar mtrfreq[cur]} is always at 100% #1177

z1atk0 opened this issue Feb 24, 2022 · 10 comments
Labels
bug Bug report or bug fix PR display: x11 Issue related to X11 backend good first issue Great issues for first-time contributors nvidia Issue or PR related to nvidia cards text Issue or PR related to `conky.text` variables

Comments

@z1atk0
Copy link
Contributor

z1atk0 commented Feb 24, 2022

Issue

An ${nvidiabar mtrfreq[cur]} is always drawn full, ie. at 100%. This is due to a logic bug in the get_nvidia_barval() function in src/nvidia.cc:

      case ATTR_FREQS_STRING:  // mtrfreq (calculate out of memfreqmax)
        if (strcmp(nvs->token, "memTransferRate") != 0) {
          // Just in case error for silly devs
          CRIT_ERR(nullptr, NULL,
                   "%s: attribute is 'ATTR_FREQS_STRING' but token is not "
                   "\"memTransferRate\" (arg: '%s')",
                   nvs->command, nvs->arg);
          return 0;
        }
        temp1 =
            get_nvidia_string_value(nvs->target, ATTR_FREQS_STRING, nvs->token,
                                    SEARCH_MAX, nvs->target_id, nvs->arg);
        temp2 = get_nvidia_string_value(nvs->target, ATTR_PERFMODES_STRING,
                                        (char *)"memTransferRatemax",
                                        SEARCH_MAX, nvs->target_id, nvs->arg);
        if (temp2 > temp1) temp1 = temp2;  // extra safe here
        value = ((float)temp1 * 100 / (float)temp2) + 0.5;
        break;

My GPU has three different performance levels 0, 1 and 2, with corresponding Graphics Clock & Memory Transfer Rate frequencies of 324 MHz & 648 MHz, 549 MHz & 3600 MHz, and 928 MHz & 5400 MHz, respectively:
Screenshot from 2022-02-24 17-58-39

In terms of conky variable values, this corresponds to ${nvidia mtrfreqmin} = 648, ${nvidia mtrfreqmax} = 5400, and ${nvidia mtrfreq[cur]} being either 648, 3600 or 5400, depending on what performance mode the GPU is currently in. Now with this in mind, the logic bug immediately becomes obvious:

        temp1 =
            get_nvidia_string_value(nvs->target, ATTR_FREQS_STRING, nvs->token,
                                    SEARCH_MAX, nvs->target_id, nvs->arg);
        temp2 = get_nvidia_string_value(nvs->target, ATTR_PERFMODES_STRING,
                                        (char *)"memTransferRatemax",
                                        SEARCH_MAX, nvs->target_id, nvs->arg);
        if (temp2 > temp1) temp1 = temp2;  // extra safe here

temp1 gets initialized with the current MTR frequency (ie. 648, 3600 or 5400), and temp2 gets initialized with the maximum possible MTR frequency the card is capable of, ie. 5400. Therefore, the if() clause is always true (except when the card is actually running at 5400 MHz, but then temp1 and temp2 are already equal anyway), temp1 gets set to 5400, and the bar value is always 5400/5400 = 1.00 or 100%. Changing the if() clause to

        if (temp1 > temp2) temp1 = temp2;  // extra safe here

(which probably was the original intention anyway, I guess) fixes the problem.

Information

conky-1.12.2, X.Org-1.18.3, NVIDIA-Linux-x86-390.147, NVIDIA-libXNVCtrl-390.138, gcc-/g++-9.2.0 on Slackware-14.2 (32bit).

conky.text = [[
[...]
$font0 GPU: ${color grey}${nvidia temp}°C @ Mode ${nvidia perfmode}, Level ${nvidia perflevelcur} ${nvidiagraph temp 8,144 00ff00 ff0000 100 -t 0}
$color Util%:   $color Fan: ${color grey}${nvidia fanlevel}% @ ${nvidia fanspeed}rpm ${nvidiabar 6,0 fanlevel}
  ${color grey}GPU    ${nvidiabar 6,120 gpuutil}$color Freq (MHz):${color grey}
  MemBW  ${nvidiabar 6,120 membwutil} GPU ${nvidiabar 6,0 gpufreqcur}
  VEng   ${nvidiabar 6,120 videoutil} Mem ${nvidiabar 6,0 memfreqcur}
  PCIeBW ${nvidiabar 6,120 pcieutil} MTR ${nvidiabar 6,0 mtrfreqcur}
$color Mem: ${color grey}${nvidia mem} MB (${nvidia memutil}%) used, ${nvidia memavail} MB free, ${nvidia memtotal} MB total
  ${nvidiagraph memutil 8,378 00ff00 ff0000 100 -t 0}
[...]
]];
@github-actions
Copy link

This issue is stale because it has been open 365 days with no activity. Remove stale label or comment, or this issue will be closed in 30 days.

@github-actions github-actions bot added the Stale Issue that requires attention because it hasn't been updated for over a year label Feb 25, 2023
@z1atk0
Copy link
Contributor Author

z1atk0 commented Mar 4, 2023

Still present in conky-1.18.1.

@github-actions github-actions bot removed the Stale Issue that requires attention because it hasn't been updated for over a year label Mar 5, 2023
Copy link

github-actions bot commented Mar 5, 2024

This issue is stale because it has been open 365 days with no activity. Remove stale label or comment, or this issue will be closed in 30 days.

@github-actions github-actions bot added the Stale Issue that requires attention because it hasn't been updated for over a year label Mar 5, 2024
@z1atk0
Copy link
Contributor Author

z1atk0 commented Mar 5, 2024

Still present in conky-1.19.8.

@brndnmtthws
Copy link
Owner

Would you like to open a PR for this?

@z1atk0
Copy link
Contributor Author

z1atk0 commented Mar 6, 2024

I'm not very git savvy, but I'll try & give it a go. Maybe I'll even manage before the next "issue is stale" warning comes around. 🙂

@github-actions github-actions bot removed the Stale Issue that requires attention because it hasn't been updated for over a year label Mar 19, 2024
@Caellian Caellian added bug Bug report or bug fix PR good first issue Great issues for first-time contributors display: x11 Issue related to X11 backend nvidia Issue or PR related to nvidia cards text Issue or PR related to `conky.text` variables labels Apr 12, 2024
@w0j0pl
Copy link
Contributor

w0j0pl commented Aug 25, 2024

Is there still a need to change this? I can do PR with the proposed change if needed ;)

@z1atk0
Copy link
Contributor Author

z1atk0 commented Aug 25, 2024

From my point of view, yes, the problem is still present in the current release. I keep the patch around locally to fix this, that's why I still haven't got around yet to do the PR myself - whenever a new version is released, I just apply the patch and be done with it. Much easier for me than fooling around with PRs (and git in general, as I said 😇).

So yes, if you could do the PR it would be very much appreciated. 👍 🙂

w0j0pl added a commit to w0j0pl/conky that referenced this issue Aug 25, 2024
Made a suggested change in logic from issue brndnmtthws#1177
@w0j0pl
Copy link
Contributor

w0j0pl commented Aug 25, 2024

Would you like to open a PR for this?

I made a commit. It's my first ever commit to the project like that, so I don't know if I did everything correct.
I didn't change a doc, because imo there is nothing to change.
I also didn't test it, because tbh, I have no idea how should I do this, but as @z1atk0 said, he made this change by himself locally, so I guess, he tested this.

brndnmtthws pushed a commit that referenced this issue Aug 27, 2024
Made a suggested change in logic from issue #1177
@brndnmtthws
Copy link
Owner

Should (hopefully) be fixed with #2018

brndnmtthws pushed a commit that referenced this issue Aug 29, 2024
* Update nvidia.cc

Made a suggested change in logic from issue #1177

* Update nvidia.cc

Made a suggested change in logic from issue #1178

* Update nvidia.cc

Made a suggested change in logic from issue #1178
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report or bug fix PR display: x11 Issue related to X11 backend good first issue Great issues for first-time contributors nvidia Issue or PR related to nvidia cards text Issue or PR related to `conky.text` variables
Projects
None yet
Development

No branches or pull requests

4 participants