| Message ID | 20170927112252.27660-1-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2017-09-27 at 11:22 +0000, 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@chris-wilson.co.uk> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Like I mentioned in the other patch, lets make future ourselves life easier and take this cleanup further; intel_uc.{c,h} intel_guc.{c,h} .h includes "intel_uc.h" intel_huc.{c,h} ditto intel_guc_ct.h includes "intel_guc.h" That way we're not driving ourselves towards the spaghetti insanity that i915_drv.h is currently. Aka. move a declaration, spend hours fixing the ordering of things. Regards, Joonas PS. Free beer offered for the one to first remove all "filename.c" sections from i915_drv.h
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@chris-wilson.co.uk> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@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
On Wed, 2017-09-27 at 14:36 +0300, Joonas Lahtinen wrote: > On Wed, 2017-09-27 at 11:22 +0000, 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@chris-wilson.co.uk> > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Ping, does this sound like a solid idea to you, Michal? Regards, Joonas > Like I mentioned in the other patch, lets make future ourselves life > easier and take this cleanup further; > > intel_uc.{c,h} > intel_guc.{c,h} .h includes "intel_uc.h" > intel_huc.{c,h} ditto > > intel_guc_ct.h includes "intel_guc.h" > > That way we're not driving ourselves towards the spaghetti insanity > that i915_drv.h is currently. Aka. move a declaration, spend hours > fixing the ordering of things. > > Regards, Joonas > > PS. Free beer offered for the one to first remove all "filename.c" > sections from i915_drv.h
On Fri, 29 Sep 2017 12:37:17 +0200, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote: > On Wed, 2017-09-27 at 14:36 +0300, Joonas Lahtinen wrote: >> On Wed, 2017-09-27 at 11:22 +0000, 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@chris-wilson.co.uk> >> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> > Cc: Chris Wilson <chris@chris-wilson.co.uk> >> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> > Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Ping, does this sound like a solid idea to you, Michal? Idea is fine (I was the one who vote for intel_huc.c) but this patch was to solve different problem: we can't define inline functions in private headers if they depend on INTEL_INFO/GEN macros as these macros requires drm_i915_private definition that is defined later in #include chain. But maybe you're right and we should start to clean our stuff first and then later focus on inline implementations. @Chris? Regards, Michal > > Regards, Joonas > >> Like I mentioned in the other patch, lets make future ourselves life >> easier and take this cleanup further; >> >> intel_uc.{c,h} >> intel_guc.{c,h} .h includes "intel_uc.h" >> intel_huc.{c,h} ditto >> >> intel_guc_ct.h includes "intel_guc.h" >> >> That way we're not driving ourselves towards the spaghetti insanity >> that i915_drv.h is currently. Aka. move a declaration, spend hours >> fixing the ordering of things. >> >> Regards, Joonas >> >> PS. Free beer offered for the one to first remove all "filename.c" >> sections from i915_drv.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