[Intel-gfx] [PATCH v2 1/2] drm/i915/uc: Move uC related types into dedicated header

Sagar Arun Kamble sagar.a.kamble at intel.com
Wed Sep 27 14:51:15 UTC 2017



On 9/27/2017 4:52 PM, Michal Wajdeczko wrote:
> In old header structure we were mixing type definitions and
> declarations that prevent us from exposing some functions
> as inline. Lets try to fix that.
>
> v2: keep old order of the structs, pull fwif.h (Sagar)
>
> Suggested-by: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Acked-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c   |   1 +
>   drivers/gpu/drm/i915/i915_drv.h       |   2 +-
>   drivers/gpu/drm/i915/i915_gem.c       |   1 +
>   drivers/gpu/drm/i915/intel_guc_ct.c   |   2 +-
>   drivers/gpu/drm/i915/intel_guc_ct.h   |   3 -
>   drivers/gpu/drm/i915/intel_guc_log.c  |   1 +
>   drivers/gpu/drm/i915/intel_uc.h       | 178 +++-------------------------------
>   drivers/gpu/drm/i915/intel_uc_types.h | 171 ++++++++++++++++++++++++++++++++
>   8 files changed, 192 insertions(+), 167 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/intel_uc_types.h
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index b4a6ac6..8f85add 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -30,6 +30,7 @@
>   #include <linux/sort.h>
>   #include <linux/sched/mm.h>
>   #include "intel_drv.h"
> +#include "intel_uc.h"
>   
>   static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
>   {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b7cba89..521af91 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -58,7 +58,6 @@
>   #include "intel_uncore.h"
>   #include "intel_bios.h"
>   #include "intel_dpll_mgr.h"
> -#include "intel_uc.h"
>   #include "intel_lrc.h"
>   #include "intel_ringbuffer.h"
>   
> @@ -73,6 +72,7 @@
>   
>   #include "i915_vma.h"
>   
> +#include "intel_uc_types.h"
>   #include "intel_gvt.h"
>   
>   /* General customization:
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 73eeb6b..fa70b7c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -35,6 +35,7 @@
>   #include "intel_drv.h"
>   #include "intel_frontbuffer.h"
>   #include "intel_mocs.h"
> +#include "intel_uc.h"
>   #include <linux/dma-fence-array.h>
>   #include <linux/kthread.h>
>   #include <linux/reservation.h>
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index c4cbec1..722e671 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -22,7 +22,7 @@
>    */
>   
>   #include "i915_drv.h"
> -#include "intel_guc_ct.h"
> +#include "intel_uc.h"
>   
>   enum { CTB_SEND = 0, CTB_RECV = 1 };
>   
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
> index 6d97f36..49c8d94 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.h
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.h
> @@ -25,9 +25,6 @@
>   #define _INTEL_GUC_CT_H_
>   
>   struct intel_guc;
> -struct i915_vma;
> -
> -#include "intel_guc_fwif.h"
>   
>   /**
>    * DOC: Command Transport (CT).
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index 6571d96..024e3b1 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -24,6 +24,7 @@
>   #include <linux/debugfs.h>
>   #include <linux/relay.h>
>   #include "i915_drv.h"
> +#include "intel_uc.h"
>   
>   static void guc_log_capture_logs(struct intel_guc *guc);
>   
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 6966349..296c52d 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -24,62 +24,8 @@
>   #ifndef _INTEL_UC_H_
>   #define _INTEL_UC_H_
>   
> -#include "intel_guc_fwif.h"
> -#include "i915_guc_reg.h"
> -#include "intel_ringbuffer.h"
> -#include "intel_guc_ct.h"
> -#include "i915_vma.h"
> +#include "intel_uc_types.h"
>   
> -struct drm_i915_gem_request;
> -
> -/*
> - * This structure primarily describes the GEM object shared with the GuC.
> - * The specs sometimes refer to this object as a "GuC context", but we use
> - * the term "client" to avoid confusion with hardware contexts. This
> - * GEM object is held for the entire lifetime of our interaction with
> - * the GuC, being allocated before the GuC is loaded with its firmware.
> - * Because there's no way to update the address used by the GuC after
> - * initialisation, the shared object must stay pinned into the GGTT as
> - * long as the GuC is in use. We also keep the first page (only) mapped
> - * into kernel address space, as it includes shared data that must be
> - * updated on every request submission.
> - *
> - * The single GEM object described here is actually made up of several
> - * separate areas, as far as the GuC is concerned. The first page (kept
> - * kmap'd) includes the "process descriptor" which holds sequence data for
> - * the doorbell, and one cacheline which actually *is* the doorbell; a
> - * write to this will "ring the doorbell" (i.e. send an interrupt to the
> - * GuC). The subsequent  pages of the client object constitute the work
> - * queue (a circular array of work items), again described in the process
> - * descriptor. Work queue pages are mapped momentarily as required.
> - */
> -struct i915_guc_client {
> -	struct i915_vma *vma;
> -	void *vaddr;
> -	struct i915_gem_context *owner;
> -	struct intel_guc *guc;
> -
> -	uint32_t engines;		/* bitmap of (host) engine ids	*/
> -	uint32_t priority;
> -	u32 stage_id;
> -	uint32_t proc_desc_offset;
> -
> -	u16 doorbell_id;
> -	unsigned long doorbell_offset;
> -
> -	spinlock_t wq_lock;
> -	/* Per-engine counts of GuC submissions */
> -	uint64_t submissions[I915_NUM_ENGINES];
> -};
> -
> -enum intel_uc_fw_status {
> -	INTEL_UC_FIRMWARE_FAIL = -1,
> -	INTEL_UC_FIRMWARE_NONE = 0,
> -	INTEL_UC_FIRMWARE_PENDING,
> -	INTEL_UC_FIRMWARE_SUCCESS
> -};
> -
> -/* User-friendly representation of an enum */
>   static inline
>   const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
>   {
> @@ -96,12 +42,6 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
>   	return "<invalid>";
>   }
>   
> -enum intel_uc_fw_type {
> -	INTEL_UC_FW_TYPE_GUC,
> -	INTEL_UC_FW_TYPE_HUC
> -};
> -
> -/* User-friendly representation of an enum */
>   static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
>   {
>   	switch (type) {
> @@ -113,93 +53,25 @@ static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
>   	return "uC";
>   }
>   
> -/*
> - * This structure encapsulates all the data needed during the process
> - * of fetching, caching, and loading the firmware image into the GuC.
> - */
> -struct intel_uc_fw {
> -	const char *path;
> -	size_t size;
> -	struct drm_i915_gem_object *obj;
> -	enum intel_uc_fw_status fetch_status;
> -	enum intel_uc_fw_status load_status;
> -
> -	uint16_t major_ver_wanted;
> -	uint16_t minor_ver_wanted;
> -	uint16_t major_ver_found;
> -	uint16_t minor_ver_found;
> -
> -	enum intel_uc_fw_type type;
> -	uint32_t header_size;
> -	uint32_t header_offset;
> -	uint32_t rsa_size;
> -	uint32_t rsa_offset;
> -	uint32_t ucode_size;
> -	uint32_t ucode_offset;
> -};
> -
> -struct intel_guc_log {
> -	uint32_t flags;
> -	struct i915_vma *vma;
> -	/* The runtime stuff gets created only when GuC logging gets enabled */
> -	struct {
> -		void *buf_addr;
> -		struct workqueue_struct *flush_wq;
> -		struct work_struct flush_work;
> -		struct rchan *relay_chan;
> -	} runtime;
> -	/* logging related stats */
> -	u32 capture_miss_count;
> -	u32 flush_interrupt_count;
> -	u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
> -	u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
> -	u32 flush_count[GUC_MAX_LOG_BUFFER];
> -};
> -
> -struct intel_guc {
> -	struct intel_uc_fw fw;
> -	struct intel_guc_log log;
> -	struct intel_guc_ct ct;
> -
> -	/* Log snapshot if GuC errors during load */
> -	struct drm_i915_gem_object *load_err_log;
> -
> -	/* intel_guc_recv interrupt related state */
> -	bool interrupts_enabled;
> -
> -	struct i915_vma *ads_vma;
> -	struct i915_vma *stage_desc_pool;
> -	void *stage_desc_pool_vaddr;
> -	struct ida stage_ids;
> -
> -	struct i915_guc_client *execbuf_client;
> -
> -	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
> -	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
> -
> -	/* GuC's FW specific registers used in MMIO send */
> -	struct {
> -		u32 base;
> -		unsigned int count;
> -		enum forcewake_domains fw_domains;
> -	} send_regs;
> -
> -	/* To serialize the intel_guc_send actions */
> -	struct mutex send_mutex;
> +static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
> +{
> +	return guc->send(guc, action, len);
> +}
>   
> -	/* GuC's FW specific send function */
> -	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
> +static inline void intel_guc_notify(struct intel_guc *guc)
> +{
> +	guc->notify(guc);
> +}
>   
> -	/* GuC's FW specific notify function */
> -	void (*notify)(struct intel_guc *guc);
> -};
> +static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> +{
> +	u32 offset = i915_ggtt_offset(vma);
>   
> -struct intel_huc {
> -	/* Generic uC firmware management */
> -	struct intel_uc_fw fw;
> +	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
> +	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
>   
> -	/* HuC-specific additions */
> -};
> +	return offset;
> +}
>   
>   /* intel_uc.c */
>   void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
> @@ -213,16 +85,6 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
>   int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
>   int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
>   
> -static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
> -{
> -	return guc->send(guc, action, len);
> -}
> -
> -static inline void intel_guc_notify(struct intel_guc *guc)
> -{
> -	guc->notify(guc);
> -}
> -
>   /* intel_guc_loader.c */
>   int intel_guc_select_fw(struct intel_guc *guc);
>   int intel_guc_init_hw(struct intel_guc *guc);
> @@ -244,14 +106,6 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
>   void i915_guc_log_register(struct drm_i915_private *dev_priv);
>   void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
>   
> -static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> -{
> -	u32 offset = i915_ggtt_offset(vma);
> -	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
> -	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
> -	return offset;
> -}
> -
>   /* intel_huc.c */
>   void intel_huc_select_fw(struct intel_huc *huc);
>   void intel_huc_init_hw(struct intel_huc *huc);
> diff --git a/drivers/gpu/drm/i915/intel_uc_types.h b/drivers/gpu/drm/i915/intel_uc_types.h
> new file mode 100644
> index 0000000..8f8bf4e
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_uc_types.h
> @@ -0,0 +1,171 @@
> +/*
> + * Copyright © 2014-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_UC_TYPES_H_
> +#define _INTEL_UC_TYPES_H_
> +
> +#include "intel_guc_fwif.h"
> +#include "intel_guc_ct.h"
> +#include "i915_guc_reg.h"
> +
> +enum intel_uc_fw_status {
> +	INTEL_UC_FIRMWARE_FAIL = -1,
> +	INTEL_UC_FIRMWARE_NONE = 0,
> +	INTEL_UC_FIRMWARE_PENDING,
> +	INTEL_UC_FIRMWARE_SUCCESS
> +};
> +
> +enum intel_uc_fw_type {
> +	INTEL_UC_FW_TYPE_GUC,
> +	INTEL_UC_FW_TYPE_HUC
> +};
> +
> +/*
> + * This structure encapsulates all the data needed during the process
> + * of fetching, caching, and loading the firmware image into the GuC.
> + */
> +struct intel_uc_fw {
> +	const char *path;
> +	size_t size;
> +	struct drm_i915_gem_object *obj;
> +	enum intel_uc_fw_status fetch_status;
> +	enum intel_uc_fw_status load_status;
> +
> +	uint16_t major_ver_wanted;
> +	uint16_t minor_ver_wanted;
> +	uint16_t major_ver_found;
> +	uint16_t minor_ver_found;
> +
> +	enum intel_uc_fw_type type;
> +	uint32_t header_size;
> +	uint32_t header_offset;
> +	uint32_t rsa_size;
> +	uint32_t rsa_offset;
> +	uint32_t ucode_size;
> +	uint32_t ucode_offset;
> +};
> +
> +struct intel_guc_log {
> +	uint32_t flags;
> +	struct i915_vma *vma;
> +	/* The runtime stuff gets created only when GuC logging gets enabled */
> +	struct {
> +		void *buf_addr;
> +		struct workqueue_struct *flush_wq;
> +		struct work_struct flush_work;
> +		struct rchan *relay_chan;
> +	} runtime;
> +	/* logging related stats */
> +	u32 capture_miss_count;
> +	u32 flush_interrupt_count;
> +	u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
> +	u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
> +	u32 flush_count[GUC_MAX_LOG_BUFFER];
> +};
> +
> +/*
> + * This structure primarily describes the GEM object shared with the GuC.
> + * The specs sometimes refer to this object as a "GuC context", but we use
> + * the term "client" to avoid confusion with hardware contexts. This
> + * GEM object is held for the entire lifetime of our interaction with
> + * the GuC, being allocated before the GuC is loaded with its firmware.
> + * Because there's no way to update the address used by the GuC after
> + * initialisation, the shared object must stay pinned into the GGTT as
> + * long as the GuC is in use. We also keep the first page (only) mapped
> + * into kernel address space, as it includes shared data that must be
> + * updated on every request submission.
> + *
> + * The single GEM object described here is actually made up of several
> + * separate areas, as far as the GuC is concerned. The first page (kept
> + * kmap'd) includes the "process descriptor" which holds sequence data for
> + * the doorbell, and one cacheline which actually *is* the doorbell; a
> + * write to this will "ring the doorbell" (i.e. send an interrupt to the
> + * GuC). The subsequent  pages of the client object constitute the work
> + * queue (a circular array of work items), again described in the process
> + * descriptor. Work queue pages are mapped momentarily as required.
> + */
> +struct i915_guc_client {
> +	struct i915_vma *vma;
> +	void *vaddr;
> +	struct i915_gem_context *owner;
> +	struct intel_guc *guc;
> +
> +	uint32_t engines;		/* bitmap of (host) engine ids	*/
> +	uint32_t priority;
> +	u32 stage_id;
> +	uint32_t proc_desc_offset;
> +
> +	u16 doorbell_id;
> +	unsigned long doorbell_offset;
> +
> +	spinlock_t wq_lock;
> +	/* Per-engine counts of GuC submissions */
> +	uint64_t submissions[I915_NUM_ENGINES];
> +};
> +
> +struct intel_guc {
> +	struct intel_uc_fw fw;
> +	struct intel_guc_log log;
> +	struct intel_guc_ct ct;
> +
> +	/* Log snapshot if GuC errors during load */
> +	struct drm_i915_gem_object *load_err_log;
> +
> +	/* intel_guc_recv interrupt related state */
> +	bool interrupts_enabled;
> +
> +	struct i915_vma *ads_vma;
> +	struct i915_vma *stage_desc_pool;
> +	void *stage_desc_pool_vaddr;
> +	struct ida stage_ids;
> +
> +	struct i915_guc_client *execbuf_client;
> +
> +	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
> +	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
> +
> +	/* GuC's FW specific registers used in MMIO send */
> +	struct {
> +		u32 base;
> +		unsigned int count;
> +		enum forcewake_domains fw_domains;
> +	} send_regs;
> +
> +	/* To serialize the intel_guc_send actions */
> +	struct mutex send_mutex;
> +
> +	/* GuC's FW specific send function */
> +	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
> +
> +	/* GuC's FW specific notify function */
> +	void (*notify)(struct intel_guc *guc);
> +};
> +
> +struct intel_huc {
> +	/* Generic uC firmware management */
> +	struct intel_uc_fw fw;
> +
> +	/* HuC-specific additions */
> +};
> +
> +#endif

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20170927/c662c02b/attachment-0001.html>


More information about the Intel-gfx mailing list