* [RFC 1/2] drm/i915/uc: Move uC related types into dedicated header
@ 2017-09-26 16:48 Michal Wajdeczko
2017-09-26 16:48 ` [RFC 2/2] drm/i915: Make intel_guc_wopcm_size() inline Michal Wajdeczko
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Michal Wajdeczko @ 2017-09-26 16:48 UTC (permalink / raw)
To: intel-gfx
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.
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>
---
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 | 1 +
drivers/gpu/drm/i915/intel_guc_log.c | 1 +
drivers/gpu/drm/i915/intel_uc.h | 180 +++-------------------------------
drivers/gpu/drm/i915/intel_uc_types.h | 172 ++++++++++++++++++++++++++++++++
7 files changed, 193 insertions(+), 165 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..cbeecd2 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -23,6 +23,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_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..b0e8849 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,23 @@ 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;
-
- /* 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);
-};
+static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
+{
+ return guc->send(guc, action, len);
+}
-struct intel_huc {
- /* Generic uC firmware management */
- struct intel_uc_fw fw;
+static inline void intel_guc_notify(struct intel_guc *guc)
+{
+ guc->notify(guc);
+}
- /* HuC-specific additions */
-};
+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_uc.c */
void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
@@ -213,16 +83,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 +104,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..fe3b19d
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_uc_types.h
@@ -0,0 +1,172 @@
+/*
+ * 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 "i915_guc_reg.h"
+#include "intel_guc_ct.h"
+
+struct i915_guc_client;
+
+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];
+};
+
+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 */
+};
+
+/*
+ * 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];
+};
+
+#endif
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread* [RFC 2/2] drm/i915: Make intel_guc_wopcm_size() inline
2017-09-26 16:48 [RFC 1/2] drm/i915/uc: Move uC related types into dedicated header Michal Wajdeczko
@ 2017-09-26 16:48 ` Michal Wajdeczko
2017-09-27 6:36 ` Sagar Arun Kamble
2017-09-26 17:53 ` ✓ Fi.CI.BAT: success for series starting with [RFC,1/2] drm/i915/uc: Move uC related types into dedicated header Patchwork
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Michal Wajdeczko @ 2017-09-26 16:48 UTC (permalink / raw)
To: intel-gfx
It's small and we are using this function sporadically.
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/intel_guc_loader.c | 11 -----------
drivers/gpu/drm/i915/intel_uc.c | 4 ++--
drivers/gpu/drm/i915/intel_uc.h | 13 ++++++++++++-
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index c9e25be..387d105a 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -250,17 +250,6 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
return ret;
}
-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;
-}
-
/*
* Load the GuC firmware blob into the MinuteIA.
*/
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 2774778..1ef2de6 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -188,7 +188,7 @@ static void fetch_uc_fw(struct drm_i915_private *dev_priv,
size = uc_fw->header_size + uc_fw->ucode_size;
/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
- if (size > intel_guc_wopcm_size(dev_priv)) {
+ if (size > intel_guc_wopcm_size(&dev_priv->guc)) {
DRM_ERROR("Firmware is too large to fit in WOPCM\n");
goto fail;
}
@@ -374,7 +374,7 @@ int intel_uc_init_hw(struct drm_i915_private *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.h b/drivers/gpu/drm/i915/intel_uc.h
index b0e8849..b65ba5c 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -63,6 +63,18 @@ static inline void intel_guc_notify(struct intel_guc *guc)
guc->notify(guc);
}
+static inline u32 intel_guc_wopcm_size(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ 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;
+}
+
static inline u32 guc_ggtt_offset(struct i915_vma *vma)
{
u32 offset = i915_ggtt_offset(vma);
@@ -88,7 +100,6 @@ int intel_guc_select_fw(struct intel_guc *guc);
int intel_guc_init_hw(struct intel_guc *guc);
int intel_guc_suspend(struct drm_i915_private *dev_priv);
int intel_guc_resume(struct drm_i915_private *dev_priv);
-u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
/* i915_guc_submission.c */
int i915_guc_submission_init(struct drm_i915_private *dev_priv);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [RFC 2/2] drm/i915: Make intel_guc_wopcm_size() inline
2017-09-26 16:48 ` [RFC 2/2] drm/i915: Make intel_guc_wopcm_size() inline Michal Wajdeczko
@ 2017-09-27 6:36 ` Sagar Arun Kamble
0 siblings, 0 replies; 8+ messages in thread
From: Sagar Arun Kamble @ 2017-09-27 6:36 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
Looks good to me.
Nitpicks:
1. s/dev_priv/i915 in intel_guc_wopcm_size
2. We are updating the parameter type of intel_guc_wopcm_size. Will it
be necessary to update that change log and intent in the commit message?
Thanks
Sagar
On 9/26/2017 10:18 PM, Michal Wajdeczko wrote:
> It's small and we are using this function sporadically.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
> drivers/gpu/drm/i915/intel_guc_loader.c | 11 -----------
> drivers/gpu/drm/i915/intel_uc.c | 4 ++--
> drivers/gpu/drm/i915/intel_uc.h | 13 ++++++++++++-
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index c9e25be..387d105a 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -250,17 +250,6 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
> return ret;
> }
>
> -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;
> -}
> -
> /*
> * Load the GuC firmware blob into the MinuteIA.
> */
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 2774778..1ef2de6 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -188,7 +188,7 @@ static void fetch_uc_fw(struct drm_i915_private *dev_priv,
> size = uc_fw->header_size + uc_fw->ucode_size;
>
> /* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
> - if (size > intel_guc_wopcm_size(dev_priv)) {
> + if (size > intel_guc_wopcm_size(&dev_priv->guc)) {
> DRM_ERROR("Firmware is too large to fit in WOPCM\n");
> goto fail;
> }
> @@ -374,7 +374,7 @@ int intel_uc_init_hw(struct drm_i915_private *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.h b/drivers/gpu/drm/i915/intel_uc.h
> index b0e8849..b65ba5c 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -63,6 +63,18 @@ static inline void intel_guc_notify(struct intel_guc *guc)
> guc->notify(guc);
> }
>
> +static inline u32 intel_guc_wopcm_size(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + 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;
> +}
> +
> static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> {
> u32 offset = i915_ggtt_offset(vma);
> @@ -88,7 +100,6 @@ int intel_guc_select_fw(struct intel_guc *guc);
> int intel_guc_init_hw(struct intel_guc *guc);
> int intel_guc_suspend(struct drm_i915_private *dev_priv);
> int intel_guc_resume(struct drm_i915_private *dev_priv);
> -u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>
> /* i915_guc_submission.c */
> int i915_guc_submission_init(struct drm_i915_private *dev_priv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [RFC,1/2] drm/i915/uc: Move uC related types into dedicated header
2017-09-26 16:48 [RFC 1/2] drm/i915/uc: Move uC related types into dedicated header Michal Wajdeczko
2017-09-26 16:48 ` [RFC 2/2] drm/i915: Make intel_guc_wopcm_size() inline Michal Wajdeczko
@ 2017-09-26 17:53 ` Patchwork
2017-09-27 2:48 ` ✗ Fi.CI.IGT: failure " Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-09-26 17:53 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: intel-gfx
== Series Details ==
Series: series starting with [RFC,1/2] drm/i915/uc: Move uC related types into dedicated header
URL : https://patchwork.freedesktop.org/series/30918/
State : success
== Summary ==
Series 30918v1 series starting with [RFC,1/2] drm/i915/uc: Move uC related types into dedicated header
https://patchwork.freedesktop.org/api/1.0/series/30918/revisions/1/mbox/
Test pm_rpm:
Subgroup basic-rte:
dmesg-warn -> PASS (fi-cfl-s) fdo#102294
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:449s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:474s
fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:416s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:513s
fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:277s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:512s
fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:489s
fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:485s
fi-cfl-s total:289 pass:223 dwarn:34 dfail:0 fail:0 skip:32 time:537s
fi-cnl-y total:289 pass:257 dwarn:0 dfail:0 fail:5 skip:27 time:644s
fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:421s
fi-glk-1 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:567s
fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:430s
fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:405s
fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:430s
fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:493s
fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:473s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:468s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:579s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:592s
fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:548s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:453s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:748s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:487s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:481s
fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:574s
fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:416s
c4c623d58e38d49e692dc9141250b35e39170e6b drm-tip: 2017y-09m-26d-16h-37m-12s UTC integration manifest
8bcc49c080ce drm/i915: Make intel_guc_wopcm_size() inline
cb01c1b473e0 drm/i915/uc: Move uC related types into dedicated header
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5822/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread* ✗ Fi.CI.IGT: failure for series starting with [RFC,1/2] drm/i915/uc: Move uC related types into dedicated header
2017-09-26 16:48 [RFC 1/2] drm/i915/uc: Move uC related types into dedicated header Michal Wajdeczko
2017-09-26 16:48 ` [RFC 2/2] drm/i915: Make intel_guc_wopcm_size() inline Michal Wajdeczko
2017-09-26 17:53 ` ✓ Fi.CI.BAT: success for series starting with [RFC,1/2] drm/i915/uc: Move uC related types into dedicated header Patchwork
@ 2017-09-27 2:48 ` Patchwork
2017-09-27 7:22 ` [RFC 1/2] " Sagar Arun Kamble
2017-09-27 9:51 ` Joonas Lahtinen
4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-09-27 2:48 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: intel-gfx
== Series Details ==
Series: series starting with [RFC,1/2] drm/i915/uc: Move uC related types into dedicated header
URL : https://patchwork.freedesktop.org/series/30918/
State : failure
== Summary ==
Test kms_flip:
Subgroup flip-vs-expired-vblank:
fail -> PASS (shard-hsw) fdo#102367
Subgroup plain-flip-ts-check-interruptible:
fail -> PASS (shard-hsw)
Test gem_exec_store:
Subgroup pages-default:
pass -> FAIL (shard-hsw)
fdo#102367 https://bugs.freedesktop.org/show_bug.cgi?id=102367
shard-hsw total:2429 pass:1334 dwarn:1 dfail:0 fail:11 skip:1083 time:9970s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5822/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFC 1/2] drm/i915/uc: Move uC related types into dedicated header
2017-09-26 16:48 [RFC 1/2] drm/i915/uc: Move uC related types into dedicated header Michal Wajdeczko
` (2 preceding siblings ...)
2017-09-27 2:48 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2017-09-27 7:22 ` Sagar Arun Kamble
2017-09-27 10:58 ` Michal Wajdeczko
2017-09-27 9:51 ` Joonas Lahtinen
4 siblings, 1 reply; 8+ messages in thread
From: Sagar Arun Kamble @ 2017-09-27 7:22 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
Like this change as now headers becomes easy to read. Minor inputs
suggested below.
On 9/26/2017 10:18 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.
>
> 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>
> ---
> 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 | 1 +
> drivers/gpu/drm/i915/intel_guc_log.c | 1 +
> drivers/gpu/drm/i915/intel_uc.h | 180 +++-------------------------------
> drivers/gpu/drm/i915/intel_uc_types.h | 172 ++++++++++++++++++++++++++++++++
> 7 files changed, 193 insertions(+), 165 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..cbeecd2 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -23,6 +23,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_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..b0e8849 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,23 @@ 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;
> -
> - /* 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);
> -};
> +static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
> +{
> + return guc->send(guc, action, len);
> +}
>
> -struct intel_huc {
> - /* Generic uC firmware management */
> - struct intel_uc_fw fw;
> +static inline void intel_guc_notify(struct intel_guc *guc)
> +{
> + guc->notify(guc);
> +}
>
> - /* HuC-specific additions */
> -};
> +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_uc.c */
> void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
> @@ -213,16 +83,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 +104,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..fe3b19d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_uc_types.h
> @@ -0,0 +1,172 @@
> +/*
> + * 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 "i915_guc_reg.h"
How about pulling intel_guc_fwif.h from intel_guc_ct.h here? I feel it
fits more here.
> +#include "intel_guc_ct.h"
> +
> +struct i915_guc_client;
Can we skip this forward declaration and ...
> +
> +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;
> +};
> +
define struct i915_guc_client here .. that way it maintains the needs of
intel_guc together.
> +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;
> +
> + /* 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 */
> +};
> +
> +/*
> + * 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];
> +};
> +
> +#endif
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFC 1/2] drm/i915/uc: Move uC related types into dedicated header
2017-09-27 7:22 ` [RFC 1/2] " Sagar Arun Kamble
@ 2017-09-27 10:58 ` Michal Wajdeczko
0 siblings, 0 replies; 8+ messages in thread
From: Michal Wajdeczko @ 2017-09-27 10:58 UTC (permalink / raw)
To: intel-gfx, Sagar Arun Kamble
On Wed, 27 Sep 2017 09:22:35 +0200, Sagar Arun Kamble
<sagar.a.kamble@intel.com> wrote:
> Like this change as now headers becomes easy to read. Minor inputs
> suggested below.
>
> On 9/26/2017 10:18 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.
>>
>> 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>
>> ---
<snip>
>> +#ifndef _INTEL_UC_TYPES_H_
>> +#define _INTEL_UC_TYPES_H_
>> +
>> +#include "i915_guc_reg.h"
> How about pulling intel_guc_fwif.h from intel_guc_ct.h here? I feel it
> fits more here.
Sure. Previously I leaved it in ct header as it was required only there.
But to simplify dependencies we can include fwif here.
>> +#include "intel_guc_ct.h"
>> +
>> +struct i915_guc_client;
> Can we skip this forward declaration and ...
Well, it looks that this forward declaration was superfluous as we need it
only for pointer definition inside intel_guc where compiler can still be
happy without explicit declaration.
> define struct i915_guc_client here .. that way it maintains the needs of
> intel_guc together.
Ok, will reverse the order but will place it right before intel_guc.
Thanks,
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC 1/2] drm/i915/uc: Move uC related types into dedicated header
2017-09-26 16:48 [RFC 1/2] drm/i915/uc: Move uC related types into dedicated header Michal Wajdeczko
` (3 preceding siblings ...)
2017-09-27 7:22 ` [RFC 1/2] " Sagar Arun Kamble
@ 2017-09-27 9:51 ` Joonas Lahtinen
4 siblings, 0 replies; 8+ messages in thread
From: Joonas Lahtinen @ 2017-09-27 9:51 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On Tue, 2017-09-26 at 16:48 +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.
>
> 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>
This is a nice improvement. I see Sagar already provided detailed
review, so please address that (it's always good idea to reduce the
amount of forward declarations). Overall, this is:
Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-09-27 10:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-26 16:48 [RFC 1/2] drm/i915/uc: Move uC related types into dedicated header Michal Wajdeczko
2017-09-26 16:48 ` [RFC 2/2] drm/i915: Make intel_guc_wopcm_size() inline Michal Wajdeczko
2017-09-27 6:36 ` Sagar Arun Kamble
2017-09-26 17:53 ` ✓ Fi.CI.BAT: success for series starting with [RFC,1/2] drm/i915/uc: Move uC related types into dedicated header Patchwork
2017-09-27 2:48 ` ✗ Fi.CI.IGT: failure " Patchwork
2017-09-27 7:22 ` [RFC 1/2] " Sagar Arun Kamble
2017-09-27 10:58 ` Michal Wajdeczko
2017-09-27 9:51 ` Joonas Lahtinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox