On 18 October 2016 at 09:49, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> On 14.10.2016 20:21, Emil Velikov wrote:
>>
>> From: Emil Velikov <emil.veli...@collabora.com>
>>
>> Currently not everyone has libudev and with follow-up patches we'll
>> completely remove the divergent codepaths.
>>
>> Use the libdrm drm device API to construct the required ID_PATH_TAG-like
>> string, to preserve the current functionality for libudev users and
>> allow others to benefit from it as well.
>>
>> v2: Drop ranty comments, pick the correct device
>>
>> Cc: Axel Davy <axel.d...@ens.fr>
>> Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
>> ---
>> src/loader/loader.c | 247
>> ++++++++++++++++++++++------------------------------
>> 1 file changed, 106 insertions(+), 141 deletions(-)
>>
>> diff --git a/src/loader/loader.c b/src/loader/loader.c
>> index ad4f946..06df05b 100644
>> --- a/src/loader/loader.c
>> +++ b/src/loader/loader.c
>
> [snip]
>>
>> @@ -321,17 +232,60 @@ static char *loader_get_dri_config_device_id(void)
>> }
>> #endif
>>
>> +static char *drm_construct_id_path_tag(drmDevicePtr device)
>> +{
>> +/* Length of "pci-xxxx_xx_xx_x\n" */
>> +#define PCI_ID_PATH_TAG_LENGTH 17
>> + char *tag = NULL;
>> +
>> + if (device->bustype == DRM_BUS_PCI) {
>> + tag = calloc(PCI_ID_PATH_TAG_LENGTH, sizeof(char));
>> + if (tag == NULL)
>> + return NULL;
>> +
>> + sprintf(tag, "pci-%04x_%02x_%02x_%1u",
>> device->businfo.pci->domain,
>> + device->businfo.pci->bus, device->businfo.pci->dev,
>> + device->businfo.pci->func);
>
>
> Defensive programming would suggest to use snprintf.
>
Correct. It's more like extra defensive in this case but will fix.
> [snip]
>
>> @@ -345,55 +299,66 @@ int loader_get_user_preferred_fd(int default_fd, int
>> *different_device)
>> return default_fd;
>> }
>>
>> - udev = udev_new();
>> - if (!udev)
>> - goto prime_clean;
>> + default_tag = drm_get_id_path_tag_for_fd(default_fd);
>> + if (default_tag == NULL)
>> + 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;
>> + num_devices = drmGetDevices(devices, MAX_DRM_DEVICES);
>> + if (num_devices < 0)
>> + goto err;
>>
>> - 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;
>> -
>> - device_name = get_render_node_from_id_path_tag(udev,
>> - prime,
>> - another_tag);
>> - if (device_name == NULL) {
>> - is_different_device = 0;
>> - goto default_device_clean;
>> + /* Hmm... detection for 2-7 seems to be broken. Oh well ...
>> + * Pick the first render device that is not our own.
>> + */
>> + for (i = 0; i < num_devices; i++) {
>> + if (devices[i]->available_nodes & 1 << DRM_NODE_RENDER &&
>> + !drm_device_matches_tag(devices[i], default_tag)) {
>> +
>> + found = true;
>> + break;
>> + }
>> + }
>> + } else {
>> + for (i = 0; i < num_devices; i++) {
>> + if (devices[i]->available_nodes & 1 << DRM_NODE_RENDER &&
>> + drm_device_matches_tag(devices[i], prime)) {
>> +
>> + found = true;
>> + break;
>> + }
>> + }
>
>
> I feel like it would be helpful to have a warning here if the device was not
> found. This could avoid some confusion when people inevitably typo their
> prime setting.
>
Original code does not have such a message, so let's add it as follow-up ?
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
