[PATCH] drm/amd/display: Fix warning about overflow

[Harry Wentland]

On 2017-10-13 04:26 AM, Michel Dänzer wrote:
> On 13/10/17 10:24 AM, Christian König wrote:
>> Am 13.10.2017 um 10:08 schrieb Michel Dänzer:
>>> On 12/10/17 07:54 PM, Harry Wentland wrote:
>>>> We're overflowing the last bit. Cast it explicitly
>>>>
>>>> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
>>>> b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
>>>> index cb94e18cc455..715dc789bb24 100644
>>>> --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
>>>> +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
>>>> @@ -1047,7 +1047,7 @@ static enum bp_result
>>>> get_embedded_panel_info_v2_1(
>>>>           lvds->lcd_timing.miscinfo & ATOM_V_REPLICATIONBY2;
>>>>       info->lcd_timing.misc_info.COMPOSITE_SYNC =
>>>>           lvds->lcd_timing.miscinfo & ATOM_COMPOSITESYNC;
>>>> -    info->lcd_timing.misc_info.INTERLACE =
>>>> +    info->lcd_timing.misc_info.INTERLACE = (uint32_t)
>>>>           lvds->lcd_timing.miscinfo & ATOM_INTERLACE;
>>> I wasn't sure offhand, so I had to write a little test program, but it
>>> looks like this doesn't work as intended: (lvds->lcd_timing.miscinfo &
>>> ATOM_INTERLACE) is an unsigned 16-bit value, which is either 0x80
>>> (ATOM_INTERLACE) or 0. Assigning that to the unsigned 1-bit field
>>> info->lcd_timing.misc_info.INTERLACE always results in 0. Casting the
>>> 16-bit value to 32 bits doesn't change that. I think what's intended
>>> here is something like info->lcd_timing.misc_info.INTERLACE =
>>> (lvds->lcd_timing.miscinfo & ATOM_INTERLACE) != 0; Looks like there's
>>> the same problem with other fields of info->lcd_timing.misc_info
>>> above. Another possible solution is making the fields of struct
>>> misc_info bool instead of uint32_t.
>>
>> While your approach works as well IIRC converting an arbitrary value
>> into a boolean is usually done with "!!".
>>
>> E.g. that should look like:
>>
>>     info->lcd_timing.misc_info.INTERLACE =
>>         !!(lvds->lcd_timing.miscinfo & ATOM_INTERLACE);
> 
> Right, that works as well, thanks.
> 

Thanks, this is really the correct approach.

Harry

>