Skip to contentSkip to conversation

drm/amd/display: Use macro for isnan check

PATCH Superseded Harry Wentland 2 participants 3 messages Nov 29, 2017 1 message not in the archive

Subsystems (4)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 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.

---
 drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c +8 -10
index b6abe0f3bb15..102b5db28402 100644
@@ -25,37 +25,35 @@
 
 #include "dcn_calc_math.h"
 
+#define isNaN(number) number != number
+
 float dcn_bw_mod(const float arg1, const float arg2)
 {
-	if (arg1 != arg1)
+	if (isNaN(arg1))
 		return arg2;
-	if (arg2 != arg2)
+	if (isNaN(arg2))
 		return arg1;
 	return arg1 - arg1 * ((int) (arg1 / arg2));
 }
 
 float dcn_bw_min2(const float arg1, const float arg2)
 {
-	if (arg1 != arg1)
+	if (isNaN(arg1))
 		return arg2;
-	if (arg2 != arg2)
+	if (isNaN(arg2))
 		return arg1;
 	return arg1 < arg2 ? arg1 : arg2;
 }
 
 unsigned int dcn_bw_max(const unsigned int arg1, const unsigned int arg2)
 {
-	if (arg1 != arg1)
-		return arg2;
-	if (arg2 != arg2)
-		return arg1;
 	return arg1 > arg2 ? arg1 : arg2;
 }
 float dcn_bw_max2(const float arg1, const float arg2)
 {
-	if (arg1 != arg1)
+	if (isNaN(arg1))
 		return arg2;
-	if (arg2 != arg2)
+	if (isNaN(arg2))
 		return arg1;
 	return arg1 > arg2 ? arg1 : arg2;
 }
Signature · 6 lines
2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 20171129202149.7880-1-harry.wentland-5C7GfCeVMHo@public.gmane.org

#2 lore raw

PATCH 2/2 drm/amd/display: Add disclaimer to BW and DML code provided by HW

This code can sometimes look troubling but we trust it as it comes from HW teams with a guarantee of correctness. Add a note to these files to explain this.

---
 drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c              |  9 +++++++++
 drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.c          |  9 +++++++++
 drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c          |  9 +++++++++
 drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c              |  9 +++++++++
 drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c         |  9 +++++++++
 drivers/gpu/drm/amd/display/dc/dml/display_rq_dlg_calc.c      |  9 +++++++++
 drivers/gpu/drm/amd/display/dc/dml/dml1_display_rq_dlg_calc.c |  9 +++++++++
 drivers/gpu/drm/amd/display/dc/dml/soc_bounding_box.c         | 10 ++++++++++
 8 files changed, 73 insertions(+)
drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c +9 -0
index 6347712db834..75afb4077651 100644
@@ -29,6 +29,15 @@
 #include "core_types.h"
 #include "dal_asic_id.h"
 
+/*
+ * NOTE:
+ *   This file is gcc-parseable HW gospel, coming straight from HW engineers.
+ *
+ * It doesn't adhere to Linux kernel style and sometimes will do thing in odd
+ * ways. Unless there is something clearly wrong with it the code should
+ * remain as-is as it provides us with a guarantee from HW that it is correct.
+ */
+
 /*******************************************************************************
  * Private Functions
  ******************************************************************************/
drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.c +9 -0
index 626f9cf8aad2..e3408f121cdc 100644
@@ -27,6 +27,15 @@
 #include "dcn_calc_auto.h"
 #include "dcn_calc_math.h"
 
+/*
+ * NOTE:
+ *   This file is gcc-parseable HW gospel, coming straight from HW engineers.
+ *
+ * It doesn't adhere to Linux kernel style and sometimes will do thing in odd
+ * ways. Unless there is something clearly wrong with it the code should
+ * remain as-is as it provides us with a guarantee from HW that it is correct.
+ */
+
 /*REVISION#250*/
 void scaler_settings_calculation(struct dcn_bw_internal_vars *v)
 {
drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c +9 -0
index 102b5db28402..2522c9e66232 100644
@@ -25,6 +25,15 @@
 
 #include "dcn_calc_math.h"
 
+/*
+ * NOTE:
+ *   This file is gcc-parseable HW gospel, coming straight from HW engineers.
+ *
+ * It doesn't adhere to Linux kernel style and sometimes will do thing in odd
+ * ways. Unless there is something clearly wrong with it the code should
+ * remain as-is as it provides us with a guarantee from HW that it is correct.
+ */
+
 #define isNaN(number) number != number
 
 float dcn_bw_mod(const float arg1, const float arg2)
drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c +9 -0
index a4fbca34bcdf..b702b87318fd 100644
@@ -33,6 +33,15 @@
 #include "dcn10/dcn10_resource.h"
 #include "dcn_calc_math.h"
 
+/*
+ * NOTE:
+ *   This file is gcc-parseable HW gospel, coming straight from HW engineers.
+ *
+ * It doesn't adhere to Linux kernel style and sometimes will do thing in odd
+ * ways. Unless there is something clearly wrong with it the code should
+ * remain as-is as it provides us with a guarantee from HW that it is correct.
+ */
+
 /* Defaults from spreadsheet rev#247 */
 const struct dcn_soc_bounding_box dcn10_soc_defaults = {
 		/* latencies */
drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c +9 -0
index 1f337ecfeab0..7d7cc04042c7 100644
@@ -28,6 +28,15 @@
 
 #include "dml_inline_defs.h"
 
+/*
+ * NOTE:
+ *   This file is gcc-parseable HW gospel, coming straight from HW engineers.
+ *
+ * It doesn't adhere to Linux kernel style and sometimes will do thing in odd
+ * ways. Unless there is something clearly wrong with it the code should
+ * remain as-is as it provides us with a guarantee from HW that it is correct.
+ */
+
 #define BPP_INVALID 0
 #define BPP_BLENDED_PIPE 0xffffffff
 static const unsigned int NumberOfStates = DC__VOLTAGE_STATES;
drivers/gpu/drm/amd/display/dc/dml/display_rq_dlg_calc.c +9 -0
index 8ba962df42e6..1c16f2828ad6 100644
@@ -27,6 +27,15 @@
 #include "display_mode_vba.h"
 #include "display_rq_dlg_calc.h"
 
+/*
+ * NOTE:
+ *   This file is gcc-parseable HW gospel, coming straight from HW engineers.
+ *
+ * It doesn't adhere to Linux kernel style and sometimes will do thing in odd
+ * ways. Unless there is something clearly wrong with it the code should
+ * remain as-is as it provides us with a guarantee from HW that it is correct.
+ */
+
 static void calculate_ttu_cursor(struct display_mode_lib *mode_lib,
 		double *refcyc_per_req_delivery_pre_cur,
 		double *refcyc_per_req_delivery_cur,
drivers/gpu/drm/amd/display/dc/dml/dml1_display_rq_dlg_calc.c +9 -0
index 1e4b1e383401..b9f4405d8de5 100644
@@ -28,6 +28,15 @@
 
 #include "dml_inline_defs.h"
 
+/*
+ * NOTE:
+ *   This file is gcc-parseable HW gospel, coming straight from HW engineers.
+ *
+ * It doesn't adhere to Linux kernel style and sometimes will do thing in odd
+ * ways. Unless there is something clearly wrong with it the code should
+ * remain as-is as it provides us with a guarantee from HW that it is correct.
+ */
+
 static unsigned int get_bytes_per_element(enum source_format_class source_format, bool is_chroma)
 {
 	unsigned int ret_val = 0;
drivers/gpu/drm/amd/display/dc/dml/soc_bounding_box.c +10 -0
index bc7d8c707221..60ecef3cc75b 100644
@@ -27,6 +27,16 @@
 #include "dc_features.h"
 
 #include "dml_inline_defs.h"
+
+/*
+ * NOTE:
+ *   This file is gcc-parseable HW gospel, coming straight from HW engineers.
+ *
+ * It doesn't adhere to Linux kernel style and sometimes will do thing in odd
+ * ways. Unless there is something clearly wrong with it the code should
+ * remain as-is as it provides us with a guarantee from HW that it is correct.
+ */
+
 void dml_socbb_set_latencies(soc_bounding_box_st *to_box, soc_bounding_box_st *from_box)
 {
 	to_box->dram_clock_change_latency_us = from_box->dram_clock_change_latency_us;
Signature · 6 lines
2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
#3 lore raw
In reply to a message not in the archive
Quoted 16 lines · Harry Wentland

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.

---
 drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c +0 -0
index b6abe0f3bb15..102b5db28402 100644

drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c line 28

Quoted 4 lines
drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c +1 -0
@@ -25,37 +25,35 @@
 
 #include "dcn_calc_math.h"
 
+#define isNaN(number) number != number

To be on the safe side, you may want to add some parens around the argument in case someone passes in an expression. E.g.,

#define isNaN(number) ((number) != (number))

Alex

Quoted 44 lines
drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c +7 -11
+
 float dcn_bw_mod(const float arg1, const float arg2)
 {
-       if (arg1 != arg1)
+       if (isNaN(arg1))
                return arg2;
-       if (arg2 != arg2)
+       if (isNaN(arg2))
                return arg1;
        return arg1 - arg1 * ((int) (arg1 / arg2));
 }
 
 float dcn_bw_min2(const float arg1, const float arg2)
 {
-       if (arg1 != arg1)
+       if (isNaN(arg1))
                return arg2;
-       if (arg2 != arg2)
+       if (isNaN(arg2))
                return arg1;
        return arg1 < arg2 ? arg1 : arg2;
 }
 
 unsigned int dcn_bw_max(const unsigned int arg1, const unsigned int arg2)
 {
-       if (arg1 != arg1)
-               return arg2;
-       if (arg2 != arg2)
-               return arg1;
        return arg1 > arg2 ? arg1 : arg2;
 }
 float dcn_bw_max2(const float arg1, const float arg2)
 {
-       if (arg1 != arg1)
+       if (isNaN(arg1))
                return arg2;
-       if (arg2 != arg2)
+       if (isNaN(arg2))
                return arg1;
        return arg1 > arg2 ? arg1 : arg2;
 }
--

2.14.1

Quoted 4 lines

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx