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

[Michel Dänzer]

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.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer