diff mbox

[v3,7/9] drm/i915/guc: Move GuC log declarations into dedicated header

Message ID 20171003163607.61680-8-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Wajdeczko Oct. 3, 2017, 4:36 p.m. UTC 
We want to keep component specific code in separate files.

Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_log.c |  1 +
 drivers/gpu/drm/i915/intel_guc_log.h | 57 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h      | 26 +---------------
 3 files changed, 59 insertions(+), 25 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h

Comments

Chris Wilson Oct. 3, 2017, 5:03 p.m. UTC | #1 
Quoting Michal Wajdeczko (2017-10-03 17:36:05)
> We want to keep component specific code in separate files.
> 
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_log.c |  1 +
>  drivers/gpu/drm/i915/intel_guc_log.h | 57 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc.h      | 26 +---------------
>  3 files changed, 59 insertions(+), 25 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index 6571d96..bd9b02e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -23,6 +23,7 @@
>   */
>  #include <linux/debugfs.h>
>  #include <linux/relay.h>
> +#include "intel_guc_log.h"
>  #include "i915_drv.h"

Can we keep a newline between <> and "", and aim for alphabetical order.
-Chris
Michal Wajdeczko Oct. 3, 2017, 6:28 p.m. UTC | #2 
On Tue, 03 Oct 2017 19:03:58 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2017-10-03 17:36:05)
>> We want to keep component specific code in separate files.
>>
>> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_guc_log.c |  1 +
>>  drivers/gpu/drm/i915/intel_guc_log.h | 57  
>> ++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_uc.h      | 26 +---------------
>>  3 files changed, 59 insertions(+), 25 deletions(-)
>>  create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
>> b/drivers/gpu/drm/i915/intel_guc_log.c
>> index 6571d96..bd9b02e 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>> @@ -23,6 +23,7 @@
>>   */
>>  #include <linux/debugfs.h>
>>  #include <linux/relay.h>
>> +#include "intel_guc_log.h"
>>  #include "i915_drv.h"
>
> Can we keep a newline between <> and "", and aim for alphabetical order.

I'll add newline in next spin, but I'm not sure about alphabetical order.

Note that Joonas wants all foo.c files to start with "foo.h" (after <>)
and then followed by other headers to make dependencies more explicit.

Michal
Chris Wilson Oct. 3, 2017, 7:48 p.m. UTC | #3 
Quoting Michal Wajdeczko (2017-10-03 19:28:02)
> On Tue, 03 Oct 2017 19:03:58 +0200, Chris Wilson  
> <chris@chris-wilson.co.uk> wrote:
> 
> > Quoting Michal Wajdeczko (2017-10-03 17:36:05)
> >> We want to keep component specific code in separate files.
> >>
> >> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_guc_log.c |  1 +
> >>  drivers/gpu/drm/i915/intel_guc_log.h | 57  
> >> ++++++++++++++++++++++++++++++++++++
> >>  drivers/gpu/drm/i915/intel_uc.h      | 26 +---------------
> >>  3 files changed, 59 insertions(+), 25 deletions(-)
> >>  create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
> >> b/drivers/gpu/drm/i915/intel_guc_log.c
> >> index 6571d96..bd9b02e 100644
> >> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> >> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> >> @@ -23,6 +23,7 @@
> >>   */
> >>  #include <linux/debugfs.h>
> >>  #include <linux/relay.h>
> >> +#include "intel_guc_log.h"
> >>  #include "i915_drv.h"
> >
> > Can we keep a newline between <> and "", and aim for alphabetical order.
> 
> I'll add newline in next spin, but I'm not sure about alphabetical order.
> 
> Note that Joonas wants all foo.c files to start with "foo.h" (after <>)
> and then followed by other headers to make dependencies more explicit.

I'm not fussed, though tbh I am more likely to forget which file I'm in
and so which special case to apply. 2 blocks of alphabetical includes is
simple enough for me to not muck up :)
-Chris
Chris Wilson Oct. 4, 2017, 5:09 p.m. UTC | #4 
Quoting Michal Wajdeczko (2017-10-03 19:28:02)
> On Tue, 03 Oct 2017 19:03:58 +0200, Chris Wilson  
> <chris@chris-wilson.co.uk> wrote:
> 
> > Quoting Michal Wajdeczko (2017-10-03 17:36:05)
> >> We want to keep component specific code in separate files.
> >>
> >> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_guc_log.c |  1 +
> >>  drivers/gpu/drm/i915/intel_guc_log.h | 57  
> >> ++++++++++++++++++++++++++++++++++++
> >>  drivers/gpu/drm/i915/intel_uc.h      | 26 +---------------
> >>  3 files changed, 59 insertions(+), 25 deletions(-)
> >>  create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
> >> b/drivers/gpu/drm/i915/intel_guc_log.c
> >> index 6571d96..bd9b02e 100644
> >> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> >> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> >> @@ -23,6 +23,7 @@
> >>   */
> >>  #include <linux/debugfs.h>
> >>  #include <linux/relay.h>
> >> +#include "intel_guc_log.h"
> >>  #include "i915_drv.h"
> >
> > Can we keep a newline between <> and "", and aim for alphabetical order.
> 
> I'll add newline in next spin, but I'm not sure about alphabetical order.
> 
> Note that Joonas wants all foo.c files to start with "foo.h" (after <>)
> and then followed by other headers to make dependencies more explicit.

Anyway, the change is trivial and clearly correct, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 6571d96..bd9b02e 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -23,6 +23,7 @@ 
  */
 #include <linux/debugfs.h>
 #include <linux/relay.h>
+#include "intel_guc_log.h"
 #include "i915_drv.h"
 
 static void guc_log_capture_logs(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h
new file mode 100644
index 0000000..099a0c8
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_log.h
@@ -0,0 +1,57 @@ 
+/*
+ * 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_GUC_LOG_H_
+#define _INTEL_GUC_LOG_H_
+
+#include <linux/workqueue.h>
+#include "intel_guc_fwif.h"
+
+struct drm_i915_private;
+struct intel_guc;
+
+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];
+};
+
+int intel_guc_log_create(struct intel_guc *guc);
+void intel_guc_log_destroy(struct intel_guc *guc);
+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);
+
+#endif
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 4fa091e..86ae507 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -29,6 +29,7 @@ 
 #include "i915_guc_reg.h"
 #include "intel_ringbuffer.h"
 #include "intel_guc_ct.h"
+#include "intel_guc_log.h"
 #include "i915_vma.h"
 #include "intel_huc.h"
 
@@ -72,24 +73,6 @@  struct i915_guc_client {
 	uint64_t submissions[I915_NUM_ENGINES];
 };
 
-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;
@@ -165,13 +148,6 @@  void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
 
-/* intel_guc_log.c */
-int intel_guc_log_create(struct intel_guc *guc);
-void intel_guc_log_destroy(struct intel_guc *guc);
-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);