thr3ads.net - Nouveau - [Nouveau] [PATCH 0/5] GPU-DRM-nouveau: Fine-tuning for five function implementations [Sep 2016]

If this information is useful, please help other people find it:
Share via:

SF Markus Elfring

2016-Sep-21 07:23 UTC

[Nouveau] [PATCH 0/5] GPU-DRM-nouveau: Fine-tuning for five function implementations

From: Markus Elfring <elfring at users.sourceforge.net>
Date: Wed, 21 Sep 2016 09:09:09 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  Use kmalloc_array() in nvbios_iccsense_parse()
  Use kmalloc_array() in gt215_link_train()
  Delete unnecessary braces
  Adjust a kzalloc() call in gt215_ram_new()
  Add space after an "if"

 drivers/gpu/drm/nouveau/nvkm/subdev/bios/iccsense.c |  4 +++-
 drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c   | 21 +++++++++------------
 2 files changed, 12 insertions(+), 13 deletions(-)

-- 
2.10.0

SF Markus Elfring

2016-Sep-21 07:24 UTC

head link

[Nouveau] [PATCH 1/5] GPU-DRM-nouveau: Use kmalloc_array() in nvbios_iccsense_parse()

From: Markus Elfring <elfring at users.sourceforge.net>
Date: Tue, 20 Sep 2016 22:22:14 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data structure by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/iccsense.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/iccsense.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/iccsense.c
index 0843280..91cf1ee 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/iccsense.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/iccsense.c
@@ -72,7 +72,9 @@ nvbios_iccsense_parse(struct nvkm_bios *bios, struct
nvbios_iccsense *iccsense)
 	}

 	iccsense->nr_entry = cnt;
-	iccsense->rail = kmalloc(sizeof(struct pwr_rail_t) * cnt, GFP_KERNEL);
+	iccsense->rail = kmalloc_array(cnt,
+				       sizeof(*iccsense->rail),
+				       GFP_KERNEL);
 	if (!iccsense->rail)
 		return -ENOMEM;

-- 
2.10.0

SF Markus Elfring

2016-Sep-21 07:25 UTC

head link

[Nouveau] [PATCH 2/5] GPU-DRM-nouveau: Use kmalloc_array() in gt215_link_train()

From: Markus Elfring <elfring at users.sourceforge.net>
Date: Tue, 20 Sep 2016 22:32:14 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
index d15ea88..dbaf577 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
@@ -170,7 +170,7 @@ gt215_link_train(struct gt215_ram *ram)
 		return -ENOSYS;

 	/* XXX: Multiple partitions? */
-	result = kmalloc(64 * sizeof(u32), GFP_KERNEL);
+	result = kmalloc_array(64, sizeof(*result), GFP_KERNEL);
 	if (!result)
 		return -ENOMEM;

-- 
2.10.0

SF Markus Elfring

2016-Sep-21 07:26 UTC

head link

[Nouveau] [PATCH 3/5] GPU-DRM-nouveau: Delete unnecessary braces

From: Markus Elfring <elfring at users.sourceforge.net>
Date: Wed, 21 Sep 2016 08:28:08 +0200

Do not use curly brackets at four source code places
where a single statement should be sufficient.

Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
index dbaf577..cb50539 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
@@ -127,12 +127,11 @@ gt215_link_train_calc(u32 *vals, struct gt215_ltrain
*train)
 	}
 
 	/* Find the best value for 0x1111e0 */
-	for (i = 0; i < 4; i++) {
+	for (i = 0; i < 4; i++)
 		if (bins[i] > qty) {
 			bin = i + 3;
 			qty = bins[i];
 		}
-	}
 
 	train->r_100720 = 0;
 	for (i = 0; i < 8; i++) {
@@ -729,9 +728,8 @@ gt215_ram_calc(struct nvkm_ram *base, u32 freq)
 		ram_mask(fuc, 0x1007e0, 0x22222222, r100760);
 	}
 
-	if (device->chipset == 0xa3 && freq > 500000) {
+	if (device->chipset == 0xa3 && freq > 500000)
 		ram_mask(fuc, 0x100700, 0x00000006, 0x00000000);
-	}
 
 	/* Final switch */
 	if (mclk.pll) {
@@ -745,12 +743,11 @@ gt215_ram_calc(struct nvkm_ram *base, u32 freq)
 	ram_nsec(fuc, 2000);
 
 	/* Set RAM MR parameters and timings */
-	for (i = 2; i >= 0; i--) {
+	for (i = 2; i >= 0; i--)
 		if (ram_rd32(fuc, mr[i]) != ram->base.mr[i]) {
 			ram_wr32(fuc, mr[i], ram->base.mr[i]);
 			ram_nsec(fuc, 1000);
 		}
-	}
 
 	ram_wr32(fuc, 0x100220[3], timing[3]);
 	ram_wr32(fuc, 0x100220[1], timing[1]);
@@ -838,11 +835,10 @@ gt215_ram_calc(struct nvkm_ram *base, u32 freq)
 	if (!next->bios.ramcfg_DLLoff)
 		nvkm_sddr2_dll_reset(fuc);
 
-	if (ram->base.type == NVKM_RAM_TYPE_GDDR3) {
+	if (ram->base.type == NVKM_RAM_TYPE_GDDR3)
 		ram_nsec(fuc, 31000);
-	} else {
+	else
 		ram_nsec(fuc, 14000);
-	}
 
 	if (ram->base.type == NVKM_RAM_TYPE_DDR3) {
 		ram_wr32(fuc, 0x100264, 0x1);
-- 
2.10.0

SF Markus Elfring

2016-Sep-21 07:28 UTC

head link

[Nouveau] [PATCH 4/5] GPU-DRM-nouveau: Adjust a kzalloc() call in gt215_ram_new()

From: Markus Elfring <elfring at users.sourceforge.net>
Date: Wed, 21 Sep 2016 08:44:38 +0200

The script "checkpatch.pl" can point out that assignments should
usually
not be performed within condition checks.
Thus move the assignment for one local variable to a separate statement
in this function.

Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
index cb50539..edbe8e4 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
@@ -940,7 +940,8 @@ gt215_ram_new(struct nvkm_fb *fb, struct nvkm_ram **pram)
 	struct gt215_ram *ram;
 	int ret, i;
 
-	if (!(ram = kzalloc(sizeof(*ram), GFP_KERNEL)))
+	ram = kzalloc(sizeof(*ram), GFP_KERNEL);
+	if (!ram)
 		return -ENOMEM;
 	*pram = &ram->base;
 
-- 
2.10.0

SF Markus Elfring

2016-Sep-21 07:29 UTC

head link

[Nouveau] [PATCH 5/5] GPU-DRM-nouveau: Add space after an "if"

From: Markus Elfring <elfring at users.sourceforge.net>
Date: Wed, 21 Sep 2016 08:58:41 +0200

Use another space character behind the keyword "if" according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
index edbe8e4..f95800d 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
@@ -256,7 +256,7 @@ gt215_link_train(struct gt215_ram *ram)
 	return ret;
 
 out:
-	if(ret == -EBUSY)
+	if (ret == -EBUSY)
 		f = NULL;
 
 	train->state = NVA3_TRAIN_UNSUPPORTED;
-- 
2.10.0

Roy Spliet

2016-Sep-21 07:49 UTC

head link

[Nouveau] [PATCH 3/5] GPU-DRM-nouveau: Delete unnecessary braces

Comments in-line. Thanks.

Roy

Op 21-09-16 om 08:26 schreef SF Markus Elfring:> From: Markus Elfring <elfring at users.sourceforge.net>
> Date: Wed, 21 Sep 2016 08:28:08 +0200
>
> Do not use curly brackets at four source code places
> where a single statement should be sufficient.
>
> Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
> ---
>   drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
> index dbaf577..cb50539 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c
> @@ -127,12 +127,11 @@ gt215_link_train_calc(u32 *vals, struct gt215_ltrain
*train)
>   	}
>   
>   	/* Find the best value for 0x1111e0 */
> -	for (i = 0; i < 4; i++) {
> +	for (i = 0; i < 4; i++)
>   		if (bins[i] > qty) {
>   			bin = i + 3;
>   			qty = bins[i];
>   		}
> -	}I'm not a fan of removing the braces around a multi-line statement, 
despite being functionally correct. It obscures the program structure on 
displays with a low effective vertical resolution.>   
>   	train->r_100720 = 0;
>   	for (i = 0; i < 8; i++) {
> @@ -729,9 +728,8 @@ gt215_ram_calc(struct nvkm_ram *base, u32 freq)
>   		ram_mask(fuc, 0x1007e0, 0x22222222, r100760);
>   	}
>   
> -	if (device->chipset == 0xa3 && freq > 500000) {
> +	if (device->chipset == 0xa3 && freq > 500000)
>   		ram_mask(fuc, 0x100700, 0x00000006, 0x00000000);
> -	}
>   
>   	/* Final switch */
>   	if (mclk.pll) {
> @@ -745,12 +743,11 @@ gt215_ram_calc(struct nvkm_ram *base, u32 freq)
>   	ram_nsec(fuc, 2000);
>   
>   	/* Set RAM MR parameters and timings */
> -	for (i = 2; i >= 0; i--) {
> +	for (i = 2; i >= 0; i--)
>   		if (ram_rd32(fuc, mr[i]) != ram->base.mr[i]) {
>   			ram_wr32(fuc, mr[i], ram->base.mr[i]);
>   			ram_nsec(fuc, 1000);
>   		}
> -	}
Idem.>   
>   	ram_wr32(fuc, 0x100220[3], timing[3]);
>   	ram_wr32(fuc, 0x100220[1], timing[1]);
> @@ -838,11 +835,10 @@ gt215_ram_calc(struct nvkm_ram *base, u32 freq)
>   	if (!next->bios.ramcfg_DLLoff)
>   		nvkm_sddr2_dll_reset(fuc);
>   
> -	if (ram->base.type == NVKM_RAM_TYPE_GDDR3) {
> +	if (ram->base.type == NVKM_RAM_TYPE_GDDR3)
>   		ram_nsec(fuc, 31000);
> -	} else {
> +	else
>   		ram_nsec(fuc, 14000);
> -	}
>   
>   	if (ram->base.type == NVKM_RAM_TYPE_DDR3) {
>   		ram_wr32(fuc, 0x100264, 0x1);
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20160921/01384c94/attachment-0001.html>

Roy Spliet

2016-Sep-21 08:09 UTC

head link

[Nouveau] [PATCH 0/5] GPU-DRM-nouveau: Fine-tuning for five function implementations

Thanks for these style fixes. Patches 1 and 2 seem good at first sight. 
Not acking because I haven't tested the patches and the implementation 
of kmalloc_array() appears to slightly differ from kmalloc(). This 
difference *should* only affect allocations larger than the ones in 
patch 1 and 2, but I'd like to see these changes tested before I can 
sleep soundly!

Patches 4 and 5 are:

Reviewed-by: Roy Spliet <nouveau at spliet.org>


Op 21-09-16 om 08:23 schreef SF Markus Elfring:> From: Markus Elfring<elfring at users.sourceforge.net>
> Date: Wed, 21 Sep 2016 09:09:09 +0200
>
> A few update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (5):
>    Use kmalloc_array() in nvbios_iccsense_parse()
>    Use kmalloc_array() in gt215_link_train()
>    Delete unnecessary braces
>    Adjust a kzalloc() call in gt215_ram_new()
>    Add space after an "if"
>
>   drivers/gpu/drm/nouveau/nvkm/subdev/bios/iccsense.c |  4 +++-
>   drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c   | 21
+++++++++------------
>   2 files changed, 12 insertions(+), 13 deletions(-)
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20160921/e09f3c42/attachment-0001.html>

Dan Carpenter

2016-Sep-21 08:47 UTC

head link

[Nouveau] [PATCH 3/5] GPU-DRM-nouveau: Delete unnecessary braces

The original style was correct, the new style is wrong. Multi-line
indents get curly braces for readability.

regards,
dan carpenter

Possibly Parallel Threads

Search for more possibly parallel threads

Nouveau - Sep 2016 - [PATCH 0/5] GPU-DRM-nouveau: Fine-tuning for five function implementations

[Nouveau] [PATCH 0/5] GPU-DRM-nouveau: Fine-tuning for five function implementations

[Nouveau] [PATCH 1/5] GPU-DRM-nouveau: Use kmalloc_array() in nvbios_iccsense_parse()

[Nouveau] [PATCH 2/5] GPU-DRM-nouveau: Use kmalloc_array() in gt215_link_train()

[Nouveau] [PATCH 3/5] GPU-DRM-nouveau: Delete unnecessary braces

[Nouveau] [PATCH 4/5] GPU-DRM-nouveau: Adjust a kzalloc() call in gt215_ram_new()

[Nouveau] [PATCH 5/5] GPU-DRM-nouveau: Add space after an "if"

[Nouveau] [PATCH 3/5] GPU-DRM-nouveau: Delete unnecessary braces

[Nouveau] [PATCH 0/5] GPU-DRM-nouveau: Fine-tuning for five function implementations

[Nouveau] [PATCH 3/5] GPU-DRM-nouveau: Delete unnecessary braces

Possibly Parallel Threads