Re: [PATCH 1/7] winemac.drv: Add macdrv_get_gpus() helper.

[Zhiyi Zhang]

On 4/23/19 12:15 AM, Chip Davis wrote:
> April 22, 2019 7:14 AM, "Zhiyi Zhang"  wrote:
> > 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>
>
> This needs to be guarded with HAVE_METAL_METAL_H.
> > #include "macdrv_cocoa.h"
> >
> > @@ -103,3 +104,170 @@ void macdrv_free_displays(struct macdrv_display* displays)
>
> [...]
> > +/***********************************************************************
> > -
> > -
> >          macdrv_get_gpus
> > -
> > -
> > -
> > - Get a list of GPU currently in the system. The first GPU is primary.
> > -
> > - 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;
>
> This too.
> > - int primary_index = 0;
> > - int gpu_count;
> > - int i;
> > - NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
> > -
> > - NSArray<id<MTLDevice>>* devices = MTLCopyAllDevices();
>
> And this. You may want to consider an alternative method of finding all the GPUs, to avoid a bunch of #ifdefs.
> > - 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);
> > -
> > - /* 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);
> > - }
> > - else
> > - {
> > -
> >    for (i = 0; i < devices.count; i++)
> > -
> >    {
> > -
> >        gpus[i].id = devices[i].registryID;
>
> The 'registryID' property is only guaranteed to be present on 10.13 and up. You'll need to check for it before using it, or else you'll take an exception.

Thanks. I will look into if I need Metal support at all.
> Chip