On 2024-04-23 09:59, Srinivasan Shanmugam wrote:
> This commit removes an unnecessary NULL check in the
> `dcn20_set_input_transfer_func` function in the `dcn20_hwseq.c` file.
> The variable `tf` is assigned the address of
> `plane_state->in_transfer_func` unconditionally, so it can never be
> `NULL`. Therefore, the check `if (tf == NULL)` is unnecessary and has
> been removed.
>
> The plane_state->in_transfer_func itself cannot be NULL because it's a
> structure, not a pointer. When we do tf =
> &plane_state->in_transfer_func;, we're getting the address of that
> structure, which will always be valid as long as plane_state itself is
> not NULL.
>
> we've already checked if plane_state is NULL with the line if (dpp_base
> == NULL || plane_state == NULL) return false;. So, if the code execution
> gets to the point where tf = &plane_state->in_transfer_func; is called,
> plane_state is guaranteed to be not NULL, and therefore tf will also not
> be NULL.
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn20/dcn20_hwseq.c
> 1094 bool dcn20_set_input_transfer_func(struct dc *dc,
> 1095 struct pipe_ctx *pipe_ctx,
> 1096 const struct dc_plane_state *plane_state)
> 1097 {
> 1098 struct dce_hwseq *hws = dc->hwseq;
> 1099 struct dpp *dpp_base = pipe_ctx->plane_res.dpp;
> 1100 const struct dc_transfer_func *tf = NULL;
> ^^^^^^^^^ This assignment is not necessary now.
>
> 1101 bool result = true;
> 1102 bool use_degamma_ram = false;
> 1103
> 1104 if (dpp_base == NULL || plane_state == NULL)
> 1105 return false;
> 1106
> 1107 hws->funcs.set_shaper_3dlut(pipe_ctx, plane_state);
> 1108 hws->funcs.set_blend_lut(pipe_ctx, plane_state);
> 1109
> 1110 tf = &plane_state->in_transfer_func;
> ^^^^^
> Before there was an if statement but now tf is assigned unconditionally
>
> 1111
> --> 1112 if (tf == NULL) {
> ^^^^^^^^^^^^^^^^^
> so these conditions are impossible.
>
> 1113 dpp_base->funcs->dpp_set_degamma(dpp_base,
> 1114 IPP_DEGAMMA_MODE_BYPASS);
> 1115 return true;
> 1116 }
> 1117
> 1118 if (tf->type == TF_TYPE_HWPWL || tf->type == TF_TYPE_DISTRIBUTED_POINTS)
> 1119 use_degamma_ram = true;
> 1120
> 1121 if (use_degamma_ram == true) {
> 1122 if (tf->type == TF_TYPE_HWPWL)
> 1123 dpp_base->funcs->dpp_program_degamma_pwl(dpp_base,
>
> Fixes the below Smatch static checker warning:
> drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn20/dcn20_hwseq.c:1112 dcn20_set_input_transfer_func() warn: address of 'plane_state->in_transfer_func' is non-NULL
>
> Fixes: 285a7054bf81 ("drm/amd/display: Remove plane and stream pointers from dc scratch")
> Cc: Wenjing Liu <wenjing.liu@xxxxxxx>
> Cc: Tom Chung <chiahsuan.chung@xxxxxxx>
> Cc: Alvin Lee <alvin.lee2@xxxxxxx>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx>
> Cc: Roman Li <roman.li@xxxxxxx>
> Cc: Qingqing Zhuo <Qingqing.Zhuo@xxxxxxx>
> Cc: Aurabindo Pillai <aurabindo.pillai@xxxxxxx>
> Cc: Harry Wentland <harry.wentland@xxxxxxx>
> Suggested-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
Previously plane_state->in_transfer_func was a pointer but that was
changed recently. Looks like this place was missed.
Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx>
Harry
> ---
> drivers/gpu/drm/amd/display/dc/hwss/dcn20/dcn20_hwseq.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn20/dcn20_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn20/dcn20_hwseq.c
> index b60ba2a110f7..58fbdd535068 100644
> --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn20/dcn20_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn20/dcn20_hwseq.c
> @@ -1109,12 +1109,6 @@ bool dcn20_set_input_transfer_func(struct dc *dc,
>
> tf = &plane_state->in_transfer_func;
>
> - if (tf == NULL) {
> - dpp_base->funcs->dpp_set_degamma(dpp_base,
> - IPP_DEGAMMA_MODE_BYPASS);
> - return true;
> - }
> -
> if (tf->type == TF_TYPE_HWPWL || tf->type == TF_TYPE_DISTRIBUTED_POINTS)
> use_degamma_ram = true;
>
