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
[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
[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
[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
[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
[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
[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
[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>
Reasonably Related Threads
- [PATCH 0/5] GPU-DRM-nouveau: Fine-tuning for five function implementations
- [PATCH 02/11] nvkm/ramgf100: Calculate timings
- [PATCH 03/11] nvkm/gddr5: MR calculation for timing table v1.0
- [PATCH 2/7] fb/ramnva3: Link training for DDR3
- All-round reclocking improvements