[Intel-gfx] [PATCH 6/7] drm/i915/guc: Move defines with size of GuC logs to intel_guc_log.h
Michal Wajdeczko
michal.wajdeczko at intel.com
Wed May 30 16:18:29 UTC 2018
On Wed, 30 May 2018 15:53:33 +0200, Piotr Piorkowski
<piotr.piorkowski at 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 at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at 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 at intel.com>
More information about the Intel-gfx
mailing list