metal : add backend function to check device family support #1547
Conversation
| @@ -1078,6 +1078,11 @@ static ggml_backend_t whisper_backend_init(const whisper_context_params & params | |||
| if (!backend_gpu) { | |||
| WHISPER_LOG_ERROR("%s: ggml_backend_metal_init() failed\n", __func__); | |||
| } | |||
| if (!ggml_backend_metal_supports_family(backend_gpu, 7)) { | |||
There was a problem hiding this comment.
I don't think this is correct. It should disable it on family 7 and older.
There was a problem hiding this comment.
Are you sure? M2 Ultra is Apple8 and it works fine
There was a problem hiding this comment.
Older would be Apple 6, 5, etc.
There was a problem hiding this comment.
Ah sorry, yes.
I thought somebody said that Apple7 works?
There was a problem hiding this comment.
Yes, should be disabled on family 6 and older. I got confused since it's disabling 7 here.
There was a problem hiding this comment.
Yup, let me know if you still think this needs to be changed.
The way it is written now it should do the following:
- Apple 1-6 -> fallback to CPU
- Apple 7, 8, 9, .. -> use GPU
The assumption is that if Apple7 is supported, then due to backwards compatibility all previous families (1-6) are also supported, so no need to check for those individually.
There was a problem hiding this comment.
the order seems to be wrong, we should check the compatibility first, then call ggml_backend_metal_init. otherwise it still crash on lower device.
There was a problem hiding this comment.
I think we should do something like this:
#ifdef GGML_USE_METAL
if (!ggml_backend_metal_supports_family(backend_gpu, 7)) {
WHISPER_LOG_ERROR("%s: Metal GPU does not support family 7 - falling back to CPU\n", __func__);
} else if (params.use_gpu) {
WHISPER_LOG_INFO("%s: using Metal backend\n", __func__);
ggml_metal_log_set_callback(whisper_log_callback_default, nullptr);
backend_gpu = ggml_backend_metal_init();
if (!backend_gpu) {
WHISPER_LOG_ERROR("%s: ggml_backend_metal_init() failed\n", __func__);
}
}
#endifbut right now, it's not possible, because we can't get the ggml_backend_t if it is not inited, I think there need some refactor
There was a problem hiding this comment.
This seems done, right now it do check in ggml_backend_metal_init to avoid crash
ref #1387
When Apple7 is not supported, we fallback to CPU automatically