diff mbox

[v4,1/5] drm/i915/guc: Move GuC WOPCM related code into separate files

Message ID 1513119394-28951-1-git-send-email-yaodong.li@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jackie Li Dec. 12, 2017, 10:56 p.m. UTC
intel_guc_reg.h should only include definition for GuC registers
and related register bits. GuC WOPCM related values should not
be defined in intel_guc_reg.h

This patch creates a better file structure by moving GuC WOPCM
related definitions int to a new header intel_guc_wopcm.h
and moving GuC WOPCM related functions to a new source file
intel_guc_wopcm.c

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
 drivers/gpu/drm/i915/Makefile          |  1 +
 drivers/gpu/drm/i915/intel_guc.c       | 11 --------
 drivers/gpu/drm/i915/intel_guc.h       |  2 +-
 drivers/gpu/drm/i915/intel_guc_reg.h   |  3 ---
 drivers/gpu/drm/i915/intel_guc_wopcm.c | 47 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc_wopcm.h | 38 +++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.c        |  2 +-
 drivers/gpu/drm/i915/intel_uc_fw.c     |  2 +-
 8 files changed, 89 insertions(+), 17 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_guc_wopcm.c
 create mode 100644 drivers/gpu/drm/i915/intel_guc_wopcm.h

Comments

Joonas Lahtinen Dec. 13, 2017, 8:19 a.m. UTC | #1
On Tue, 2017-12-12 at 14:56 -0800, Jackie Li wrote:
> intel_guc_reg.h should only include definition for GuC registers
> and related register bits. GuC WOPCM related values should not
> be defined in intel_guc_reg.h
> 
> This patch creates a better file structure by moving GuC WOPCM
> related definitions int to a new header intel_guc_wopcm.h
> and moving GuC WOPCM related functions to a new source file
> intel_guc_wopcm.c
> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Jackie Li <yaodong.li@intel.com>

<SNIP>

> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -218,7 +218,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>  	}
>  
>  	/* init WOPCM */
> -	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
> +	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(guc));

This is a write-once register, the code needs to be refactored to
account that somebody (like an ugly BIOS) wrote it already and we have
to live with that value. Otherwise we're digging a hole for future
selves.

We should also verify that the write sticks as we expect.

Regards, Joonas
Jackie Li Dec. 13, 2017, 6:29 p.m. UTC | #2
On 12/13/2017 12:19 AM, Joonas Lahtinen wrote:
> On Tue, 2017-12-12 at 14:56 -0800, Jackie Li wrote:
>> intel_guc_reg.h should only include definition for GuC registers
>> and related register bits. GuC WOPCM related values should not
>> be defined in intel_guc_reg.h
>>
>> This patch creates a better file structure by moving GuC WOPCM
>> related definitions int to a new header intel_guc_wopcm.h
>> and moving GuC WOPCM related functions to a new source file
>> intel_guc_wopcm.c
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Signed-off-by: Jackie Li <yaodong.li@intel.com>
> <SNIP>
>
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -218,7 +218,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>>   	}
>>   
>>   	/* init WOPCM */
>> -	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
>> +	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(guc));
> This is a write-once register, the code needs to be refactored to
> account that somebody (like an ugly BIOS) wrote it already and we have
> to live with that value. Otherwise we're digging a hole for future
> selves.
It's the way the current code works. I will work out a new patch to
do the code refactoring. Anyhow, this is a good catch! Thanks!
>
> We should also verify that the write sticks as we expect.
For the verification - the following dynamic partitioning patch will
guarantee the correctness of this size value. As for the successful
register updating, I will address it within the new patch code refactoring
patch.
>
> Regards, Joonas
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 091aef2..1588b2e 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -87,6 +87,7 @@  i915-y += intel_uc.o \
 	  intel_guc_fw.o \
 	  intel_guc_log.o \
 	  intel_guc_submission.o \
+	  intel_guc_wopcm.o \
 	  intel_huc.o
 
 # autogenerated null render state
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 177ee69..76519a7 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -370,14 +370,3 @@  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
 	i915_gem_object_put(obj);
 	return vma;
 }
-
-u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
-{
-	u32 wopcm_size = GUC_WOPCM_TOP;
-
-	/* On BXT, the top of WOPCM is reserved for RC6 context */
-	if (IS_GEN9_LP(dev_priv))
-		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
-
-	return wopcm_size;
-}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 5985672..bfdc887 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -31,6 +31,7 @@ 
 #include "intel_guc_ct.h"
 #include "intel_guc_log.h"
 #include "intel_guc_reg.h"
+#include "intel_guc_wopcm.h"
 #include "intel_uc_fw.h"
 #include "i915_vma.h"
 
@@ -126,6 +127,5 @@  int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 int intel_guc_suspend(struct drm_i915_private *dev_priv);
 int intel_guc_resume(struct drm_i915_private *dev_priv);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
index 19a9247..144cd74 100644
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -76,9 +76,6 @@ 
 
 /* Defines WOPCM space available to GuC firmware */
 #define GUC_WOPCM_SIZE			_MMIO(0xc050)
-/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
-#define   GUC_WOPCM_TOP			  (0x80 << 12)	/* 512KB */
-#define   BXT_GUC_WOPCM_RC6_RESERVED	  (0x10 << 12)	/* 64KB  */
 
 /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
 #define GUC_GGTT_TOP			0xFEE00000
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
new file mode 100644
index 0000000..87643a0
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
@@ -0,0 +1,47 @@ 
+/*
+ * Copyright © 2017 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 "intel_guc_wopcm.h"
+#include "i915_drv.h"
+
+/*
+ * intel_guc_wopcm_size() - Get the size of GuC WOPCM.
+ * @guc: intel guc.
+ *
+ * Get the platform specific GuC WOPCM size.
+ *
+ * Return: size of the GuC WOPCM.
+ */
+u32 intel_guc_wopcm_size(struct intel_guc *guc)
+{
+	struct drm_i915_private *i915 = guc_to_i915(guc);
+
+	u32 wopcm_size = GUC_WOPCM_TOP;
+
+	/* On BXT, the top of WOPCM is reserved for RC6 context */
+	if (IS_GEN9_LP(i915))
+		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
+
+	return wopcm_size;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.h b/drivers/gpu/drm/i915/intel_guc_wopcm.h
new file mode 100644
index 0000000..04d61c8
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.h
@@ -0,0 +1,38 @@ 
+/*
+ * Copyright © 2017 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_GUC_WOPCM_H_
+#define _INTEL_GUC_WOPCM_H_
+
+#include <linux/types.h>
+
+struct intel_guc;
+
+/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
+#define GUC_WOPCM_TOP			  (0x80 << 12)	/* 512KB */
+#define BXT_GUC_WOPCM_RC6_RESERVED	  (0x10 << 12)	/* 64KB  */
+
+u32 intel_guc_wopcm_size(struct intel_guc *guc);
+
+#endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 461047c..44deb80 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -218,7 +218,7 @@  int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	}
 
 	/* init WOPCM */
-	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
+	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(guc));
 	I915_WRITE(DMA_GUC_WOPCM_OFFSET,
 		   GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC);
 
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 784eff9..24945cf 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -97,7 +97,7 @@  void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 
 	/* Header and uCode will be loaded to WOPCM */
 	size = uc_fw->header_size + uc_fw->ucode_size;
-	if (size > intel_guc_wopcm_size(dev_priv)) {
+	if (size > intel_guc_wopcm_size(&dev_priv->guc)) {
 		DRM_WARN("%s: Firmware is too large to fit in WOPCM\n",
 			 intel_uc_fw_type_repr(uc_fw->type));
 		err = -E2BIG;