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

[Axel Davy]

On 12/10/2016 19:15, Emil Velikov wrote:
> 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 :-\
Do you suggest to have DRI_PRIME=2 (up to 8) be the same than DRI_PRIME=1 ?
>
>>> +      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
>

I'm not sure to understand exactly what you want to say.

Here is a summary of current behaviour.


device_id (env var, or dri conf) can be set to "1" (another device than 
default) or to an id path tag pci-XXX. The legacy env var DRI_PRIME 
overrides the setting if set (but is used the same way exactly).

If never the value doesn't match 1, or a valid id path tag, you get the 
default device.

If you set 1, it picks any device that has a different id path tag than 
the default device. This is obviously limited if you get many devices in 
the computer, and in this case you should rather use id path tags.

Else it tries to pick the device with the specified id path tag.

For the case when the specified id path tag is the id path tag of the 
default device, the default device is taken. It was decided to implement 
by taking the render node of the device in this case. I don't remember 
exactly why it was chosen instead of just taking the default fd. I think 
returning the default fd is fine as well.

Were you suggesting that the call to get_render_node_from_id_path_tag is 
useless when we detect that the id path tag is the same than for the 
default device ? In that case the code is equivalent to just take the 
render-node version of device. If you decide to just return the default 
fd in that situation, you can just avoid the call.


Axel

Axel