Re: [PATCH] drm/amdgpu/display: Remove unnecessary typecasts and fix build issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2022-06-07 13:58, Aurabindo Pillai wrote:
> 
> 
> On 2022-06-07 11:34, Leo wrote:
>>
>>
>> On 2022-06-07 05:40, Chandan Vurdigere Nataraj wrote:
>>> [Why]
>>> Getting below errors:
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1414:5: error: implicit conversion from enumeration type 'enum scan_direction_class' to different enumeration type 'enum dm_rotation_angle' [-Werror,-Wenum-conversion]
>>>                                  mode_lib->vba.SourceScan[k],
>>>                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1744:22: error: implicit conversion from enumeration type 'enum scan_direction_class' to different enumeration type 'enum dm_rotation_angle' [-Werror,-Wenum-conversion]
>>>                          && (!(!IsVertical(mode_lib->vba.SourceScan[k])) || mode_lib->vba.DCCEnable[k] == true)) {
>>>                                 ~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> 2 errors generated.
>>>
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_util_32.c:5484:18: error: implicit conversion from enumeration type 'RequestType' to different enumeration type 'enum RequestType' [-Werror,-Wenum-conversion]
>>>                          RequestLuma = REQ_256Bytes;
>>>                                      ~ ^~~~~~~~~~~~
>>> 18 errors of similar kind
>>>
>>> [How]
>>> 1. Add typecast at relevant places
>>> 2. Move the enum RequestType definition ahead of declarations
>>>
>>> Signed-off-by: Chandan Vurdigere Nataraj <chandan.vurdigerenataraj@xxxxxxx>
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c
>>> index b77a1ae792d1..5828e60f291d 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c
>>> @@ -1411,7 +1411,7 @@ static void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerforman
>>>                   v->BytePerPixelC[k],
>>>                   v->BytePerPixelDETY[k],
>>>                   v->BytePerPixelDETC[k],
>>> -                mode_lib->vba.SourceScan[k],
>>> +                (enum dm_rotation_angle) mode_lib->vba.SourceScan[k],
>>
>> Hi Jay,
>>
>> This seems fishy, dm_rotation_angle and scan_dirrection_class are very different enums.
>> Comparing dml32_CalculateDCCConfiguration() with CalculateDCCConfiguration(), it seems dm_rotation_angle is new for DCN32. Is passing vba.SourceScan correct here?
> 
> I agree. It should be typecast to scan_direction_class

>From the build error, it looks like dml32_CalculateDCCConfiguration() wants type `enum dm_rotation_amgle`, but vba.SourceScan[n] is of type `enum scan_direction_class`. The error is complaining that we're implicitly casting from scan_direction_class to dm_rotation_angle. In otherwords, this was an issue prior to Chandan's fix.

I think we'll need to address this separately, but for now this fixes a build on chromeos with clang 14 that I've been able to reproduce. Elevating my ack to:

Reviewed-by: Leo Li <sunpeng.li@xxxxxxx>

Thanks!
Leo

>>
>> One more comment below.
>>
>>>                   /* Output */
>>>                   &v->DCCYMaxUncompressedBlock[k],
>>>                   &v->DCCCMaxUncompressedBlock[k],
>>> @@ -1741,7 +1741,8 @@ void dml32_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_l
>>>       mode_lib->vba.SourceFormatPixelAndScanSupport = true;
>>>       for (k = 0; k <= mode_lib->vba.NumberOfActiveSurfaces - 1; k++) {
>>>           if (mode_lib->vba.SurfaceTiling[k] == dm_sw_linear
>>> -            && (!(!IsVertical(mode_lib->vba.SourceScan[k])) || mode_lib->vba.DCCEnable[k] == true)) {
>>> +            && (!(!IsVertical((enum dm_rotation_angle) mode_lib->vba.SourceScan[k]))
>>> +                || mode_lib->vba.DCCEnable[k] == true)) {
>>>               mode_lib->vba.SourceFormatPixelAndScanSupport = false;
>>>           }
>>>       }
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
>>> index 6509a84eeb64..07f3a85f8edf 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
>>> @@ -5314,9 +5314,15 @@ void dml32_CalculateDCCConfiguration(
>>>           unsigned int        *IndependentBlockLuma,
>>>           unsigned int        *IndependentBlockChroma)
>>>   {
>>> +    typedef enum {
>>> +        REQ_256Bytes,
>>> +        REQ_128BytesNonContiguous,
>>> +        REQ_128BytesContiguous,
>>> +        REQ_NA
>>> +    } RequestType;
>>>   -    enum RequestType   RequestLuma;
>>> -    enum RequestType   RequestChroma;
>>> +    RequestType   RequestLuma;
>>> +    RequestType   RequestChroma;
>>
>> This might need a wider cleanup, enum RequestType is defined in display_mode_enums.h and is already included in all the display_mode_vba*.c files I've come across. Unless I'm missing something, we shouldn't need to redefine RequestType.
>>
>> That said, there doesn't seem to be any functional change, and it fixes a build error. So
> 
> I'm unable to repro this error. Are you using clang? If so, which version ?
> 
>> Acked-by: Leo Li <sunpeng.li@xxxxxxx>
>>
>> Thanks,
>> Leo
>>
>>>         unsigned int   segment_order_horz_contiguous_luma;
>>>       unsigned int   segment_order_horz_contiguous_chroma;
>>> @@ -5350,13 +5356,6 @@ void dml32_CalculateDCCConfiguration(
>>>       double   detile_buf_vp_horz_limit;
>>>       double   detile_buf_vp_vert_limit;
>>>   -    typedef enum {
>>> -        REQ_256Bytes,
>>> -        REQ_128BytesNonContiguous,
>>> -        REQ_128BytesContiguous,
>>> -        REQ_NA
>>> -    } RequestType;
>>> -
>>>       yuv420 = ((SourcePixelFormat == dm_420_8 || SourcePixelFormat == dm_420_10 ||
>>>               SourcePixelFormat == dm_420_12) ? 1 : 0);
>>>       horz_div_l = 1;
>>> @@ -5527,11 +5526,11 @@ void dml32_CalculateDCCConfiguration(
>>>               RequestChroma = REQ_128BytesContiguous;
>>>       }
>>>   -    if (RequestLuma == (enum RequestType) REQ_256Bytes) {
>>> +    if (RequestLuma == REQ_256Bytes) {
>>>           *MaxUncompressedBlockLuma = 256;
>>>           *MaxCompressedBlockLuma = 256;
>>>           *IndependentBlockLuma = 0;
>>> -    } else if (RequestLuma == (enum RequestType) REQ_128BytesContiguous) {
>>> +    } else if (RequestLuma == REQ_128BytesContiguous) {
>>>           *MaxUncompressedBlockLuma = 256;
>>>           *MaxCompressedBlockLuma = 128;
>>>           *IndependentBlockLuma = 128;
>>> @@ -5541,11 +5540,11 @@ void dml32_CalculateDCCConfiguration(
>>>           *IndependentBlockLuma = 64;
>>>       }
>>>   -    if (RequestChroma == (enum RequestType) REQ_256Bytes) {
>>> +    if (RequestChroma == REQ_256Bytes) {
>>>           *MaxUncompressedBlockChroma = 256;
>>>           *MaxCompressedBlockChroma = 256;
>>>           *IndependentBlockChroma = 0;
>>> -    } else if (RequestChroma == (enum RequestType) REQ_128BytesContiguous) {
>>> +    } else if (RequestChroma == REQ_128BytesContiguous) {
>>>           *MaxUncompressedBlockChroma = 256;
>>>           *MaxCompressedBlockChroma = 128;
>>>           *IndependentBlockChroma = 128;
>>




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux