Re: [PATCH v9 1/7] drm/i915/guc: Move GuC WOPCM related code into separate files

["Michal Wajdeczko" <michal.wajdeczko@xxxxxxxxx>]

On Fri, 09 Feb 2018 00:03:49 +0100, Jackie Li <yaodong.li@xxxxxxxxx> wrote:

> intel_guc_reg.h should only include definition for GuC registers and
> related register bits. Non-register related GuC WOPCM macro definitions
> should not be defined in intel_guc_reg.h.
>
> This patch cleans up intel_guc_reg.h and moves GuC WOPCM related code  
> into
> new created separate files as future patches will increase the complexity
> of determining the GuC WOPCM offset and size.
>
> v8:
>  - Fixed naming, coding style issues and typo in commit message (Sagar)
>  - Updated commit message to explain why we need create new file for GuC
>    WOPCM related code (Chris)
>
> v9:
>  - Corrected kernel-doc format (Sagar)
>  - Simplified commit message (Michal)
>  - Updated license of new created files to use SPDX (Michal)
>  - Updated macros and added more comments to be reader friendly (Michal)
>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> (v8)
> Signed-off-by: Jackie Li <yaodong.li@xxxxxxxxx>
> ---
>  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   |  4 ---
>  drivers/gpu/drm/i915/intel_guc_wopcm.c | 48  
> ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_guc_wopcm.h | 45  
> +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc.c        |  2 +-
>  drivers/gpu/drm/i915/intel_uc_fw.c     |  2 +-
>  8 files changed, 97 insertions(+), 18 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_guc_wopcm.c
>  create mode 100644 drivers/gpu/drm/i915/intel_guc_wopcm.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile  
> b/drivers/gpu/drm/i915/Makefile
> index f55cc02..418d96b 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -89,6 +89,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 21140cc..9f45e6d 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -509,14 +509,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 52856a9..9e0a97e 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"
> @@ -130,6 +131,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..1f52fb8 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -68,7 +68,6 @@
>  #define DMA_GUC_WOPCM_OFFSET		_MMIO(0xc340)
>  #define   HUC_LOADING_AGENT_VCR		  (0<<1)
>  #define   HUC_LOADING_AGENT_GUC		  (1<<1)
> -#define   GUC_WOPCM_OFFSET_VALUE	  0x80000	/* 512KB */
>  #define GUC_MAX_IDLE_COUNT		_MMIO(0xC3E4)
> #define HUC_STATUS2             _MMIO(0xD3B0)
> @@ -76,9 +75,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..c3d54f8
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> @@ -0,0 +1,48 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright © 2017-2018 Intel Corporation
> + *

Do we really want to keep legacy license text?
Note that in other file (intel_lrc_reg.h) we have only SPDX tag.

> + * 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 size = GUC_WOPCM_TOP;
> +
> +	/* On BXT, the top of WOPCM is reserved for RC6 context */
> +	if (IS_GEN9_LP(i915))
> +		size -= BXT_GUC_WOPCM_RC6_RESERVED;
> +
> +	return 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..7323604
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.h
> @@ -0,0 +1,45 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright © 2017-2018 Intel Corporation
> + *

Same here.

> + * 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;
> +
> +/* 512KB static offset from WOPCM base. */
> +#define GUC_WOPCM_OFFSET_VALUE		(512 << 10)
> +/*
> + * 512KB static GuC WOPCM size from GUC_WOPCM_OFFSET_VALUE to the end  
> of GuC
> + * WOPCM. GuC addresses below GUC_WOPCM_TOP don't map through the GTT.
> + */
> +#define GUC_WOPCM_TOP			(512 << 10)
> +#define BXT_GUC_WOPCM_RC6_RESERVED	(64 << 10)
> +
> +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 9f1bac6..1b2831b 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -340,7 +340,7 @@ int intel_uc_init_hw(struct drm_i915_private  
> *dev_priv)
>  	gen9_reset_guc_interrupts(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;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx