sensors: Dont add DTS sensors when OCC inband sensors are available

Message ID 1520215388-27436-1-git-send-email-shilpa.bhat@linux.vnet.ibm.com
State New
Headers show
Series
  • sensors: Dont add DTS sensors when OCC inband sensors are available
Related show

Commit Message

Shilpasri G Bhat March 5, 2018, 2:03 a.m.
There are two sets of core temperature sensors today. One is DTS scom
based core temperature sensors and the second group is the sensors
provided by OCC. DTS is the highest temperature among the different
temperature zones in the core while OCC core temperature sensors are
the average temperature of the core. DTS sensors are read directly by
the host by SCOMing the DTS sensors while OCC sensors are read and
updated by OCC to main memory.

Reading DTS sensors by SCOMing is a heavy and slower operation as
compared to reading OCC sensors which is as good as reading memory.
So dont add DTS sensors when OCC sensors are available.

Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
---
 core/init.c       |  4 +++-
 core/sensor.c     |  1 -
 hw/occ-sensor.c   | 14 ++++++++------
 include/skiboot.h |  2 +-
 4 files changed, 12 insertions(+), 9 deletions(-)

Comments

Vaidyanathan Srinivasan March 7, 2018, 5:40 a.m. | #1
* Shilpa Bhat <shilpa.bhat@linux.vnet.ibm.com> [2018-03-05 07:33:08]:

> There are two sets of core temperature sensors today. One is DTS scom
> based core temperature sensors and the second group is the sensors
> provided by OCC. DTS is the highest temperature among the different
> temperature zones in the core while OCC core temperature sensors are
> the average temperature of the core. DTS sensors are read directly by
> the host by SCOMing the DTS sensors while OCC sensors are read and
> updated by OCC to main memory.
> 
> Reading DTS sensors by SCOMing is a heavy and slower operation as
> compared to reading OCC sensors which is as good as reading memory.
> So dont add DTS sensors when OCC sensors are available.

Also OCC code has additional logic to avoid waking up sleeping cores
and also adjust for calibration errors in DTS sensors.  Basically OCC
has chip topology knowledge and it can obtain temperature from nearest
sensor if a core is clock gated or in power save mode.
 
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>

Acked-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>

> ---
>  core/init.c       |  4 +++-
>  core/sensor.c     |  1 -
>  hw/occ-sensor.c   | 14 ++++++++------
>  include/skiboot.h |  2 +-
>  4 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/core/init.c b/core/init.c
> index 83f42f5..29c3546 100644
> --- a/core/init.c
> +++ b/core/init.c
> @@ -50,6 +50,7 @@
>  #include <libstb/trustedboot.h>
>  #include <phys-map.h>
>  #include <imc.h>
> +#include <dts.h>
>  
>  enum proc_gen proc_gen;
>  unsigned int pcie_max_link_speed;
> @@ -501,7 +502,8 @@ void __noreturn load_and_boot_kernel(bool is_reboot)
>  		 * OCC takes few secs to boot.  Call this as late as
>  		 * as possible to avoid delay.
>  		 */
> -		occ_sensors_init();
> +		if (!occ_sensors_init())
> +			dts_sensor_create_nodes(sensor_node);

We fallback to xscom based DTS reads if OCC is not providing this
data.

>  	} else {
>  		/* fdt will be rebuilt */
> diff --git a/core/sensor.c b/core/sensor.c
> index 9bc204a..c3fa319 100644
> --- a/core/sensor.c
> +++ b/core/sensor.c
> @@ -144,7 +144,6 @@ void sensor_init(void)
>  	dt_add_property_string(sensor_node, "compatible", "ibm,opal-sensor");
>  	dt_add_property_cells(sensor_node, "#address-cells", 1);
>  	dt_add_property_cells(sensor_node, "#size-cells", 0);
> -	dts_sensor_create_nodes(sensor_node);
>  
>  	/* Register OPAL interface */
>  	opal_register(OPAL_SENSOR_READ, opal_sensor_read, 3);
> diff --git a/hw/occ-sensor.c b/hw/occ-sensor.c
> index 090e0f0..d1cac33 100644
> --- a/hw/occ-sensor.c
> +++ b/hw/occ-sensor.c
> @@ -491,7 +491,7 @@ static void add_sensor_node(const char *loc, const char *type, int i, int attr,
>  	*phandle = node->phandle;
>  }
>  
> -void occ_sensors_init(void)
> +bool occ_sensors_init(void)
>  {
>  	struct proc_chip *chip;
>  	struct dt_node *sg, *exports;
> @@ -500,13 +500,13 @@ void occ_sensors_init(void)
>  
>  	/* OCC inband sensors is only supported in P9 */
>  	if (proc_gen != proc_gen_p9)
> -		return;
> +		return false;
>  
>  	/* Sensors are copied to BAR2 OCC Common Area */
>  	chip = next_chip(NULL);
>  	if (!chip->occ_common_base) {
>  		prerror("OCC: Unassigned OCC Common Area. No sensors found\n");
> -		return;
> +		return false;
>  	}
>  
>  	occ_sensor_base = chip->occ_common_base + OCC_SENSOR_DATA_BLOCK_OFFSET;
> @@ -514,7 +514,7 @@ void occ_sensors_init(void)
>  	sg = dt_new(opal_node, "sensor-groups");
>  	if (!sg) {
>  		prerror("OCC: Failed to create sensor groups node\n");
> -		return;
> +		return false;
>  	}
>  	dt_add_property_string(sg, "compatible", "ibm,opal-sensor-group");
>  	dt_add_property_cells(sg, "#address-cells", 1);
> @@ -593,14 +593,16 @@ void occ_sensors_init(void)
>  	}
>  
>  	if (!occ_num)
> -		return;
> +		return false;
>  
>  	exports = dt_find_by_path(dt_root, "/ibm,opal/firmware/exports");
>  	if (!exports) {
>  		prerror("OCC: dt node /ibm,opal/firmware/exports not found\n");
> -		return;
> +		return false;
>  	}
>  
>  	dt_add_property_u64s(exports, "occ_inband_sensors", occ_sensor_base,
>  			     OCC_SENSOR_DATA_BLOCK_SIZE * occ_num);
> +
> +	return true;
>  }
> diff --git a/include/skiboot.h b/include/skiboot.h
> index 1180a17..601eab0 100644
> --- a/include/skiboot.h
> +++ b/include/skiboot.h
> @@ -352,7 +352,7 @@ extern int fake_nvram_start_read(void *dst, uint32_t src, uint32_t len);
>  extern int fake_nvram_write(uint32_t offset, void *src, uint32_t size);
>  
>  /* OCC Inband Sensors */
> -extern void occ_sensors_init(void);
> +extern bool occ_sensors_init(void);
>  extern int occ_sensor_read(u32 handle, u64 *data);
>  extern int occ_sensor_group_clear(u32 group_hndl, int token);
>  extern void occ_add_sensor_groups(struct dt_node *sg, u32  *phandles,
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot

Patch

diff --git a/core/init.c b/core/init.c
index 83f42f5..29c3546 100644
--- a/core/init.c
+++ b/core/init.c
@@ -50,6 +50,7 @@ 
 #include <libstb/trustedboot.h>
 #include <phys-map.h>
 #include <imc.h>
+#include <dts.h>
 
 enum proc_gen proc_gen;
 unsigned int pcie_max_link_speed;
@@ -501,7 +502,8 @@  void __noreturn load_and_boot_kernel(bool is_reboot)
 		 * OCC takes few secs to boot.  Call this as late as
 		 * as possible to avoid delay.
 		 */
-		occ_sensors_init();
+		if (!occ_sensors_init())
+			dts_sensor_create_nodes(sensor_node);
 
 	} else {
 		/* fdt will be rebuilt */
diff --git a/core/sensor.c b/core/sensor.c
index 9bc204a..c3fa319 100644
--- a/core/sensor.c
+++ b/core/sensor.c
@@ -144,7 +144,6 @@  void sensor_init(void)
 	dt_add_property_string(sensor_node, "compatible", "ibm,opal-sensor");
 	dt_add_property_cells(sensor_node, "#address-cells", 1);
 	dt_add_property_cells(sensor_node, "#size-cells", 0);
-	dts_sensor_create_nodes(sensor_node);
 
 	/* Register OPAL interface */
 	opal_register(OPAL_SENSOR_READ, opal_sensor_read, 3);
diff --git a/hw/occ-sensor.c b/hw/occ-sensor.c
index 090e0f0..d1cac33 100644
--- a/hw/occ-sensor.c
+++ b/hw/occ-sensor.c
@@ -491,7 +491,7 @@  static void add_sensor_node(const char *loc, const char *type, int i, int attr,
 	*phandle = node->phandle;
 }
 
-void occ_sensors_init(void)
+bool occ_sensors_init(void)
 {
 	struct proc_chip *chip;
 	struct dt_node *sg, *exports;
@@ -500,13 +500,13 @@  void occ_sensors_init(void)
 
 	/* OCC inband sensors is only supported in P9 */
 	if (proc_gen != proc_gen_p9)
-		return;
+		return false;
 
 	/* Sensors are copied to BAR2 OCC Common Area */
 	chip = next_chip(NULL);
 	if (!chip->occ_common_base) {
 		prerror("OCC: Unassigned OCC Common Area. No sensors found\n");
-		return;
+		return false;
 	}
 
 	occ_sensor_base = chip->occ_common_base + OCC_SENSOR_DATA_BLOCK_OFFSET;
@@ -514,7 +514,7 @@  void occ_sensors_init(void)
 	sg = dt_new(opal_node, "sensor-groups");
 	if (!sg) {
 		prerror("OCC: Failed to create sensor groups node\n");
-		return;
+		return false;
 	}
 	dt_add_property_string(sg, "compatible", "ibm,opal-sensor-group");
 	dt_add_property_cells(sg, "#address-cells", 1);
@@ -593,14 +593,16 @@  void occ_sensors_init(void)
 	}
 
 	if (!occ_num)
-		return;
+		return false;
 
 	exports = dt_find_by_path(dt_root, "/ibm,opal/firmware/exports");
 	if (!exports) {
 		prerror("OCC: dt node /ibm,opal/firmware/exports not found\n");
-		return;
+		return false;
 	}
 
 	dt_add_property_u64s(exports, "occ_inband_sensors", occ_sensor_base,
 			     OCC_SENSOR_DATA_BLOCK_SIZE * occ_num);
+
+	return true;
 }
diff --git a/include/skiboot.h b/include/skiboot.h
index 1180a17..601eab0 100644
--- a/include/skiboot.h
+++ b/include/skiboot.h
@@ -352,7 +352,7 @@  extern int fake_nvram_start_read(void *dst, uint32_t src, uint32_t len);
 extern int fake_nvram_write(uint32_t offset, void *src, uint32_t size);
 
 /* OCC Inband Sensors */
-extern void occ_sensors_init(void);
+extern bool occ_sensors_init(void);
 extern int occ_sensor_read(u32 handle, u64 *data);
 extern int occ_sensor_group_clear(u32 group_hndl, int token);
 extern void occ_add_sensor_groups(struct dt_node *sg, u32  *phandles,