[PATCH] gpu: drm: radeon: refactor code

[PATCH] gpu: drm: radeon: refactor code

[Gustavo A. R. Silva]

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 <garsilva@embeddedor.com>
---
 drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 222a1fa..7235d0c 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -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);
-- 
2.5.0

Re: [PATCH] gpu: drm: radeon: refactor code

[Christian König]

Am 17.05.2017 um 04:20 schrieb Gustavo A. R. Silva:
> 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 <garsilva@embeddedor.com>

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 
<christian.koenig@amd.com> on it, but I'm not sure if Alex will pick it up.

Regards,
Christian.

> ---
>   drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 8 +-------
>   1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> index 222a1fa..7235d0c 100644
> --- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> +++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> @@ -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);

RE: [PATCH] gpu: drm: radeon: refactor code

[Deucher, Alexander]

> -----Original Message-----
> From: Koenig, Christian
> Sent: Wednesday, May 17, 2017 4:23 AM
> To: Gustavo A. R. Silva; Deucher, Alexander; David Airlie
> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] gpu: drm: radeon: refactor code
> 
> Am 17.05.2017 um 04:20 schrieb Gustavo A. R. Silva:
> > 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 <garsilva@embeddedor.com>
> 
> 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
> <christian.koenig@amd.com> 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

> 
> Regards,
> Christian.
> 
> > ---
> >   drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 8 +-------
> >   1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> > index 222a1fa..7235d0c 100644
> > --- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> > +++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> > @@ -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);
>