[PATCH 2/2] xfree86: attempt to autoconfig gpu slave devices

[Aaron Plattner]

On 03/31/2015 12:12 AM, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> This allows us to skip the screen section, the first
> Device section will get assigned to the screen,
> any remaining ones will get assigned to the GPUDevice
> sections for the screen.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>   hw/xfree86/common/xf86Config.c | 31 ++++++++++++++++++++++++++-----
>   1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
> index 8992208..a1ab7b5 100644
> --- a/hw/xfree86/common/xf86Config.c
> +++ b/hw/xfree86/common/xf86Config.c
> @@ -1825,13 +1825,34 @@ configScreen(confScreenPtr screenp, XF86ConfScreenPtr conf_screen, int scrnum,
>           screenp->device = NULL;
>       }
>
> -    for (i = 0; i < conf_screen->num_gpu_devices; i++) {
> -        screenp->gpu_devices[i] = xnfcalloc(1, sizeof(GDevRec));
> -        if (configDevice(screenp->gpu_devices[i], conf_screen->scrn_gpu_devices[i], TRUE, TRUE)) {
> -            screenp->gpu_devices[i]->myScreenSection = screenp;
> +    if (conf_screen->num_gpu_devices == 0) {
> +        XF86ConfDevicePtr sdevice = xf86configptr->conf_device_lst->list.next;
> +
> +        for (i = 0; i < 4; i++) {
> +
> +            if (!sdevice)
> +                break;
> +            FIND_SUITABLE (XF86ConfDevicePtr, sdevice, conf_screen->scrn_gpu_devices[i]);
> +            if (!conf_screen->scrn_gpu_devices[i])
> +                break;
> +            screenp->gpu_devices[i] = xnfcalloc(1, sizeof(GDevRec));
> +            if (configDevice(screenp->gpu_devices[i], conf_screen->scrn_gpu_devices[i], TRUE, TRUE)) {
> +                screenp->gpu_devices[i]->myScreenSection = screenp;
> +            }
> +            sdevice = sdevice->list.next;

If FIND_SUITABLE decides that sdevice isn't suitable but some later 
device is, won't this loop end up finding the same suitable device 
multiple times?  It seems like this should be

   sdevice = conf_screen->scrn_gpu_devices[i]->list.next;

instead since we know that none of the devices between sdevice and 
conf_screen->scrn_gpu_devices[i] were suitable.

-- Aaron

>           }
> +        screenp->num_gpu_devices = i;
> +
> +    } else {
> +        for (i = 0; i < conf_screen->num_gpu_devices; i++) {
> +            screenp->gpu_devices[i] = xnfcalloc(1, sizeof(GDevRec));
> +            if (configDevice(screenp->gpu_devices[i], conf_screen->scrn_gpu_devices[i], TRUE, TRUE)) {
> +                screenp->gpu_devices[i]->myScreenSection = screenp;
> +            }
> +        }
> +        screenp->num_gpu_devices = conf_screen->num_gpu_devices;
>       }
> -    screenp->num_gpu_devices = conf_screen->num_gpu_devices;
> +
>       screenp->options = conf_screen->scrn_option_lst;
>
>       /*
>