[PATCH 0/4] drm/amd/display: Base changes for isolating FPU operation in a single place
Christian König
christian.koenig at amd.com
Wed Mar 31 12:49:42 UTC 2021
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
>
More information about the dri-devel
mailing list