Hi Rodrigo,
I'm not so happy about the whole recursion thing, but I think that is
something which can be worked on later on.
Apart from that the approach sounds solid to me.
Regards,
Christian.
Am 31.03.21 um 14:24 schrieb Rodrigo Siqueira:
Hi,
In the display core, we utilize floats and doubles units for calculating
modesetting parameters. One side effect of our approach to use double-precision
is the fact that we spread multiple FPU access across our driver, which means
that we can accidentally clobber user space FPU state.
# Challenges
1. Keep in mind that this FPU code is ingrained in our display driver and
performs several crucial tasks. Additionally, we already have multiple
architectures available in the kernel and a large set of users; in other words,
we prefer to avoid a radical approach that might break our user's system.
2. We share our display code with Windows; thus, we need to maintain the
interoperability between these two systems.
3. We need a mechanism for identifying which function uses FPU registers;
fortunately, Peter Zijlstra wrote a series a couple of months ago where he
introduced an FPU check for objtool. I used the following command for
identifying the potential FPU usage:
./tools/objtool/objtool check -Ffa "drivers/gpu/drm/amd/display/dc/ANY_FILE.o"
4. Since our code heavily relies on FPU and the fact that we spread
kernel_fpu_begin/end across multiple functions, we can have some complex
scenarios that will require code refactoring. However, we want to avoid
complicated changes since this is a formula to introduce regressions; we want
something that allows us to fix it in small, safe, and reliable steps.
# Our approach
For trying to solve this problem, we came up with the following strategy:
1. Keep in mind that we are using kernel_fpu_begin/end spread in various areas
and sometimes across multiple functions. If we try to move some of the
functions to an isolated place, we can generate a situation where we can call
the FPU protection more than once, causing multiple warnings. We can deal with
this problem by adding a thin management layer around the kernel_fpu_begin/end
used inside the display.
2. We will need a trace mechanism for this FPU management inside our display
code.
3. After we get the thin layer that manages FPU, we can start to move each
function that uses FPU to the centralized place. Our DQE runs multiple tests in
different ASICs every week; we can take advantage of this to ensure that our
FPU patches work does not introduce any regression. The idea is to work on a
specific part of the code every week (e.g., week 1: DCN2, week 1: DCN2.1,
etc.).
4. Finally, after we can isolate the FPU operations in a single place, we can
altogether remove the FPU flags from other files and eliminate an unnecessary
code introduced to deal with this problem.
# This series
To maintain the interoperability between multiple OSes, we already have a
define named DC_FP_START/END, which is a straightforward wrapper to
kernel_fpu_begin/end in the Linux side. In this series, I decided to expand the
scope of this DC_FP_* wrapper to trace FPU entrance and exit in the display
code, but I also add a mechanism for managing the entrance and exit of
kernel_fpu_begin/end. You can see the details on how I did that in the last two
patches.
I also isolate a simple function that requires FPU access to demonstrate my
strategy for isolating this FPU access in a single place. If this series gets
accepted, the following steps consist of moving all FPU functions weekly until
we isolate everything in the fpu_operation folder.
Best Regards
Rodrigo Siqueira
Rodrigo Siqueira (4):
drm/amd/display: Introduce FPU directory inside DC
drm/amd/display: Add FPU event trace
drm/amd/display: Add ref count control for FPU utilization
drm/amd/display: Add DC_FP helper to check FPU state
.../gpu/drm/amd/display/amdgpu_dm/Makefile | 3 +-
.../amd/display/amdgpu_dm/amdgpu_dm_trace.h | 24 ++++
.../gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 111 ++++++++++++++++++
.../gpu/drm/amd/display/amdgpu_dm/dc_fpu.h | 34 ++++++
drivers/gpu/drm/amd/display/dc/Makefile | 1 +
drivers/gpu/drm/amd/display/dc/dc_trace.h | 3 +
.../drm/amd/display/dc/dcn20/dcn20_resource.c | 41 +------
.../drm/amd/display/dc/dcn20/dcn20_resource.h | 2 -
.../drm/amd/display/dc/dcn21/dcn21_resource.c | 2 +
.../amd/display/dc/fpu_operations/Makefile | 58 +++++++++
.../drm/amd/display/dc/fpu_operations/dcn2x.c | 106 +++++++++++++++++
.../drm/amd/display/dc/fpu_operations/dcn2x.h | 33 ++++++
drivers/gpu/drm/amd/display/dc/os_types.h | 6 +-
13 files changed, 381 insertions(+), 43 deletions(-)
create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.h
create mode 100644 drivers/gpu/drm/amd/display/dc/fpu_operations/Makefile
create mode 100644 drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.c
create mode 100644 drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.h
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
