| Message ID | 990112183d2bc344bd921bb55eee2f8cc2cd8bd5.1582752490.git.melissa.srw@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/display: dc_link: cleaning up some code style issues | expand |
Hi, First of all, thank you for your patch. I just have one tiny comment inline. On 02/26, Melissa Wen wrote: > Coding style clean up on enable_link_dp function as suggested by > checkpatch.pl: > > CHECK: Lines should not end with a '(' > WARNING: line over 80 characters > WARNING: suspect code indent for conditional statements (8, 24) > CHECK: braces {} should be used on all arms of this statement > ERROR: else should follow close brace '}' > CHECK: Comparison to NULL could be written > "link->preferred_training_settings.fec_enable" > > Signed-off-by: Melissa Wen <melissa.srw@gmail.com> > --- > drivers/gpu/drm/amd/display/dc/core/dc_link.c | 32 +++++++++---------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c > index a09119c10d7c..0f28b5694144 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c > @@ -1480,9 +1480,8 @@ static void enable_stream_features(struct pipe_ctx *pipe_ctx) > } > } > > -static enum dc_status enable_link_dp( > - struct dc_state *state, > - struct pipe_ctx *pipe_ctx) > +static enum dc_status enable_link_dp(struct dc_state *state, > + struct pipe_ctx *pipe_ctx) > { > struct dc_stream_state *stream = pipe_ctx->stream; > enum dc_status status; > @@ -1512,27 +1511,28 @@ static enum dc_status enable_link_dp( > > pipe_ctx->stream_res.pix_clk_params.requested_sym_clk = > link_settings.link_rate * LINK_RATE_REF_FREQ_IN_KHZ; > - if (state->clk_mgr && !apply_seamless_boot_optimization) > - state->clk_mgr->funcs->update_clocks(state->clk_mgr, state, false); > + if (state->clk_mgr && !apply_seamless_boot_optimization) { > + state->clk_mgr->funcs->update_clocks(state->clk_mgr, > + state, false); > + } This `if` condition only has one action, which means that you don't need to add `{}` in the above statement. See: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces Thanks > > skip_video_pattern = true; > > if (link_settings.link_rate == LINK_RATE_LOW) > - skip_video_pattern = false; > - > - if (perform_link_training_with_retries( > - &link_settings, > - skip_video_pattern, > - LINK_TRAINING_ATTEMPTS, > - pipe_ctx, > - pipe_ctx->stream->signal)) { > + skip_video_pattern = false; > + > + if (perform_link_training_with_retries(&link_settings, > + skip_video_pattern, > + LINK_TRAINING_ATTEMPTS, > + pipe_ctx, > + pipe_ctx->stream->signal)) { > link->cur_link_settings = link_settings; > status = DC_OK; > - } > - else > + } else { > status = DC_FAIL_DP_LINK_TRAINING; > + } > > - if (link->preferred_training_settings.fec_enable != NULL) > + if (link->preferred_training_settings.fec_enable) > fec_enable = *link->preferred_training_settings.fec_enable; > else > fec_enable = true; > -- > 2.25.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7CRodrigo.Siqueira%40amd.com%7Ccbf2adb12548404e917208d7bb0842d2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637183516476617662&sdata=%2FyyxWqZVGvbVAnr1OWCKi0y5Sdl5j5Le6C3dmYoaNy4%3D&reserved=0
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c index a09119c10d7c..0f28b5694144 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c @@ -1480,9 +1480,8 @@ static void enable_stream_features(struct pipe_ctx *pipe_ctx) } } -static enum dc_status enable_link_dp( - struct dc_state *state, - struct pipe_ctx *pipe_ctx) +static enum dc_status enable_link_dp(struct dc_state *state, + struct pipe_ctx *pipe_ctx) { struct dc_stream_state *stream = pipe_ctx->stream; enum dc_status status; @@ -1512,27 +1511,28 @@ static enum dc_status enable_link_dp( pipe_ctx->stream_res.pix_clk_params.requested_sym_clk = link_settings.link_rate * LINK_RATE_REF_FREQ_IN_KHZ; - if (state->clk_mgr && !apply_seamless_boot_optimization) - state->clk_mgr->funcs->update_clocks(state->clk_mgr, state, false); + if (state->clk_mgr && !apply_seamless_boot_optimization) { + state->clk_mgr->funcs->update_clocks(state->clk_mgr, + state, false); + } skip_video_pattern = true; if (link_settings.link_rate == LINK_RATE_LOW) - skip_video_pattern = false; - - if (perform_link_training_with_retries( - &link_settings, - skip_video_pattern, - LINK_TRAINING_ATTEMPTS, - pipe_ctx, - pipe_ctx->stream->signal)) { + skip_video_pattern = false; + + if (perform_link_training_with_retries(&link_settings, + skip_video_pattern, + LINK_TRAINING_ATTEMPTS, + pipe_ctx, + pipe_ctx->stream->signal)) { link->cur_link_settings = link_settings; status = DC_OK; - } - else + } else { status = DC_FAIL_DP_LINK_TRAINING; + } - if (link->preferred_training_settings.fec_enable != NULL) + if (link->preferred_training_settings.fec_enable) fec_enable = *link->preferred_training_settings.fec_enable; else fec_enable = true;
Coding style clean up on enable_link_dp function as suggested by checkpatch.pl: CHECK: Lines should not end with a '(' WARNING: line over 80 characters WARNING: suspect code indent for conditional statements (8, 24) CHECK: braces {} should be used on all arms of this statement ERROR: else should follow close brace '}' CHECK: Comparison to NULL could be written "link->preferred_training_settings.fec_enable" Signed-off-by: Melissa Wen <melissa.srw@gmail.com> --- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-)