diff mbox series

[02/33] drm/i915: Introduce struct intel_gt as replacement for anonymous i915->gt

Message ID 20190619132459.25351-3-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Implicit dev_priv removal and GT compartmentalization | expand

Commit Message

Tvrtko Ursulin June 19, 2019, 1:24 p.m. UTC 
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We have long been slighlty annoyed by the anonymous i915->gt.

Promote it to a separate structure and give it its own header.

This is a first step towards cleaning up the separation between i915 and gt.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_gt_types.h | 53 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h          | 34 +--------------
 2 files changed, 55 insertions(+), 32 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_types.h

Comments

Jani Nikula June 19, 2019, 1:48 p.m. UTC | #1 
On Wed, 19 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> We have long been slighlty annoyed by the anonymous i915->gt.
>
> Promote it to a separate structure and give it its own header.
>
> This is a first step towards cleaning up the separation between i915 and gt.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_types.h | 53 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h          | 34 +--------------
>  2 files changed, 55 insertions(+), 32 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_types.h
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> new file mode 100644
> index 000000000000..dcdb18e0dd84
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -0,0 +1,53 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright © 2019 Intel Corporation
> + */

http://patchwork.freedesktop.org/patch/msgid/20190615043142.GA1890@nishad

BR,
Jani.

> +
> +#ifndef __INTEL_GT_TYPES__
> +#define __INTEL_GT_TYPES__
> +
> +#include <linux/ktime.h>
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +#include "i915_vma.h"
> +#include "intel_wakeref.h"
> +
> +struct intel_gt {
> +	struct i915_gt_timelines {
> +		struct mutex mutex; /* protects list, tainted by GPU */
> +		struct list_head active_list;
> +
> +		/* Pack multiple timelines' seqnos into the same page */
> +		spinlock_t hwsp_lock;
> +		struct list_head hwsp_free_list;
> +	} timelines;
> +
> +	struct list_head active_rings;
> +
> +	struct intel_wakeref wakeref;
> +
> +	struct list_head closed_vma;
> +	spinlock_t closed_lock; /* guards the list of closed_vma */
> +
> +	/**
> +	 * Is the GPU currently considered idle, or busy executing
> +	 * userspace requests? Whilst idle, we allow runtime power
> +	 * management to power down the hardware and display clocks.
> +	 * In order to reduce the effect on performance, there
> +	 * is a slight delay before we do so.
> +	 */
> +	intel_wakeref_t awake;
> +
> +	struct blocking_notifier_head pm_notifications;
> +
> +	ktime_t last_init_time;
> +
> +	struct i915_vma *scratch;
> +};
> +
> +#endif /* __INTEL_GT_TYPES_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bc909ec5d9c3..be433894ea28 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -72,6 +72,7 @@
>  
>  #include "gt/intel_lrc.h"
>  #include "gt/intel_engine.h"
> +#include "gt/intel_gt_types.h"
>  #include "gt/intel_workarounds.h"
>  
>  #include "intel_device_info.h"
> @@ -1824,38 +1825,7 @@ struct drm_i915_private {
>  	} perf;
>  
>  	/* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
> -	struct {
> -		struct i915_gt_timelines {
> -			struct mutex mutex; /* protects list, tainted by GPU */
> -			struct list_head active_list;
> -
> -			/* Pack multiple timelines' seqnos into the same page */
> -			spinlock_t hwsp_lock;
> -			struct list_head hwsp_free_list;
> -		} timelines;
> -
> -		struct list_head active_rings;
> -
> -		struct intel_wakeref wakeref;
> -
> -		struct list_head closed_vma;
> -		spinlock_t closed_lock; /* guards the list of closed_vma */
> -
> -		/**
> -		 * Is the GPU currently considered idle, or busy executing
> -		 * userspace requests? Whilst idle, we allow runtime power
> -		 * management to power down the hardware and display clocks.
> -		 * In order to reduce the effect on performance, there
> -		 * is a slight delay before we do so.
> -		 */
> -		intel_wakeref_t awake;
> -
> -		struct blocking_notifier_head pm_notifications;
> -
> -		ktime_t last_init_time;
> -
> -		struct i915_vma *scratch;
> -	} gt;
> +	struct intel_gt gt;
>  
>  	struct {
>  		struct notifier_block pm_notifier;
Chris Wilson June 19, 2019, 1:51 p.m. UTC | #2 
Quoting Jani Nikula (2019-06-19 14:48:30)
> On Wed, 19 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >
> > We have long been slighlty annoyed by the anonymous i915->gt.
> >
> > Promote it to a separate structure and give it its own header.
> >
> > This is a first step towards cleaning up the separation between i915 and gt.
> >
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_types.h | 53 ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_drv.h          | 34 +--------------
> >  2 files changed, 55 insertions(+), 32 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_types.h
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > new file mode 100644
> > index 000000000000..dcdb18e0dd84
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > @@ -0,0 +1,53 @@
> > +/*
> > + * SPDX-License-Identifier: MIT
> > + *
> > + * Copyright © 2019 Intel Corporation
> > + */
> 
> http://patchwork.freedesktop.org/patch/msgid/20190615043142.GA1890@nishad

I utterly abhor that. Breaking prior coding style and consistency just
for the sake of a perl script. I want the copyright information as part
of the licence grant (as it is who is giving the licence grant in the first
place).
-Chris
Jani Nikula June 19, 2019, 2:20 p.m. UTC | #3 
On Wed, 19 Jun 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2019-06-19 14:48:30)
>> On Wed, 19 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> >
>> > We have long been slighlty annoyed by the anonymous i915->gt.
>> >
>> > Promote it to a separate structure and give it its own header.
>> >
>> > This is a first step towards cleaning up the separation between i915 and gt.
>> >
>> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  drivers/gpu/drm/i915/gt/intel_gt_types.h | 53 ++++++++++++++++++++++++
>> >  drivers/gpu/drm/i915/i915_drv.h          | 34 +--------------
>> >  2 files changed, 55 insertions(+), 32 deletions(-)
>> >  create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_types.h
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> > new file mode 100644
>> > index 000000000000..dcdb18e0dd84
>> > --- /dev/null
>> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> > @@ -0,0 +1,53 @@
>> > +/*
>> > + * SPDX-License-Identifier: MIT
>> > + *
>> > + * Copyright © 2019 Intel Corporation
>> > + */
>> 
>> http://patchwork.freedesktop.org/patch/msgid/20190615043142.GA1890@nishad
>
> I utterly abhor that. Breaking prior coding style and consistency just
> for the sake of a perl script. I want the copyright information as part
> of the licence grant (as it is who is giving the licence grant in the first
> place).

Aesthetically speaking, I'm with you.

It's just that the powers that be have in their infinite wisdom decided
on the one true style (*) to add SPDX headers.

I'll look the other way, but I'm also not going to block patches adding
new files with "conforming" headers. I'm not sure how long we'll be able
to fend off patches converting existing headers, especially given that
SPDX headers were (and I think are being) added directly to Linus' tree
bypassing subsystem trees.

Fair enough?


BR,
Jani.


(*) Ahem, the two true styles, depending on whether it's a .c or a .h.
Tvrtko Ursulin June 19, 2019, 4:18 p.m. UTC | #4 
On 19/06/2019 15:20, Jani Nikula wrote:
> On Wed, 19 Jun 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Quoting Jani Nikula (2019-06-19 14:48:30)
>>> On Wed, 19 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> We have long been slighlty annoyed by the anonymous i915->gt.
>>>>
>>>> Promote it to a separate structure and give it its own header.
>>>>
>>>> This is a first step towards cleaning up the separation between i915 and gt.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>> ---
>>>>   drivers/gpu/drm/i915/gt/intel_gt_types.h | 53 ++++++++++++++++++++++++
>>>>   drivers/gpu/drm/i915/i915_drv.h          | 34 +--------------
>>>>   2 files changed, 55 insertions(+), 32 deletions(-)
>>>>   create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_types.h
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>>> new file mode 100644
>>>> index 000000000000..dcdb18e0dd84
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>>> @@ -0,0 +1,53 @@
>>>> +/*
>>>> + * SPDX-License-Identifier: MIT
>>>> + *
>>>> + * Copyright © 2019 Intel Corporation
>>>> + */
>>>
>>> http://patchwork.freedesktop.org/patch/msgid/20190615043142.GA1890@nishad
>>
>> I utterly abhor that. Breaking prior coding style and consistency just
>> for the sake of a perl script. I want the copyright information as part
>> of the licence grant (as it is who is giving the licence grant in the first
>> place).
> 
> Aesthetically speaking, I'm with you.
> 
> It's just that the powers that be have in their infinite wisdom decided
> on the one true style (*) to add SPDX headers.
> 
> I'll look the other way, but I'm also not going to block patches adding
> new files with "conforming" headers. I'm not sure how long we'll be able
> to fend off patches converting existing headers, especially given that
> SPDX headers were (and I think are being) added directly to Linus' tree
> bypassing subsystem trees.
> 
> Fair enough?

I can convert this before merging no problem. If nothing else will keep 
checkpatch quieter so at least some benefit.

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
new file mode 100644
index 000000000000..dcdb18e0dd84
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -0,0 +1,53 @@ 
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __INTEL_GT_TYPES__
+#define __INTEL_GT_TYPES__
+
+#include <linux/ktime.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include "i915_vma.h"
+#include "intel_wakeref.h"
+
+struct intel_gt {
+	struct i915_gt_timelines {
+		struct mutex mutex; /* protects list, tainted by GPU */
+		struct list_head active_list;
+
+		/* Pack multiple timelines' seqnos into the same page */
+		spinlock_t hwsp_lock;
+		struct list_head hwsp_free_list;
+	} timelines;
+
+	struct list_head active_rings;
+
+	struct intel_wakeref wakeref;
+
+	struct list_head closed_vma;
+	spinlock_t closed_lock; /* guards the list of closed_vma */
+
+	/**
+	 * Is the GPU currently considered idle, or busy executing
+	 * userspace requests? Whilst idle, we allow runtime power
+	 * management to power down the hardware and display clocks.
+	 * In order to reduce the effect on performance, there
+	 * is a slight delay before we do so.
+	 */
+	intel_wakeref_t awake;
+
+	struct blocking_notifier_head pm_notifications;
+
+	ktime_t last_init_time;
+
+	struct i915_vma *scratch;
+};
+
+#endif /* __INTEL_GT_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bc909ec5d9c3..be433894ea28 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -72,6 +72,7 @@ 
 
 #include "gt/intel_lrc.h"
 #include "gt/intel_engine.h"
+#include "gt/intel_gt_types.h"
 #include "gt/intel_workarounds.h"
 
 #include "intel_device_info.h"
@@ -1824,38 +1825,7 @@  struct drm_i915_private {
 	} perf;
 
 	/* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
-	struct {
-		struct i915_gt_timelines {
-			struct mutex mutex; /* protects list, tainted by GPU */
-			struct list_head active_list;
-
-			/* Pack multiple timelines' seqnos into the same page */
-			spinlock_t hwsp_lock;
-			struct list_head hwsp_free_list;
-		} timelines;
-
-		struct list_head active_rings;
-
-		struct intel_wakeref wakeref;
-
-		struct list_head closed_vma;
-		spinlock_t closed_lock; /* guards the list of closed_vma */
-
-		/**
-		 * Is the GPU currently considered idle, or busy executing
-		 * userspace requests? Whilst idle, we allow runtime power
-		 * management to power down the hardware and display clocks.
-		 * In order to reduce the effect on performance, there
-		 * is a slight delay before we do so.
-		 */
-		intel_wakeref_t awake;
-
-		struct blocking_notifier_head pm_notifications;
-
-		ktime_t last_init_time;
-
-		struct i915_vma *scratch;
-	} gt;
+	struct intel_gt gt;
 
 	struct {
 		struct notifier_block pm_notifier;