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

Features from lasybear/sph-sgminer_x11mod #217

Closed
wants to merge 5 commits into from
Closed

Features from lasybear/sph-sgminer_x11mod #217

wants to merge 5 commits into from

Conversation

tupieurods
Copy link
Contributor

Extend time to sick state
Gpu sets idle, if all pools dead
Total uptime length in first string
show-coindiff option
P.S. Sorry for second pull request, first time selected wrong branch

@tupieurods
Copy link
Contributor Author

Can i get a list of required fixes in my commits? As i understand first thing: return WATCHDOG_SICK_TIME to 120.

@tupieurods
Copy link
Contributor Author

What should i fix next?

@mrbrdo
Copy link
Contributor

mrbrdo commented Jun 3, 2014

It looks much better now.
I don't understand the use case of the exit frequencies ("idle"). Why is this useful, do you expect long periods of time where the GPUs are idling? I don't see why this would happen.

@tupieurods
Copy link
Contributor Author

Internet crash, for example. Or user configured just one/two pools for mining and it's offline.

@mrbrdo
Copy link
Contributor

mrbrdo commented Jun 3, 2014

And the point is just to save power?

@tupieurods
Copy link
Contributor Author

Yes. Actually, i don't care about electricity, because i pay 0.04$ per kW/h. But someone pays more, a lot of more.

@mrbrdo
Copy link
Contributor

mrbrdo commented Jun 3, 2014

I don't think this is the right approach to this. We can simply disable all devices. Perhaps it's also necessary to shutdown threads, but I don't think so. Is this even an actual problem, do the devices actually work on anything if there is no new work from pool? They don't just idle?

@tupieurods
Copy link
Contributor Author

No, they don't work. Tried to unplug internet connection: clocks dropped, gpu usage: 0.

@mrbrdo
Copy link
Contributor

mrbrdo commented Jun 3, 2014

so then there is no need for that change?

@tupieurods
Copy link
Contributor Author

I'm using sgminer version with this fix. I will try to run version without.

@tupieurods
Copy link
Contributor Author

Same behavior. Maybe depends from driver, looks like we don't need it. Should i remove it or you will remove it in merger process?
Also i have question about git usage. Currently my fork isn't updated to this repo(x13mod added), should i do merge in my fork or rebase?

@mrbrdo
Copy link
Contributor

mrbrdo commented Jun 3, 2014

I'll take care of it in the evening.

Normally you would do a rebase yeah, and force push that. Similarly you could rebase and update existing commits if needed, instead of adding new commits.

@veox veox modified the milestones: 6.0, 5.0 Jun 3, 2014
@tupieurods
Copy link
Contributor Author

Sorry, did some experiments with my git and as result i lost all your comments in commits. I'm so sorry.
Restored some of them(i lost only my answers, because github didn't mail them to me):
_[sgminer] show-coindiff option added_
mrbrdo: you are hardcoding values that are not hardcoded anymore. that's the whole point of this branch
mrbrdo:man.. there is numerator in algorithm struct... see for example https://github.com/tupieurods/sgminer/blob/v5_0/sgminer.c#L4136
Also, please rename this new function to get_work_blockdiff and use it in set_blockdiff instead of having the same code 2 times.
_[sgminer] Extend time to sick state_
mrbrdo:How does this affect Scrypt and other coins? Why is this necessary in the first place?
mrbrdo:It could indicate some other issue. Also some user is talking about stratum issues with nicehash but this has been fixed in this build. Also just because this was a problem with the original build doesn't mean it's necessarily a problem with this one. Unless we are sure what is going on I am not happy with choosing arbitrary values to fix something.
mrbrdo:good catch, it seems it should be 120 and say 2 minutes.

@mrbrdo
Copy link
Contributor

mrbrdo commented Jun 4, 2014

Yeah no worries, this is normal if you rebase and force push...

@mrbrdo
Copy link
Contributor

mrbrdo commented Jun 4, 2014

I will merge all except GPU idle thing. Also next time please base your stuff on v5_0 branch instead of master.

@mrbrdo
Copy link
Contributor

mrbrdo commented Jun 4, 2014

@mrbrdo mrbrdo closed this Jun 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants