Re: [PATCH 1/3] tests/amdgpu: Fix various warnings

[Christian König <christian.koenig@xxxxxxx>]

Am 06.12.19 um 05:32 schrieb Luben Tuikov:
> This patch fixes the following warnings:
> -Wformat=
> -Wmaybe-uninitialized
> -Wmisleading-indentation
> -Wstringop-truncation
> -Wunused-function
> -Wunused-variable
>
> It also removes forward declarations and moves
> global functions to the bottom, keeping locals
> at the top, in ras_tests.c.
>
> Signed-off-by: Luben Tuikov <luben.tuikov@xxxxxxx>

Those cleanups should probably happen on the upstream master.

The the RAS development branch we probably have tons of stuff which will 
never go upstream.

Apart from that the patch is Acked-by: Christian König 
<christian.koenig@xxxxxxx>.

Regards,
Christian.

> ---
>   amdgpu/amdgpu_bo.c           |   2 -
>   tests/amdgpu/basic_tests.c   |   7 +-
>   tests/amdgpu/bo_tests.c      |   9 +-
>   tests/amdgpu/cs_tests.c      |   1 +
>   tests/amdgpu/ras_tests.c     | 241 +++++++++++++++++------------------
>   tests/amdgpu/syncobj_tests.c |   2 +-
>   6 files changed, 124 insertions(+), 138 deletions(-)
>
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> index c54687ed..d6ea0e74 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -396,7 +396,6 @@ unlock:
>   drm_public int amdgpu_get_fb_id(amdgpu_device_handle dev, unsigned int *fb_id)
>   {
>   	drmModeResPtr mode_res;
> -	int count_crtcs;
>   	drmModeCrtcPtr mode_crtc;
>   	int current_id = 0;
>   	int r = 0;
> @@ -421,7 +420,6 @@ drm_public int amdgpu_get_fb_id(amdgpu_device_handle dev, unsigned int *fb_id)
>   	if (!mode_res)
>   		return EFAULT;
>   
> -	count_crtcs = mode_res->count_crtcs;
>   	for (i = 0; i < mode_res->count_crtcs; i++) {
>   		mode_crtc = drmModeGetCrtc(fd, mode_res->crtcs[i]);
>   		if (mode_crtc) {
> diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c
> index c3c09702..cce0640a 100644
> --- a/tests/amdgpu/basic_tests.c
> +++ b/tests/amdgpu/basic_tests.c
> @@ -3231,7 +3231,7 @@ static void amdgpu_memcpy_draw(amdgpu_device_handle device_handle,
>   	int bo_cmd_size = 4096;
>   	struct amdgpu_cs_request ibs_request = {0};
>   	struct amdgpu_cs_ib_info ib_info= {0};
> -	uint32_t hang_state, hangs, expired;
> +	uint32_t expired;
>   	amdgpu_bo_list_handle bo_list;
>   	struct amdgpu_cs_fence fence_status = {0};
>   
> @@ -3479,10 +3479,11 @@ amdgpu_direct_gma_bo_alloc_and_map(amdgpu_device_handle dev, amdgpu_device_handl
>   			goto error_va_map;
>   	}
>   
> -	if (cpu)
> +	if (cpu) {
>   		r = amdgpu_bo_cpu_map(buf_handle_import, cpu);
>   		if(r)
>   			goto error_va_map;
> +	}
>   
>   	*bo = buf_handle;
>   	*bo_peer = buf_handle_import;
> @@ -3610,7 +3611,7 @@ static void amdgpu_direct_gma_mmap(amdgpu_device_handle dev,
>   	amdgpu_bo_handle *buf_handle_import;
>   	volatile uint8_t **ptr;
>   	struct drm_amdgpu_capability cap;
> -	uint64_t size = 4096, phys_addr, remain;
> +	uint64_t size = 4096, remain;
>   	int i, j, r, tst_loop = 20;
>   
>   	buf_handle = calloc(tst_loop, sizeof(*buf_handle));
> diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
> index 27048c88..7fcabb85 100644
> --- a/tests/amdgpu/bo_tests.c
> +++ b/tests/amdgpu/bo_tests.c
> @@ -325,8 +325,7 @@ static void amdgpu_bo_find_by_cpu_mapping(void)
>   }
>   static void amdgpu_get_fb_id_and_handle(void)
>   {
> -	uint32_t *ptr;
> -	int i, r;
> +	int r;
>   	unsigned int fb_id;
>   	struct amdgpu_bo_import_result output;
>   
> @@ -352,7 +351,7 @@ static void amdgpu_bo_ssg(void)
>   	int i, j, fd;
>   	uint64_t pattern = 0xdeadbeef12345678, out;
>   	void *buf;
> -	bool write_is_ok;
> +	bool write_is_ok = false;
>   
>   	CU_ASSERT(!amdgpu_query_capability(device_handle, &cap));
>   	if(!(cap.flag & AMDGPU_CAPABILITY_SSG_FLAG)) {
> @@ -363,7 +362,7 @@ static void amdgpu_bo_ssg(void)
>   	if (buf_size > cap.direct_gma_size << 20)
>   		buf_size = cap.direct_gma_size << 20;
>   
> -	printf("SSG read/write block size 0x%x\n", buf_size);
> +	printf("SSG read/write block size 0x%lx\n", buf_size);
>   
>   	CU_ASSERT((fd = open(in_file, O_WRONLY | O_CREAT, S_IRWXU)) >= 0);
>   	for (i = 0; i < buf_size; i += sizeof(pattern)) {
> @@ -413,7 +412,7 @@ static void amdgpu_bo_ssg(void)
>   	for (i = 0; i < 3; i++) {
>   		struct timespec ts1, ts2;
>   		double a, b, c;
> -		bool write_is_same;
> +		bool write_is_same = false;
>   
>   		CU_ASSERT((fd = open(out_file, O_WRONLY | O_CREAT | O_DIRECT, S_IRWXU)) >= 0);
>   
> diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c
> index c0903a2a..10124c15 100644
> --- a/tests/amdgpu/cs_tests.c
> +++ b/tests/amdgpu/cs_tests.c
> @@ -362,6 +362,7 @@ static void amdgpu_cs_uvd_decode(void)
>   		bs_addr = fb_addr + 4*1024;
>   	dpb_addr = ALIGN(bs_addr + sizeof(uvd_bitstream), 4*1024);
>   
> +	ctx_addr = 0;
>   	if (family_id >= AMDGPU_FAMILY_VI) {
>   		if ((family_id == AMDGPU_FAMILY_AI) ||
>   		    (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A ||
> diff --git a/tests/amdgpu/ras_tests.c b/tests/amdgpu/ras_tests.c
> index c1c543c1..d714be73 100644
> --- a/tests/amdgpu/ras_tests.c
> +++ b/tests/amdgpu/ras_tests.c
> @@ -522,124 +522,6 @@ static int amdgpu_ras_lookup_id(drmDevicePtr device)
>   	return -1;
>   }
>   
> -CU_BOOL suite_ras_tests_enable(void)
> -{
> -	amdgpu_device_handle device_handle;
> -	uint32_t  major_version;
> -	uint32_t  minor_version;
> -	int i;
> -	drmDevicePtr device;
> -
> -	for (i = 0; i < MAX_CARDS_SUPPORTED && drm_amdgpu[i] >= 0; i++) {
> -		if (amdgpu_device_initialize(drm_amdgpu[i], &major_version,
> -					&minor_version, &device_handle))
> -			continue;
> -
> -		if (drmGetDevice2(drm_amdgpu[i],
> -					DRM_DEVICE_GET_PCI_REVISION,
> -					&device))
> -			continue;
> -
> -		if (device->bustype == DRM_BUS_PCI &&
> -				amdgpu_ras_lookup_capability(device_handle)) {
> -			amdgpu_device_deinitialize(device_handle);
> -			return CU_TRUE;
> -		}
> -
> -		if (amdgpu_device_deinitialize(device_handle))
> -			continue;
> -	}
> -
> -	return CU_FALSE;
> -}
> -
> -int suite_ras_tests_init(void)
> -{
> -	drmDevicePtr device;
> -	amdgpu_device_handle device_handle;
> -	uint32_t  major_version;
> -	uint32_t  minor_version;
> -	uint32_t  capability;
> -	struct ras_test_mask test_mask;
> -	int id;
> -	int i;
> -	int r;
> -
> -	for (i = 0; i < MAX_CARDS_SUPPORTED && drm_amdgpu[i] >= 0; i++) {
> -		r = amdgpu_device_initialize(drm_amdgpu[i], &major_version,
> -				&minor_version, &device_handle);
> -		if (r)
> -			continue;
> -
> -		if (drmGetDevice2(drm_amdgpu[i],
> -					DRM_DEVICE_GET_PCI_REVISION,
> -					&device)) {
> -			amdgpu_device_deinitialize(device_handle);
> -			continue;
> -		}
> -
> -		if (device->bustype != DRM_BUS_PCI) {
> -			amdgpu_device_deinitialize(device_handle);
> -			continue;
> -		}
> -
> -		capability = amdgpu_ras_lookup_capability(device_handle);
> -		if (capability == 0) {
> -			amdgpu_device_deinitialize(device_handle);
> -			continue;
> -
> -		}
> -
> -		id = amdgpu_ras_lookup_id(device);
> -		if (id == -1) {
> -			amdgpu_device_deinitialize(device_handle);
> -			continue;
> -		}
> -
> -		test_mask = amdgpu_ras_get_test_mask(device);
> -
> -		devices[devices_count++] = (struct amdgpu_ras_data) {
> -			device_handle, id, capability, test_mask,
> -		};
> -	}
> -
> -	if (devices_count == 0)
> -		return CUE_SINIT_FAILED;
> -
> -	return CUE_SUCCESS;
> -}
> -
> -int suite_ras_tests_clean(void)
> -{
> -	int r;
> -	int i;
> -	int ret = CUE_SUCCESS;
> -
> -	for (i = 0; i < devices_count; i++) {
> -		r = amdgpu_device_deinitialize(devices[i].device_handle);
> -		if (r)
> -			ret = CUE_SCLEAN_FAILED;
> -	}
> -	return ret;
> -}
> -
> -static void amdgpu_ras_disable_test(void);
> -static void amdgpu_ras_enable_test(void);
> -static void amdgpu_ras_inject_test(void);
> -static void amdgpu_ras_query_test(void);
> -static void amdgpu_ras_basic_test(void);
> -
> -CU_TestInfo ras_tests[] = {
> -	{ "ras basic test",	amdgpu_ras_basic_test },
> -	{ "ras query test",	amdgpu_ras_query_test },
> -	{ "ras inject test",	amdgpu_ras_inject_test },
> -	{ "ras disable test",	amdgpu_ras_disable_test },
> -#if 0
> -	{ "ras enable test",	amdgpu_ras_enable_test },
> -#endif
> -	CU_TEST_INFO_NULL,
> -};
> -
>   //helpers
>   
>   static int test_card;
> @@ -648,10 +530,8 @@ static char debugfs_path[1024];
>   static uint32_t ras_mask;
>   static amdgpu_device_handle device_handle;
>   
> -static int set_test_card(int card)
> +static void set_test_card(int card)
>   {
> -	int i;
> -
>   	test_card = card;
>   	sprintf(sysfs_path, "/sys/class/drm/card%d/device/ras/", devices[card].id);
>   	sprintf(debugfs_path, "/sys/kernel/debug/dri/%d/ras/", devices[card].id);
> @@ -660,8 +540,6 @@ static int set_test_card(int card)
>   	ras_block_mask_inject = devices[card].test_mask.inject_mask;
>   	ras_block_mask_query = devices[card].test_mask.query_mask;
>   	ras_block_mask_basic = devices[card].test_mask.basic_mask;
> -
> -	return 0;
>   }
>   
>   static const char *get_ras_sysfs_root(void)
> @@ -742,7 +620,6 @@ static int amdgpu_ras_query_err_count(enum amdgpu_ras_block block,
>   {
>   	char buf[64];
>   	char name[1024];
> -	int ret;
>   
>   	*ue = *ce = 0;
>   
> @@ -779,7 +656,7 @@ static int amdgpu_ras_inject(enum amdgpu_ras_block block,
>   	inject->head.block = block;
>   	inject->head.type = type;
>   	inject->head.sub_block_index = sub_block;
> -	strncpy(inject->head.name, ras_block_str(block), 32);
> +	strncpy(inject->head.name, ras_block_str(block), sizeof(inject->head.name)-1);
>   	inject->address = address;
>   	inject->value = value;
>   
> @@ -956,8 +833,6 @@ static void amdgpu_ras_query_test(void)
>   
>   static void amdgpu_ras_basic_test(void)
>   {
> -	unsigned long ue, ce;
> -	char name[1024];
>   	int ret;
>   	int i;
>   	int j;
> @@ -1000,3 +875,115 @@ static void amdgpu_ras_basic_test(void)
>   		}
>   	}
>   }
> +
> +CU_TestInfo ras_tests[] = {
> +	{ "ras basic test",	amdgpu_ras_basic_test },
> +	{ "ras query test",	amdgpu_ras_query_test },
> +	{ "ras inject test",	amdgpu_ras_inject_test },
> +	{ "ras disable test",	amdgpu_ras_disable_test },
> +#if 0
> +	{ "ras enable test",	amdgpu_ras_enable_test },
> +#endif
> +	CU_TEST_INFO_NULL,
> +};
> +
> +CU_BOOL suite_ras_tests_enable(void)
> +{
> +	amdgpu_device_handle device_handle;
> +	uint32_t  major_version;
> +	uint32_t  minor_version;
> +	int i;
> +	drmDevicePtr device;
> +
> +	for (i = 0; i < MAX_CARDS_SUPPORTED && drm_amdgpu[i] >= 0; i++) {
> +		if (amdgpu_device_initialize(drm_amdgpu[i], &major_version,
> +					&minor_version, &device_handle))
> +			continue;
> +
> +		if (drmGetDevice2(drm_amdgpu[i],
> +					DRM_DEVICE_GET_PCI_REVISION,
> +					&device))
> +			continue;
> +
> +		if (device->bustype == DRM_BUS_PCI &&
> +				amdgpu_ras_lookup_capability(device_handle)) {
> +			amdgpu_device_deinitialize(device_handle);
> +			return CU_TRUE;
> +		}
> +
> +		if (amdgpu_device_deinitialize(device_handle))
> +			continue;
> +	}
> +
> +	return CU_FALSE;
> +}
> +
> +int suite_ras_tests_init(void)
> +{
> +	drmDevicePtr device;
> +	amdgpu_device_handle device_handle;
> +	uint32_t  major_version;
> +	uint32_t  minor_version;
> +	uint32_t  capability;
> +	struct ras_test_mask test_mask;
> +	int id;
> +	int i;
> +	int r;
> +
> +	for (i = 0; i < MAX_CARDS_SUPPORTED && drm_amdgpu[i] >= 0; i++) {
> +		r = amdgpu_device_initialize(drm_amdgpu[i], &major_version,
> +				&minor_version, &device_handle);
> +		if (r)
> +			continue;
> +
> +		if (drmGetDevice2(drm_amdgpu[i],
> +					DRM_DEVICE_GET_PCI_REVISION,
> +					&device)) {
> +			amdgpu_device_deinitialize(device_handle);
> +			continue;
> +		}
> +
> +		if (device->bustype != DRM_BUS_PCI) {
> +			amdgpu_device_deinitialize(device_handle);
> +			continue;
> +		}
> +
> +		capability = amdgpu_ras_lookup_capability(device_handle);
> +		if (capability == 0) {
> +			amdgpu_device_deinitialize(device_handle);
> +			continue;
> +
> +		}
> +
> +		id = amdgpu_ras_lookup_id(device);
> +		if (id == -1) {
> +			amdgpu_device_deinitialize(device_handle);
> +			continue;
> +		}
> +
> +		test_mask = amdgpu_ras_get_test_mask(device);
> +
> +		devices[devices_count++] = (struct amdgpu_ras_data) {
> +			device_handle, id, capability, test_mask,
> +		};
> +	}
> +
> +	if (devices_count == 0)
> +		return CUE_SINIT_FAILED;
> +
> +	return CUE_SUCCESS;
> +}
> +
> +int suite_ras_tests_clean(void)
> +{
> +	int r;
> +	int i;
> +	int ret = CUE_SUCCESS;
> +
> +	for (i = 0; i < devices_count; i++) {
> +		r = amdgpu_device_deinitialize(devices[i].device_handle);
> +		if (r)
> +			ret = CUE_SCLEAN_FAILED;
> +	}
> +	return ret;
> +}
> diff --git a/tests/amdgpu/syncobj_tests.c b/tests/amdgpu/syncobj_tests.c
> index 869ed88e..3a7b38eb 100644
> --- a/tests/amdgpu/syncobj_tests.c
> +++ b/tests/amdgpu/syncobj_tests.c
> @@ -96,7 +96,7 @@ static int syncobj_command_submission_helper(uint32_t syncobj_handle, bool
>   	struct amdgpu_cs_fence fence_status;
>   	amdgpu_bo_list_handle bo_list;
>   	amdgpu_va_handle va_handle;
> -	uint32_t expired, flags;
> +	uint32_t expired;
>   	int i, r;
>   	uint64_t seq_no;
>   	static uint32_t *ptr;

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx