Re: [RFC v4 08/14] drm/i915/vm_bind: Abstract out common execbuf functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 22, 2022 at 12:54:09PM +0300, Jani Nikula wrote:
On Wed, 21 Sep 2022, Niranjana Vishwanathapura <niranjana.vishwanathapura@xxxxxxxxx> wrote:
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer_common.h b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer_common.h
new file mode 100644
index 000000000000..725febfd6a53
--- /dev/null
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer_common.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#ifndef __I915_GEM_EXECBUFFER_COMMON_H
+#define __I915_GEM_EXECBUFFER_COMMON_H
+
+#include <drm/drm_syncobj.h>
+
+#include "gt/intel_context.h"

You don't need these includes. Most of it can be handled using forward
declarations. You'll need <linux/types.h>


Thanks Jani,
Sure, here and everywhere I will remove the unwanted includes and use
forward declarations instead.

+
+struct eb_fence {
+	struct drm_syncobj *syncobj;
+	struct dma_fence *dma_fence;
+	u64 value;
+	struct dma_fence_chain *chain_fence;
+};
+
+int __eb_pin_engine(struct intel_context *ce, struct i915_gem_ww_ctx *ww,
+		    bool throttle, bool nonblock);
+void __eb_unpin_engine(struct intel_context *ce);
+int __eb_select_engine(struct intel_context *ce);
+void __eb_put_engine(struct intel_context *context, struct intel_gt *gt);
+
+struct intel_context *
+eb_find_context(struct intel_context *context, unsigned int context_number);
+
+int add_timeline_fence(struct drm_file *file, u32 handle, u64 point,
+		       struct eb_fence *f, bool wait, bool signal);
+void put_fence_array(struct eb_fence *fences, u64 num_fences);
+int await_fence_array(struct eb_fence *fences, u64 num_fences,
+		      struct i915_request *rq);
+void signal_fence_array(struct eb_fence *fences, u64 num_fences,
+			struct dma_fence * const fence);
+
+int eb_requests_add(struct i915_request **requests, unsigned int num_batches,
+		    struct intel_context *context, struct i915_sched_attr sched,

struct i915_sched_attr is passed by value, so you either need to turn
that into a pointer, or you need the definition. The definition is just
a wrapper around an int. (For strict type safety or for future proofing
or what, I don't know.) And this all brings me to my pet peeve about
gem/gt headers.

To get that definition of a struct wrapper around an int, you need to
include i915_scheduler_types.h, which recursively includes a total of 16
headers. Touch any of those files, and you get a rebuild butterfly
effect.

28% of i915 header files, when modified, cause the rebuild of 83% of the
driver. Please let's not make it worse.


Ok. I think it is passed by value as it is just a wrapper around an int.
I am just moving this function to a separate file.
Will keep it as such, but will forward declare it insted of including
the i915_scheduler_types.h.

Regards,
Niranjana


BR,
Jani.

+		    int err);
+void eb_requests_get(struct i915_request **requests, unsigned int num_batches);
+void eb_requests_put(struct i915_request **requests, unsigned int num_batches);
+
+struct dma_fence *__eb_composite_fence_create(struct i915_request **requests,
+					      unsigned int num_batches,
+					      struct intel_context *context);
+
+#endif /* __I915_GEM_EXECBUFFER_COMMON_H */

--
Jani Nikula, Intel Open Source Graphics Center



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux