diff mbox

[v2,5/6] drm/i915: Add support to parse DMI table and get platform memory info

Message ID 1453911003-9856-5-git-send-email-shobhit.kumar@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kumar, Shobhit Jan. 27, 2016, 4:10 p.m. UTC
This is needed for WM computation workaround for arbitrated display
bandwidth.

v2: Address Matt's review comments
    - Be more paranoid while dmi decoding
    - Also add support for decoding speed from configured memory speed
      if availble in DMI memory entry

Cc: matthew.d.roper@intel.com
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 47 +++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
 2 files changed, 53 insertions(+)

Comments

Matt Roper Feb. 10, 2016, 10:29 p.m. UTC | #1
On Wed, Jan 27, 2016 at 09:40:02PM +0530, Shobhit Kumar wrote:
> This is needed for WM computation workaround for arbitrated display
> bandwidth.
> 
> v2: Address Matt's review comments
>     - Be more paranoid while dmi decoding
>     - Also add support for decoding speed from configured memory speed
>       if availble in DMI memory entry
> 
> Cc: matthew.d.roper@intel.com
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 47 +++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index d70d96f..320143b 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -49,6 +49,7 @@
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/oom.h>
> +#include <linux/dmi.h>
>  
>  
>  static int i915_getparam(struct drm_device *dev, void *data,
> @@ -855,6 +856,49 @@ static void intel_init_dpio(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> +static void dmi_decode_memory_info(const struct dmi_header *hdr, void *priv)
> +{
> +	struct drm_i915_private *dev_priv = (struct drm_i915_private *) priv;
> +	const u8 *data = (const u8 *) hdr;
> +	uint16_t size, mem_speed;
> +
> +#define DMI_CONF_MEM_SPEED_OFFSET	0x20
> +#define DMI_MEM_SPEED_OFFSET		0x15
> +#define DMI_MEM_SIZE_OFFSET		0x0C
> +
> +	if (hdr->type == DMI_ENTRY_MEM_DEVICE) {
> +		/* Found a memory channel ? */
> +		size = (uint16_t) (*((uint16_t *)(data + DMI_MEM_SIZE_OFFSET)));

It might be nicer/cleaner to copy over the memdev_dmi_entry struct from
drivers/edac/i7core_edac.c and cast the data pointer into that to avoid
all the pointer arithmetic.  But all of your calculations look correct
to me, so probably not a huge deal either way.

> +		if (size == 0)
> +			return;
> +
> +		dev_priv->dmi.mem_channel++;
> +
> +		/* Get the speed */
> +		if (hdr->length > DMI_CONF_MEM_SPEED_OFFSET)
> +			mem_speed =
> +				(uint16_t) (*((uint16_t *)(data + DMI_CONF_MEM_SPEED_OFFSET)));
> +		else if (hdr->length > DMI_MEM_SPEED_OFFSET)
> +			mem_speed =
> +				(uint16_t) (*((uint16_t *)(data + DMI_MEM_SPEED_OFFSET)));

I think we have more layers of casting here than necessary?

> +		else
> +			mem_speed = -1;

mem_speed is a uint, so down below it's actually going to pass the
mem_speed > 0 tests, which I don't think was your intent.


Matt

> +
> +		/*
> +		 * Check all channels have same speed
> +		 * else mark speed as invalid
> +		 */
> +		if (dev_priv->dmi.mem_speed == 0) {
> +			if (mem_speed > 0)
> +				dev_priv->dmi.mem_speed = mem_speed;
> +			else
> +				dev_priv->dmi.mem_speed = -1;
> +		} else if (dev_priv->dmi.mem_speed > 0 &&
> +					dev_priv->dmi.mem_speed != mem_speed)
> +			dev_priv->dmi.mem_speed = -1;
> +	}
> +}
> +
>  /**
>   * i915_driver_load - setup chip and create an initial config
>   * @dev: DRM device
> @@ -882,6 +926,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	dev->dev_private = dev_priv;
>  	dev_priv->dev = dev;
>  
> +	/* walk the dmi device table for getting platform memory information */
> +	dmi_walk(dmi_decode_memory_info, (void *) dev_priv);
> +
>  	/* Setup the write-once "constant" device info */
>  	device_info = (struct intel_device_info *)&dev_priv->info;
>  	memcpy(device_info, info, sizeof(dev_priv->info));
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 211af53..b040e7a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1968,6 +1968,12 @@ struct drm_i915_private {
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
>  	 */
> +
> +	/* DMI data for memory bandwidth calculation */
> +	struct {
> +		uint16_t mem_channel;
> +		int16_t mem_speed;
> +	} dmi;
>  };
>  
>  static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> -- 
> 2.5.0
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index d70d96f..320143b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -49,6 +49,7 @@ 
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/oom.h>
+#include <linux/dmi.h>
 
 
 static int i915_getparam(struct drm_device *dev, void *data,
@@ -855,6 +856,49 @@  static void intel_init_dpio(struct drm_i915_private *dev_priv)
 	}
 }
 
+static void dmi_decode_memory_info(const struct dmi_header *hdr, void *priv)
+{
+	struct drm_i915_private *dev_priv = (struct drm_i915_private *) priv;
+	const u8 *data = (const u8 *) hdr;
+	uint16_t size, mem_speed;
+
+#define DMI_CONF_MEM_SPEED_OFFSET	0x20
+#define DMI_MEM_SPEED_OFFSET		0x15
+#define DMI_MEM_SIZE_OFFSET		0x0C
+
+	if (hdr->type == DMI_ENTRY_MEM_DEVICE) {
+		/* Found a memory channel ? */
+		size = (uint16_t) (*((uint16_t *)(data + DMI_MEM_SIZE_OFFSET)));
+		if (size == 0)
+			return;
+
+		dev_priv->dmi.mem_channel++;
+
+		/* Get the speed */
+		if (hdr->length > DMI_CONF_MEM_SPEED_OFFSET)
+			mem_speed =
+				(uint16_t) (*((uint16_t *)(data + DMI_CONF_MEM_SPEED_OFFSET)));
+		else if (hdr->length > DMI_MEM_SPEED_OFFSET)
+			mem_speed =
+				(uint16_t) (*((uint16_t *)(data + DMI_MEM_SPEED_OFFSET)));
+		else
+			mem_speed = -1;
+
+		/*
+		 * Check all channels have same speed
+		 * else mark speed as invalid
+		 */
+		if (dev_priv->dmi.mem_speed == 0) {
+			if (mem_speed > 0)
+				dev_priv->dmi.mem_speed = mem_speed;
+			else
+				dev_priv->dmi.mem_speed = -1;
+		} else if (dev_priv->dmi.mem_speed > 0 &&
+					dev_priv->dmi.mem_speed != mem_speed)
+			dev_priv->dmi.mem_speed = -1;
+	}
+}
+
 /**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
@@ -882,6 +926,9 @@  int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	dev->dev_private = dev_priv;
 	dev_priv->dev = dev;
 
+	/* walk the dmi device table for getting platform memory information */
+	dmi_walk(dmi_decode_memory_info, (void *) dev_priv);
+
 	/* Setup the write-once "constant" device info */
 	device_info = (struct intel_device_info *)&dev_priv->info;
 	memcpy(device_info, info, sizeof(dev_priv->info));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 211af53..b040e7a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1968,6 +1968,12 @@  struct drm_i915_private {
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
 	 */
+
+	/* DMI data for memory bandwidth calculation */
+	struct {
+		uint16_t mem_channel;
+		int16_t mem_speed;
+	} dmi;
 };
 
 static inline struct drm_i915_private *to_i915(const struct drm_device *dev)