RE: [PATCH] drm/amd/amdgpu: Remove redundant else branch in amdgpu_encoders.c
[AMD Official Use Only - General]
-Original Message-
From: Alex Deucher
Sent: Thursday, May 11, 2023 7:55 AM
To: SHANMUGAM, SRINIVASAN
Cc: Koenig, Christian ; Deucher, Alexander
; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: Remove redundant else branch in
amdgpu_encoders.c
On Tue, May 9, 2023 at 1:17 AM SHANMUGAM, SRINIVASAN
wrote:
>
> [AMD Official Use Only - General]
>
>
>
> -Original Message-
> From: Alex Deucher
> Sent: Monday, May 8, 2023 9:27 PM
> To: SHANMUGAM, SRINIVASAN
> Cc: Koenig, Christian ; Deucher, Alexander
> ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amd/amdgpu: Remove redundant else branch in
> amdgpu_encoders.c
>
> On Mon, May 8, 2023 at 11:29 AM Srinivasan Shanmugam
> wrote:
> >
> > Adhere to Linux kernel coding style.
> >
> > Reported by checkpatch:
> >
> > WARNING: else is not generally useful after a break or return
> >
>
> What about the else in the previous case statement?
>
> Alex
>
> Hi Alex,
>
> Thanks a lot for your feedbacks,
>
> the else in the previous case ie., is binded to if statement ie., "if
> (amdgpu_connector->use_digital) {", am I correct please?, please correct me,
> if my understanding is wrong? & the best solution with your tips pls, so that
> I can edit & resend the patch please?
>
Yes that one. It follows a similar pattern to the case you changed.
Shouldn't checkpatch warn on both?
Alex
So sorry Alex, couldn't reply instantly, was occupied with something else.
Yes Alex, somehow checkpatch was pointing only to below, hence avoided multiple
return statements with a break without affecting functionality & proposed a v2
patch: https://patchwork.freedesktop.org/patch/537520/?series=117468=2
WARNING: else is not generally useful after a break or return
#245: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c:245:
+ return false;
+ else {
Srini
> Much appreciate for your help in advance,
>
> > Cc: Christian König
> > Cc: Alex Deucher
> > Signed-off-by: Srinivasan Shanmugam
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 26
> > ++--
> > 1 file changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > index c96e458ed088..049e9976ff34 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > @@ -242,19 +242,18 @@ bool amdgpu_dig_monitor_is_duallink(struct
> > drm_encoder *encoder,
> > if ((dig_connector->dp_sink_type ==
> > CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
> > (dig_connector->dp_sink_type ==
> > CONNECTOR_OBJECT_ID_eDP))
> > return false;
> > - else {
> > - /* HDMI 1.3 supports up to 340 Mhz over single link
> > */
> > - if (connector->display_info.is_hdmi) {
> > - if (pixel_clock > 34)
> > - return true;
> > - else
> > - return false;
> > - } else {
> > - if (pixel_clock > 165000)
> > - return true;
> > - else
> > - return false;
> > - }
> > +
> > + /* HDMI 1.3 supports up to 340 Mhz over single link */
> > + if (connector->display_info.is_hdmi) {
> > + if (pixel_clock > 34)
> > + return true;
> > + else
> > + return false;
> > + } else {
> > + if (pixel_clock > 165000)
> > + return true;
> > + else
> > + return false;
> > }
> > default:
> > return false;
> > --
> > 2.25.1
> >
Re: [PATCH] drm/amd/amdgpu: Remove redundant else branch in amdgpu_encoders.c
On Tue, May 9, 2023 at 1:17 AM SHANMUGAM, SRINIVASAN
wrote:
>
> [AMD Official Use Only - General]
>
>
>
> -Original Message-
> From: Alex Deucher
> Sent: Monday, May 8, 2023 9:27 PM
> To: SHANMUGAM, SRINIVASAN
> Cc: Koenig, Christian ; Deucher, Alexander
> ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amd/amdgpu: Remove redundant else branch in
> amdgpu_encoders.c
>
> On Mon, May 8, 2023 at 11:29 AM Srinivasan Shanmugam
> wrote:
> >
> > Adhere to Linux kernel coding style.
> >
> > Reported by checkpatch:
> >
> > WARNING: else is not generally useful after a break or return
> >
>
> What about the else in the previous case statement?
>
> Alex
>
> Hi Alex,
>
> Thanks a lot for your feedbacks,
>
> the else in the previous case ie., is binded to if statement ie., "if
> (amdgpu_connector->use_digital) {", am I correct please?, please correct me,
> if my understanding is wrong? & the best solution with your tips pls, so that
> I can edit & resend the patch please?
>
Yes that one. It follows a similar pattern to the case you changed.
Shouldn't checkpatch warn on both?
Alex
> Much appreciate for your help in advance,
>
> > Cc: Christian König
> > Cc: Alex Deucher
> > Signed-off-by: Srinivasan Shanmugam
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 26
> > ++--
> > 1 file changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > index c96e458ed088..049e9976ff34 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > @@ -242,19 +242,18 @@ bool amdgpu_dig_monitor_is_duallink(struct
> > drm_encoder *encoder,
> > if ((dig_connector->dp_sink_type ==
> > CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
> > (dig_connector->dp_sink_type ==
> > CONNECTOR_OBJECT_ID_eDP))
> > return false;
> > - else {
> > - /* HDMI 1.3 supports up to 340 Mhz over single link
> > */
> > - if (connector->display_info.is_hdmi) {
> > - if (pixel_clock > 34)
> > - return true;
> > - else
> > - return false;
> > - } else {
> > - if (pixel_clock > 165000)
> > - return true;
> > - else
> > - return false;
> > - }
> > +
> > + /* HDMI 1.3 supports up to 340 Mhz over single link */
> > + if (connector->display_info.is_hdmi) {
> > + if (pixel_clock > 34)
> > + return true;
> > + else
> > + return false;
> > + } else {
> > + if (pixel_clock > 165000)
> > + return true;
> > + else
> > + return false;
> > }
> > default:
> > return false;
> > --
> > 2.25.1
> >
RE: [PATCH] drm/amd/amdgpu: Remove redundant else branch in amdgpu_encoders.c
[AMD Official Use Only - General]
-Original Message-
From: Alex Deucher
Sent: Monday, May 8, 2023 9:27 PM
To: SHANMUGAM, SRINIVASAN
Cc: Koenig, Christian ; Deucher, Alexander
; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: Remove redundant else branch in
amdgpu_encoders.c
On Mon, May 8, 2023 at 11:29 AM Srinivasan Shanmugam
wrote:
>
> Adhere to Linux kernel coding style.
>
> Reported by checkpatch:
>
> WARNING: else is not generally useful after a break or return
>
What about the else in the previous case statement?
Alex
Hi Alex,
Thanks a lot for your feedbacks,
the else in the previous case ie., is binded to if statement ie., "if
(amdgpu_connector->use_digital) {", am I correct please?, please correct me, if
my understanding is wrong? & the best solution with your tips pls, so that I
can edit & resend the patch please?
Much appreciate for your help in advance,
> Cc: Christian König
> Cc: Alex Deucher
> Signed-off-by: Srinivasan Shanmugam
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 26
> ++--
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> index c96e458ed088..049e9976ff34 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> @@ -242,19 +242,18 @@ bool amdgpu_dig_monitor_is_duallink(struct drm_encoder
> *encoder,
> if ((dig_connector->dp_sink_type ==
> CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
> (dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP))
> return false;
> - else {
> - /* HDMI 1.3 supports up to 340 Mhz over single link */
> - if (connector->display_info.is_hdmi) {
> - if (pixel_clock > 34)
> - return true;
> - else
> - return false;
> - } else {
> - if (pixel_clock > 165000)
> - return true;
> - else
> - return false;
> - }
> +
> + /* HDMI 1.3 supports up to 340 Mhz over single link */
> + if (connector->display_info.is_hdmi) {
> + if (pixel_clock > 34)
> + return true;
> + else
> + return false;
> + } else {
> + if (pixel_clock > 165000)
> + return true;
> + else
> + return false;
> }
> default:
> return false;
> --
> 2.25.1
>
Re: [PATCH] drm/amd/amdgpu: Remove redundant else branch in amdgpu_encoders.c
On Mon, May 8, 2023 at 11:29 AM Srinivasan Shanmugam
wrote:
>
> Adhere to Linux kernel coding style.
>
> Reported by checkpatch:
>
> WARNING: else is not generally useful after a break or return
>
What about the else in the previous case statement?
Alex
> Cc: Christian König
> Cc: Alex Deucher
> Signed-off-by: Srinivasan Shanmugam
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 26 ++--
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> index c96e458ed088..049e9976ff34 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> @@ -242,19 +242,18 @@ bool amdgpu_dig_monitor_is_duallink(struct drm_encoder
> *encoder,
> if ((dig_connector->dp_sink_type ==
> CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
> (dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP))
> return false;
> - else {
> - /* HDMI 1.3 supports up to 340 Mhz over single link */
> - if (connector->display_info.is_hdmi) {
> - if (pixel_clock > 34)
> - return true;
> - else
> - return false;
> - } else {
> - if (pixel_clock > 165000)
> - return true;
> - else
> - return false;
> - }
> +
> + /* HDMI 1.3 supports up to 340 Mhz over single link */
> + if (connector->display_info.is_hdmi) {
> + if (pixel_clock > 34)
> + return true;
> + else
> + return false;
> + } else {
> + if (pixel_clock > 165000)
> + return true;
> + else
> + return false;
> }
> default:
> return false;
> --
> 2.25.1
>
