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

From: Deucher, Alexander
Date: Tue May 23 2017 - 11:35:29 EST


> -----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@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxx>
>
> 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@xxxxxxx> 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);
>