Skip to contentSkip to conversation

gpu: drm: radeon: refactor code

PATCH In review Gustavo A. R. Silva 3 participants 3 messages May 17, 2017 – May 23, 2017

Subsystems (2)
DRM drivers Radeon and amdgpu DRM drivers
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 gpu: drm: radeon: refactor code

Local variable _color_ is assigned to a constant value and it is never updated again. Remove this variable and refactor the code it affects.

Addresses-Coverity-ID: 1226745

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
drivers/gpu/drm/radeon/radeon_legacy_encoders.c +1 -7
index 222a1fa..7235d0c 100644
@@ -640,7 +640,6 @@ static enum drm_connector_status radeon_legacy_primary_dac_detect(struct drm_enc
 	uint32_t vclk_ecp_cntl, crtc_ext_cntl;
 	uint32_t dac_ext_cntl, dac_cntl, dac_macro_cntl, tmp;
 	enum drm_connector_status found = connector_status_disconnected;
-	bool color = true;
 
 	/* just don't bother on RN50 those chip are often connected to remoting
 	 * console hw and often we get failure to load detect those. So to make

@@ -665,12 +664,7 @@ static enum drm_connector_status radeon_legacy_primary_dac_detect(struct drm_enc
 	WREG32(RADEON_CRTC_EXT_CNTL, tmp);
 
 	tmp = RADEON_DAC_FORCE_BLANK_OFF_EN |
-		RADEON_DAC_FORCE_DATA_EN;
-
-	if (color)
-		tmp |= RADEON_DAC_FORCE_DATA_SEL_RGB;
-	else
-		tmp |= RADEON_DAC_FORCE_DATA_SEL_G;
+		RADEON_DAC_FORCE_DATA_EN | RADEON_DAC_FORCE_DATA_SEL_RGB;
 
 	if (ASIC_IS_R300(rdev))
 		tmp |= (0x1b6 << RADEON_DAC_FORCE_DATA_SHIFT);
Signature · 1 line
2.5.0
maintainer
#1 lore raw

Am 17.05.2017 um 04:20 schrieb Gustavo A. R. Silva:

Quoted 6 lines

Local variable _color_ is assigned to a constant value and it is never updated again. Remove this variable and refactor the code it affects.

Addresses-Coverity-ID: 1226745

Signed-off-by Gustavo A. R. Silva <[email protected]>

Mhm, on the one hand it looks like a valid cleanup. On the other that is legacy code we haven't touched in a while.

Feel free to put my Reviewed-by: Christian König <[email protected]> on it, but I'm not sure if Alex will pick it up.

Regards,
Christian.

Quoted 20 lines
---
  drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 8 +-------
  1 file changed, 1 insertion(+), 7 deletions(-)
drivers/gpu/drm/radeon/radeon_legacy_encoders.c +0 -1
index 222a1fa..7235d0c 100644
@@ -640,7 +640,6 @@ static enum drm_connector_status radeon_legacy_primary_dac_detect(struct drm_enc
  	uint32_t vclk_ecp_cntl, crtc_ext_cntl;
  	uint32_t dac_ext_cntl, dac_cntl, dac_macro_cntl, tmp;
  	enum drm_connector_status found = connector_status_disconnected;
-	bool color = true;
 
  	/* just don't bother on RN50 those chip are often connected to remoting
  	 * console hw and often we get failure to load detect those. So to make

@@ -665,12 +664,7 @@ static enum drm_connector_status radeon_legacy_primary_dac_detect(struct drm_enc
  	WREG32(RADEON_CRTC_EXT_CNTL, tmp);
 
  	tmp = RADEON_DAC_FORCE_BLANK_OFF_EN |

drivers/gpu/drm/radeon/radeon_legacy_encoders.c static enum drm_connector_status radeon_legacy_primary_dac_detect(struct drm_enc · line 670

Quoted 10 lines
drivers/gpu/drm/radeon/radeon_legacy_encoders.c +1 -6
-		RADEON_DAC_FORCE_DATA_EN;
-
-	if (color)
-		tmp |= RADEON_DAC_FORCE_DATA_SEL_RGB;
-	else
-		tmp |= RADEON_DAC_FORCE_DATA_SEL_G;
+		RADEON_DAC_FORCE_DATA_EN | RADEON_DAC_FORCE_DATA_SEL_RGB;
 
  	if (ASIC_IS_R300(rdev))
  		tmp |= (0x1b6 << RADEON_DAC_FORCE_DATA_SHIFT);

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

maintainer
#2 lore raw
Quoted 13 lines

-----Original Message-----
From: Koenig, Christian
Sent: Wednesday, May 17, 2017 4:23 AM
To: Gustavo A. R. Silva; Deucher, Alexander; David Airlie
Cc: [email protected]; [email protected]; linux- [email protected]
Subject: Re: [PATCH] gpu: drm: radeon: refactor code

Am 17.05.2017 um 04:20 schrieb Gustavo A. R. Silva:

Quoted 4 lines

Local variable _color_ is assigned to a constant value and it is never updated again. Remove this variable and refactor the code it affects.

Quoted 8 lines
Quoted 2 lines

Addresses-Coverity-ID: 1226745

Signed-off-by Gustavo A. R. Silva <[email protected]>

Mhm, on the one hand it looks like a valid cleanup. On the other that is legacy code we haven't touched in a while.

Feel free to put my Reviewed-by: Christian König <[email protected]> on it, but I'm not sure if Alex will pick it up.

It's like that to show how to do color vs. mono load detection. It's not something we supported, but others using the hw may be interested.

Alex

Quoted 29 lines

Regards,
Christian.

Review

Quoted 5 lines
---
  drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 8 +-------
  1 file changed, 1 insertion(+), 7 deletions(-)
(quoted diff) +0 -0

b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c

drivers/gpu/drm/radeon/radeon_legacy_encoders.c static enum drm_connector_status

Quoted 4 lines

index 222a1fa..7235d0c 100644

drivers/gpu/drm/radeon/radeon_legacy_encoders.c +0 -0
@@ -640,7 +640,6 @@ static enum drm_connector_status

radeon_legacy_primary_dac_detect(struct drm_enc

drivers/gpu/drm/radeon/radeon_legacy_encoders.c static enum drm_connector_status radeon_legacy_primary_dac_detect(struct drm_enc

Quoted 3 lines
drivers/gpu/drm/radeon/radeon_legacy_encoders.c +0 -0
  	uint32_t vclk_ecp_cntl, crtc_ext_cntl;
  	uint32_t dac_ext_cntl, dac_cntl, dac_macro_cntl, tmp;
  	enum drm_connector_status found =

connector_status_disconnected;

drivers/gpu/drm/radeon/radeon_legacy_encoders.c static enum drm_connector_status radeon_legacy_primary_dac_detect(struct drm_enc

Quoted 3 lines
drivers/gpu/drm/radeon/radeon_legacy_encoders.c +0 -1
-	bool color = true;
 
  	/* just don't bother on RN50 those chip are often connected to

remoting

drivers/gpu/drm/radeon/radeon_legacy_encoders.c static enum drm_connector_status · line 645

Quoted 1 line
drivers/gpu/drm/radeon/radeon_legacy_encoders.c +0 -0
  	 * console hw and often we get failure to load detect those. So to

make

drivers/gpu/drm/radeon/radeon_legacy_encoders.c static enum drm_connector_status

Quoted 1 line
drivers/gpu/drm/radeon/radeon_legacy_encoders.c +0 -0
@@ -665,12 +664,7 @@ static enum drm_connector_status

radeon_legacy_primary_dac_detect(struct drm_enc

Quoted 2 lines
drivers/gpu/drm/radeon/radeon_legacy_encoders.c +0 -0
  	WREG32(RADEON_CRTC_EXT_CNTL, tmp);
Quoted 13 lines

drivers/gpu/drm/radeon/radeon_legacy_encoders.c static enum drm_connector_status radeon_legacy_primary_dac_detect(struct drm_enc

Quoted 8 lines
drivers/gpu/drm/radeon/radeon_legacy_encoders.c +1 -6
  	tmp = RADEON_DAC_FORCE_BLANK_OFF_EN |
-		RADEON_DAC_FORCE_DATA_EN;
-
-	if (color)
-		tmp |= RADEON_DAC_FORCE_DATA_SEL_RGB;
-	else
-		tmp |= RADEON_DAC_FORCE_DATA_SEL_G;
+		RADEON_DAC_FORCE_DATA_EN |

RADEON_DAC_FORCE_DATA_SEL_RGB;

Quoted 3 lines
drivers/gpu/drm/radeon/radeon_legacy_encoders.c +0 -0
  	if (ASIC_IS_R300(rdev))
  		tmp |= (0x1b6 << RADEON_DAC_FORCE_DATA_SHIFT);

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx