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 >
