[PATCH v2] drm/amd/amdgpu: Remove redundant else branch in amdgpu_encoders.c

[Deucher, Alexander]

[AMD Official Use Only - General]

> -----Original Message-----
> From: SHANMUGAM, SRINIVASAN
> <SRINIVASAN.SHANMUGAM at amd.com>
> Sent: Wednesday, May 17, 2023 6:07 AM
> To: Koenig, Christian <Christian.Koenig at amd.com>; Deucher, Alexander
> <Alexander.Deucher at amd.com>
> Cc: amd-gfx at lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> <SRINIVASAN.SHANMUGAM at amd.com>
> Subject: [PATCH v2] drm/amd/amdgpu: Remove redundant else branch in
> amdgpu_encoders.c
> 
> Adhere to Linux kernel coding style.
> 
> Reported by checkpatch:
> 
> WARNING: else is not generally useful after a break or return
> 
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
> 
> v2:
>    - Avoid mulitple return statements in amdgpu_dig_monitor_is_duallink
>      function without affecting functionality
>    - Fix following warnings:
>         WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> 	WARNING: Missing a blank line after declarations
> #74: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c:74:
> +                       struct amdgpu_connector *amdgpu_connector =
> to_amdgpu_connector(connector);
> +                       amdgpu_encoder->active_device =
> + amdgpu_encoder->devices & amdgpu_connector->devices;
> 
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 41 ++++++++++++---
> -----
>  1 file changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> index c96e458ed088..8c17a74f001a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> @@ -71,6 +71,7 @@ void amdgpu_encoder_set_active_device(struct
> drm_encoder *encoder)
>  	drm_for_each_connector_iter(connector, &iter) {
>  		if (connector->encoder == encoder) {
>  			struct amdgpu_connector *amdgpu_connector =
> to_amdgpu_connector(connector);
> +
>  			amdgpu_encoder->active_device =
> amdgpu_encoder->devices & amdgpu_connector->devices;
>  			DRM_DEBUG_KMS("setting active device to %08x
> from %08x %08x for encoder %d\n",
>  				  amdgpu_encoder->active_device,
> amdgpu_encoder->devices, @@ -166,12 +167,12 @@ void
> amdgpu_panel_mode_fixup(struct drm_encoder *encoder,  {
>  	struct amdgpu_encoder *amdgpu_encoder =
> to_amdgpu_encoder(encoder);
>  	struct drm_display_mode *native_mode = &amdgpu_encoder-
> >native_mode;
> -	unsigned hblank = native_mode->htotal - native_mode->hdisplay;
> -	unsigned vblank = native_mode->vtotal - native_mode->vdisplay;
> -	unsigned hover = native_mode->hsync_start - native_mode-
> >hdisplay;
> -	unsigned vover = native_mode->vsync_start - native_mode-
> >vdisplay;
> -	unsigned hsync_width = native_mode->hsync_end - native_mode-
> >hsync_start;
> -	unsigned vsync_width = native_mode->vsync_end - native_mode-
> >vsync_start;
> +	unsigned int hblank = native_mode->htotal - native_mode-
> >hdisplay;
> +	unsigned int vblank = native_mode->vtotal - native_mode->vdisplay;
> +	unsigned int hover = native_mode->hsync_start - native_mode-
> >hdisplay;
> +	unsigned int vover = native_mode->vsync_start - native_mode-
> >vdisplay;
> +	unsigned int hsync_width = native_mode->hsync_end -
> native_mode->hsync_start;
> +	unsigned int vsync_width = native_mode->vsync_end -
> +native_mode->vsync_start;
> 

This part looks fine.

>  	adjusted_mode->clock = native_mode->clock;
>  	adjusted_mode->flags = native_mode->flags; @@ -208,6 +209,7 @@
> bool amdgpu_dig_monitor_is_duallink(struct drm_encoder *encoder,
>  	struct drm_connector *connector;
>  	struct amdgpu_connector *amdgpu_connector;
>  	struct amdgpu_connector_atom_dig *dig_connector;
> +	bool ret;
> 
>  	connector = amdgpu_get_connector_for_encoder(encoder);
>  	/* if we don't have an active device yet, just use one of @@ -224,39
> +226,44 @@ bool amdgpu_dig_monitor_is_duallink(struct drm_encoder
> *encoder,
>  			/* HDMI 1.3 supports up to 340 Mhz over single link
> */
>  			if (connector->display_info.is_hdmi) {
>  				if (pixel_clock > 340000)
> -					return true;
> +					ret = true;
>  				else
> -					return false;
> +					ret = false;
>  			} else {
>  				if (pixel_clock > 165000)
> -					return true;
> +					ret = true;
>  				else
> -					return false;
> +					ret =  false;
>  			}
>  		} else
> -			return false;
> +			ret = false;
> +		break;
>  	case DRM_MODE_CONNECTOR_DVID:
>  	case DRM_MODE_CONNECTOR_HDMIA:
>  	case DRM_MODE_CONNECTOR_DisplayPort:
>  		dig_connector = amdgpu_connector->con_priv;
>  		if ((dig_connector->dp_sink_type ==
> CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
>  		    (dig_connector->dp_sink_type ==
> CONNECTOR_OBJECT_ID_eDP))
> -			return false;
> +			ret = false;
>  		else {
>  			/* HDMI 1.3 supports up to 340 Mhz over single link
> */
>  			if (connector->display_info.is_hdmi) {
>  				if (pixel_clock > 340000)
> -					return true;
> +					ret = true;
>  				else
> -					return false;
> +					ret = false;
>  			} else {
>  				if (pixel_clock > 165000)
> -					return true;
> +					ret = true;
>  				else
> -					return false;
> +					ret = false;
>  			}
>  		}
> +		break;
>  	default:
> -		return false;
> +		ret = false;
> +		break;
>  	}
> +
> +	return ret;

These hunks make the code less readable.  I'd suggest dropping them.

Alex

>  }
> --
> 2.25.1