Hi Alex,
Things look a lot better imho. There's some ideas below, for the
future if you/others see value in them. Please do not worry about
those now.
On 24 January 2017 at 22:29, Alex Xie <AlexBin.Xie at amd.com> wrote:
> Verify the vender ID and driver name.
> Open all AMDGPU devices.
> Provide an option to open render node.
>
> Tested as root: PASS
> Tested as non-privileged user:
> All tests failed as expected
>
> v2: Return value in the ene of function amdgpu_open_devices.
> Check the return value of amdgpu_open_devices.
> amdgpu_test is not for USB device for the time being.
> Get the name of node from function drmGetDevices2.
> Drop the legacy drmAvailable() from the test.
>
> Signed-off-by: Alex Xie <AlexBin.Xie at amd.com>
> ---
> tests/amdgpu/amdgpu_test.c | 145 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 115 insertions(+), 30 deletions(-)
>
> diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
> index 71f357c..d2b00d4 100644
> --- a/tests/amdgpu/amdgpu_test.c
> +++ b/tests/amdgpu/amdgpu_test.c
> @@ -115,6 +115,111 @@ static const char usage[] = "Usage: %s [-hl] [<-s <suite id>> [-t <test id>]]\n"
> /** Specified options strings for getopt */
> static const char options[] = "hls:t:";
>
> +/* Open AMD devices.
> + * Return the number of AMD device openned.
> + */
> +static int amdgpu_open_devices(int open_render_node)
> +{
> + drmDevicePtr devices[MAX_CARDS_SUPPORTED];
> + int ret;
> + int i;
> + int drm_node;
> + int amd_index = 0;
> + int drm_count;
> + int fd;
> + drmVersionPtr version;
> +
> + drm_count = drmGetDevices2(0, devices, MAX_CARDS_SUPPORTED);
> +
> + for (i = 0; i < drm_count; i++) {
> + /* If this is not PCI device, skip*/
> + if (devices[i]->bustype != DRM_BUS_PCI)
> + continue;
> +
> + /* If this is not AMD GPU vender ID, skip*/
> + if (devices[i]->deviceinfo.pci->vendor_id != 0x1002)
> + continue;
> +
Once the filtering is done, it may be that only 2 of the
MAX_CARDS_SUPPORTED acquired fit the criteria.
One could fetch arbitrary large (all?) devices and then cap if/as needed.
> +
> + /* This node is not available. */
> + if (fd < 0) continue;
Normally continue should be on the next line.
> +/* Print AMD devices information */
> +static void amdgpu_print_devices()
> +{
> + int i;
> + for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
> + if (drm_amdgpu[i] >=0) {
> + /** Display version of DRM driver */
> + drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
Since we've done this above one might as well store it and cleanups at
amdgpu_close_devices() time.
It comes more applicable as you use drmGetDevice2 to get the device
info with later patch(es). Might as well store all the info that we
fetch during amdgpu_open_devices() ?
Thanks
Emil
