On 4/23/19 11:56 AM, Ken Thomases wrote:
On Apr 22, 2019, at 7:13 AM, Zhiyi Zhang zzhang@codeweavers.com wrote:
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com
Maybe Mac driver would draw more attention :)
dlls/winemac.drv/cocoa_display.m | 168 +++++++++++++++++++++++++++++++++++++++ dlls/winemac.drv/macdrv_cocoa.h | 17 ++++ 2 files changed, 185 insertions(+)
diff --git a/dlls/winemac.drv/cocoa_display.m b/dlls/winemac.drv/cocoa_display.m index 93a0fbca35..569cd32352 100644 --- a/dlls/winemac.drv/cocoa_display.m +++ b/dlls/winemac.drv/cocoa_display.m @@ -19,6 +19,7 @@ */
#import <AppKit/AppKit.h> +#import <Metal/Metal.h> #include "macdrv_cocoa.h"
@@ -103,3 +104,170 @@ void macdrv_free_displays(struct macdrv_display* displays) { free(displays); }
+/***********************************************************************
get_entry_property_uint32
- Get an IO registry entry property of type uint32 and store it in value parameter.
- Return 0 on failure.
Nothing actually uses the return value.
- */
+static uint32_t get_entry_property_uint32(io_registry_entry_t entry, CFStringRef property_name, uint32_t* value) +{
- CFDataRef data = IORegistryEntrySearchCFProperty(entry, kIOServicePlane, property_name, kCFAllocatorDefault,
kIORegistryIterateRecursively | kIORegistryIterateParents);- if (!data)
return 0;You should check that `data` is actually a CFData reference by comparing CFGetTypeID(data) == CFDataGetTypeID(). You should also check that its length == sizeof(uint32_t).
- const uint32_t* bytes = (const uint32_t*)(CFDataGetBytePtr(data));
- *value = *bytes;
Probably better to use CFDataGetBytes() to copy the bytes out to value. The byte pointer might not be aligned properly and that's not really kosher.
- CFRelease(data);
- return 1;
+}
+/***********************************************************************
get_entry_property_string
- Get an IO registry entry property of type string and write it in buffer parameter.
- Return the size written into the buffer.
Again, nothing uses the return value.
- */
+static size_t get_entry_property_string(io_registry_entry_t entry, CFStringRef property_name, char* buffer,
size_t buffer_size)+{
- size_t length;
- CFDataRef data = IORegistryEntrySearchCFProperty(entry, kIOServicePlane, property_name, kCFAllocatorDefault,
kIORegistryIterateRecursively | kIORegistryIterateParents);- if (!data)
return 0;Again, verify that it's a CFData.
- length = CFDataGetLength(data);
- if (length > buffer_size)
return 0;This early return leaks `data`.
- memcpy(buffer, CFDataGetBytePtr(data), length);
Again, probably best to use CFDataGetBytes().
The character buffer in `data` is not guaranteed to be null-terminated. For one of the GPUs in my Mac, it's not. So, the length check should account for an extra byte and then you should manually null-terminate `buffer`.
- CFRelease(data);
- return length;
+}
+/***********************************************************************
macdrv_get_gpu_info_from_entry
- Get GPU information from an IO registry entry.
- */
+static void macdrv_get_gpu_info_from_entry(struct macdrv_gpu* gpu, io_registry_entry_t entry) +{
- get_entry_property_uint32(entry, CFSTR("vendor-id"), &gpu->vendor_id);
- get_entry_property_uint32(entry, CFSTR("device-id"), &gpu->device_id);
- get_entry_property_uint32(entry, CFSTR("subsystem-id"), &gpu->subsys_id);
- get_entry_property_uint32(entry, CFSTR("revision-id"), &gpu->revision_id);
- get_entry_property_string(entry, CFSTR("model"), gpu->name, sizeof(gpu->name));
+}
+/***********************************************************************
macdrv_get_gpu_info_from_display_id
- Get GPU information from a display id.
- This is a fallback for 32bit build since Metal doesn't support 32bit.
- Thus registryID can't be used.
- */
+void macdrv_get_gpu_info_from_display_id(struct macdrv_gpu* gpu, CGDirectDisplayID display_id)
This should be static.
+{
- io_registry_entry_t entry = CGDisplayIOServicePort(display_id);
On my Mac, this entry seems to be an IOFramebuffer entry a couple of levels below the GPU device entry. You should probably traverse the parent chain to look for an IOPCIDevice entry.
To the extent that this works, it's probably because of the kIORegistryIterateParents option in the property search. I worry that an intervening entry with a same-named property might break things.
Yes, it relies on kIORegistryIterateParents in the property search. The parent of the an framebuffer is the GPU.
- macdrv_get_gpu_info_from_entry(gpu, entry);
+}
+/***********************************************************************
macdrv_get_gpu_info_from_registry_id
- Get GPU information from a Metal device registry id.
- */
+void macdrv_get_gpu_info_from_registry_id(struct macdrv_gpu* gpu, uint64_t registry_id)
Should be static.
+{
- /* entry is IOGraphicsAccelerator. Parent of the entry is the GPU */
- io_registry_entry_t entry =
IOServiceGetMatchingService(kIOMasterPortDefault, IORegistryEntryIDMatching(registry_id));- if (!entry)
return;- io_registry_entry_t parent;
- if (IORegistryEntryGetParentEntry(entry, kIOServicePlane, &parent) == kIOReturnSuccess)
If you implement the search for an IOPCIDevice ancestor I recommended above, you should probably reuse it here, too. I'm not sure how reliable it is that the GPU is the immediate parent.
Parent is already the GPU here. MoltenVK uses this method too. I am not sure GPU is the immediate parent too but I don't think Apple would change this often? Anyway, searching for an IOPCIDevice entry makes sense, I will use that.
- {
macdrv_get_gpu_info_from_entry(gpu, entry);
Ehh, this should be macdrv_get_gpu_info_from_entry(gpu, parent);
In any case, you didn't use `parent` in this call so looking it up was pointless. ;) Again, this probably worked because of kIORegistryIterateParents.
IOObjectRelease(parent);- }
- IOObjectRelease(entry);
+}
+/***********************************************************************
macdrv_get_gpus
- Get a list of GPU currently in the system. The first GPU is primary.
"list of GPUs" (plural)
- Call macdrv_free_gpus() when you are done using the data.
- Return -1 on failure with parameters unchanged.
- */
+int macdrv_get_gpus(struct macdrv_gpu** new_gpus, int* count) +{
- struct macdrv_gpu* gpus;
- CGDirectDisplayID primary_display;
- id<MTLDevice> primary_device;
- int primary_index = 0;
- int gpu_count;
- int i;
- NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
- NSArray<id<MTLDevice>>* devices = MTLCopyAllDevices();
You leak this array. You need to either [devices release] at the end or [devices autorelease] here.
Thanks. I thought putting a [[NSAutoreleasePool alloc] init] should be enough to tell object-c to auto release it memory.
- gpu_count = devices.count ? devices.count : 1;
- gpus = calloc(gpu_count, sizeof(*gpus));
- if (!gpus)
return -1;- primary_display = CGMainDisplayID();
- primary_device = CGDirectDisplayCopyCurrentMetalDevice(primary_display);
Similarly, you leak this device object.
There's a question as to what should be considered the primary GPU. On Macs with automatic graphics switching, the integrated GPU might be current for the main display at initialization time. We'd then consider that primary. If we ever support apps specifying which GPU they want to use for graphics, we probably don't want to tell them that the integrated GPU is primary.
The primary GPU here is the one that drives the main display, which is where the dock is shown. The primary concept is the Windows one, where you choose a monitor to be primary.
I'm not sure there's a good fix, though. Metal doesn't let us list which devices could theoretically drive which displays. MTLDevice does have the lowPower property, but that doesn't quite tell us what we need to know.
I've read somewhere that calling MTLCreateSystemDefaultDevice() will provoke a switch to the discrete GPU. We could do that to determine what GPU will be primary if we're doing Metal or OpenGL rendering, but I don't know if our process will allow a switch back to the integrate GPU once we release that device or if it's marked as needing the discrete GPU for the rest of its life.
- /* 32bit build. Metal is unsupported. Report only one GPU. Use the primary display to get GPU info */
- if (!devices.count)
- {
gpus[0].id = 0;macdrv_get_gpu_info_from_display_id(&gpus[0], primary_display);Hmm. Why only report one GPU for this case? Given that we can look up an IOPCIDevice entry from a display ID, we could enumerate all of the display IDs and accumulate all of the unique GPUs.
This also means we might not need Metal support at all. Is io_registry_entry_t can be used to comparison or it is just a temporary handle? I am not sure.
- }
- else
- {
for (i = 0; i < devices.count; i++){gpus[i].id = devices[i].registryID;macdrv_get_gpu_info_from_registry_id(&gpus[i], gpus[i].id);if (gpus[i].id == primary_device.registryID)primary_index = i;}/* Make sure the first GPU is primary */if (primary_index){struct macdrv_gpu tmp;tmp = gpus[0];gpus[0] = gpus[primary_index];gpus[primary_index] = tmp;}- }
- [pool release];
- *new_gpus = gpus;
- *count = gpu_count;
- return 0;
+}
+/***********************************************************************
macdrv_free_gpus
- Free a GPU list allocated from macdrv_get_gpus()
- */
+void macdrv_free_gpus(struct macdrv_gpu* gpus) +{
- free(gpus);
+}