All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
@ 2014-07-31  1:59 jeff.mcgee
  2014-07-31  1:59 ` [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV jeff.mcgee
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: jeff.mcgee @ 2014-07-31  1:59 UTC (permalink / raw)
  To: intel-gfx

From: Jeff McGee <jeff.mcgee@intel.com>

Define a struct to capture information on the device's Slice/Subslice/EU
(SSEU) configuration. Add this struct to the main device info struct.
Define a packed bitfield form for the SSEU info and share it with
userspace via a new GETPARAM option.

Starting with Cherryview, devices may have a varying number of EU for
a given ID due to creative fusing. The surest way to determine the
configuration is by reading fuses which is best done in the kernel and
communicated to userspace. The immediate need from userspace is to
determine the number of threads of compute work that can be safely
submitted.

The definition of SSEU as a new drm/i915 component, with its own header
file and soon-to-be source file, is in anticipation of lots of upcoming
code for its management, particularly the power gating functionality.

Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c   |  3 +++
 drivers/gpu/drm/i915/i915_drv.h   |  3 +++
 drivers/gpu/drm/i915/intel_sseu.h | 40 +++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h       | 18 ++++++++++++++++++
 4 files changed, 64 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/intel_sseu.h

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2e7f03a..f581848 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1025,6 +1025,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_CMD_PARSER_VERSION:
 		value = i915_cmd_parser_get_version();
 		break;
+	case I915_PARAM_SSEU_INFO:
+		value = INTEL_INFO(dev)->sseu.gp_sseu_info;
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 18c9ad8..01adafd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -45,6 +45,7 @@
 #include <linux/intel-iommu.h>
 #include <linux/kref.h>
 #include <linux/pm_qos.h>
+#include "intel_sseu.h"
 
 /* General customization:
  */
@@ -562,6 +563,8 @@ struct intel_device_info {
 	int trans_offsets[I915_MAX_TRANSCODERS];
 	int palette_offsets[I915_MAX_PIPES];
 	int cursor_offsets[I915_MAX_PIPES];
+	/* Slice/Subslice/EU info */
+	struct intel_sseu_info sseu;
 };
 
 #undef DEFINE_FLAG
diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
new file mode 100644
index 0000000..7db7175
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_sseu.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+#ifndef _INTEL_SSEU_H_
+#define _INTEL_SSEU_H_
+
+struct intel_sseu_info {
+	/* Total slice count */
+	unsigned int slice_cnt;
+	/* Total subslice count */
+	unsigned int subslice_cnt;
+	/* Total execution unit count */
+	unsigned int eu_cnt;
+	/* Thread count per EU */
+	unsigned int threads_per_eu;
+	/* Bit field representation for I915_PARAM_SSEU_INFO */
+	u32 gp_sseu_info;
+};
+
+#endif
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index ff57f07..b99c1a2 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -171,6 +171,23 @@ typedef struct _drm_i915_sarea {
 #define I915_BOX_TEXTURE_LOAD  0x8
 #define I915_BOX_LOST_CONTEXT  0x10
 
+/*
+ * Slice/Subslice/EU Info
+ * - Accessed via GETPARAM ioctl option I915_PARAM_SSEU_INFO
+ * - SLICE_CNT: total slice count
+ * - SUBSLICE_CNT: total subslice count
+ * - EU_CNT: total execution unit count
+ * - THREADS_PER_EU: thread count per EU
+*/
+#define I915_SSEU_INFO_SLICE_CNT_MASK		0xf
+#define I915_SSEU_INFO_SLICE_CNT_SHIFT		0
+#define I915_SSEU_INFO_SUBSLICE_CNT_MASK	(0x3f<<4)
+#define I915_SSEU_INFO_SUBSLICE_CNT_SHIFT	4
+#define I915_SSEU_INFO_EU_CNT_MASK		(0xff<<10)
+#define I915_SSEU_INFO_EU_CNT_SHIFT		10
+#define I915_SSEU_INFO_THREADS_PER_EU_MASK	(0xf<<18)
+#define I915_SSEU_INFO_THREADS_PER_EU_SHIFT	18
+
 /* I915 specific ioctls
  * The device specific ioctl range is 0x40 to 0x79.
  */
@@ -340,6 +357,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
 #define I915_PARAM_HAS_WT     	 	 27
 #define I915_PARAM_CMD_PARSER_VERSION	 28
+#define I915_PARAM_SSEU_INFO		 29
 
 typedef struct drm_i915_getparam {
 	int param;
-- 
2.0.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
  2014-07-31  1:59 [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM jeff.mcgee
@ 2014-07-31  1:59 ` jeff.mcgee
  2014-07-31 11:55   ` Mcaulay, Alistair
  2014-08-04  8:22   ` Daniel Vetter
  2014-07-31 11:53 ` [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM Mcaulay, Alistair
  2014-08-04  8:20 ` Daniel Vetter
  2 siblings, 2 replies; 16+ messages in thread
From: jeff.mcgee @ 2014-07-31  1:59 UTC (permalink / raw)
  To: intel-gfx

From: Jeff McGee <jeff.mcgee@intel.com>

Cherryview can have different SSEU configurations within a given PCI
ID, so we collect the info from the fuse register.

I don't currently have access to a CHV, much less one with an interesting
fuse config. So I have compile-checked this only!

Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/Makefile     |  3 +-
 drivers/gpu/drm/i915/i915_dma.c   |  2 ++
 drivers/gpu/drm/i915/i915_reg.h   | 13 ++++++++
 drivers/gpu/drm/i915/intel_sseu.c | 64 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_sseu.h |  2 ++
 5 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/intel_sseu.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 91bd167..9a0f411 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \
 	  i915_irq.o \
 	  i915_trace_points.o \
 	  intel_ringbuffer.o \
-	  intel_uncore.o
+	  intel_uncore.o \
+	  intel_sseu.o
 
 # autogenerated null render state
 i915-y += intel_renderstate_gen6.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f581848..384ef65 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 			goto out_gem_unload;
 	}
 
+	intel_sseu_init(dev);
+
 	intel_power_domains_init(dev_priv);
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 28e21ed..24a2d56 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5624,6 +5624,19 @@ enum punit_power_well {
 #define GEN7_MISCCPCTL			(0x9424)
 #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
 
+/* Fuse readout registers for GT */
+#define CHV_FUSE_GT					0x182168
+#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0		10
+#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1		11
+#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK		(0xf<<16)
+#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT		16
+#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK		(0xf<<20)
+#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT		20
+#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK		(0xf<<24)
+#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT		24
+#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK		(0xf<<28)
+#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT		28
+
 /* IVYBRIDGE DPF */
 #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
 #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c
new file mode 100644
index 0000000..6ba4830
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_sseu.c
@@ -0,0 +1,64 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+#include <linux/bitops.h>
+#include "i915_drv.h"
+#include "intel_sseu.h"
+
+void intel_sseu_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_sseu_info sseu_info;
+	u32 gp = 0;
+
+	/* Collect SSEU info per device */
+	if (IS_CHERRYVIEW(dev)) {
+		u32 fuse, mask_ss, mask_eu;
+
+		fuse = I915_READ(CHV_FUSE_GT);
+		mask_ss = fuse & (CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 |
+				  CHV_FUSE_GT_SUBSLICE_DISABLE_SS1);
+		mask_eu = fuse & (CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK |
+				  CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK |
+				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK |
+				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK);
+		sseu_info.slice_cnt = 1;
+		sseu_info.subslice_cnt = 2 - hweight32(mask_ss);
+		sseu_info.eu_cnt = 16 - hweight32(mask_eu);
+		sseu_info.threads_per_eu = 7;
+	}
+
+	/* Pack SSEU info bitfield for I915_PARAM_SSEU_INFO */
+	gp |= (sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) &
+	       I915_SSEU_INFO_SLICE_CNT_MASK;
+	gp |= (sseu_info.subslice_cnt << I915_SSEU_INFO_SUBSLICE_CNT_SHIFT) &
+	       I915_SSEU_INFO_SUBSLICE_CNT_MASK;
+	gp |= (sseu_info.eu_cnt << I915_SSEU_INFO_EU_CNT_SHIFT) &
+	       I915_SSEU_INFO_EU_CNT_MASK;
+	gp |= (sseu_info.threads_per_eu << I915_SSEU_INFO_THREADS_PER_EU_SHIFT) &
+	       I915_SSEU_INFO_THREADS_PER_EU_MASK;
+	sseu_info.gp_sseu_info = gp;
+
+	/* Copy SSEU info to the const device info with pointer magic */
+	*(struct intel_sseu_info *)&dev_priv->info.sseu = sseu_info;
+}
diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
index 7db7175..0257358 100644
--- a/drivers/gpu/drm/i915/intel_sseu.h
+++ b/drivers/gpu/drm/i915/intel_sseu.h
@@ -37,4 +37,6 @@ struct intel_sseu_info {
 	u32 gp_sseu_info;
 };
 
+extern void intel_sseu_init(struct drm_device *dev);
+
 #endif
-- 
2.0.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
  2014-07-31  1:59 [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM jeff.mcgee
  2014-07-31  1:59 ` [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV jeff.mcgee
@ 2014-07-31 11:53 ` Mcaulay, Alistair
  2014-07-31 23:16   ` Jeff McGee
  2014-08-04  8:20 ` Daniel Vetter
  2 siblings, 1 reply; 16+ messages in thread
From: Mcaulay, Alistair @ 2014-07-31 11:53 UTC (permalink / raw)
  To: Mcgee, Jeff, intel-gfx

Hi Jeff,

These patches look like they solve the problem well. I've added some comments in amongst the code.

Thanks,
Alistair.

-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of jeff.mcgee@intel.com
Sent: Thursday, July 31, 2014 3:00 AM
To: intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM

From: Jeff McGee <jeff.mcgee@intel.com>

Define a struct to capture information on the device's Slice/Subslice/EU
(SSEU) configuration. Add this struct to the main device info struct.
Define a packed bitfield form for the SSEU info and share it with userspace via a new GETPARAM option.

Starting with Cherryview, devices may have a varying number of EU for a given ID due to creative fusing. The surest way to determine the configuration is by reading fuses which is best done in the kernel and communicated to userspace. The immediate need from userspace is to determine the number of threads of compute work that can be safely submitted.

The definition of SSEU as a new drm/i915 component, with its own header file and soon-to-be source file, is in anticipation of lots of upcoming code for its management, particularly the power gating functionality.

Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c   |  3 +++
 drivers/gpu/drm/i915/i915_drv.h   |  3 +++
 drivers/gpu/drm/i915/intel_sseu.h | 40 +++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h       | 18 ++++++++++++++++++
 4 files changed, 64 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/intel_sseu.h

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2e7f03a..f581848 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1025,6 +1025,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_CMD_PARSER_VERSION:
 		value = i915_cmd_parser_get_version();
 		break;
+	case I915_PARAM_SSEU_INFO:
+		value = INTEL_INFO(dev)->sseu.gp_sseu_info;
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 18c9ad8..01adafd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -45,6 +45,7 @@
 #include <linux/intel-iommu.h>
 #include <linux/kref.h>
 #include <linux/pm_qos.h>
+#include "intel_sseu.h"
 
 /* General customization:
  */
@@ -562,6 +563,8 @@ struct intel_device_info {
 	int trans_offsets[I915_MAX_TRANSCODERS];
 	int palette_offsets[I915_MAX_PIPES];
 	int cursor_offsets[I915_MAX_PIPES];
+	/* Slice/Subslice/EU info */
+	struct intel_sseu_info sseu;
 };
 
 #undef DEFINE_FLAG
diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
new file mode 100644
index 0000000..7db7175
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_sseu.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person 
+obtaining a
+ * copy of this software and associated documentation files (the 
+"Software"),
+ * to deal in the Software without restriction, including without 
+limitation
+ * the rights to use, copy, modify, merge, publish, distribute, 
+sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom 
+the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the 
+next
+ * paragraph) shall be included in all copies or substantial portions 
+of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
+EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
+MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
+SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
+OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
+ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
+DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+#ifndef _INTEL_SSEU_H_
+#define _INTEL_SSEU_H_
+
+struct intel_sseu_info {
+	/* Total slice count */
+	unsigned int slice_cnt;
+	/* Total subslice count */
+	unsigned int subslice_cnt;
+	/* Total execution unit count */
+	unsigned int eu_cnt;
+	/* Thread count per EU */
+	unsigned int threads_per_eu;
+	/* Bit field representation for I915_PARAM_SSEU_INFO */
+	u32 gp_sseu_info;
+};
+
+#endif
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index ff57f07..b99c1a2 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -171,6 +171,23 @@ typedef struct _drm_i915_sarea {  #define I915_BOX_TEXTURE_LOAD  0x8  #define I915_BOX_LOST_CONTEXT  0x10
 
+/*
+ * Slice/Subslice/EU Info
+ * - Accessed via GETPARAM ioctl option I915_PARAM_SSEU_INFO
+ * - SLICE_CNT: total slice count
+ * - SUBSLICE_CNT: total subslice count
+ * - EU_CNT: total execution unit count
+ * - THREADS_PER_EU: thread count per EU */
+#define I915_SSEU_INFO_SLICE_CNT_MASK		0xf
+#define I915_SSEU_INFO_SLICE_CNT_SHIFT		0
+#define I915_SSEU_INFO_SUBSLICE_CNT_MASK	(0x3f<<4)

Why not use the shift defines here?
#define I915_SSEU_INFO_SUBSLICE_CNT_MASK	(0x3f<< I915_SSEU_INFO_SUBSLICE_CNT_SHIFT)
etc

+#define I915_SSEU_INFO_SUBSLICE_CNT_SHIFT	4
+#define I915_SSEU_INFO_EU_CNT_MASK		(0xff<<10)
+#define I915_SSEU_INFO_EU_CNT_SHIFT		10
+#define I915_SSEU_INFO_THREADS_PER_EU_MASK	(0xf<<18)
+#define I915_SSEU_INFO_THREADS_PER_EU_SHIFT	18

There are 10 bits left unused here. Is it likely another field could be added later?
If not, the masks could be widened to be more future proof.

+
 /* I915 specific ioctls
  * The device specific ioctl range is 0x40 to 0x79.
  */
@@ -340,6 +357,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
 #define I915_PARAM_HAS_WT     	 	 27
 #define I915_PARAM_CMD_PARSER_VERSION	 28
+#define I915_PARAM_SSEU_INFO		 29
 
 typedef struct drm_i915_getparam {
 	int param;
--
2.0.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
  2014-07-31  1:59 ` [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV jeff.mcgee
@ 2014-07-31 11:55   ` Mcaulay, Alistair
  2014-07-31 23:18     ` Jeff McGee
  2014-08-04  8:18     ` Daniel Vetter
  2014-08-04  8:22   ` Daniel Vetter
  1 sibling, 2 replies; 16+ messages in thread
From: Mcaulay, Alistair @ 2014-07-31 11:55 UTC (permalink / raw)
  To: Mcgee, Jeff, intel-gfx

Hi Jeff,

Some more comments in the code.

Thanks,
Alistair.

-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of jeff.mcgee@intel.com
Sent: Thursday, July 31, 2014 3:00 AM
To: intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV

From: Jeff McGee <jeff.mcgee@intel.com>

Cherryview can have different SSEU configurations within a given PCI ID, so we collect the info from the fuse register.

I don't currently have access to a CHV, much less one with an interesting fuse config. So I have compile-checked this only!

Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/Makefile     |  3 +-
 drivers/gpu/drm/i915/i915_dma.c   |  2 ++
 drivers/gpu/drm/i915/i915_reg.h   | 13 ++++++++
 drivers/gpu/drm/i915/intel_sseu.c | 64 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_sseu.h |  2 ++
 5 files changed, 83 insertions(+), 1 deletion(-)  create mode 100644 drivers/gpu/drm/i915/intel_sseu.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91bd167..9a0f411 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \
 	  i915_irq.o \
 	  i915_trace_points.o \
 	  intel_ringbuffer.o \
-	  intel_uncore.o
+	  intel_uncore.o \
+	  intel_sseu.o
 
 # autogenerated null render state
 i915-y += intel_renderstate_gen6.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f581848..384ef65 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 			goto out_gem_unload;
 	}
 
+	intel_sseu_init(dev);
+
 	intel_power_domains_init(dev_priv);
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 28e21ed..24a2d56 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5624,6 +5624,19 @@ enum punit_power_well {
 #define GEN7_MISCCPCTL			(0x9424)
 #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
 
+/* Fuse readout registers for GT */
+#define CHV_FUSE_GT					0x182168
+#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0		10
+#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1		11

These should be:
#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0		(1 << 10)
#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1		(1 << 11)

+#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK		(0xf<<16)
+#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT		16

Why not use the shift define here?
#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK		(0xf<<CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT)
and for the others.

+#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK		(0xf<<20)
+#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT		20
+#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK		(0xf<<24)
+#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT		24
+#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK		(0xf<<28)
+#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT		28
+
 /* IVYBRIDGE DPF */
 #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
 #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c
new file mode 100644
index 0000000..6ba4830
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_sseu.c
@@ -0,0 +1,64 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person 
+obtaining a
+ * copy of this software and associated documentation files (the 
+"Software"),
+ * to deal in the Software without restriction, including without 
+limitation
+ * the rights to use, copy, modify, merge, publish, distribute, 
+sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom 
+the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the 
+next
+ * paragraph) shall be included in all copies or substantial portions 
+of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
+EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
+MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
+SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
+OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
+ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
+DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+#include <linux/bitops.h>
+#include "i915_drv.h"
+#include "intel_sseu.h"
+
+void intel_sseu_init(struct drm_device *dev) {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_sseu_info sseu_info;
+	u32 gp = 0;
+
+	/* Collect SSEU info per device */
+	if (IS_CHERRYVIEW(dev)) {
+		u32 fuse, mask_ss, mask_eu;
+
+		fuse = I915_READ(CHV_FUSE_GT);
+		mask_ss = fuse & (CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 |
+				  CHV_FUSE_GT_SUBSLICE_DISABLE_SS1);
+		mask_eu = fuse & (CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK |
+				  CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK |
+				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK |
+				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK);
+		sseu_info.slice_cnt = 1;
+		sseu_info.subslice_cnt = 2 - hweight32(mask_ss);
+		sseu_info.eu_cnt = 16 - hweight32(mask_eu);
+		sseu_info.threads_per_eu = 7;
+	}
+
+	/* Pack SSEU info bitfield for I915_PARAM_SSEU_INFO */
+	gp |= (sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) &
+	       I915_SSEU_INFO_SLICE_CNT_MASK;

In theory, there should be no need to use the mask, unless something has gone wrong.
Maybe:

WARN_ON((sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) > I915_SSEU_INFO_SLICE_CNT_MASK );
etc


+	gp |= (sseu_info.subslice_cnt << I915_SSEU_INFO_SUBSLICE_CNT_SHIFT) &
+	       I915_SSEU_INFO_SUBSLICE_CNT_MASK;
+	gp |= (sseu_info.eu_cnt << I915_SSEU_INFO_EU_CNT_SHIFT) &
+	       I915_SSEU_INFO_EU_CNT_MASK;
+	gp |= (sseu_info.threads_per_eu << I915_SSEU_INFO_THREADS_PER_EU_SHIFT) &
+	       I915_SSEU_INFO_THREADS_PER_EU_MASK;
+	sseu_info.gp_sseu_info = gp;
+
+	/* Copy SSEU info to the const device info with pointer magic */
+	*(struct intel_sseu_info *)&dev_priv->info.sseu = sseu_info; }
diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
index 7db7175..0257358 100644
--- a/drivers/gpu/drm/i915/intel_sseu.h
+++ b/drivers/gpu/drm/i915/intel_sseu.h
@@ -37,4 +37,6 @@ struct intel_sseu_info {
 	u32 gp_sseu_info;
 };
 
+extern void intel_sseu_init(struct drm_device *dev);
+
 #endif
--
2.0.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
  2014-07-31 11:53 ` [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM Mcaulay, Alistair
@ 2014-07-31 23:16   ` Jeff McGee
  0 siblings, 0 replies; 16+ messages in thread
From: Jeff McGee @ 2014-07-31 23:16 UTC (permalink / raw)
  To: Mcaulay, Alistair; +Cc: intel-gfx

On Thu, Jul 31, 2014 at 04:53:34AM -0700, Mcaulay, Alistair wrote:
> Hi Jeff,
> 
> These patches look like they solve the problem well. I've added some comments in amongst the code.
> 
> Thanks,
> Alistair.
> 
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of jeff.mcgee@intel.com
> Sent: Thursday, July 31, 2014 3:00 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
> 
> From: Jeff McGee <jeff.mcgee@intel.com>
> 
> Define a struct to capture information on the device's Slice/Subslice/EU
> (SSEU) configuration. Add this struct to the main device info struct.
> Define a packed bitfield form for the SSEU info and share it with userspace via a new GETPARAM option.
> 
> Starting with Cherryview, devices may have a varying number of EU for a given ID due to creative fusing. The surest way to determine the configuration is by reading fuses which is best done in the kernel and communicated to userspace. The immediate need from userspace is to determine the number of threads of compute work that can be safely submitted.
> 
> The definition of SSEU as a new drm/i915 component, with its own header file and soon-to-be source file, is in anticipation of lots of upcoming code for its management, particularly the power gating functionality.
> 
> Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c   |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h   |  3 +++
>  drivers/gpu/drm/i915/intel_sseu.h | 40 +++++++++++++++++++++++++++++++++++++++
>  include/uapi/drm/i915_drm.h       | 18 ++++++++++++++++++
>  4 files changed, 64 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/intel_sseu.h
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2e7f03a..f581848 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1025,6 +1025,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_CMD_PARSER_VERSION:
>  		value = i915_cmd_parser_get_version();
>  		break;
> +	case I915_PARAM_SSEU_INFO:
> +		value = INTEL_INFO(dev)->sseu.gp_sseu_info;
> +		break;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 18c9ad8..01adafd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -45,6 +45,7 @@
>  #include <linux/intel-iommu.h>
>  #include <linux/kref.h>
>  #include <linux/pm_qos.h>
> +#include "intel_sseu.h"
>  
>  /* General customization:
>   */
> @@ -562,6 +563,8 @@ struct intel_device_info {
>  	int trans_offsets[I915_MAX_TRANSCODERS];
>  	int palette_offsets[I915_MAX_PIPES];
>  	int cursor_offsets[I915_MAX_PIPES];
> +	/* Slice/Subslice/EU info */
> +	struct intel_sseu_info sseu;
>  };
>  
>  #undef DEFINE_FLAG
> diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
> new file mode 100644
> index 0000000..7db7175
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_sseu.h
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the 
> +next
> + * paragraph) shall be included in all copies or substantial portions 
> +of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> +OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
> +ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> +DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +#ifndef _INTEL_SSEU_H_
> +#define _INTEL_SSEU_H_
> +
> +struct intel_sseu_info {
> +	/* Total slice count */
> +	unsigned int slice_cnt;
> +	/* Total subslice count */
> +	unsigned int subslice_cnt;
> +	/* Total execution unit count */
> +	unsigned int eu_cnt;
> +	/* Thread count per EU */
> +	unsigned int threads_per_eu;
> +	/* Bit field representation for I915_PARAM_SSEU_INFO */
> +	u32 gp_sseu_info;
> +};
> +
> +#endif
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index ff57f07..b99c1a2 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -171,6 +171,23 @@ typedef struct _drm_i915_sarea {  #define I915_BOX_TEXTURE_LOAD  0x8  #define I915_BOX_LOST_CONTEXT  0x10
>  
> +/*
> + * Slice/Subslice/EU Info
> + * - Accessed via GETPARAM ioctl option I915_PARAM_SSEU_INFO
> + * - SLICE_CNT: total slice count
> + * - SUBSLICE_CNT: total subslice count
> + * - EU_CNT: total execution unit count
> + * - THREADS_PER_EU: thread count per EU */
> +#define I915_SSEU_INFO_SLICE_CNT_MASK		0xf
> +#define I915_SSEU_INFO_SLICE_CNT_SHIFT		0
> +#define I915_SSEU_INFO_SUBSLICE_CNT_MASK	(0x3f<<4)
> 
> Why not use the shift defines here?
> #define I915_SSEU_INFO_SUBSLICE_CNT_MASK	(0x3f<< I915_SSEU_INFO_SUBSLICE_CNT_SHIFT)
> etc
> 

The macro names are so long right now (in order to be descriptive) that
doing this will exceed 80 character line length, and so is a readability
issue for some. Otherwise I agree with you. I'll see what I can do with
the names and perhaps ask for a pass on the line length.
-Jeff

> +#define I915_SSEU_INFO_SUBSLICE_CNT_SHIFT	4
> +#define I915_SSEU_INFO_EU_CNT_MASK		(0xff<<10)
> +#define I915_SSEU_INFO_EU_CNT_SHIFT		10
> +#define I915_SSEU_INFO_THREADS_PER_EU_MASK	(0xf<<18)
> +#define I915_SSEU_INFO_THREADS_PER_EU_SHIFT	18
> 
> There are 10 bits left unused here. Is it likely another field could be added later?
> If not, the masks could be widened to be more future proof.
> 

Yes, I think with power gating, we will need additional flags here. I
feel like the ranges here are pretty future proof: max slice = 15,
max subslice = 63, max eu = 255, max threads per eu = 15. We could
probably dedicate a few more bits to these and still have room for flags.
Any suggestions on what range to increase?
-Jeff

> +
>  /* I915 specific ioctls
>   * The device specific ioctl range is 0x40 to 0x79.
>   */
> @@ -340,6 +357,7 @@ typedef struct drm_i915_irq_wait {
>  #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
>  #define I915_PARAM_HAS_WT     	 	 27
>  #define I915_PARAM_CMD_PARSER_VERSION	 28
> +#define I915_PARAM_SSEU_INFO		 29
>  
>  typedef struct drm_i915_getparam {
>  	int param;
> --
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
  2014-07-31 11:55   ` Mcaulay, Alistair
@ 2014-07-31 23:18     ` Jeff McGee
  2014-08-04  8:18     ` Daniel Vetter
  1 sibling, 0 replies; 16+ messages in thread
From: Jeff McGee @ 2014-07-31 23:18 UTC (permalink / raw)
  To: Mcaulay, Alistair; +Cc: intel-gfx

On Thu, Jul 31, 2014 at 04:55:58AM -0700, Mcaulay, Alistair wrote:
> Hi Jeff,
> 
> Some more comments in the code.
> 
> Thanks,
> Alistair.
> 
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of jeff.mcgee@intel.com
> Sent: Thursday, July 31, 2014 3:00 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
> 
> From: Jeff McGee <jeff.mcgee@intel.com>
> 
> Cherryview can have different SSEU configurations within a given PCI ID, so we collect the info from the fuse register.
> 
> I don't currently have access to a CHV, much less one with an interesting fuse config. So I have compile-checked this only!
> 
> Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile     |  3 +-
>  drivers/gpu/drm/i915/i915_dma.c   |  2 ++
>  drivers/gpu/drm/i915/i915_reg.h   | 13 ++++++++
>  drivers/gpu/drm/i915/intel_sseu.c | 64 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_sseu.h |  2 ++
>  5 files changed, 83 insertions(+), 1 deletion(-)  create mode 100644 drivers/gpu/drm/i915/intel_sseu.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91bd167..9a0f411 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \
>  	  i915_irq.o \
>  	  i915_trace_points.o \
>  	  intel_ringbuffer.o \
> -	  intel_uncore.o
> +	  intel_uncore.o \
> +	  intel_sseu.o
>  
>  # autogenerated null render state
>  i915-y += intel_renderstate_gen6.o \
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f581848..384ef65 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  			goto out_gem_unload;
>  	}
>  
> +	intel_sseu_init(dev);
> +
>  	intel_power_domains_init(dev_priv);
>  
>  	if (drm_core_check_feature(dev, DRIVER_MODESET)) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 28e21ed..24a2d56 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5624,6 +5624,19 @@ enum punit_power_well {
>  #define GEN7_MISCCPCTL			(0x9424)
>  #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
>  
> +/* Fuse readout registers for GT */
> +#define CHV_FUSE_GT					0x182168
> +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0		10
> +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1		11
> 
> These should be:
> #define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0		(1 << 10)
> #define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1		(1 << 11)
> 
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK		(0xf<<16)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT		16
> 
> Why not use the shift define here?
> #define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK		(0xf<<CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT)
> and for the others.
> 

See answer in previous patch
-Jeff

> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK		(0xf<<20)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT		20
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK		(0xf<<24)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT		24
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK		(0xf<<28)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT		28
> +
>  /* IVYBRIDGE DPF */
>  #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
>  #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
> diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c
> new file mode 100644
> index 0000000..6ba4830
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_sseu.c
> @@ -0,0 +1,64 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the 
> +next
> + * paragraph) shall be included in all copies or substantial portions 
> +of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> +OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
> +ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> +DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +#include <linux/bitops.h>
> +#include "i915_drv.h"
> +#include "intel_sseu.h"
> +
> +void intel_sseu_init(struct drm_device *dev) {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_sseu_info sseu_info;
> +	u32 gp = 0;
> +
> +	/* Collect SSEU info per device */
> +	if (IS_CHERRYVIEW(dev)) {
> +		u32 fuse, mask_ss, mask_eu;
> +
> +		fuse = I915_READ(CHV_FUSE_GT);
> +		mask_ss = fuse & (CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 |
> +				  CHV_FUSE_GT_SUBSLICE_DISABLE_SS1);
> +		mask_eu = fuse & (CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK |
> +				  CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK |
> +				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK |
> +				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK);
> +		sseu_info.slice_cnt = 1;
> +		sseu_info.subslice_cnt = 2 - hweight32(mask_ss);
> +		sseu_info.eu_cnt = 16 - hweight32(mask_eu);
> +		sseu_info.threads_per_eu = 7;
> +	}
> +
> +	/* Pack SSEU info bitfield for I915_PARAM_SSEU_INFO */
> +	gp |= (sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) &
> +	       I915_SSEU_INFO_SLICE_CNT_MASK;
> 
> In theory, there should be no need to use the mask, unless something has gone wrong.
> Maybe:
> 
> WARN_ON((sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) > I915_SSEU_INFO_SLICE_CNT_MASK );
> etc
> 
> 

I agree. Will make similar change in next version.
-Jeff

> +	gp |= (sseu_info.subslice_cnt << I915_SSEU_INFO_SUBSLICE_CNT_SHIFT) &
> +	       I915_SSEU_INFO_SUBSLICE_CNT_MASK;
> +	gp |= (sseu_info.eu_cnt << I915_SSEU_INFO_EU_CNT_SHIFT) &
> +	       I915_SSEU_INFO_EU_CNT_MASK;
> +	gp |= (sseu_info.threads_per_eu << I915_SSEU_INFO_THREADS_PER_EU_SHIFT) &
> +	       I915_SSEU_INFO_THREADS_PER_EU_MASK;
> +	sseu_info.gp_sseu_info = gp;
> +
> +	/* Copy SSEU info to the const device info with pointer magic */
> +	*(struct intel_sseu_info *)&dev_priv->info.sseu = sseu_info; }
> diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
> index 7db7175..0257358 100644
> --- a/drivers/gpu/drm/i915/intel_sseu.h
> +++ b/drivers/gpu/drm/i915/intel_sseu.h
> @@ -37,4 +37,6 @@ struct intel_sseu_info {
>  	u32 gp_sseu_info;
>  };
>  
> +extern void intel_sseu_init(struct drm_device *dev);
> +
>  #endif
> --
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
  2014-07-31 11:55   ` Mcaulay, Alistair
  2014-07-31 23:18     ` Jeff McGee
@ 2014-08-04  8:18     ` Daniel Vetter
  1 sibling, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-04  8:18 UTC (permalink / raw)
  To: Mcaulay, Alistair; +Cc: intel-gfx

On Thu, Jul 31, 2014 at 11:55:58AM +0000, Mcaulay, Alistair wrote:
> Hi Jeff,
> 
> Some more comments in the code.

Can you please fix your mailer to properly quote? Just doing in-line
comments makes it really hard to find them.
-Daniel

> 
> Thanks,
> Alistair.
> 
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of jeff.mcgee@intel.com
> Sent: Thursday, July 31, 2014 3:00 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
> 
> From: Jeff McGee <jeff.mcgee@intel.com>
> 
> Cherryview can have different SSEU configurations within a given PCI ID, so we collect the info from the fuse register.
> 
> I don't currently have access to a CHV, much less one with an interesting fuse config. So I have compile-checked this only!
> 
> Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile     |  3 +-
>  drivers/gpu/drm/i915/i915_dma.c   |  2 ++
>  drivers/gpu/drm/i915/i915_reg.h   | 13 ++++++++
>  drivers/gpu/drm/i915/intel_sseu.c | 64 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_sseu.h |  2 ++
>  5 files changed, 83 insertions(+), 1 deletion(-)  create mode 100644 drivers/gpu/drm/i915/intel_sseu.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91bd167..9a0f411 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \
>  	  i915_irq.o \
>  	  i915_trace_points.o \
>  	  intel_ringbuffer.o \
> -	  intel_uncore.o
> +	  intel_uncore.o \
> +	  intel_sseu.o
>  
>  # autogenerated null render state
>  i915-y += intel_renderstate_gen6.o \
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f581848..384ef65 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  			goto out_gem_unload;
>  	}
>  
> +	intel_sseu_init(dev);
> +
>  	intel_power_domains_init(dev_priv);
>  
>  	if (drm_core_check_feature(dev, DRIVER_MODESET)) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 28e21ed..24a2d56 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5624,6 +5624,19 @@ enum punit_power_well {
>  #define GEN7_MISCCPCTL			(0x9424)
>  #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
>  
> +/* Fuse readout registers for GT */
> +#define CHV_FUSE_GT					0x182168
> +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0		10
> +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1		11
> 
> These should be:
> #define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0		(1 << 10)
> #define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1		(1 << 11)
> 
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK		(0xf<<16)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT		16
> 
> Why not use the shift define here?
> #define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK		(0xf<<CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT)
> and for the others.
> 
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK		(0xf<<20)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT		20
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK		(0xf<<24)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT		24
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK		(0xf<<28)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT		28
> +
>  /* IVYBRIDGE DPF */
>  #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
>  #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
> diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c
> new file mode 100644
> index 0000000..6ba4830
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_sseu.c
> @@ -0,0 +1,64 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the 
> +next
> + * paragraph) shall be included in all copies or substantial portions 
> +of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> +OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
> +ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> +DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +#include <linux/bitops.h>
> +#include "i915_drv.h"
> +#include "intel_sseu.h"
> +
> +void intel_sseu_init(struct drm_device *dev) {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_sseu_info sseu_info;
> +	u32 gp = 0;
> +
> +	/* Collect SSEU info per device */
> +	if (IS_CHERRYVIEW(dev)) {
> +		u32 fuse, mask_ss, mask_eu;
> +
> +		fuse = I915_READ(CHV_FUSE_GT);
> +		mask_ss = fuse & (CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 |
> +				  CHV_FUSE_GT_SUBSLICE_DISABLE_SS1);
> +		mask_eu = fuse & (CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK |
> +				  CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK |
> +				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK |
> +				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK);
> +		sseu_info.slice_cnt = 1;
> +		sseu_info.subslice_cnt = 2 - hweight32(mask_ss);
> +		sseu_info.eu_cnt = 16 - hweight32(mask_eu);
> +		sseu_info.threads_per_eu = 7;
> +	}
> +
> +	/* Pack SSEU info bitfield for I915_PARAM_SSEU_INFO */
> +	gp |= (sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) &
> +	       I915_SSEU_INFO_SLICE_CNT_MASK;
> 
> In theory, there should be no need to use the mask, unless something has gone wrong.
> Maybe:
> 
> WARN_ON((sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) > I915_SSEU_INFO_SLICE_CNT_MASK );
> etc
> 
> 
> +	gp |= (sseu_info.subslice_cnt << I915_SSEU_INFO_SUBSLICE_CNT_SHIFT) &
> +	       I915_SSEU_INFO_SUBSLICE_CNT_MASK;
> +	gp |= (sseu_info.eu_cnt << I915_SSEU_INFO_EU_CNT_SHIFT) &
> +	       I915_SSEU_INFO_EU_CNT_MASK;
> +	gp |= (sseu_info.threads_per_eu << I915_SSEU_INFO_THREADS_PER_EU_SHIFT) &
> +	       I915_SSEU_INFO_THREADS_PER_EU_MASK;
> +	sseu_info.gp_sseu_info = gp;
> +
> +	/* Copy SSEU info to the const device info with pointer magic */
> +	*(struct intel_sseu_info *)&dev_priv->info.sseu = sseu_info; }
> diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
> index 7db7175..0257358 100644
> --- a/drivers/gpu/drm/i915/intel_sseu.h
> +++ b/drivers/gpu/drm/i915/intel_sseu.h
> @@ -37,4 +37,6 @@ struct intel_sseu_info {
>  	u32 gp_sseu_info;
>  };
>  
> +extern void intel_sseu_init(struct drm_device *dev);
> +
>  #endif
> --
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
  2014-07-31  1:59 [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM jeff.mcgee
  2014-07-31  1:59 ` [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV jeff.mcgee
  2014-07-31 11:53 ` [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM Mcaulay, Alistair
@ 2014-08-04  8:20 ` Daniel Vetter
  2014-08-05 14:03   ` Jeff McGee
  2 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2014-08-04  8:20 UTC (permalink / raw)
  To: jeff.mcgee; +Cc: intel-gfx

On Wed, Jul 30, 2014 at 08:59:46PM -0500, jeff.mcgee@intel.com wrote:
> From: Jeff McGee <jeff.mcgee@intel.com>
> 
> Define a struct to capture information on the device's Slice/Subslice/EU
> (SSEU) configuration. Add this struct to the main device info struct.
> Define a packed bitfield form for the SSEU info and share it with
> userspace via a new GETPARAM option.
> 
> Starting with Cherryview, devices may have a varying number of EU for
> a given ID due to creative fusing. The surest way to determine the
> configuration is by reading fuses which is best done in the kernel and
> communicated to userspace. The immediate need from userspace is to
> determine the number of threads of compute work that can be safely
> submitted.
> 
> The definition of SSEU as a new drm/i915 component, with its own header
> file and soon-to-be source file, is in anticipation of lots of upcoming
> code for its management, particularly the power gating functionality.
> 
> Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c   |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h   |  3 +++
>  drivers/gpu/drm/i915/intel_sseu.h | 40 +++++++++++++++++++++++++++++++++++++++
>  include/uapi/drm/i915_drm.h       | 18 ++++++++++++++++++
>  4 files changed, 64 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/intel_sseu.h
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 2e7f03a..f581848 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1025,6 +1025,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_CMD_PARSER_VERSION:
>  		value = i915_cmd_parser_get_version();
>  		break;
> +	case I915_PARAM_SSEU_INFO:
> +		value = INTEL_INFO(dev)->sseu.gp_sseu_info;
> +		break;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 18c9ad8..01adafd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -45,6 +45,7 @@
>  #include <linux/intel-iommu.h>
>  #include <linux/kref.h>
>  #include <linux/pm_qos.h>
> +#include "intel_sseu.h"
>  
>  /* General customization:
>   */
> @@ -562,6 +563,8 @@ struct intel_device_info {
>  	int trans_offsets[I915_MAX_TRANSCODERS];
>  	int palette_offsets[I915_MAX_PIPES];
>  	int cursor_offsets[I915_MAX_PIPES];
> +	/* Slice/Subslice/EU info */
> +	struct intel_sseu_info sseu;
>  };
>  
>  #undef DEFINE_FLAG
> diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
> new file mode 100644
> index 0000000..7db7175
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_sseu.h
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +#ifndef _INTEL_SSEU_H_
> +#define _INTEL_SSEU_H_
> +
> +struct intel_sseu_info {
> +	/* Total slice count */
> +	unsigned int slice_cnt;
> +	/* Total subslice count */
> +	unsigned int subslice_cnt;
> +	/* Total execution unit count */
> +	unsigned int eu_cnt;
> +	/* Thread count per EU */
> +	unsigned int threads_per_eu;
> +	/* Bit field representation for I915_PARAM_SSEU_INFO */
> +	u32 gp_sseu_info;
> +};
> +
> +#endif
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index ff57f07..b99c1a2 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -171,6 +171,23 @@ typedef struct _drm_i915_sarea {
>  #define I915_BOX_TEXTURE_LOAD  0x8
>  #define I915_BOX_LOST_CONTEXT  0x10
>  
> +/*
> + * Slice/Subslice/EU Info
> + * - Accessed via GETPARAM ioctl option I915_PARAM_SSEU_INFO
> + * - SLICE_CNT: total slice count
> + * - SUBSLICE_CNT: total subslice count
> + * - EU_CNT: total execution unit count
> + * - THREADS_PER_EU: thread count per EU
> +*/
> +#define I915_SSEU_INFO_SLICE_CNT_MASK		0xf
> +#define I915_SSEU_INFO_SLICE_CNT_SHIFT		0
> +#define I915_SSEU_INFO_SUBSLICE_CNT_MASK	(0x3f<<4)
> +#define I915_SSEU_INFO_SUBSLICE_CNT_SHIFT	4
> +#define I915_SSEU_INFO_EU_CNT_MASK		(0xff<<10)
> +#define I915_SSEU_INFO_EU_CNT_SHIFT		10
> +#define I915_SSEU_INFO_THREADS_PER_EU_MASK	(0xf<<18)
> +#define I915_SSEU_INFO_THREADS_PER_EU_SHIFT	18

Tbh this looks a bit too tricky, I'd just allocate a pile of getparm
numbers, one for each - they're cheap.

Also, usual broken record request: I need open-source userspace using
this (mesa, ddx, libva).
-Daniel

> +
>  /* I915 specific ioctls
>   * The device specific ioctl range is 0x40 to 0x79.
>   */
> @@ -340,6 +357,7 @@ typedef struct drm_i915_irq_wait {
>  #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
>  #define I915_PARAM_HAS_WT     	 	 27
>  #define I915_PARAM_CMD_PARSER_VERSION	 28
> +#define I915_PARAM_SSEU_INFO		 29
>  
>  typedef struct drm_i915_getparam {
>  	int param;
> -- 
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
  2014-07-31  1:59 ` [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV jeff.mcgee
  2014-07-31 11:55   ` Mcaulay, Alistair
@ 2014-08-04  8:22   ` Daniel Vetter
  2014-08-05 13:47     ` Jeff McGee
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2014-08-04  8:22 UTC (permalink / raw)
  To: jeff.mcgee; +Cc: intel-gfx

On Wed, Jul 30, 2014 at 08:59:47PM -0500, jeff.mcgee@intel.com wrote:
> From: Jeff McGee <jeff.mcgee@intel.com>
> 
> Cherryview can have different SSEU configurations within a given PCI
> ID, so we collect the info from the fuse register.
> 
> I don't currently have access to a CHV, much less one with an interesting
> fuse config. So I have compile-checked this only!
> 
> Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile     |  3 +-
>  drivers/gpu/drm/i915/i915_dma.c   |  2 ++
>  drivers/gpu/drm/i915/i915_reg.h   | 13 ++++++++
>  drivers/gpu/drm/i915/intel_sseu.c | 64 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_sseu.h |  2 ++
>  5 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_sseu.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 91bd167..9a0f411 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \
>  	  i915_irq.o \
>  	  i915_trace_points.o \
>  	  intel_ringbuffer.o \
> -	  intel_uncore.o
> +	  intel_uncore.o \
> +	  intel_sseu.o
>  
>  # autogenerated null render state
>  i915-y += intel_renderstate_gen6.o \
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index f581848..384ef65 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  			goto out_gem_unload;
>  	}
>  
> +	intel_sseu_init(dev);
> +
>  	intel_power_domains_init(dev_priv);
>  
>  	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 28e21ed..24a2d56 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5624,6 +5624,19 @@ enum punit_power_well {
>  #define GEN7_MISCCPCTL			(0x9424)
>  #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
>  
> +/* Fuse readout registers for GT */
> +#define CHV_FUSE_GT					0x182168
> +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0		10
> +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1		11
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK		(0xf<<16)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT		16
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK		(0xf<<20)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT		20
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK		(0xf<<24)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT		24
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK		(0xf<<28)
> +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT		28
> +
>  /* IVYBRIDGE DPF */
>  #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
>  #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
> diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c
> new file mode 100644
> index 0000000..6ba4830
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_sseu.c
> @@ -0,0 +1,64 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +#include <linux/bitops.h>
> +#include "i915_drv.h"
> +#include "intel_sseu.h"
> +
> +void intel_sseu_init(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_sseu_info sseu_info;
> +	u32 gp = 0;
> +
> +	/* Collect SSEU info per device */
> +	if (IS_CHERRYVIEW(dev)) {
> +		u32 fuse, mask_ss, mask_eu;
> +
> +		fuse = I915_READ(CHV_FUSE_GT);
> +		mask_ss = fuse & (CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 |
> +				  CHV_FUSE_GT_SUBSLICE_DISABLE_SS1);
> +		mask_eu = fuse & (CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK |
> +				  CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK |
> +				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK |
> +				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK);
> +		sseu_info.slice_cnt = 1;
> +		sseu_info.subslice_cnt = 2 - hweight32(mask_ss);
> +		sseu_info.eu_cnt = 16 - hweight32(mask_eu);
> +		sseu_info.threads_per_eu = 7;
> +	}
> +
> +	/* Pack SSEU info bitfield for I915_PARAM_SSEU_INFO */
> +	gp |= (sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) &
> +	       I915_SSEU_INFO_SLICE_CNT_MASK;
> +	gp |= (sseu_info.subslice_cnt << I915_SSEU_INFO_SUBSLICE_CNT_SHIFT) &
> +	       I915_SSEU_INFO_SUBSLICE_CNT_MASK;
> +	gp |= (sseu_info.eu_cnt << I915_SSEU_INFO_EU_CNT_SHIFT) &
> +	       I915_SSEU_INFO_EU_CNT_MASK;
> +	gp |= (sseu_info.threads_per_eu << I915_SSEU_INFO_THREADS_PER_EU_SHIFT) &
> +	       I915_SSEU_INFO_THREADS_PER_EU_MASK;
> +	sseu_info.gp_sseu_info = gp;

This leaves the question how userspace can figure out whether we actually
support this. You need to return -EINVAL or something for all platforms
where this is not yet implemented.

> +
> +	/* Copy SSEU info to the const device info with pointer magic */
> +	*(struct intel_sseu_info *)&dev_priv->info.sseu = sseu_info;

I've thought gcc just does a memcpy for plain strut assignments, i.e.

+	dev_priv->info.sseu = sseu_info;

Cheers, Daniel

> +}
> diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
> index 7db7175..0257358 100644
> --- a/drivers/gpu/drm/i915/intel_sseu.h
> +++ b/drivers/gpu/drm/i915/intel_sseu.h
> @@ -37,4 +37,6 @@ struct intel_sseu_info {
>  	u32 gp_sseu_info;
>  };
>  
> +extern void intel_sseu_init(struct drm_device *dev);
> +
>  #endif
> -- 
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
  2014-08-05 13:47     ` Jeff McGee
@ 2014-08-05 13:41       ` Damien Lespiau
  2014-08-06 14:40         ` Jeff McGee
  0 siblings, 1 reply; 16+ messages in thread
From: Damien Lespiau @ 2014-08-05 13:41 UTC (permalink / raw)
  To: Daniel Vetter, intel-gfx

On Tue, Aug 05, 2014 at 08:47:54AM -0500, Jeff McGee wrote:
> > > +
> > > +	/* Copy SSEU info to the const device info with pointer magic */
> > > +	*(struct intel_sseu_info *)&dev_priv->info.sseu = sseu_info;
> > 
> > I've thought gcc just does a memcpy for plain strut assignments, i.e.
> > 
> > +	dev_priv->info.sseu = sseu_info;

That's what we have intel_device_info_runtime_init(), the SSEU init
function should be called from there.

-- 
Damien

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
  2014-08-04  8:22   ` Daniel Vetter
@ 2014-08-05 13:47     ` Jeff McGee
  2014-08-05 13:41       ` Damien Lespiau
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff McGee @ 2014-08-05 13:47 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Mon, Aug 04, 2014 at 10:22:55AM +0200, Daniel Vetter wrote:
> On Wed, Jul 30, 2014 at 08:59:47PM -0500, jeff.mcgee@intel.com wrote:
> > From: Jeff McGee <jeff.mcgee@intel.com>
> > 
> > Cherryview can have different SSEU configurations within a given PCI
> > ID, so we collect the info from the fuse register.
> > 
> > I don't currently have access to a CHV, much less one with an interesting
> > fuse config. So I have compile-checked this only!
> > 
> > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile     |  3 +-
> >  drivers/gpu/drm/i915/i915_dma.c   |  2 ++
> >  drivers/gpu/drm/i915/i915_reg.h   | 13 ++++++++
> >  drivers/gpu/drm/i915/intel_sseu.c | 64 +++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_sseu.h |  2 ++
> >  5 files changed, 83 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/i915/intel_sseu.c
> > 
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 91bd167..9a0f411 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \
> >  	  i915_irq.o \
> >  	  i915_trace_points.o \
> >  	  intel_ringbuffer.o \
> > -	  intel_uncore.o
> > +	  intel_uncore.o \
> > +	  intel_sseu.o
> >  
> >  # autogenerated null render state
> >  i915-y += intel_renderstate_gen6.o \
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index f581848..384ef65 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> >  			goto out_gem_unload;
> >  	}
> >  
> > +	intel_sseu_init(dev);
> > +
> >  	intel_power_domains_init(dev_priv);
> >  
> >  	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 28e21ed..24a2d56 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5624,6 +5624,19 @@ enum punit_power_well {
> >  #define GEN7_MISCCPCTL			(0x9424)
> >  #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
> >  
> > +/* Fuse readout registers for GT */
> > +#define CHV_FUSE_GT					0x182168
> > +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0		10
> > +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1		11
> > +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK		(0xf<<16)
> > +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT		16
> > +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK		(0xf<<20)
> > +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT		20
> > +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK		(0xf<<24)
> > +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT		24
> > +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK		(0xf<<28)
> > +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT		28
> > +
> >  /* IVYBRIDGE DPF */
> >  #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
> >  #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
> > diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c
> > new file mode 100644
> > index 0000000..6ba4830
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/intel_sseu.c
> > @@ -0,0 +1,64 @@
> > +/*
> > + * Copyright © 2014 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + */
> > +#include <linux/bitops.h>
> > +#include "i915_drv.h"
> > +#include "intel_sseu.h"
> > +
> > +void intel_sseu_init(struct drm_device *dev)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct intel_sseu_info sseu_info;
> > +	u32 gp = 0;
> > +
> > +	/* Collect SSEU info per device */
> > +	if (IS_CHERRYVIEW(dev)) {
> > +		u32 fuse, mask_ss, mask_eu;
> > +
> > +		fuse = I915_READ(CHV_FUSE_GT);
> > +		mask_ss = fuse & (CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 |
> > +				  CHV_FUSE_GT_SUBSLICE_DISABLE_SS1);
> > +		mask_eu = fuse & (CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK |
> > +				  CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK |
> > +				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK |
> > +				  CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK);
> > +		sseu_info.slice_cnt = 1;
> > +		sseu_info.subslice_cnt = 2 - hweight32(mask_ss);
> > +		sseu_info.eu_cnt = 16 - hweight32(mask_eu);
> > +		sseu_info.threads_per_eu = 7;
> > +	}
> > +
> > +	/* Pack SSEU info bitfield for I915_PARAM_SSEU_INFO */
> > +	gp |= (sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) &
> > +	       I915_SSEU_INFO_SLICE_CNT_MASK;
> > +	gp |= (sseu_info.subslice_cnt << I915_SSEU_INFO_SUBSLICE_CNT_SHIFT) &
> > +	       I915_SSEU_INFO_SUBSLICE_CNT_MASK;
> > +	gp |= (sseu_info.eu_cnt << I915_SSEU_INFO_EU_CNT_SHIFT) &
> > +	       I915_SSEU_INFO_EU_CNT_MASK;
> > +	gp |= (sseu_info.threads_per_eu << I915_SSEU_INFO_THREADS_PER_EU_SHIFT) &
> > +	       I915_SSEU_INFO_THREADS_PER_EU_MASK;
> > +	sseu_info.gp_sseu_info = gp;
> 
> This leaves the question how userspace can figure out whether we actually
> support this. You need to return -EINVAL or something for all platforms
> where this is not yet implemented.
> 

Agree. I'll add that.
-Jeff

> > +
> > +	/* Copy SSEU info to the const device info with pointer magic */
> > +	*(struct intel_sseu_info *)&dev_priv->info.sseu = sseu_info;
> 
> I've thought gcc just does a memcpy for plain strut assignments, i.e.
> 
> +	dev_priv->info.sseu = sseu_info;
> 
> Cheers, Daniel
> 

This is necessary due to the const-ness of dev_priv->info. The
straight-forward copy either fails to compile or issues a nasty
warning (can't remember which right now). This same method is used in
i915_driver_load to initialize dev_priv->info.
-Jeff

> > +}
> > diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
> > index 7db7175..0257358 100644
> > --- a/drivers/gpu/drm/i915/intel_sseu.h
> > +++ b/drivers/gpu/drm/i915/intel_sseu.h
> > @@ -37,4 +37,6 @@ struct intel_sseu_info {
> >  	u32 gp_sseu_info;
> >  };
> >  
> > +extern void intel_sseu_init(struct drm_device *dev);
> > +
> >  #endif
> > -- 
> > 2.0.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
  2014-08-04  8:20 ` Daniel Vetter
@ 2014-08-05 14:03   ` Jeff McGee
  2014-08-05 14:50     ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff McGee @ 2014-08-05 14:03 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Mon, Aug 04, 2014 at 10:20:37AM +0200, Daniel Vetter wrote:
> On Wed, Jul 30, 2014 at 08:59:46PM -0500, jeff.mcgee@intel.com wrote:
> > From: Jeff McGee <jeff.mcgee@intel.com>
> > 
> > Define a struct to capture information on the device's Slice/Subslice/EU
> > (SSEU) configuration. Add this struct to the main device info struct.
> > Define a packed bitfield form for the SSEU info and share it with
> > userspace via a new GETPARAM option.
> > 
> > Starting with Cherryview, devices may have a varying number of EU for
> > a given ID due to creative fusing. The surest way to determine the
> > configuration is by reading fuses which is best done in the kernel and
> > communicated to userspace. The immediate need from userspace is to
> > determine the number of threads of compute work that can be safely
> > submitted.
> > 
> > The definition of SSEU as a new drm/i915 component, with its own header
> > file and soon-to-be source file, is in anticipation of lots of upcoming
> > code for its management, particularly the power gating functionality.
> > 
> > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c   |  3 +++
> >  drivers/gpu/drm/i915/i915_drv.h   |  3 +++
> >  drivers/gpu/drm/i915/intel_sseu.h | 40 +++++++++++++++++++++++++++++++++++++++
> >  include/uapi/drm/i915_drm.h       | 18 ++++++++++++++++++
> >  4 files changed, 64 insertions(+)
> >  create mode 100644 drivers/gpu/drm/i915/intel_sseu.h
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 2e7f03a..f581848 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1025,6 +1025,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
> >  	case I915_PARAM_CMD_PARSER_VERSION:
> >  		value = i915_cmd_parser_get_version();
> >  		break;
> > +	case I915_PARAM_SSEU_INFO:
> > +		value = INTEL_INFO(dev)->sseu.gp_sseu_info;
> > +		break;
> >  	default:
> >  		DRM_DEBUG("Unknown parameter %d\n", param->param);
> >  		return -EINVAL;
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 18c9ad8..01adafd 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -45,6 +45,7 @@
> >  #include <linux/intel-iommu.h>
> >  #include <linux/kref.h>
> >  #include <linux/pm_qos.h>
> > +#include "intel_sseu.h"
> >  
> >  /* General customization:
> >   */
> > @@ -562,6 +563,8 @@ struct intel_device_info {
> >  	int trans_offsets[I915_MAX_TRANSCODERS];
> >  	int palette_offsets[I915_MAX_PIPES];
> >  	int cursor_offsets[I915_MAX_PIPES];
> > +	/* Slice/Subslice/EU info */
> > +	struct intel_sseu_info sseu;
> >  };
> >  
> >  #undef DEFINE_FLAG
> > diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
> > new file mode 100644
> > index 0000000..7db7175
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/intel_sseu.h
> > @@ -0,0 +1,40 @@
> > +/*
> > + * Copyright © 2014 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + */
> > +#ifndef _INTEL_SSEU_H_
> > +#define _INTEL_SSEU_H_
> > +
> > +struct intel_sseu_info {
> > +	/* Total slice count */
> > +	unsigned int slice_cnt;
> > +	/* Total subslice count */
> > +	unsigned int subslice_cnt;
> > +	/* Total execution unit count */
> > +	unsigned int eu_cnt;
> > +	/* Thread count per EU */
> > +	unsigned int threads_per_eu;
> > +	/* Bit field representation for I915_PARAM_SSEU_INFO */
> > +	u32 gp_sseu_info;
> > +};
> > +
> > +#endif
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index ff57f07..b99c1a2 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -171,6 +171,23 @@ typedef struct _drm_i915_sarea {
> >  #define I915_BOX_TEXTURE_LOAD  0x8
> >  #define I915_BOX_LOST_CONTEXT  0x10
> >  
> > +/*
> > + * Slice/Subslice/EU Info
> > + * - Accessed via GETPARAM ioctl option I915_PARAM_SSEU_INFO
> > + * - SLICE_CNT: total slice count
> > + * - SUBSLICE_CNT: total subslice count
> > + * - EU_CNT: total execution unit count
> > + * - THREADS_PER_EU: thread count per EU
> > +*/
> > +#define I915_SSEU_INFO_SLICE_CNT_MASK		0xf
> > +#define I915_SSEU_INFO_SLICE_CNT_SHIFT		0
> > +#define I915_SSEU_INFO_SUBSLICE_CNT_MASK	(0x3f<<4)
> > +#define I915_SSEU_INFO_SUBSLICE_CNT_SHIFT	4
> > +#define I915_SSEU_INFO_EU_CNT_MASK		(0xff<<10)
> > +#define I915_SSEU_INFO_EU_CNT_SHIFT		10
> > +#define I915_SSEU_INFO_THREADS_PER_EU_MASK	(0xf<<18)
> > +#define I915_SSEU_INFO_THREADS_PER_EU_SHIFT	18
> 
> Tbh this looks a bit too tricky, I'd just allocate a pile of getparm
> numbers, one for each - they're cheap.
> 

I can do that. Wasn't sure what our general preference was concerning
this issue. Packing them seems efficient but I suppose separating them
is simpler and more future-proof. And if I separate, I will just supply
the value that is needed for the immediate concern (total EU to determine
thread limits) and add other values later on need, instead of carving out
space for them now.
-Jeff

> Also, usual broken record request: I need open-source userspace using
> this (mesa, ddx, libva).
> -Daniel
> 

This is kind of chicken-and-egg problem that I haven't been through. I
assume that we build new interfaces up the stack (kernel->libdrm->usermode).
Any tips or docs on how to proceed?
-Jeff
> > +
> >  /* I915 specific ioctls
> >   * The device specific ioctl range is 0x40 to 0x79.
> >   */
> > @@ -340,6 +357,7 @@ typedef struct drm_i915_irq_wait {
> >  #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
> >  #define I915_PARAM_HAS_WT     	 	 27
> >  #define I915_PARAM_CMD_PARSER_VERSION	 28
> > +#define I915_PARAM_SSEU_INFO		 29
> >  
> >  typedef struct drm_i915_getparam {
> >  	int param;
> > -- 
> > 2.0.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
  2014-08-05 14:03   ` Jeff McGee
@ 2014-08-05 14:50     ` Daniel Vetter
  2014-08-06 14:43       ` Jeff McGee
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2014-08-05 14:50 UTC (permalink / raw)
  To: Daniel Vetter, intel-gfx

On Tue, Aug 5, 2014 at 4:03 PM, Jeff McGee <jeff.mcgee@intel.com> wrote:
>> Also, usual broken record request: I need open-source userspace using
>> this (mesa, ddx, libva).
>> -Daniel
>>
>
> This is kind of chicken-and-egg problem that I haven't been through. I
> assume that we build new interfaces up the stack (kernel->libdrm->usermode).
> Any tips or docs on how to proceed?

You need to have the full thing ready with kernel, igt, libdrm, and
umd driver patches (and maybe even testcases for the new umd feature,
e.g. for some of the arb robustness stuff we've done). Then you post
them all to relevant mailing lists and go through the review process
for all parts. Once that's done and _all_ parts are reviewed, merging
starts with kernel+igt, then libdrm + libdrm release, then umd.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
  2014-08-05 13:41       ` Damien Lespiau
@ 2014-08-06 14:40         ` Jeff McGee
  0 siblings, 0 replies; 16+ messages in thread
From: Jeff McGee @ 2014-08-06 14:40 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On Tue, Aug 05, 2014 at 02:41:54PM +0100, Damien Lespiau wrote:
> On Tue, Aug 05, 2014 at 08:47:54AM -0500, Jeff McGee wrote:
> > > > +
> > > > +	/* Copy SSEU info to the const device info with pointer magic */
> > > > +	*(struct intel_sseu_info *)&dev_priv->info.sseu = sseu_info;
> > > 
> > > I've thought gcc just does a memcpy for plain strut assignments, i.e.
> > > 
> > > +	dev_priv->info.sseu = sseu_info;
> 
> That's what we have intel_device_info_runtime_init(), the SSEU init
> function should be called from there.
> 
> -- 
> Damien

Agree. I will move the call to intel_device_info_runtime_init().
-Jeff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
  2014-08-05 14:50     ` Daniel Vetter
@ 2014-08-06 14:43       ` Jeff McGee
  2014-08-06 15:02         ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff McGee @ 2014-08-06 14:43 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, Aug 05, 2014 at 04:50:16PM +0200, Daniel Vetter wrote:
> On Tue, Aug 5, 2014 at 4:03 PM, Jeff McGee <jeff.mcgee@intel.com> wrote:
> >> Also, usual broken record request: I need open-source userspace using
> >> this (mesa, ddx, libva).
> >> -Daniel
> >>
> >
> > This is kind of chicken-and-egg problem that I haven't been through. I
> > assume that we build new interfaces up the stack (kernel->libdrm->usermode).
> > Any tips or docs on how to proceed?
> 
> You need to have the full thing ready with kernel, igt, libdrm, and
> umd driver patches (and maybe even testcases for the new umd feature,
> e.g. for some of the arb robustness stuff we've done). Then you post
> them all to relevant mailing lists and go through the review process
> for all parts. Once that's done and _all_ parts are reviewed, merging
> starts with kernel+igt, then libdrm + libdrm release, then umd.
> -Daniel

Thanks Daniel. Is it acceptable to get the kernel part merged with only
an igt test (technically an open-source userspace component)? I think
probably not, but have to ask.
-Jeff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
  2014-08-06 14:43       ` Jeff McGee
@ 2014-08-06 15:02         ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06 15:02 UTC (permalink / raw)
  To: Daniel Vetter, intel-gfx

On Wed, Aug 06, 2014 at 09:43:44AM -0500, Jeff McGee wrote:
> On Tue, Aug 05, 2014 at 04:50:16PM +0200, Daniel Vetter wrote:
> > On Tue, Aug 5, 2014 at 4:03 PM, Jeff McGee <jeff.mcgee@intel.com> wrote:
> > >> Also, usual broken record request: I need open-source userspace using
> > >> this (mesa, ddx, libva).
> > >> -Daniel
> > >>
> > >
> > > This is kind of chicken-and-egg problem that I haven't been through. I
> > > assume that we build new interfaces up the stack (kernel->libdrm->usermode).
> > > Any tips or docs on how to proceed?
> > 
> > You need to have the full thing ready with kernel, igt, libdrm, and
> > umd driver patches (and maybe even testcases for the new umd feature,
> > e.g. for some of the arb robustness stuff we've done). Then you post
> > them all to relevant mailing lists and go through the review process
> > for all parts. Once that's done and _all_ parts are reviewed, merging
> > starts with kernel+igt, then libdrm + libdrm release, then umd.
> > -Daniel
> 
> Thanks Daniel. Is it acceptable to get the kernel part merged with only
> an igt test (technically an open-source userspace component)? I think
> probably not, but have to ask.

Not really. The entire idea behind this is that only once we have the full
stack picture available we can properly assess whether the interface
fullfills the goal. That requires an actual implementation of the feature
for the entire stack.

And Dave Airlie (as the drm upstream maintainer) requires that that entire
pile is open-source.

So we really don't have wiggle room here. And personally I don't want to
open the opportunity for discussions for each specific feature on whether
a bare-bones demonstration test in igt is good enough or not, it really
should be something shippable to users.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-08-06 15:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-31  1:59 [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM jeff.mcgee
2014-07-31  1:59 ` [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV jeff.mcgee
2014-07-31 11:55   ` Mcaulay, Alistair
2014-07-31 23:18     ` Jeff McGee
2014-08-04  8:18     ` Daniel Vetter
2014-08-04  8:22   ` Daniel Vetter
2014-08-05 13:47     ` Jeff McGee
2014-08-05 13:41       ` Damien Lespiau
2014-08-06 14:40         ` Jeff McGee
2014-07-31 11:53 ` [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM Mcaulay, Alistair
2014-07-31 23:16   ` Jeff McGee
2014-08-04  8:20 ` Daniel Vetter
2014-08-05 14:03   ` Jeff McGee
2014-08-05 14:50     ` Daniel Vetter
2014-08-06 14:43       ` Jeff McGee
2014-08-06 15:02         ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.