[PATCH] drm/amd/display: only collect data if debug gamut_remap is available

[Melissa Wen]

On 13/05/2025 15:27, Alex Hung wrote:
>
>
> On 4/25/25 14:49, Melissa Wen wrote:
>> Color gamut_remap state log may be not avaiable for some hw versions, so
>> prevent null pointer dereference by checking it there is a function to
>> collect data for this hw version.
>>
>> Signed-off-by: Melissa Wen <mwen at igalia.com>
>> ---
>>   .../amd/display/dc/hwss/dcn20/dcn20_hwseq.c   | 26 +++++++++++++------
>>   .../amd/display/dc/hwss/dcn30/dcn30_hwseq.c   | 24 ++++++++++++-----
>>   2 files changed, 35 insertions(+), 15 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 5e78b553adbd..b0f9e46e202a 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
>> @@ -76,6 +76,7 @@ void dcn20_log_color_state(struct dc *dc,
>>   {
>>       struct dc_context *dc_ctx = dc->ctx;
>>       struct resource_pool *pool = dc->res_pool;
>> +    bool is_gamut_remap_available;
>>       int i;
>>         DTN_INFO("DPP:  DGAM mode  SHAPER mode  3DLUT mode 3DLUT bit 
>> depth"
>> @@ -89,15 +90,15 @@ void dcn20_log_color_state(struct dc *dc,
>>           struct dcn_dpp_state s = {0};
>>             dpp->funcs->dpp_read_state(dpp, &s);
>> -        dpp->funcs->dpp_get_gamut_remap(dpp, &s.gamut_remap);
>> +        if (dpp->funcs->dpp_get_gamut_remap) {
>> +            dpp->funcs->dpp_get_gamut_remap(dpp, &s.gamut_remap);
>> +            is_gamut_remap_available = true;
>> +        }
>>             if (!s.is_enabled)
>>               continue;
>>   -        DTN_INFO("[%2d]:  %8s  %11s  %10s  %15s  %10s  %9s %12s  "
>> -             "%010lld %010lld %010lld %010lld "
>> -             "%010lld %010lld %010lld %010lld "
>> -             "%010lld %010lld %010lld %010lld",
>> +        DTN_INFO("[%2d]:  %8s  %11s  %10s  %15s  %10s  %9s",
>>               dpp->inst,
>>               (s.dgam_lut_mode == 0) ? "Bypass" :
>>                ((s.dgam_lut_mode == 1) ? "sRGB" :
>> @@ -114,10 +115,17 @@ void dcn20_log_color_state(struct dc *dc,
>>               (s.lut3d_bit_depth <= 0) ? "12-bit" : "10-bit",
>>               (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
>>               (s.rgam_lut_mode == 1) ? "RAM A" :
>> -             ((s.rgam_lut_mode == 1) ? "RAM B" : "Bypass"),
>> +             ((s.rgam_lut_mode == 1) ? "RAM B" : "Bypass"));
>> +
>> +        if (is_gamut_remap_available) {
>
> is_gamut_remap_available can be uninitialized before used.

Ops. Let me send a new version.
>
>> +            DTN_INFO("  %12s  "
>> +                 "%010lld %010lld %010lld %010lld "
>> +                 "%010lld %010lld %010lld %010lld "
>> +                 "%010lld %010lld %010lld %010lld",
>> +
>>               (s.gamut_remap.gamut_adjust_type == 0) ? "Bypass" :
>> -             ((s.gamut_remap.gamut_adjust_type == 1) ? "HW" :
>> -                                   "SW"),
>> +                ((s.gamut_remap.gamut_adjust_type == 1) ? "HW" :
>> +                                      "SW"),
>>               s.gamut_remap.temperature_matrix[0].value,
>>               s.gamut_remap.temperature_matrix[1].value,
>>               s.gamut_remap.temperature_matrix[2].value,
>> @@ -130,6 +138,8 @@ void dcn20_log_color_state(struct dc *dc,
>>               s.gamut_remap.temperature_matrix[9].value,
>>               s.gamut_remap.temperature_matrix[10].value,
>>               s.gamut_remap.temperature_matrix[11].value);
>> +        }
>> +
>>           DTN_INFO("\n");
>>       }
>>       DTN_INFO("\n");
>> diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c 
>> b/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
>> index e89ebfda4873..931cd2e886b8 100644
>> --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
>> +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
>> @@ -74,6 +74,7 @@ void dcn30_log_color_state(struct dc *dc,
>>   {
>>       struct dc_context *dc_ctx = dc->ctx;
>>       struct resource_pool *pool = dc->res_pool;
>> +    bool is_gamut_remap_available;
>>       int i;
>>         DTN_INFO("DPP:  DGAM ROM  DGAM ROM type  DGAM LUT SHAPER mode"
>> @@ -88,16 +89,16 @@ void dcn30_log_color_state(struct dc *dc,
>>           struct dcn_dpp_state s = {0};
>>             dpp->funcs->dpp_read_state(dpp, &s);
>> -        dpp->funcs->dpp_get_gamut_remap(dpp, &s.gamut_remap);
>> +
>> +        if (dpp->funcs->dpp_get_gamut_remap) {
>> +            dpp->funcs->dpp_get_gamut_remap(dpp, &s.gamut_remap);
>> +            is_gamut_remap_available = true;
>> +        }
>>             if (!s.is_enabled)
>>               continue;
>>   -        DTN_INFO("[%2d]:  %7x  %13s  %8s  %11s  %10s  %15s %10s  %9s"
>> -             "  %12s  "
>> -             "%010lld %010lld %010lld %010lld "
>> -             "%010lld %010lld %010lld %010lld "
>> -             "%010lld %010lld %010lld %010lld",
>> +        DTN_INFO("[%2d]:  %7x  %13s  %8s  %11s  %10s  %15s %10s  %9s",
>>               dpp->inst,
>>               s.pre_dgam_mode,
>>               (s.pre_dgam_select == 0) ? "sRGB" :
>> @@ -121,7 +122,14 @@ void dcn30_log_color_state(struct dc *dc,
>>               (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
>>               (s.rgam_lut_mode == 0) ? "Bypass" :
>>                ((s.rgam_lut_mode == 1) ? "RAM A" :
>> -                           "RAM B"),
>> +                           "RAM B"));
>> +
>> +        if (is_gamut_remap_available) {
>
> is_gamut_remap_available can be uninitialized before used.
>
>> +            DTN_INFO("  %12s  "
>> +                 "%010lld %010lld %010lld %010lld "
>> +                 "%010lld %010lld %010lld %010lld "
>> +                 "%010lld %010lld %010lld %010lld",
>> +
>>               (s.gamut_remap.gamut_adjust_type == 0) ? "Bypass" :
>>                   ((s.gamut_remap.gamut_adjust_type == 1) ? "HW" :
>>                                         "SW"),
>> @@ -137,6 +145,8 @@ void dcn30_log_color_state(struct dc *dc,
>>               s.gamut_remap.temperature_matrix[9].value,
>>               s.gamut_remap.temperature_matrix[10].value,
>>               s.gamut_remap.temperature_matrix[11].value);
>> +        }
>> +
>>           DTN_INFO("\n");
>>       }
>>       DTN_INFO("\n");
>