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....@amd.com>
> Cc: Tom Chung <chiahsuan.ch...@amd.com>
> Cc: Alvin Lee <alvin.l...@amd.com>
> Cc: Rodrigo Siqueira <rodrigo.sique...@amd.com>
> Cc: Roman Li <roman...@amd.com>
> Cc: Qingqing Zhuo <qingqing.z...@amd.com>
> Cc: Aurabindo Pillai <aurabindo.pil...@amd.com>
> Cc: Harry Wentland <harry.wentl...@amd.com>
> Suggested-by: Dan Carpenter <dan.carpen...@linaro.org>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmu...@amd.com>
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.wentl...@amd.com>
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;
>
