On Wed, 04 Oct 2017 19:17:21 +0200, Chris Wilson
<chris@xxxxxxxxxxxxxxxxxx> wrote:
Quoting Michal Wajdeczko (2017-10-03 17:36:07)
We want to keep GuC specific code in separated files.
v2: move all functions in single patch (Joonas)
fix old checkpatch issues (Sagar)
v3: rebased
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> #1
---
<snip>
+#ifndef _INTEL_GUC_H_
+#define _INTEL_GUC_H_
+
+#include "intel_uncore.h"
+#include "intel_guc_fwif.h"
+#include "intel_guc_ct.h"
+#include "intel_guc_log.h"
+#include "intel_uc_fw.h"
+#include "i915_guc_reg.h"
+#include "i915_vma.h"
Hmm. Are they required for the header? Or for the C body?
For the header.
#include "intel_uncore.h" --> enum forcewake_domains
#include "intel_guc_fwif.h" --> GUC_NUM_DOORBELLS
#include "intel_guc_ct.h" --> struct intel_guc_ct
#include "intel_guc_log.h" --> struct intel_guc_log
#include "intel_uc_fw.h" --> struct intel_uc_fw
#include "i915_guc_reg.h" --> GUC_WOPCM_TOP
#include "i915_vma.h" --> i915_ggtt_offset
+
+struct intel_guc {
+ struct intel_uc_fw fw;
+ struct intel_guc_log log;
+ struct intel_guc_ct ct;
Most of the embedded types ^^ will not require all of these headers.
Their requirements are handled in their own .h
+
+ /* 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);
But we do need <linux/bitmap.h>
No, only <linux/types.h>
https://cgit.freedesktop.org/drm-tip/tree/include/linux/types.h#n9
+ uint32_t db_cacheline; /* Cyclic counter mod pagesize
*/
Too soon to s/uint32_t/u32/ ?
Yes.
+
+ /* 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)
Unusual line splitting.
Will fix.
+{
+ return guc->send(guc, action, len);
+}
+
+static inline void intel_guc_notify(struct intel_guc *guc)
+{
+ guc->notify(guc);
+}
+
+static inline u32 guc_ggtt_offset(struct i915_vma *vma)
+{
+ u32 offset = i915_ggtt_offset(vma);
So here we required i915_vma.h, ok.
It's pretty much a carbon copy, so
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
However, there's still plenty to polish.
-Chris
Thanks,
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
