Re: [PATCH 02/28] drm/amd/display: Rework dsc to isolate FPU operations

2020-06-08 Thread Christian König

Am 08.06.20 um 06:59 schrieb Qingqing Zhuo:

From: Rodrigo Siqueira 

When we want to use float point operation on Linux
we need to use within special kernel protection
(`kernel_fpu_{begin,end}()`.), otherwise the kernel
can clobber userspace FPU register state. For detecting
these issues we use a tool named objtool (with -Ffa
flags) to highlight the FPU problems, all warnings can
be summed up as follows:

./tools/objtool/objtool check -Ffa
drivers/gpu/drm/amd/display/dc/dml/dml_common_defs.o

[..] dc/dsc/rc_calc.o: warning: objtool: get_qp_set()+0x2f8:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc.o: warning: objtool: dsc_roundf()+0x5:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc.o: warning: objtool: dsc_ceil()+0x5:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc.o: warning: objtool: get_ofs_set()+0x3eb:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc.o: warning: objtool: calc_rc_params()+0x3c:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/dc_dsc.o: warning: objtool:
get_dsc_bandwidth_range.isra.0()+0x8d:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/dc_dsc.o: warning: objtool: setup_dsc_config()+0x2ef:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc_dpi.o: warning: objtool:copy_pps_fields()+0xbb:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc_dpi.o: warning: objtool:
dscc_compute_dsc_parameters()+0x7b:
FPU instruction outside of kernel_fpu_{begin,end}()

This commit fixes the above issues by rework DSC as described:

1. Isolate all FPU operations in a single file;
2. Use FPU flags only in the file that handles FPU operations;
3. Isolate all functions that require float point operation in static
functions;
4. Add a mid-layer function that does not use any float point operation,
and that could be safely invoked in other parts of the code.
5. Keep float point operation under DC_FP_{START/END} macro.

CC: Christian König 
CC: Alexander Deucher 
CC: Peter Zijlstra 
CC: Tony Cheng 
CC: Harry Wentland 
Signed-off-by: Rodrigo Siqueira 
Reviewed-by: Mikita Lipski 
Acked-by: Qingqing Zhuo 
---
  drivers/gpu/drm/amd/display/dc/dsc/Makefile   |   2 -
  drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c   |  18 +--
  drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c  | 151 +-
  drivers/gpu/drm/amd/display/dc/dsc/rc_calc.h  |   5 +-
  .../gpu/drm/amd/display/dc/dsc/rc_calc_dpi.c  |  27 +---
  5 files changed, 153 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dsc/Makefile 
b/drivers/gpu/drm/amd/display/dc/dsc/Makefile
index 3f66868df171..ea29cf95d470 100644
--- a/drivers/gpu/drm/amd/display/dc/dsc/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dsc/Makefile
@@ -28,8 +28,6 @@ endif
  endif
  
  CFLAGS_$(AMDDALPATH)/dc/dsc/rc_calc.o := $(dsc_ccflags)

-CFLAGS_$(AMDDALPATH)/dc/dsc/rc_calc_dpi.o := $(dsc_ccflags)
-CFLAGS_$(AMDDALPATH)/dc/dsc/dc_dsc.o := $(dsc_ccflags)
  
  DSC = dc_dsc.o rc_calc.o rc_calc_dpi.o
  
diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c

index 0ea6662a1563..0c7f247bb7de 100644
--- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
+++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
@@ -22,10 +22,12 @@
   * Author: AMD
   */
  
+#include 

  #include "dc_hw_types.h"
  #include "dsc.h"
  #include 
  #include "dc.h"
+#include "rc_calc.h"
  
  /* This module's internal functions */
  
@@ -304,22 +306,6 @@ static inline uint32_t dsc_div_by_10_round_up(uint32_t value)

return (value + 9) / 10;
  }
  
-static inline uint32_t calc_dsc_bpp_x16(uint32_t stream_bandwidth_kbps, uint32_t pix_clk_100hz, uint32_t bpp_increment_div)

-{
-   uint32_t dsc_target_bpp_x16;
-   float f_dsc_target_bpp;
-   float f_stream_bandwidth_100bps = stream_bandwidth_kbps * 10.0f;
-   uint32_t precision = bpp_increment_div; // bpp_increment_div is 
actually precision
-
-   f_dsc_target_bpp = f_stream_bandwidth_100bps / pix_clk_100hz;
-
-   // Round down to the nearest precision stop to bring it into DSC spec 
range
-   dsc_target_bpp_x16 = (uint32_t)(f_dsc_target_bpp * precision);
-   dsc_target_bpp_x16 = (dsc_target_bpp_x16 * 16) / precision;
-
-   return dsc_target_bpp_x16;
-}
-
  /* Get DSC bandwidth range based on [min_bpp, max_bpp] target bitrate range, 
and timing's pixel clock
   * and uncompressed bandwidth.
   */
diff --git a/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c 
b/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c
index 03ae15946c6d..667afbc260f9 100644
--- a/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c
+++ b/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c
@@ -23,6 +23,7 @@
   * Authors: AMD
   *
   */
+#include 
  
  #include "os_types.h"

  #include "rc_calc.h"
@@ -40,7 +41,8 @@
break
  
  
-void get_qp_set(qp_set qps, enum colour_mode cm, enum bits_per_comp bpc, enum max_min max_min, float bpp)


[PATCH 02/28] drm/amd/display: Rework dsc to isolate FPU operations

2020-06-07 Thread Qingqing Zhuo
From: Rodrigo Siqueira 

When we want to use float point operation on Linux
we need to use within special kernel protection
(`kernel_fpu_{begin,end}()`.), otherwise the kernel
can clobber userspace FPU register state. For detecting
these issues we use a tool named objtool (with -Ffa
flags) to highlight the FPU problems, all warnings can
be summed up as follows:

./tools/objtool/objtool check -Ffa
drivers/gpu/drm/amd/display/dc/dml/dml_common_defs.o

[..] dc/dsc/rc_calc.o: warning: objtool: get_qp_set()+0x2f8:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc.o: warning: objtool: dsc_roundf()+0x5:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc.o: warning: objtool: dsc_ceil()+0x5:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc.o: warning: objtool: get_ofs_set()+0x3eb:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc.o: warning: objtool: calc_rc_params()+0x3c:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/dc_dsc.o: warning: objtool:
get_dsc_bandwidth_range.isra.0()+0x8d:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/dc_dsc.o: warning: objtool: setup_dsc_config()+0x2ef:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc_dpi.o: warning: objtool:copy_pps_fields()+0xbb:
FPU instruction outside of kernel_fpu_{begin,end}()
[..] dc/dsc/rc_calc_dpi.o: warning: objtool:
dscc_compute_dsc_parameters()+0x7b:
FPU instruction outside of kernel_fpu_{begin,end}()

This commit fixes the above issues by rework DSC as described:

1. Isolate all FPU operations in a single file;
2. Use FPU flags only in the file that handles FPU operations;
3. Isolate all functions that require float point operation in static
   functions;
4. Add a mid-layer function that does not use any float point operation,
   and that could be safely invoked in other parts of the code.
5. Keep float point operation under DC_FP_{START/END} macro.

CC: Christian König 
CC: Alexander Deucher 
CC: Peter Zijlstra 
CC: Tony Cheng 
CC: Harry Wentland 
Signed-off-by: Rodrigo Siqueira 
Reviewed-by: Mikita Lipski 
Acked-by: Qingqing Zhuo 
---
 drivers/gpu/drm/amd/display/dc/dsc/Makefile   |   2 -
 drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c   |  18 +--
 drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c  | 151 +-
 drivers/gpu/drm/amd/display/dc/dsc/rc_calc.h  |   5 +-
 .../gpu/drm/amd/display/dc/dsc/rc_calc_dpi.c  |  27 +---
 5 files changed, 153 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dsc/Makefile 
b/drivers/gpu/drm/amd/display/dc/dsc/Makefile
index 3f66868df171..ea29cf95d470 100644
--- a/drivers/gpu/drm/amd/display/dc/dsc/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dsc/Makefile
@@ -28,8 +28,6 @@ endif
 endif
 
 CFLAGS_$(AMDDALPATH)/dc/dsc/rc_calc.o := $(dsc_ccflags)
-CFLAGS_$(AMDDALPATH)/dc/dsc/rc_calc_dpi.o := $(dsc_ccflags)
-CFLAGS_$(AMDDALPATH)/dc/dsc/dc_dsc.o := $(dsc_ccflags)
 
 DSC = dc_dsc.o rc_calc.o rc_calc_dpi.o
 
diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c 
b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
index 0ea6662a1563..0c7f247bb7de 100644
--- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
+++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
@@ -22,10 +22,12 @@
  * Author: AMD
  */
 
+#include 
 #include "dc_hw_types.h"
 #include "dsc.h"
 #include 
 #include "dc.h"
+#include "rc_calc.h"
 
 /* This module's internal functions */
 
@@ -304,22 +306,6 @@ static inline uint32_t dsc_div_by_10_round_up(uint32_t 
value)
return (value + 9) / 10;
 }
 
-static inline uint32_t calc_dsc_bpp_x16(uint32_t stream_bandwidth_kbps, 
uint32_t pix_clk_100hz, uint32_t bpp_increment_div)
-{
-   uint32_t dsc_target_bpp_x16;
-   float f_dsc_target_bpp;
-   float f_stream_bandwidth_100bps = stream_bandwidth_kbps * 10.0f;
-   uint32_t precision = bpp_increment_div; // bpp_increment_div is 
actually precision
-
-   f_dsc_target_bpp = f_stream_bandwidth_100bps / pix_clk_100hz;
-
-   // Round down to the nearest precision stop to bring it into DSC spec 
range
-   dsc_target_bpp_x16 = (uint32_t)(f_dsc_target_bpp * precision);
-   dsc_target_bpp_x16 = (dsc_target_bpp_x16 * 16) / precision;
-
-   return dsc_target_bpp_x16;
-}
-
 /* Get DSC bandwidth range based on [min_bpp, max_bpp] target bitrate range, 
and timing's pixel clock
  * and uncompressed bandwidth.
  */
diff --git a/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c 
b/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c
index 03ae15946c6d..667afbc260f9 100644
--- a/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c
+++ b/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c
@@ -23,6 +23,7 @@
  * Authors: AMD
  *
  */
+#include 
 
 #include "os_types.h"
 #include "rc_calc.h"
@@ -40,7 +41,8 @@
break
 
 
-void get_qp_set(qp_set qps, enum colour_mode cm, enum bits_per_comp bpc, enum 
max_min max_min, float bpp)
+static void get_qp_set(qp_set qps, enum colour_mode cm, enum bits_per_comp bpc,
+