New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 800873 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Nonsensical code in AMD GPU driver

Project Member Reported by groeck@chromium.org, Jan 10 2018

Issue description

gcc 7.2 complains when compiling AMD GPU code in chromeos-4.14.

drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calc_math.c: In function 'dcn_bw_max': 
>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calc_math.c:48:11: warning: self-comparison always evaluates to false [-Wtautological-compare] 
     if (arg1 != arg1) 
              ^~ 
   drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calc_math.c:50:11: warning: self-comparison always evaluates to false [-Wtautological-compare] 
     if (arg2 != arg2) 
              ^~ 

There is a lot of similar code in this file. Maybe there is a logic behind it, but I don't see what it might be.

On a side note, that file declares various 'float' variables and functions. Is that permitted nowadays in the kernel ?
 
These are NaN checks.  It was clarified in:
https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next&id=3f4e3a282e1235a9ab46d063b0902c00b6afc6a9

The bandwidth validation code we get from the hw teams uses floating point.  It's generally frowned on in the kernel, but is used in some cases.  In this case, we haven't validated or implemented a fixed point version.  DCN is only used on APUs right now so it will always be x86 architecture.

Comment 2 by groeck@chromium.org, Jan 10 2018

#1: That may make sense for floats, but I don't think it does for unsigned int, which is what gcc complains about (but that explains why gcc doesn't complain about the floats).



The int check was removed in the commit I mentioned in comment 1.

Comment 4 by groeck@chromium.org, Jan 10 2018

#3: Yes, I just noticed. Is that patch in the queue for chromeos-4.14 ?

I'm not sure.

Comment 6 by groeck@chromium.org, Jan 10 2018

Owner: groeck@chromium.org
Status: Assigned (was: Untriaged)
Ok, let's leave this open then for tracking. I'll assign to myself to make sure it gets addressed.

Comment 7 by dbehr@chromium.org, Jan 11 2018

After I will finish the whole large AMD patchlist (last batch of commits going through CQ right now). I will go back and compare what we have with 4.15 and drm-next etc and see what else we should apply in our tree.
There will be definitely more things to fix like b/71640610

Comment 8 by groeck@chromium.org, Feb 13 2018

Status: Started (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 14 2018

Labels: merge-merged-chromeos-4.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/08da2becd344bd169b2af6a15ddf4acc5080d43f

commit 08da2becd344bd169b2af6a15ddf4acc5080d43f
Author: Harry Wentland <harry.wentland@amd.com>
Date: Wed Feb 14 08:48:23 2018

UPSTREAM: drm/amd/display: Use macro for isnan check

In code provided by HW teams we do a NaN check on floats
by comparing the number against itself. This confuses most
people including myself. Macro it out to make it self-explanatory.

Don't do a NaN check for int.

v2: parantheses around 'number' expression

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

BUG= chromium:800873 
TEST=Build and boot

Change-Id: I2e4c610b13a9c6e83f26db85a980a4b2647233ff
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 3f4e3a282e1235a9ab46d063b0902c00b6afc6a9)
Reviewed-on: https://chromium-review.googlesource.com/917312
Reviewed-by: Dominik Behr <dbehr@chromium.org>

[modify] https://crrev.com/08da2becd344bd169b2af6a15ddf4acc5080d43f/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c

Status: Fixed (was: Started)

Sign in to add a comment