[PATCH 0/3] GPU-DRM-Radeon: Fine-tuning for three function implementations

[PATCH 0/3] GPU-DRM-Radeon: Fine-tuning for three function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 May 2017 22:00:02 +0200

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

Markus Elfring (3):
  Use seq_putc() in radeon_sa_bo_dump_debug_info()
  Use seq_puts() in radeon_debugfs_pm_info()
  Use seq_puts() in r100_debugfs_cp_csq_fifo()

 drivers/gpu/drm/radeon/r100.c      | 6 +++---
 drivers/gpu/drm/radeon/radeon_pm.c | 5 +++--
 drivers/gpu/drm/radeon/radeon_sa.c | 9 +++------
 3 files changed, 9 insertions(+), 11 deletions(-)

-- 
2.12.2

[PATCH 1/3] GPU-DRM-Radeon: Use seq_putc() in radeon_sa_bo_dump_debug_info()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 May 2017 21:35:48 +0200

A few single characters should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/radeon/radeon_sa.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
index 197b157b73d0..67bc3618798d 100644
--- a/drivers/gpu/drm/radeon/radeon_sa.c
+++ b/drivers/gpu/drm/radeon/radeon_sa.c
@@ -406,18 +406,15 @@ void radeon_sa_bo_dump_debug_info(struct radeon_sa_manager *sa_manager,
 	list_for_each_entry(i, &sa_manager->olist, olist) {
 		uint64_t soffset = i->soffset + sa_manager->gpu_addr;
 		uint64_t eoffset = i->eoffset + sa_manager->gpu_addr;
-		if (&i->olist == sa_manager->hole) {
-			seq_printf(m, ">");
-		} else {
-			seq_printf(m, " ");
-		}
+
+		seq_putc(m, (&i->olist == sa_manager->hole) ? '>' : ' ');
 		seq_printf(m, "[0x%010llx 0x%010llx] size %8lld",
 			   soffset, eoffset, eoffset - soffset);
 		if (i->fence) {
 			seq_printf(m, " protected by 0x%016llx on ring %d",
 				   i->fence->seq, i->fence->ring);
 		}
-		seq_printf(m, "\n");
+		seq_putc(m, '\n');
 	}
 	spin_unlock(&sa_manager->wq.lock);
 }
-- 
2.12.2

[PATCH 2/3] GPU-DRM-Radeon: Use seq_puts() in radeon_debugfs_pm_info()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 May 2017 21:50:14 +0200

Two strings which did not contain data format specifications should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

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

diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
index 326ad068c15a..f84ddcc426c1 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -1869,13 +1869,14 @@ static int radeon_debugfs_pm_info(struct seq_file *m, void *data)
 
 	if  ((rdev->flags & RADEON_IS_PX) &&
 	     (ddev->switch_power_state != DRM_SWITCH_POWER_ON)) {
-		seq_printf(m, "PX asic powered off\n");
+		seq_puts(m, "PX asic powered off\n");
 	} else if (rdev->pm.dpm_enabled) {
 		mutex_lock(&rdev->pm.mutex);
 		if (rdev->asic->dpm.debugfs_print_current_performance_level)
 			radeon_dpm_debugfs_print_current_performance_level(rdev, m);
 		else
-			seq_printf(m, "Debugfs support not implemented for this asic\n");
+			seq_puts(m,
+				 "Debugfs support not implemented for this asic\n");
 		mutex_unlock(&rdev->pm.mutex);
 	} else {
 		seq_printf(m, "default engine clock: %u0 kHz\n", rdev->pm.default_sclk);
-- 
2.12.2

[PATCH 3/3] GPU-DRM-Radeon: Use seq_puts() in r100_debugfs_cp_csq_fifo()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 May 2017 21:54:49 +0200

Strings which did not contain data format specifications should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/radeon/r100.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index c31e660e35db..09b88738aa07 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -2992,19 +2992,19 @@ static int r100_debugfs_cp_csq_fifo(struct seq_file *m, void *data)
 	seq_printf(m, "Indirect2 wptr %u\n", ib2_wptr);
 	/* FIXME: 0, 128, 640 depends on fifo setup see cp_init_kms
 	 * 128 = indirect1_start * 8 & 640 = indirect2_start * 8 */
-	seq_printf(m, "Ring fifo:\n");
+	seq_puts(m, "Ring fifo:\n");
 	for (i = 0; i < 256; i++) {
 		WREG32(RADEON_CP_CSQ_ADDR, i << 2);
 		tmp = RREG32(RADEON_CP_CSQ_DATA);
 		seq_printf(m, "rfifo[%04d]=0x%08X\n", i, tmp);
 	}
-	seq_printf(m, "Indirect1 fifo:\n");
+	seq_puts(m, "Indirect1 fifo:\n");
 	for (i = 256; i <= 512; i++) {
 		WREG32(RADEON_CP_CSQ_ADDR, i << 2);
 		tmp = RREG32(RADEON_CP_CSQ_DATA);
 		seq_printf(m, "ib1fifo[%04d]=0x%08X\n", i, tmp);
 	}
-	seq_printf(m, "Indirect2 fifo:\n");
+	seq_puts(m, "Indirect2 fifo:\n");
 	for (i = 640; i < ib1_wptr; i++) {
 		WREG32(RADEON_CP_CSQ_ADDR, i << 2);
 		tmp = RREG32(RADEON_CP_CSQ_DATA);
-- 
2.12.2

Re: [PATCH 0/3] GPU-DRM-Radeon: Fine-tuning for three function implementations

[Christian König]

Am 02.05.2017 um 22:04 schrieb SF Markus Elfring:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 2 May 2017 22:00:02 +0200
>
> Three update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (3):
>    Use seq_putc() in radeon_sa_bo_dump_debug_info()
>    Use seq_puts() in radeon_debugfs_pm_info()
>    Use seq_puts() in r100_debugfs_cp_csq_fifo()

Reviewed-by: Christian König <christian.koenig@amd.com>

>
>   drivers/gpu/drm/radeon/r100.c      | 6 +++---
>   drivers/gpu/drm/radeon/radeon_pm.c | 5 +++--
>   drivers/gpu/drm/radeon/radeon_sa.c | 9 +++------
>   3 files changed, 9 insertions(+), 11 deletions(-)
>

Re: [PATCH 0/3] GPU-DRM-Radeon: Fine-tuning for three function implementations

[Michel Dänzer]

On 03/05/17 09:46 PM, Christian König wrote:
> Am 02.05.2017 um 22:04 schrieb SF Markus Elfring:
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Tue, 2 May 2017 22:00:02 +0200
>>
>> Three update suggestions were taken into account
>> from static source code analysis.
>>
>> Markus Elfring (3):
>>    Use seq_putc() in radeon_sa_bo_dump_debug_info()
>>    Use seq_puts() in radeon_debugfs_pm_info()
>>    Use seq_puts() in r100_debugfs_cp_csq_fifo()
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>

Based on
https://lists.freedesktop.org/archives/dri-devel/2017-May/140837.html
and followups, I'm afraid we'll have to make sure Markus' patches have
been tested adequately before applying them.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 0/3] GPU-DRM-Radeon: Fine-tuning for three function implementations

[Christian König]

Am 10.05.2017 um 02:23 schrieb Michel Dänzer:
> On 03/05/17 09:46 PM, Christian König wrote:
>> Am 02.05.2017 um 22:04 schrieb SF Markus Elfring:
>>> From: Markus Elfring <elfring@users.sourceforge.net>
>>> Date: Tue, 2 May 2017 22:00:02 +0200
>>>
>>> Three update suggestions were taken into account
>>> from static source code analysis.
>>>
>>> Markus Elfring (3):
>>>     Use seq_putc() in radeon_sa_bo_dump_debug_info()
>>>     Use seq_puts() in radeon_debugfs_pm_info()
>>>     Use seq_puts() in r100_debugfs_cp_csq_fifo()
>> Reviewed-by: Christian König <christian.koenig@amd.com>
> Based on
> https://lists.freedesktop.org/archives/dri-devel/2017-May/140837.html
> and followups, I'm afraid we'll have to make sure Markus' patches have
> been tested adequately before applying them.

I can't judge the background of that decision, but at least those tree 
patches for radeon looked trivial to me.

I don't see much what could go wrong when merging them. On the other 
hand you are right that trying to find hardware to test that stuff could 
be challenging.

Christian.

Re: [PATCH 0/3] GPU-DRM-Radeon: Fine-tuning for three function implementations

[Michel Dänzer]

On 10/05/17 08:30 PM, Christian König wrote:
> Am 10.05.2017 um 02:23 schrieb Michel Dänzer:
>> On 03/05/17 09:46 PM, Christian König wrote:
>>> Am 02.05.2017 um 22:04 schrieb SF Markus Elfring:
>>>> From: Markus Elfring <elfring@users.sourceforge.net>
>>>> Date: Tue, 2 May 2017 22:00:02 +0200
>>>>
>>>> Three update suggestions were taken into account
>>>> from static source code analysis.
>>>>
>>>> Markus Elfring (3):
>>>>     Use seq_putc() in radeon_sa_bo_dump_debug_info()
>>>>     Use seq_puts() in radeon_debugfs_pm_info()
>>>>     Use seq_puts() in r100_debugfs_cp_csq_fifo()
>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>> Based on
>> https://lists.freedesktop.org/archives/dri-devel/2017-May/140837.html
>> and followups, I'm afraid we'll have to make sure Markus' patches have
>> been tested adequately before applying them.
> 
> I can't judge the background of that decision, but at least those tree
> patches for radeon looked trivial to me.

Which is part of the issue, see also
https://lists.freedesktop.org/archives/dri-devel/2017-May/140694.html
and other posts in that thread.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 0/3] GPU-DRM-Radeon: Fine-tuning for three function implementations

[Laurent Pinchart]

Hi Christian,

On Wednesday 10 May 2017 13:30:37 Christian König wrote:
> Am 10.05.2017 um 02:23 schrieb Michel Dänzer:
> > On 03/05/17 09:46 PM, Christian König wrote:
> >> Am 02.05.2017 um 22:04 schrieb SF Markus Elfring:
> >>> From: Markus Elfring <elfring@users.sourceforge.net>
> >>> Date: Tue, 2 May 2017 22:00:02 +0200
> >>> 
> >>> Three update suggestions were taken into account
> >>> from static source code analysis.
> >>> 
> >>> Markus Elfring (3):
> >>>     Use seq_putc() in radeon_sa_bo_dump_debug_info()
> >>>     Use seq_puts() in radeon_debugfs_pm_info()
> >>>     Use seq_puts() in r100_debugfs_cp_csq_fifo()
> >> 
> >> Reviewed-by: Christian König <christian.koenig@amd.com>
> > 
> > Based on
> > https://lists.freedesktop.org/archives/dri-devel/2017-May/140837.html
> > and followups, I'm afraid we'll have to make sure Markus' patches have
> > been tested adequately before applying them.
> 
> I can't judge the background of that decision, but at least those tree
> patches for radeon looked trivial to me.
> 
> I don't see much what could go wrong when merging them.

For what it's worth, I've once applied a patch from Markus for the uvcvideo 
driver that seemed trivial but ended up introducing a breakage that I hadn't 
caught during review. I recommend rejecting Markus' patches that are not clear 
improvements, and carefully testing the others.

> On the other hand you are right that trying to find hardware to test that
> stuff could be challenging.

-- 
Regards,

Laurent Pinchart