[Intel-gfx] [PATCH v2 7/7] drm/i915/guc: Add support for define guc_log_size in megabytes.

[Michal Wajdeczko]

On Mon, 04 Jun 2018 16:19:47 +0200, Piotr Piorkowski  
<piotr.piorkowski at intel.com> wrote:

> At this moment we can define GuC logs sizes only using pages.
> But GuC also allows use for this values expressed in megabytes.
> Lets add support for define guc_log_size in megabytes when we
> debug of GuC.
>
> v2:
> - change buffers size to more friendly (Michał Wajdeczko)
> - merge statements in guc_ctl_log_params_flags() (Michał Wajdeczko)
>
> 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     | 8 ++++++++
>  drivers/gpu/drm/i915/intel_guc_log.h | 6 ++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c  
> b/drivers/gpu/drm/i915/intel_guc.c
> index 68b94c23f26b..27a23f7ee4f8 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -263,7 +263,13 @@ 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;
> +	#if (((CRASH_BUFFER_SIZE) % (1 << 20)) == 0)
> +	#define UNIT (1 << 20)

btw, there are some nice predefined macros that can be used here
instead of magic shifts ;)

	#if (CRASH_BUFFER_SIZE % SZ_1M) == 0
	#define UNIT SZ_1M
...
	#define UNIT SZ_4K

> +	#define FLAG GUC_LOG_ALLOC_IN_MEGABYTE
> +	#else
>  	#define UNIT (4 << 10)
> +	#define FLAG 0
> +	#endif
> 	BUILD_BUG_ON(!CRASH_BUFFER_SIZE);
>  	BUILD_BUG_ON(!IS_ALIGNED(CRASH_BUFFER_SIZE, UNIT));
> @@ -281,12 +287,14 @@ static u32 guc_ctl_log_params_flags(struct  
> intel_guc *guc)
> 	flags = GUC_LOG_VALID |
>  		GUC_LOG_NOTIFY_ON_HALF_FULL |
> +		FLAG |
>  		((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
> +	#undef FLAG

I'm afraid that someone may view FLAG/UNIT as too generic,
maybe better to wait for other review comments..

> 	return flags;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.h  
> b/drivers/gpu/drm/i915/intel_guc_log.h
> index 4ebb19f87b54..4feaeba1be1e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.h
> +++ b/drivers/gpu/drm/i915/intel_guc_log.h
> @@ -34,9 +34,15 @@
> struct intel_guc;
> +#ifdef DRM_I915_DEBUG_GUC

you probably wanted to use CONFIG_DRM_I915_DEBUG_GUC here

> +#define CRASH_BUFFER_SIZE	(2 * 1024 * 1024)
> +#define DPC_BUFFER_SIZE		(8 * 1024 * 1024)
> +#define ISR_BUFFER_SIZE		(8 * 1024 * 1024)

other macros are also available:

SZ_2M
SZ_8M

> +#else
>  #define CRASH_BUFFER_SIZE	(8 * 1024)
>  #define DPC_BUFFER_SIZE		(32 * 1024)
>  #define ISR_BUFFER_SIZE		(32 * 1024)

SZ_8K
SZ_32K

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

with #ifdef CONFIG fixed, and
preferably with SZ_xx macros, this is

Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com>