[PATCH] drm/amd/display: Fix identical code for different branches

[PATCH] drm/amd/display: Fix identical code for different branches

[Len Baker]

The ternary expression:

vrr->state == VRR_STATE_ACTIVE_VARIABLE ? max_refresh :	max_refresh;

has identical then and else expressions. So, simplify the code.

Addresses-Coverity-ID: 1471122 ("Identical code for different branches")
Fixes: 9bc4162665827 ("drm/amd/display: Implement VSIF V3 extended refresh rate feature")
Signed-off-by: Len Baker <len.baker@gmx.com>
---
 drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
index 3f4f44b44e6a..54374c7d309b 100644
--- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
+++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
@@ -613,9 +613,8 @@ static void build_vrr_infopacket_data_v3(const struct mod_vrr_params *vrr,
 			(vrr->state == VRR_STATE_INACTIVE) ? min_refresh :
 			max_refresh; // Non-fs case, program nominal range

-	max_programmed = (vrr->state == VRR_STATE_ACTIVE_FIXED) ? fixed_refresh :
-			(vrr->state == VRR_STATE_ACTIVE_VARIABLE) ? max_refresh :
-			max_refresh;// Non-fs case, program nominal range
+	max_programmed = (vrr->state == VRR_STATE_ACTIVE_FIXED) ?
+		fixed_refresh : max_refresh;

 	/* PB7 = FreeSync Minimum refresh rate (Hz) */
 	infopacket->sb[7] = min_programmed & 0xFF;
--
2.25.1

[PATCH] drm/amd/display: Fix identical code for different branches

[Len Baker]

The branches of the "if" statement are the same. So remove the
unnecessary if and goto statements.

Addresses-Coverity-ID: 1456916 ("Identical code for different branches")
Fixes: 4c283fdac08ab ("drm/amd/display: Add HDCP module")
Signed-off-by: Len Baker <len.baker@gmx.com>
---
 drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
index de872e7958b0..d0c565567102 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
@@ -305,10 +305,8 @@ static enum mod_hdcp_status wait_for_ready(struct mod_hdcp *hdcp,
 				hdcp, "bcaps_read"))
 			goto out;
 	}
-	if (!mod_hdcp_execute_and_set(check_ksv_ready,
-			&input->ready_check, &status,
-			hdcp, "ready_check"))
-		goto out;
+	mod_hdcp_execute_and_set(check_ksv_ready, &input->ready_check, &status,
+				 hdcp, "ready_check");
 out:
 	return status;
 }
--
2.25.1

Re: [PATCH] drm/amd/display: Fix identical code for different branches

[Joe Perches]

On Sun, 2021-07-11 at 19:24 +0200, Len Baker wrote:
> The branches of the "if" statement are the same. So remove the
> unnecessary if and goto statements.
> 
> Addresses-Coverity-ID: 1456916 ("Identical code for different branches")
> Fixes: 4c283fdac08ab ("drm/amd/display: Add HDCP module")
> Signed-off-by: Len Baker <len.baker@gmx.com>

I'm not a big fan of this type of change.

It's currently the same style used for six tests in this function
and changing this last one would just make it harder to see the
code blocks as consistent.

I doubt any reasonable compiler would produce different objects.

> diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
[]
> @@ -305,10 +305,8 @@ static enum mod_hdcp_status wait_for_ready(struct mod_hdcp *hdcp,
>  				hdcp, "bcaps_read"))
>  			goto out;
>  	}
> -	if (!mod_hdcp_execute_and_set(check_ksv_ready,
> -			&input->ready_check, &status,
> -			hdcp, "ready_check"))
> -		goto out;
> +	mod_hdcp_execute_and_set(check_ksv_ready, &input->ready_check, &status,
> +				 hdcp, "ready_check");
>  out:
>  	return status;
>  }
> --
> 2.25.1
> 

Re: [PATCH] drm/amd/display: Fix identical code for different branches

[Len Baker]

On Sun, Jul 11, 2021 at 10:45:48AM -0700, Joe Perches wrote:
> On Sun, 2021-07-11 at 19:24 +0200, Len Baker wrote:
> > The branches of the "if" statement are the same. So remove the
> > unnecessary if and goto statements.
> >
> > Addresses-Coverity-ID: 1456916 ("Identical code for different branches")
> > Fixes: 4c283fdac08ab ("drm/amd/display: Add HDCP module")
> > Signed-off-by: Len Baker <len.baker@gmx.com>
>
> I'm not a big fan of this type of change.
>
> It's currently the same style used for six tests in this function
> and changing this last one would just make it harder to see the
> code blocks as consistent.
>
> I doubt any reasonable compiler would produce different objects.

Ok, thanks for the review. I leave it as is.

> > diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
> []
> > @@ -305,10 +305,8 @@ static enum mod_hdcp_status wait_for_ready(struct mod_hdcp *hdcp,
> >  				hdcp, "bcaps_read"))
> >  			goto out;
> >  	}
> > -	if (!mod_hdcp_execute_and_set(check_ksv_ready,
> > -			&input->ready_check, &status,
> > -			hdcp, "ready_check"))
> > -		goto out;
> > +	mod_hdcp_execute_and_set(check_ksv_ready, &input->ready_check, &status,
> > +				 hdcp, "ready_check");
> >  out:
> >  	return status;
> >  }
> > --
> > 2.25.1
> >

Thanks,
Len