On Fri, 16 Oct 2020 at 04:42, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, Oct 15, 2020 at 10:51 AM Linus Torvalds > <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > Thanks, looks good to me [..] > > Uhhuh. I already pushed things out, but my clang build (which I don't > do between each merge) shows a problem: > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:650:8: > warning: logical not is only applied to the left hand side of this > comparison [-Wlogical-not-parentheses] > && !params[i].clock_force_enable == DSC_CLK_FORCE_DEFAULT) { > ^ ~~ > > and I think clang is entirely right to complain about that code. > > Yes, the code may be correct, but even if it's correct, that's a > really odd way to write things. > > Anyway, what it does is: > > !params[i].clock_force_enable > > turns 0 into 1, and anything that isn't 0 into 0. > > And DSC_CLK_FORCE_DEFAULT has a value of 0, so what that line actually means is > > (params[i].clock_force_enable == 0) == 0 > > which obviously is > > params[i].clock_force_enable != 0 > > which in this case is the same as > > params[i].clock_force_enable != DSC_CLK_FORCE_DEFAULT > > which may be what the code actually meant to do. > > So I suspect it does what it meant to do, but only because > DSC_CLK_FORCE_DEFAULT also happens to be 0, which also acts as a > boolean 'false'. > > But it's also possible that the '!' is a left-over, and the code > actually _meant_ to do the exact reverse of that. I have no idea. > > This odd code was introduced by commit 0749ddeb7d6c ("drm/amd/display: > Add DSC force disable to dsc_clock_en debugfs entry"), and can we > please agree to not write this kind of obfuscated C code? I've asked Alex to direct send you any fix for you to apply once he's had a chance to validate it, Dave. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel