[Mesa-dev] [PATCH 05/16] loader: reimplement loader_get_user_preferred_fd via libdrm

[Emil Velikov]

On 11 October 2016 at 19:54, Axel Davy <axel.davy at ens.fr> wrote:
> On 11/10/2016 20:31, Emil Velikov wrote:
>>
>>   -   udev = udev_new();
>> -   if (!udev)
>> -      goto prime_clean;
>> +   if (drmGetDevice(default_fd, &device) != 0)
>> +      goto err;
>>   -   default_device_id_path_tag = get_id_path_tag_from_fd(udev,
>> default_fd);
>> -   if (!default_device_id_path_tag)
>> -      goto udev_clean;
>> -
>> -   is_different_device = 1;
>>      /* two format are supported:
>>       * "1": choose any other card than the card used by default.
>>       * id_path_tag: (for example "pci-0000_02_00_0") choose the card
>>       * with this id_path_tag.
>>       */
>>      if (!strcmp(prime,"1")) {
>> -      free(prime);
>> -      prime = strdup(default_device_id_path_tag);
>> -      /* request a card with a different card than the default card */
>> -      another_tag = 1;
>> -   } else if (!strcmp(default_device_id_path_tag, prime))
>> -      /* we are to get a new fd (render-node) of the same device */
>> -      is_different_device = 0;
>> +      /* Hmm... detection for 2-7 seems to be broken. Nicely done. */
>
> For DRI2, DRI_PRIME takes a number corresponding to the device number as
> configured in xorg (and you have xrandr commands to configure the
> behaviour). This doesn't work with the DRI3 scheme. However for
> retrocompatibility, after several discussions, we decided to let DRI2's most
> used command 'DRI_PRIME=1' to still sorta work on DRI3, but the meaning
> changed: DRI_PRIME=1 => 'give me any device that is not the compositor/xorg
> device'
No argument/objection against DRI_PRIME with DRI3, yet the current
hard coded DRI_PRIME=1 _only_ check is iffy. As per the spec there can
be 8 devices (0-8) with 0 always being the xorg/compositor one and 1-7
as 'other' devices. Yet here we only consider 1 and ignore everything
else :-\

>>
>> +      is_different_device = 1;
>> +   } else {
>> +      default_tag = drm_construct_id_path_tag(device);
>> +      if (default_tag == NULL)
>> +         goto err;
>>   -   device_name = get_render_node_from_id_path_tag(udev,
>> -                                                  prime,
>> -                                                  another_tag);
>> -   if (device_name == NULL) {
>> +      if (strcmp(default_tag, prime) != 0) {
>> +         free(default_tag);
>> +         goto err;
>> +      }
>> +
>> +      free(default_tag);
>> +      /* we are to get a new fd (render-node) of the same device */
>>         is_different_device = 0;
>> -      goto default_device_clean;
>> +      // XXX: WTF ^^ so one uses the new model only to point to the exact
>> same
>> +      // device and not the other more/less powerful GPU ?
>
> This case if when one has in his configuration file that the app should run
> on device XXX, which happens to be the one in use by the compositor.
> This case needed to be handled, and while it may not very seem useful now,
> it could be in the future an interesting case (On wayland, you could have
> the compositor use a card on some screens, and another card on the other,
> etc. In this case it makes sense for the user to specify the card, and it
> may or may not be the card used for the given screen).
>
There's something lost in the translation here:

Currently in case of non DRI_PRIME (i.e. path tag) we dive into
get_render_node_from_id_path_tag(), loop and trigger on !another_tag
&& !strcmp(id_path_tag, id_path_tag_tmp) (another_tag is zero here).

Or in other words the function returns the render node, of the exact
_same_ device. Which makes the whole new method of using pci-...
dubious since one cannot select the 'other' device :-\

That is unless I'm missing something ?

-Emil