diff mbox

[6/7] drm/i915/guc: Move defines with size of GuC logs to intel_guc_log.h

Message ID 20180530135334.25113-6-piotr.piorkowski@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Piotr Piórkowski May 30, 2018, 1:53 p.m. UTC 
At this moment, we have defined GuC logs sizes in intel_guc_fwif.h, but
as these values are related directly to the GuC logs, and not to API of
GuC parameters, we should move these defines to intel_guc_log.h.

Signed-off-by: Piotr Piórkowski <piotr.piorkowski@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_guc.c      | 28 +++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_guc_fwif.h | 20 +++-----------------
 drivers/gpu/drm/i915/intel_guc_log.c  | 28 +++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_guc_log.h  |  9 +++------
 4 files changed, 54 insertions(+), 31 deletions(-)

Comments

Michal Wajdeczko May 30, 2018, 4:18 p.m. UTC | #1 
On Wed, 30 May 2018 15:53:33 +0200, Piotr Piorkowski  
<piotr.piorkowski@intel.com> wrote:

> At this moment, we have defined GuC logs sizes in intel_guc_fwif.h, but
> as these values are related directly to the GuC logs, and not to API of
> GuC parameters, we should move these defines to intel_guc_log.h.
>
> Signed-off-by: Piotr Piórkowski <piotr.piorkowski@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_guc.c      | 28 +++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_guc_fwif.h | 20 +++-----------------
>  drivers/gpu/drm/i915/intel_guc_log.c  | 28 +++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_guc_log.h  |  9 +++------
>  4 files changed, 54 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c  
> b/drivers/gpu/drm/i915/intel_guc.c
> index 3b45f06b1aa2..e15047fedb45 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -263,13 +263,31 @@ static u32 guc_ctl_log_params_flags(struct  
> intel_guc *guc)
>  	u32 offset = intel_guc_ggtt_offset(guc, guc->log.vma) >> PAGE_SHIFT;
>  	u32 flags;
> -	/* each allocated unit is a page */
> -	flags = GUC_LOG_VALID | GUC_LOG_NOTIFY_ON_HALF_FULL |
> -		(GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT) |
> -		(GUC_LOG_DPC_PAGES << GUC_LOG_DPC_SHIFT) |
> -		(GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) |
> +	#define UNIT (4 << 10)
> +
> +	BUILD_BUG_ON(!CRASH_BUFFER_SIZE);
> +	BUILD_BUG_ON(!IS_ALIGNED(CRASH_BUFFER_SIZE, UNIT));
> +	BUILD_BUG_ON(!DPC_BUFFER_SIZE);
> +	BUILD_BUG_ON(!IS_ALIGNED(DPC_BUFFER_SIZE, UNIT));
> +	BUILD_BUG_ON(!ISR_BUFFER_SIZE);
> +	BUILD_BUG_ON(!IS_ALIGNED(ISR_BUFFER_SIZE, UNIT));
> +
> +	BUILD_BUG_ON((CRASH_BUFFER_SIZE/UNIT - 1) >
> +			(GUC_LOG_CRASH_MASK >> GUC_LOG_CRASH_SHIFT));
> +	BUILD_BUG_ON((DPC_BUFFER_SIZE/UNIT - 1) >
> +			(GUC_LOG_DPC_MASK >> GUC_LOG_DPC_SHIFT));
> +	BUILD_BUG_ON((ISR_BUFFER_SIZE/UNIT - 1) >
> +			(GUC_LOG_ISR_MASK >> GUC_LOG_ISR_SHIFT));
> +
> +	flags = GUC_LOG_VALID |
> +		GUC_LOG_NOTIFY_ON_HALF_FULL |
> +		((CRASH_BUFFER_SIZE/UNIT - 1) << GUC_LOG_CRASH_SHIFT) |
> +		((DPC_BUFFER_SIZE/UNIT - 1) << GUC_LOG_DPC_SHIFT) |
> +		((ISR_BUFFER_SIZE/UNIT - 1) << GUC_LOG_ISR_SHIFT) |
>  		(offset << GUC_LOG_BUF_ADDR_SHIFT);
> +	#undef UNIT
> +
>  	return flags;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h  
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 0867ba76d445..1a0f2a39cef9 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -84,12 +84,12 @@
>  #define   GUC_LOG_VALID			(1 << 0)
>  #define   GUC_LOG_NOTIFY_ON_HALF_FULL	(1 << 1)
>  #define   GUC_LOG_ALLOC_IN_MEGABYTE	(1 << 3)
> -#define   GUC_LOG_CRASH_PAGES		1
>  #define   GUC_LOG_CRASH_SHIFT		4
> -#define   GUC_LOG_DPC_PAGES		7
> +#define   GUC_LOG_CRASH_MASK		(0x1 << GUC_LOG_CRASH_SHIFT)
>  #define   GUC_LOG_DPC_SHIFT		6
> -#define   GUC_LOG_ISR_PAGES		7
> +#define   GUC_LOG_DPC_MASK	        (0x7 << GUC_LOG_DPC_SHIFT)
>  #define   GUC_LOG_ISR_SHIFT		9
> +#define   GUC_LOG_ISR_MASK	        (0x7 << GUC_LOG_ISR_SHIFT)
>  #define   GUC_LOG_BUF_ADDR_SHIFT	12
> #define GUC_CTL_PAGE_FAULT_CONTROL	5
> @@ -532,20 +532,6 @@ enum guc_log_buffer_type {
>  };
> /**
> - * DOC: GuC Log buffer Layout
> - *
> - * Page0  +-------------------------------+
> - *        |   ISR state header (32 bytes) |
> - *        |      DPC state header         |
> - *        |   Crash dump state header     |
> - * Page1  +-------------------------------+
> - *        |           ISR logs            |
> - * Page9  +-------------------------------+
> - *        |           DPC logs            |
> - * Page17 +-------------------------------+
> - *        |         Crash Dump logs       |
> - *        +-------------------------------+
> - *
>   * Below state structure is used for coordination of retrieval of GuC  
> firmware
>   * logs. Separate state is maintained for each log buffer type.
>   * read_ptr points to the location where i915 read last in log buffer  
> and
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
> b/drivers/gpu/drm/i915/intel_guc_log.c
> index a751025b6ad7..1c4a8de9c305 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -27,6 +27,28 @@
>  #include "intel_guc_log.h"
>  #include "i915_drv.h"
> +/*
> + *  GuC Log buffer Layout
> + *
> + *  +===============================+ 00B
> + *  |    Crash dump state header    |
> + *  +-------------------------------+ 32B
> + *  |       DPC state header        |
> + *  +-------------------------------+ 64B
> + *  |       ISR state header        |
> + *  +-------------------------------+ 96B
> + *  |                               |
> + *  +===============================+ PAGE_SIZE (4KB)
> + *  |        Crash Dump logs        |
> + *  +===============================+ + CRASH_SIZE
> + *  |           DPC logs            |
> + *  +===============================+ + DPC_SIZE
> + *  |           ISR logs            |
> + *  +===============================+ + ISR_SIZE
> + */
> +#define GUC_LOG_SIZE	(PAGE_SIZE + CRASH_BUFFER_SIZE + DPC_BUFFER_SIZE +  
> \
> +			ISR_BUFFER_SIZE)

maybe we can drop this define completely and:
- sum sizes explicitly in intel_guc_log_create()
- use log->vma.size in guc_log_relay_create()

> +
>  static void guc_log_capture_logs(struct intel_guc_log *log);
> /**
> @@ -215,11 +237,11 @@ static unsigned int guc_get_log_buffer_size(enum  
> guc_log_buffer_type type)
>  {
>  	switch (type) {
>  	case GUC_ISR_LOG_BUFFER:
> -		return (GUC_LOG_ISR_PAGES + 1) * PAGE_SIZE;
> +		return ISR_BUFFER_SIZE;
>  	case GUC_DPC_LOG_BUFFER:
> -		return (GUC_LOG_DPC_PAGES + 1) * PAGE_SIZE;
> +		return DPC_BUFFER_SIZE;
>  	case GUC_CRASH_DUMP_LOG_BUFFER:
> -		return (GUC_LOG_CRASH_PAGES + 1) * PAGE_SIZE;
> +		return CRASH_BUFFER_SIZE;
>  	default:
>  		MISSING_CASE(type);
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.h  
> b/drivers/gpu/drm/i915/intel_guc_log.h
> index 995aeb29a3b9..1b3afdae6d0d 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.h
> +++ b/drivers/gpu/drm/i915/intel_guc_log.h
> @@ -34,12 +34,9 @@
> struct intel_guc;
> -/*
> - * The first page is to save log buffer state. Allocate one
> - * extra page for others in case for overlap
> - */
> -#define GUC_LOG_SIZE	((1 + GUC_LOG_DPC_PAGES + 1 + GUC_LOG_ISR_PAGES + \
> -			  1 + GUC_LOG_CRASH_PAGES + 1) << PAGE_SHIFT)
> +#define CRASH_BUFFER_SIZE	8192
> +#define DPC_BUFFER_SIZE		32768
> +#define ISR_BUFFER_SIZE		32768

bikeshed: maybe something more friendly like (32 * 1024) ?

> /*
>   * While we're using plain log level in i915, GuC controls are much  
> more...

with GUC_LOG_SIZE removed,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 3b45f06b1aa2..e15047fedb45 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -263,13 +263,31 @@  static u32 guc_ctl_log_params_flags(struct intel_guc *guc)
 	u32 offset = intel_guc_ggtt_offset(guc, guc->log.vma) >> PAGE_SHIFT;
 	u32 flags;
 
-	/* each allocated unit is a page */
-	flags = GUC_LOG_VALID | GUC_LOG_NOTIFY_ON_HALF_FULL |
-		(GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT) |
-		(GUC_LOG_DPC_PAGES << GUC_LOG_DPC_SHIFT) |
-		(GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) |
+	#define UNIT (4 << 10)
+
+	BUILD_BUG_ON(!CRASH_BUFFER_SIZE);
+	BUILD_BUG_ON(!IS_ALIGNED(CRASH_BUFFER_SIZE, UNIT));
+	BUILD_BUG_ON(!DPC_BUFFER_SIZE);
+	BUILD_BUG_ON(!IS_ALIGNED(DPC_BUFFER_SIZE, UNIT));
+	BUILD_BUG_ON(!ISR_BUFFER_SIZE);
+	BUILD_BUG_ON(!IS_ALIGNED(ISR_BUFFER_SIZE, UNIT));
+
+	BUILD_BUG_ON((CRASH_BUFFER_SIZE/UNIT - 1) >
+			(GUC_LOG_CRASH_MASK >> GUC_LOG_CRASH_SHIFT));
+	BUILD_BUG_ON((DPC_BUFFER_SIZE/UNIT - 1) >
+			(GUC_LOG_DPC_MASK >> GUC_LOG_DPC_SHIFT));
+	BUILD_BUG_ON((ISR_BUFFER_SIZE/UNIT - 1) >
+			(GUC_LOG_ISR_MASK >> GUC_LOG_ISR_SHIFT));
+
+	flags = GUC_LOG_VALID |
+		GUC_LOG_NOTIFY_ON_HALF_FULL |
+		((CRASH_BUFFER_SIZE/UNIT - 1) << GUC_LOG_CRASH_SHIFT) |
+		((DPC_BUFFER_SIZE/UNIT - 1) << GUC_LOG_DPC_SHIFT) |
+		((ISR_BUFFER_SIZE/UNIT - 1) << GUC_LOG_ISR_SHIFT) |
 		(offset << GUC_LOG_BUF_ADDR_SHIFT);
 
+	#undef UNIT
+
 	return flags;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 0867ba76d445..1a0f2a39cef9 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -84,12 +84,12 @@ 
 #define   GUC_LOG_VALID			(1 << 0)
 #define   GUC_LOG_NOTIFY_ON_HALF_FULL	(1 << 1)
 #define   GUC_LOG_ALLOC_IN_MEGABYTE	(1 << 3)
-#define   GUC_LOG_CRASH_PAGES		1
 #define   GUC_LOG_CRASH_SHIFT		4
-#define   GUC_LOG_DPC_PAGES		7
+#define   GUC_LOG_CRASH_MASK		(0x1 << GUC_LOG_CRASH_SHIFT)
 #define   GUC_LOG_DPC_SHIFT		6
-#define   GUC_LOG_ISR_PAGES		7
+#define   GUC_LOG_DPC_MASK	        (0x7 << GUC_LOG_DPC_SHIFT)
 #define   GUC_LOG_ISR_SHIFT		9
+#define   GUC_LOG_ISR_MASK	        (0x7 << GUC_LOG_ISR_SHIFT)
 #define   GUC_LOG_BUF_ADDR_SHIFT	12
 
 #define GUC_CTL_PAGE_FAULT_CONTROL	5
@@ -532,20 +532,6 @@  enum guc_log_buffer_type {
 };
 
 /**
- * DOC: GuC Log buffer Layout
- *
- * Page0  +-------------------------------+
- *        |   ISR state header (32 bytes) |
- *        |      DPC state header         |
- *        |   Crash dump state header     |
- * Page1  +-------------------------------+
- *        |           ISR logs            |
- * Page9  +-------------------------------+
- *        |           DPC logs            |
- * Page17 +-------------------------------+
- *        |         Crash Dump logs       |
- *        +-------------------------------+
- *
  * Below state structure is used for coordination of retrieval of GuC firmware
  * logs. Separate state is maintained for each log buffer type.
  * read_ptr points to the location where i915 read last in log buffer and
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index a751025b6ad7..1c4a8de9c305 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -27,6 +27,28 @@ 
 #include "intel_guc_log.h"
 #include "i915_drv.h"
 
+/*
+ *  GuC Log buffer Layout
+ *
+ *  +===============================+ 00B
+ *  |    Crash dump state header    |
+ *  +-------------------------------+ 32B
+ *  |       DPC state header        |
+ *  +-------------------------------+ 64B
+ *  |       ISR state header        |
+ *  +-------------------------------+ 96B
+ *  |                               |
+ *  +===============================+ PAGE_SIZE (4KB)
+ *  |        Crash Dump logs        |
+ *  +===============================+ + CRASH_SIZE
+ *  |           DPC logs            |
+ *  +===============================+ + DPC_SIZE
+ *  |           ISR logs            |
+ *  +===============================+ + ISR_SIZE
+ */
+#define GUC_LOG_SIZE	(PAGE_SIZE + CRASH_BUFFER_SIZE + DPC_BUFFER_SIZE + \
+			ISR_BUFFER_SIZE)
+
 static void guc_log_capture_logs(struct intel_guc_log *log);
 
 /**
@@ -215,11 +237,11 @@  static unsigned int guc_get_log_buffer_size(enum guc_log_buffer_type type)
 {
 	switch (type) {
 	case GUC_ISR_LOG_BUFFER:
-		return (GUC_LOG_ISR_PAGES + 1) * PAGE_SIZE;
+		return ISR_BUFFER_SIZE;
 	case GUC_DPC_LOG_BUFFER:
-		return (GUC_LOG_DPC_PAGES + 1) * PAGE_SIZE;
+		return DPC_BUFFER_SIZE;
 	case GUC_CRASH_DUMP_LOG_BUFFER:
-		return (GUC_LOG_CRASH_PAGES + 1) * PAGE_SIZE;
+		return CRASH_BUFFER_SIZE;
 	default:
 		MISSING_CASE(type);
 	}
diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h
index 995aeb29a3b9..1b3afdae6d0d 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.h
+++ b/drivers/gpu/drm/i915/intel_guc_log.h
@@ -34,12 +34,9 @@ 
 
 struct intel_guc;
 
-/*
- * The first page is to save log buffer state. Allocate one
- * extra page for others in case for overlap
- */
-#define GUC_LOG_SIZE	((1 + GUC_LOG_DPC_PAGES + 1 + GUC_LOG_ISR_PAGES + \
-			  1 + GUC_LOG_CRASH_PAGES + 1) << PAGE_SHIFT)
+#define CRASH_BUFFER_SIZE	8192
+#define DPC_BUFFER_SIZE		32768
+#define ISR_BUFFER_SIZE		32768
 
 /*
  * While we're using plain log level in i915, GuC controls are much more...