Re: [PATCH libdrm] amdgpu: dynamically detect number of drm devices

["Zhang, Jerry (Junwei)" <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>]

On 04/20/2018 02:58 PM, Christian König wrote:
> Am 20.04.2018 um 04:04 schrieb Zhang, Jerry (Junwei):
>> On 04/20/2018 09:50 AM, Zhang, Jerry (Junwei) wrote:
>>> On 04/19/2018 07:06 PM, Christian König wrote:
>>>> Wouldn't it be simpler to just set MAX_CARDS_SUPPORTED to 128?
>>>
>>> Perhaps it's 64 for card number.
>>> Although CONTROL node is not used now, but only 64 slots are reserved for each
>>> type.
>>>
>>> otherwise, we may prepare a patch to update to 128.
>>> How do you think that?
>>
>> On second thought, it will touch much code to clean CONTROL node.
>> In current stage, updating the number looks the best way.
>> But 64 seems reasonable for now.
>
> My last status is that control nodes where dropped and so we are back to 128
> devices in theory, but I have to admit that I didn't follow the discussion closely.

At that time, it only dropped the CONTROL node register, but many cleanup work 
did not perform, since it's a little bit work to do, waiting for a while to make 
sure it's really not used anywhere.

Anyway, it's fine for current change to extend the number for card.

Jerry

>
> Anyway I've already pushed the patch upstream which changes that to 128 and as
> far as I can see we waste only around 256bytes even if 64 would be sufficient,
> so there is no strong reason to change that once more.
>
> Christian.
>
>>
>> Jerry
>>
>>>
>>> Jerry
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>> Am 19.04.2018 um 12:12 schrieb Xiaojie Yuan:
>>>>> Change-Id: I36764951bebbcbf06cf84dd43ee946a34ec7b100
>>>>> Signed-off-by: Xiaojie Yuan <Xiaojie.Yuan@amd.com>
>>>>> ---
>>>>>   tests/amdgpu/amdgpu_test.c | 44 ++++++++++++++++++++++++++++----------
>>>>>   tests/amdgpu/amdgpu_test.h |  7 +-----
>>>>>   2 files changed, 34 insertions(+), 17 deletions(-)
>>>>>
>>>>> diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
>>>>> index 96fcd687..f7ac4ab4 100644
>>>>> --- a/tests/amdgpu/amdgpu_test.c
>>>>> +++ b/tests/amdgpu/amdgpu_test.c
>>>>> @@ -61,7 +61,8 @@
>>>>>    *  Open handles for amdgpu devices
>>>>>    *
>>>>>    */
>>>>> -int drm_amdgpu[MAX_CARDS_SUPPORTED];
>>>>> +int *drm_amdgpu;
>>>>> +size_t num_drm_devices;
>>>>>   /** Open render node to test */
>>>>>   int open_render_node = 0;    /* By default run most tests on primary node */
>>>>> @@ -238,16 +239,16 @@ static const char options[]   = "hlrps:t:b:d:f";
>>>>>    */
>>>>>   static int amdgpu_open_devices(int open_render_node)
>>>>>   {
>>>>> -    drmDevicePtr devices[MAX_CARDS_SUPPORTED];
>>>>> +    drmDevicePtr *devices;
>>>>>       int i;
>>>>>       int drm_node;
>>>>>       int amd_index = 0;
>>>>>       int drm_count;
>>>>> +    int drm_count2;
>>>>>       int fd;
>>>>>       drmVersionPtr version;
>>>>> -    drm_count = drmGetDevices2(0, devices, MAX_CARDS_SUPPORTED);
>>>>> -
>>>>> +    drm_count = drmGetDevices2(0, NULL, 0);
>>>>>       if (drm_count < 0) {
>>>>>           fprintf(stderr,
>>>>>               "drmGetDevices2() returned an error %d\n",
>>>>> @@ -255,6 +256,27 @@ static int amdgpu_open_devices(int open_render_node)
>>>>>           return 0;
>>>>>       }
>>>>> +    devices = calloc(drm_count, sizeof(drmDevicePtr));
>>>>> +    if (!devices) {
>>>>> +        goto end;
>>>>> +    }
>>>>> +
>>>>> +    drm_amdgpu = calloc(drm_count, sizeof(int));
>>>>> +    if (!drm_amdgpu) {
>>>>> +        goto end;
>>>>> +    }
>>>>> +
>>>>> +    for (i = 0; i < drm_count; i++)
>>>>> +        drm_amdgpu[i] = -1;
>>>>> +
>>>>> +    drm_count2 = drmGetDevices2(0, devices, drm_count);
>>>>> +    if (drm_count2 != drm_count) {
>>>>> +        fprintf(stderr, "number of drm devices changed");
>>>>> +        goto end;
>>>>> +    }
>>>>> +
>>>>> +    num_drm_devices = drm_count;
>>>>> +
>>>>>       for (i = 0; i < drm_count; i++) {
>>>>>           /* If this is not PCI device, skip*/
>>>>>           if (devices[i]->bustype != DRM_BUS_PCI)
>>>>> @@ -302,7 +324,9 @@ static int amdgpu_open_devices(int open_render_node)
>>>>>           amd_index++;
>>>>>       }
>>>>> +end:
>>>>>       drmFreeDevices(devices, drm_count);
>>>>> +    free(devices);
>>>>>       return amd_index;
>>>>>   }
>>>>> @@ -311,9 +335,11 @@ static int amdgpu_open_devices(int open_render_node)
>>>>>   static void amdgpu_close_devices()
>>>>>   {
>>>>>       int i;
>>>>> -    for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
>>>>> +    for (i = 0; i < num_drm_devices; i++)
>>>>>           if (drm_amdgpu[i] >=0)
>>>>>               close(drm_amdgpu[i]);
>>>>> +
>>>>> +    free(drm_amdgpu);
>>>>>   }
>>>>>   /* Print AMD devices information */
>>>>> @@ -339,7 +365,7 @@ static void amdgpu_print_devices()
>>>>>       /* Display information of AMD devices */
>>>>>       printf("Devices:\n");
>>>>> -    for (i = 0; i < MAX_CARDS_SUPPORTED && drm_amdgpu[i] >=0; i++)
>>>>> +    for (i = 0; i < num_drm_devices && drm_amdgpu[i] >=0; i++)
>>>>>           if (drmGetDevice2(drm_amdgpu[i],
>>>>>               DRM_DEVICE_GET_PCI_REVISION,
>>>>>               &device) == 0) {
>>>>> @@ -377,7 +403,7 @@ static int amdgpu_find_device(uint8_t bus, uint16_t dev)
>>>>>       int i;
>>>>>       drmDevicePtr device;
>>>>> -    for (i = 0; i < MAX_CARDS_SUPPORTED && drm_amdgpu[i] >= 0; i++) {
>>>>> +    for (i = 0; i < num_drm_devices && drm_amdgpu[i] >= 0; i++) {
>>>>>           if (drmGetDevice2(drm_amdgpu[i],
>>>>>               DRM_DEVICE_GET_PCI_REVISION,
>>>>>               &device) == 0) {
>>>>> @@ -456,10 +482,6 @@ int main(int argc, char **argv)
>>>>>       int display_list = 0;
>>>>>       int force_run = 0;
>>>>> -    for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
>>>>> -        drm_amdgpu[i] = -1;
>>>>> -
>>>>> -
>>>>>       /* Parse command line string */
>>>>>       opterr = 0;        /* Do not print error messages from getopt */
>>>>>       while ((c = getopt(argc, argv, options)) != -1) {
>>>>> diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h
>>>>> index 62875736..8a604fe4 100644
>>>>> --- a/tests/amdgpu/amdgpu_test.h
>>>>> +++ b/tests/amdgpu/amdgpu_test.h
>>>>> @@ -27,13 +27,8 @@
>>>>>   #include "amdgpu.h"
>>>>>   #include "amdgpu_drm.h"
>>>>> -/**
>>>>> - * Define max. number of card in system which we are able to handle
>>>>> - */
>>>>> -#define MAX_CARDS_SUPPORTED     4
>>>>> -
>>>>>   /* Forward reference for array to keep "drm" handles */
>>>>> -extern int drm_amdgpu[MAX_CARDS_SUPPORTED];
>>>>> +extern int *drm_amdgpu;
>>>>>   /* Global variables */
>>>>>   extern int open_render_node;
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx