[AMD Official Use Only - General]
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Thursday, May 11, 2023 7:55 AM
To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@xxxxxxx>
Cc: Koenig, Christian <Christian.Koenig@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
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 <SRINIVASAN.SHANMUGAM@xxxxxxx> wrote:
>
> [AMD Official Use Only - General]
>
>
>
> -----Original Message-----
> From: Alex Deucher <alexdeucher@xxxxxxxxx>
> Sent: Monday, May 8, 2023 9:27 PM
> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@xxxxxxx>
> Cc: Koenig, Christian <Christian.Koenig@xxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> 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 <srinivasan.shanmugam@xxxxxxx> 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&rev=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 <christian.koenig@xxxxxxx>
> > Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> > Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> > ---
> > 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 > 340000)
> > - 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 > 340000)
> > + return true;
> > + else
> > + return false;
> > + } else {
> > + if (pixel_clock > 165000)
> > + return true;
> > + else
> > + return false;
> > }
> > default:
> > return false;
> > --
> > 2.25.1
> >
