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 |
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;
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
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.
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 --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;