| Message ID | 20240217202015.2034288-1-tuliomf09@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/display: clean unnecessary braces | expand |
Hi Túlio, First of all thanks for your patch. See my comments inline. On 2/17/24 13:20, Túlio Fernandes wrote: > Clean unnecessary braces in dc/dcn32/dcn32_resource_helpers.c and dc/dcn32/dcn201_link_encoder.c Did you identify this issue with checkpatch? If so, I recommend you paste the error message in the commit message. Iirc, checkpatch provides the file and the function name, which can make this commit message more informative. Also, wrap the commit message under 70 characters. > > Signed-off-by: Túlio Fernandes <tuliomf09@gmail.com> > --- > .../display/dc/dcn32/dcn32_resource_helpers.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c > index 87760600e154..e179dea148e7 100644 > --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c > +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c > @@ -110,14 +110,12 @@ uint32_t dcn32_helper_calculate_num_ways_for_subvp( > struct dc_state *context) > { > if (context->bw_ctx.bw.dcn.mall_subvp_size_bytes > 0) { > - if (dc->debug.force_subvp_num_ways) { > + if (dc->debug.force_subvp_num_ways) > return dc->debug.force_subvp_num_ways; > - } else { > + else > return dcn32_helper_mall_bytes_to_ways(dc, context->bw_ctx.bw.dcn.mall_subvp_size_bytes); > - } > - } else { > + } else Actually, we want to keep the braces around the else part to keep the braces balanced with the if condition. Thanks Siqueira > return 0; > - } > } > > void dcn32_merge_pipes_for_subvp(struct dc *dc, > @@ -250,9 +248,9 @@ bool dcn32_is_psr_capable(struct pipe_ctx *pipe) > { > bool psr_capable = false; > > - if (pipe->stream && pipe->stream->link->psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED) { > + if (pipe->stream && pipe->stream->link->psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED) > psr_capable = true; > - } > + > return psr_capable; > } > > @@ -278,9 +276,9 @@ static void override_det_for_subvp(struct dc *dc, struct dc_state *context, uint > if (pipe_ctx->stream && pipe_ctx->plane_state && dc_state_get_pipe_subvp_type(context, pipe_ctx) != SUBVP_PHANTOM) { > if (dcn32_allow_subvp_high_refresh_rate(dc, context, pipe_ctx)) { > > - if (pipe_ctx->stream->timing.v_addressable == 1080 && pipe_ctx->stream->timing.h_addressable == 1920) { > + if (pipe_ctx->stream->timing.v_addressable == 1080 && pipe_ctx->stream->timing.h_addressable == 1920) > fhd_count++; > - } > + > subvp_high_refresh_count++; > } > }
Am 21.02.24 um 19:01 schrieb Rodrigo Siqueira Jordao: > [SNIP] >> diff --git >> a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c >> b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c >> index 87760600e154..e179dea148e7 100644 >> --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c >> +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c >> @@ -110,14 +110,12 @@ uint32_t >> dcn32_helper_calculate_num_ways_for_subvp( >> struct dc_state *context) >> { >> if (context->bw_ctx.bw.dcn.mall_subvp_size_bytes > 0) { >> - if (dc->debug.force_subvp_num_ways) { >> + if (dc->debug.force_subvp_num_ways) >> return dc->debug.force_subvp_num_ways; >> - } else { >> + else >> return dcn32_helper_mall_bytes_to_ways(dc, >> context->bw_ctx.bw.dcn.mall_subvp_size_bytes); >> - } >> - } else { >> + } else > > Actually, we want to keep the braces around the else part to keep the > braces balanced with the if condition. Yeah, and checkpatch actually complains about that. E.g. you shouldn't use "} else" or "else {", but always "} else {". So the patch is actually a bit bogus and introduces new coding style warnings. Regards, Christian. > > Thanks > Siqueira > >> return 0; >> - } >> } >> void dcn32_merge_pipes_for_subvp(struct dc *dc, >> @@ -250,9 +248,9 @@ bool dcn32_is_psr_capable(struct pipe_ctx *pipe) >> { >> bool psr_capable = false; >> - if (pipe->stream && >> pipe->stream->link->psr_settings.psr_version != >> DC_PSR_VERSION_UNSUPPORTED) { >> + if (pipe->stream && pipe->stream->link->psr_settings.psr_version >> != DC_PSR_VERSION_UNSUPPORTED) >> psr_capable = true; >> - } >> + >> return psr_capable; >> } >> @@ -278,9 +276,9 @@ static void override_det_for_subvp(struct dc >> *dc, struct dc_state *context, uint >> if (pipe_ctx->stream && pipe_ctx->plane_state && >> dc_state_get_pipe_subvp_type(context, pipe_ctx) != SUBVP_PHANTOM) { >> if (dcn32_allow_subvp_high_refresh_rate(dc, context, >> pipe_ctx)) { >> - if (pipe_ctx->stream->timing.v_addressable == 1080 >> && pipe_ctx->stream->timing.h_addressable == 1920) { >> + if (pipe_ctx->stream->timing.v_addressable == 1080 >> && pipe_ctx->stream->timing.h_addressable == 1920) >> fhd_count++; >> - } >> + >> subvp_high_refresh_count++; >> } >> } >
Hi, Christian! Ok, thanks for clarifying this for me. I'll continue analyzing the files here, now based on these points. Best regards Em qui., 22 de fev. de 2024 às 06:33, Christian König <christian.koenig@amd.com> escreveu: > > Am 21.02.24 um 19:01 schrieb Rodrigo Siqueira Jordao: > > [SNIP] > >> diff --git > >> a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c > >> b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c > >> index 87760600e154..e179dea148e7 100644 > >> --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c > >> +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c > >> @@ -110,14 +110,12 @@ uint32_t > >> dcn32_helper_calculate_num_ways_for_subvp( > >> struct dc_state *context) > >> { > >> if (context->bw_ctx.bw.dcn.mall_subvp_size_bytes > 0) { > >> - if (dc->debug.force_subvp_num_ways) { > >> + if (dc->debug.force_subvp_num_ways) > >> return dc->debug.force_subvp_num_ways; > >> - } else { > >> + else > >> return dcn32_helper_mall_bytes_to_ways(dc, > >> context->bw_ctx.bw.dcn.mall_subvp_size_bytes); > >> - } > >> - } else { > >> + } else > > > > Actually, we want to keep the braces around the else part to keep the > > braces balanced with the if condition. > > Yeah, and checkpatch actually complains about that. E.g. you shouldn't > use "} else" or "else {", but always "} else {". > > So the patch is actually a bit bogus and introduces new coding style > warnings. > > Regards, > Christian. > > > > > Thanks > > Siqueira > > > >> return 0; > >> - } > >> } > >> void dcn32_merge_pipes_for_subvp(struct dc *dc, > >> @@ -250,9 +248,9 @@ bool dcn32_is_psr_capable(struct pipe_ctx *pipe) > >> { > >> bool psr_capable = false; > >> - if (pipe->stream && > >> pipe->stream->link->psr_settings.psr_version != > >> DC_PSR_VERSION_UNSUPPORTED) { > >> + if (pipe->stream && pipe->stream->link->psr_settings.psr_version > >> != DC_PSR_VERSION_UNSUPPORTED) > >> psr_capable = true; > >> - } > >> + > >> return psr_capable; > >> } > >> @@ -278,9 +276,9 @@ static void override_det_for_subvp(struct dc > >> *dc, struct dc_state *context, uint > >> if (pipe_ctx->stream && pipe_ctx->plane_state && > >> dc_state_get_pipe_subvp_type(context, pipe_ctx) != SUBVP_PHANTOM) { > >> if (dcn32_allow_subvp_high_refresh_rate(dc, context, > >> pipe_ctx)) { > >> - if (pipe_ctx->stream->timing.v_addressable == 1080 > >> && pipe_ctx->stream->timing.h_addressable == 1920) { > >> + if (pipe_ctx->stream->timing.v_addressable == 1080 > >> && pipe_ctx->stream->timing.h_addressable == 1920) > >> fhd_count++; > >> - } > >> + > >> subvp_high_refresh_count++; > >> } > >> } > > >
diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c index 87760600e154..e179dea148e7 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c @@ -110,14 +110,12 @@ uint32_t dcn32_helper_calculate_num_ways_for_subvp( struct dc_state *context) { if (context->bw_ctx.bw.dcn.mall_subvp_size_bytes > 0) { - if (dc->debug.force_subvp_num_ways) { + if (dc->debug.force_subvp_num_ways) return dc->debug.force_subvp_num_ways; - } else { + else return dcn32_helper_mall_bytes_to_ways(dc, context->bw_ctx.bw.dcn.mall_subvp_size_bytes); - } - } else { + } else return 0; - } } void dcn32_merge_pipes_for_subvp(struct dc *dc, @@ -250,9 +248,9 @@ bool dcn32_is_psr_capable(struct pipe_ctx *pipe) { bool psr_capable = false; - if (pipe->stream && pipe->stream->link->psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED) { + if (pipe->stream && pipe->stream->link->psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED) psr_capable = true; - } + return psr_capable; } @@ -278,9 +276,9 @@ static void override_det_for_subvp(struct dc *dc, struct dc_state *context, uint if (pipe_ctx->stream && pipe_ctx->plane_state && dc_state_get_pipe_subvp_type(context, pipe_ctx) != SUBVP_PHANTOM) { if (dcn32_allow_subvp_high_refresh_rate(dc, context, pipe_ctx)) { - if (pipe_ctx->stream->timing.v_addressable == 1080 && pipe_ctx->stream->timing.h_addressable == 1920) { + if (pipe_ctx->stream->timing.v_addressable == 1080 && pipe_ctx->stream->timing.h_addressable == 1920) fhd_count++; - } + subvp_high_refresh_count++; } }
Clean unnecessary braces in dc/dcn32/dcn32_resource_helpers.c and dc/dcn32/dcn201_link_encoder.c Signed-off-by: Túlio Fernandes <tuliomf09@gmail.com> --- .../display/dc/dcn32/dcn32_resource_helpers.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)