Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1444839 > unrolled thread

[PATCH 0/8] drm/amdgpu: Fine-tuning for three function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 0/8] drm/amdgpu: Fine-tuning for three function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 16:40 +0200
    [PATCH 2/8] drm/amdgpu/powerplay: Delete unnecessary checks before  the function call "kfree" SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 16:50 +0200
    [PATCH 1/8] drm/amdgpu: Delete an unnecessary check before  drm_gem_object_unreference_unlocked() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 16:50 +0200
    [PATCH 4/8] drm/amdgpu: Delete a variable in  amdgpu_cgs_acpi_eval_object() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 17:00 +0200
      [PATCH 5/8] drm/amdgpu: Delete an unnecessary variable initialisation  in amdgpu_cgs_acpi_eval_object() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 17:00 +0200
    [PATCH 3/8] drm/amdgpu: One function call less in  amdgpu_cgs_acpi_eval_object() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 17:00 +0200
    [PATCH 6/8] drm/amdgpu: Change assignment for a variable in  amdgpu_cgs_acpi_eval_object() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 17:10 +0200
    [PATCH 7/8] drm/amd/powerplay: Change assignment for a buffer  variable in phm_dispatch_table() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 17:10 +0200
      Re: [PATCH 7/8] drm/amd/powerplay: Change assignment for a buffer  variable in phm_dispatch_table() walter harms <wharms@bfs.de> - 2016-07-17 17:10 +0200
    [PATCH 5/8] drm/amdgpu: Delete an unnecessary variable initialisation  in amdgpu_cgs_acpi_eval_object() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 17:10 +0200
    [PATCH 8/8] drm/amd/powerplay: Delete an unnecessary variable  initialisation in phm_dispatch_table() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 17:20 +0200

#1444839 — [PATCH 0/8] drm/amdgpu: Fine-tuning for three function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 16:23:21 +0200

Further update suggestions were taken into account after patches
were applied from static source code analysis.

Markus Elfring (8):
  Delete an unnecessary check before drm_gem_object_unreference_unlocked()
  Delete unnecessary checks before the function call "kfree"
  One function call less in amdgpu_cgs_acpi_eval_object() after error detection
  Delete a variable in amdgpu_cgs_acpi_eval_object()
  Delete an unnecessary variable initialisation in amdgpu_cgs_acpi_eval_object()
  Change assignment for a variable in amdgpu_cgs_acpi_eval_object()
  Change assignment for a buffer variable in phm_dispatch_table()
  Delete an unnecessary variable initialisation in phm_dispatch_table()

 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c            | 28 ++++++++++------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c        |  4 +---
 .../gpu/drm/amd/powerplay/hwmgr/functiontables.c   | 12 ++++------
 3 files changed, 19 insertions(+), 25 deletions(-)

-- 
2.9.1

[toc] | [next] | [standalone]

#1444841 — [PATCH 2/8] drm/amdgpu/powerplay: Delete unnecessary checks before the function call "kfree"

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 12:38:12 +0200

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c              | 3 +--
 drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c | 5 +----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index cf6f49f..6f11bc1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -1053,8 +1053,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 	}
 
 error:
-	if (obj != NULL)
-		kfree(obj);
+	kfree(obj);
 	kfree((void *)input.pointer);
 	return result;
 }
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c b/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
index 7a705ce..024e22e 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
@@ -76,10 +76,7 @@ int phm_dispatch_table(struct pp_hwmgr *hwmgr,
 	}
 
 	result = phm_run_table(hwmgr, rt_table, input, output, temp_storage);
-
-	if (NULL != temp_storage)
-		kfree(temp_storage);
-
+	kfree(temp_storage);
 	return result;
 }
 
-- 
2.9.1

[toc] | [prev] | [next] | [standalone]

#1444844 — [PATCH 1/8] drm/amdgpu: Delete an unnecessary check before drm_gem_object_unreference_unlocked()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 11:28:36 +0200

The drm_gem_object_unreference_unlocked() function tests whether
its argument is NULL and then returns immediately.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index a6eecf6..2a07b15 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -516,9 +516,7 @@ static void amdgpu_user_framebuffer_destroy(struct drm_framebuffer *fb)
 {
 	struct amdgpu_framebuffer *amdgpu_fb = to_amdgpu_framebuffer(fb);
 
-	if (amdgpu_fb->obj) {
-		drm_gem_object_unreference_unlocked(amdgpu_fb->obj);
-	}
+	drm_gem_object_unreference_unlocked(amdgpu_fb->obj);
 	drm_framebuffer_cleanup(fb);
 	kfree(amdgpu_fb);
 }
-- 
2.9.1

[toc] | [prev] | [next] | [standalone]

#1444845 — [PATCH 4/8] drm/amdgpu: Delete a variable in amdgpu_cgs_acpi_eval_object()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 14:00:28 +0200

The local variable "func_no" was assigned a value at two places.
But it was not read within this function. Thus delete it.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index 705bfa2..f5de510 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -909,7 +909,6 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 	uint32_t i, count;
 	acpi_status status;
 	int result = 0;
-	uint32_t func_no = 0xFFFFFFFF;
 
 	handle = ACPI_HANDLE(&adev->pdev->dev);
 	if (!handle)
@@ -926,7 +925,6 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 		if (info->pinput_argument == NULL)
 			return -EINVAL;
 		argument = info->pinput_argument;
-		func_no = argument->value;
 		for (i = 0; i < info->input_count; i++) {
 			if (((argument->type == ACPI_TYPE_STRING) ||
 			     (argument->type == ACPI_TYPE_BUFFER)) &&
-- 
2.9.1

[toc] | [prev] | [next] | [standalone]

#1444847 — [PATCH 5/8] drm/amdgpu: Delete an unnecessary variable initialisation in amdgpu_cgs_acpi_eval_object()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 14:54:12 +0200

The variable "argument" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index f5de510..47f2a43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -905,7 +905,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *params, *obj;
 	uint8_t name[5] = {'\0'};
-	struct cgs_acpi_method_argument *argument = NULL;
+	struct cgs_acpi_method_argument *argument;
 	uint32_t i, count;
 	acpi_status status;
 	int result = 0;
-- 
2.9.1

[toc] | [prev] | [next] | [standalone]

#1444846 — [PATCH 3/8] drm/amdgpu: One function call less in amdgpu_cgs_acpi_eval_object() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 13:43:44 +0200

The kfree() function was called in one case by the
amdgpu_cgs_acpi_eval_object() function during error handling
even if the passed variable "obj" contained a null pointer.

* Adjust jump targets according to the Linux coding style convention.

* Delete unnecessary initialisations for the variables "obj"
  and "params" then.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index 6f11bc1..705bfa2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -903,8 +903,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 	acpi_handle handle;
 	struct acpi_object_list input;
 	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
-	union acpi_object *params = NULL;
-	union acpi_object *obj = NULL;
+	union acpi_object *params, *obj;
 	uint8_t name[5] = {'\0'};
 	struct cgs_acpi_method_argument *argument = NULL;
 	uint32_t i, count;
@@ -996,7 +995,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 
 	if (ACPI_FAILURE(status)) {
 		result = -EIO;
-		goto error;
+		goto free_input;
 	}
 
 	/* return the output info */
@@ -1006,7 +1005,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 		if ((obj->type != ACPI_TYPE_PACKAGE) ||
 			(obj->package.count != count)) {
 			result = -EIO;
-			goto error;
+			goto free_obj;
 		}
 		params = obj->package.elements;
 	} else
@@ -1014,13 +1013,13 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 
 	if (params == NULL) {
 		result = -EIO;
-		goto error;
+		goto free_obj;
 	}
 
 	for (i = 0; i < count; i++) {
 		if (argument->type != params->type) {
 			result = -EIO;
-			goto error;
+			goto free_obj;
 		}
 		switch (params->type) {
 		case ACPI_TYPE_INTEGER:
@@ -1030,7 +1029,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 			if ((params->string.length != argument->data_length) ||
 				(params->string.pointer == NULL)) {
 				result = -EIO;
-				goto error;
+				goto free_obj;
 			}
 			strncpy(argument->pointer,
 				params->string.pointer,
@@ -1039,7 +1038,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 		case ACPI_TYPE_BUFFER:
 			if (params->buffer.pointer == NULL) {
 				result = -EIO;
-				goto error;
+				goto free_obj;
 			}
 			memcpy(argument->pointer,
 				params->buffer.pointer,
@@ -1052,8 +1051,9 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 		params++;
 	}
 
-error:
+free_obj:
 	kfree(obj);
+free_input:
 	kfree((void *)input.pointer);
 	return result;
 }
-- 
2.9.1

[toc] | [prev] | [next] | [standalone]

#1444856 — [PATCH 6/8] drm/amdgpu: Change assignment for a variable in amdgpu_cgs_acpi_eval_object()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 15:05:45 +0200

Indicate successful function execution only at the end.
Thus omit initialisation for the variable "result" at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index 47f2a43..57859bd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -908,7 +908,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 	struct cgs_acpi_method_argument *argument;
 	uint32_t i, count;
 	acpi_status status;
-	int result = 0;
+	int result;
 
 	handle = ACPI_HANDLE(&adev->pdev->dev);
 	if (!handle)
@@ -1049,6 +1049,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 		params++;
 	}
 
+	result = 0;
 free_obj:
 	kfree(obj);
 free_input:
-- 
2.9.1

[toc] | [prev] | [next] | [standalone]

#1444857 — [PATCH 7/8] drm/amd/powerplay: Change assignment for a buffer variable in phm_dispatch_table()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 15:36:36 +0200

The variable "temp_storage" was eventually reassigned with a pointer.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c b/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
index 024e22e..735aeb0 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
@@ -60,7 +60,7 @@ int phm_dispatch_table(struct pp_hwmgr *hwmgr,
 		       void *input, void *output)
 {
 	int result = 0;
-	void *temp_storage = NULL;
+	void *temp_storage;
 
 	if (hwmgr == NULL || rt_table == NULL) {
 		printk(KERN_ERR "[ powerplay ] Invalid Parameter!\n");
@@ -73,7 +73,8 @@ int phm_dispatch_table(struct pp_hwmgr *hwmgr,
 			printk(KERN_ERR "[ powerplay ] Could not allocate table temporary storage\n");
 			return -ENOMEM;
 		}
-	}
+	} else
+		temp_storage = NULL;
 
 	result = phm_run_table(hwmgr, rt_table, input, output, temp_storage);
 	kfree(temp_storage);
-- 
2.9.1

[toc] | [prev] | [next] | [standalone]

#1445055 — Re: [PATCH 7/8] drm/amd/powerplay: Change assignment for a buffer variable in phm_dispatch_table()

Am 16.07.2016 17:08, schrieb SF Markus Elfring:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 16 Jul 2016 15:36:36 +0200
> 
> The variable "temp_storage" was eventually reassigned with a pointer.
> Thus omit the explicit initialisation at the beginning.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c b/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
> index 024e22e..735aeb0 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
> @@ -60,7 +60,7 @@ int phm_dispatch_table(struct pp_hwmgr *hwmgr,
>  		       void *input, void *output)
>  {
>  	int result = 0;
> -	void *temp_storage = NULL;
> +	void *temp_storage;
>  
>  	if (hwmgr == NULL || rt_table == NULL) {
>  		printk(KERN_ERR "[ powerplay ] Invalid Parameter!\n");
> @@ -73,7 +73,8 @@ int phm_dispatch_table(struct pp_hwmgr *hwmgr,
>  			printk(KERN_ERR "[ powerplay ] Could not allocate table temporary storage\n");
>  			return -ENOMEM;
>  		}
> -	}
> +	} else
> +		temp_storage = NULL;
>  
>  	result = phm_run_table(hwmgr, rt_table, input, output, temp_storage);
>  	kfree(temp_storage);


the handling of rt_table->storage_size == 0 is so better visible.

IMHO in this case the function could return directly either with -EINVAL;
or with 0;  -> more direct more obvious.

if (rt_table->storage_size == 0 )
	return 0;

re,
 wh

[toc] | [prev] | [next] | [standalone]

#1444861 — [PATCH 5/8] drm/amdgpu: Delete an unnecessary variable initialisation in amdgpu_cgs_acpi_eval_object()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 14:54:12 +0200

The variable "argument" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index f5de510..47f2a43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -905,7 +905,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device,
 	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *params, *obj;
 	uint8_t name[5] = {'\0'};
-	struct cgs_acpi_method_argument *argument = NULL;
+	struct cgs_acpi_method_argument *argument;
 	uint32_t i, count;
 	acpi_status status;
 	int result = 0;
-- 
2.9.1

[toc] | [prev] | [next] | [standalone]

#1444864 — [PATCH 8/8] drm/amd/powerplay: Delete an unnecessary variable initialisation in phm_dispatch_table()

From 0bf6f3c40786e12d3d42672f1d56296b30e17ac9 Mon Sep 17 00:00:00 2001
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 15:50:18 +0200
Subject: [PATCH 8/8] drm/amd/powerplay: Delete an unnecessary variable
 initialisation in phm_dispatch_table()

The variable "result" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c b/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
index 735aeb0..fdfed63 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c
@@ -59,7 +59,7 @@ int phm_dispatch_table(struct pp_hwmgr *hwmgr,
 		       struct phm_runtime_table_header *rt_table,
 		       void *input, void *output)
 {
-	int result = 0;
+	int result;
 	void *temp_storage;
 
 	if (hwmgr == NULL || rt_table == NULL) {
-- 
2.9.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web