| Message ID | 20180509120419.6733-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Chris Wilson <chris@chris-wilson.co.uk> writes: > In the next few patches, we want to abuse tasklet to avoid ksoftirqd > latency along critical paths. To make that abuse easily to swallow, > first coat the tasklet in a little syntactic sugar. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 8 +-- > drivers/gpu/drm/i915/i915_irq.c | 2 +- > drivers/gpu/drm/i915/i915_tasklet.h | 78 +++++++++++++++++++++ > drivers/gpu/drm/i915/intel_engine_cs.c | 11 ++- > drivers/gpu/drm/i915/intel_guc_submission.c | 8 +-- > drivers/gpu/drm/i915/intel_lrc.c | 18 ++--- > drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +- > 7 files changed, 104 insertions(+), 24 deletions(-) > create mode 100644 drivers/gpu/drm/i915/i915_tasklet.h > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 89bf5d67cb74..98481e150e61 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3043,9 +3043,9 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) > * common case of recursively being called from set-wedged from inside > * i915_reset. > */ > - if (!atomic_read(&engine->execlists.tasklet.count)) > - tasklet_kill(&engine->execlists.tasklet); > - tasklet_disable(&engine->execlists.tasklet); > + if (i915_tasklet_is_enabled(&engine->execlists.tasklet)) > + i915_tasklet_flush(&engine->execlists.tasklet); > + i915_tasklet_disable(&engine->execlists.tasklet); > > /* > * We're using worker to queue preemption requests from the tasklet in > @@ -3265,7 +3265,7 @@ void i915_gem_reset(struct drm_i915_private *dev_priv, > > void i915_gem_reset_finish_engine(struct intel_engine_cs *engine) > { > - tasklet_enable(&engine->execlists.tasklet); > + i915_tasklet_enable(&engine->execlists.tasklet); > kthread_unpark(engine->breadcrumbs.signaler); > > intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL); > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index f9bc3aaa90d0..f8aff5a5aa83 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1477,7 +1477,7 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir) > } > > if (tasklet) > - tasklet_hi_schedule(&execlists->tasklet); > + i915_tasklet_schedule(&execlists->tasklet); > } > > static void gen8_gt_irq_ack(struct drm_i915_private *i915, > diff --git a/drivers/gpu/drm/i915/i915_tasklet.h b/drivers/gpu/drm/i915/i915_tasklet.h > new file mode 100644 > index 000000000000..c9f41a5ebb91 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_tasklet.h > @@ -0,0 +1,78 @@ > +/* > + * SPDX-License-Identifier: GPL-2.0 > + * > + * Copyright © 2018 Intel Corporation > + */ > + > +#ifndef _I915_TASKLET_H_ > +#define _I915_TASKLET_H_ > + > +#include <linux/atomic.h> > +#include <linux/interrupt.h> > + > +/** > + * struct i915_tasklet - wrapper around tasklet_struct > + * > + * We want to abuse tasklets slightly, such as calling them directly. In some > + * cases, this requires some auxiliary tracking so subclass the tasklet_struct > + * so that we have a central place and helpers. > + */ > +struct i915_tasklet { > + struct tasklet_struct base; > +}; > + > +static inline void i915_tasklet_init(struct i915_tasklet *t, > + void (*func)(unsigned long), > + unsigned long data) > +{ > + tasklet_init(&t->base, func, data); > +} > + > +static inline bool i915_tasklet_is_scheduled(const struct i915_tasklet *t) > +{ > + return test_bit(TASKLET_STATE_SCHED, &t->base.state); > +} > + > +static inline bool i915_tasklet_is_locked(const struct i915_tasklet *t) why not is_running() ? > +{ > + return test_bit(TASKLET_STATE_RUN, &t->base.state); > +} > + > +static inline bool i915_tasklet_is_enabled(const struct i915_tasklet *t) > +{ > + return likely(!atomic_read(&t->base.count)); I am concerned that we bury the sign of racy nature out of sight and then it comes and bites us. -Mika > +} > + > +static inline void i915_tasklet_schedule(struct i915_tasklet *t) > +{ > + tasklet_hi_schedule(&t->base); > +} > + > +static inline void i915_tasklet_flush(struct i915_tasklet *t) > +{ > + tasklet_kill(&t->base); > +} > + > +static inline void i915_tasklet_enable(struct i915_tasklet *t) > +{ > + tasklet_enable(&t->base); > +} > + > +static inline void i915_tasklet_disable(struct i915_tasklet *t) > +{ > + tasklet_disable(&t->base); > +} > + > +static inline void i915_tasklet_set_func(struct i915_tasklet *t, > + void (*func)(unsigned long data), > + unsigned long data) > +{ > + tasklet_disable(&t->base); What does the disable/enable pair buy us here? It looks that you want trylock and unlock so that you can safely change the func/data. -Mika > + > + t->base.func = func; > + t->base.data = data; > + > + tasklet_enable(&t->base); > +} > + > +#endif /* _I915_TASKLET_H_ */ > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index 70325e0824e3..5f1118ea96d8 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -1032,7 +1032,7 @@ void intel_engines_park(struct drm_i915_private *i915) > for_each_engine(engine, i915, id) { > /* Flush the residual irq tasklets first. */ > intel_engine_disarm_breadcrumbs(engine); > - tasklet_kill(&engine->execlists.tasklet); > + i915_tasklet_flush(&engine->execlists.tasklet); > > /* > * We are committed now to parking the engines, make sure there > @@ -1249,9 +1249,8 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine, > intel_read_status_page(engine, intel_hws_csb_write_index(engine->i915)), > yesno(test_bit(ENGINE_IRQ_EXECLIST, > &engine->irq_posted)), > - yesno(test_bit(TASKLET_STATE_SCHED, > - &engine->execlists.tasklet.state)), > - enableddisabled(!atomic_read(&engine->execlists.tasklet.count))); > + yesno(i915_tasklet_is_scheduled(&engine->execlists.tasklet)), > + enableddisabled(i915_tasklet_is_enabled(&engine->execlists.tasklet))); > if (read >= GEN8_CSB_ENTRIES) > read = 0; > if (write >= GEN8_CSB_ENTRIES) > @@ -1479,7 +1478,7 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine) > if (!intel_engine_supports_stats(engine)) > return -ENODEV; > > - tasklet_disable(&execlists->tasklet); > + i915_tasklet_disable(&execlists->tasklet); > write_seqlock_irqsave(&engine->stats.lock, flags); > > if (unlikely(engine->stats.enabled == ~0)) { > @@ -1505,7 +1504,7 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine) > > unlock: > write_sequnlock_irqrestore(&engine->stats.lock, flags); > - tasklet_enable(&execlists->tasklet); > + i915_tasklet_enable(&execlists->tasklet); > > return err; > } > diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c > index 2feb65096966..a7afc976c3b9 100644 > --- a/drivers/gpu/drm/i915/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/intel_guc_submission.c > @@ -591,7 +591,7 @@ static void inject_preempt_context(struct work_struct *work) > if (WARN_ON(intel_guc_send(guc, data, ARRAY_SIZE(data)))) { > execlists_clear_active(&engine->execlists, > EXECLISTS_ACTIVE_PREEMPT); > - tasklet_schedule(&engine->execlists.tasklet); > + i915_tasklet_schedule(&engine->execlists.tasklet); > } > } > > @@ -1263,10 +1263,10 @@ int intel_guc_submission_enable(struct intel_guc *guc) > guc_interrupts_capture(dev_priv); > > for_each_engine(engine, dev_priv, id) { > - struct intel_engine_execlists * const execlists = > - &engine->execlists; > + i915_tasklet_set_func(&engine->execlists.tasklet, > + guc_submission_tasklet, > + (unsigned long)engine); > > - execlists->tasklet.func = guc_submission_tasklet; > engine->park = guc_submission_park; > engine->unpark = guc_submission_unpark; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 7f98dda3c929..539fa03d7600 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1165,7 +1165,7 @@ static void queue_request(struct intel_engine_cs *engine, > static void __submit_queue(struct intel_engine_cs *engine, int prio) > { > engine->execlists.queue_priority = prio; > - tasklet_hi_schedule(&engine->execlists.tasklet); > + i915_tasklet_schedule(&engine->execlists.tasklet); > } > > static void submit_queue(struct intel_engine_cs *engine, int prio) > @@ -1778,7 +1778,7 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine) > > /* After a GPU reset, we may have requests to replay */ > if (execlists->first) > - tasklet_schedule(&execlists->tasklet); > + i915_tasklet_schedule(&execlists->tasklet); > > return 0; > } > @@ -2182,9 +2182,8 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *engine) > * Tasklet cannot be active at this point due intel_mark_active/idle > * so this is just for documentation. > */ > - if (WARN_ON(test_bit(TASKLET_STATE_SCHED, > - &engine->execlists.tasklet.state))) > - tasklet_kill(&engine->execlists.tasklet); > + if (WARN_ON(i915_tasklet_is_scheduled(&engine->execlists.tasklet))) > + i915_tasklet_flush(&engine->execlists.tasklet); > > dev_priv = engine->i915; > > @@ -2209,7 +2208,10 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine) > engine->submit_request = execlists_submit_request; > engine->cancel_requests = execlists_cancel_requests; > engine->schedule = execlists_schedule; > - engine->execlists.tasklet.func = execlists_submission_tasklet; > + > + i915_tasklet_set_func(&engine->execlists.tasklet, > + execlists_submission_tasklet, > + (unsigned long)engine); > > engine->park = NULL; > engine->unpark = NULL; > @@ -2303,8 +2305,8 @@ logical_ring_setup(struct intel_engine_cs *engine) > > engine->execlists.fw_domains = fw_domains; > > - tasklet_init(&engine->execlists.tasklet, > - execlists_submission_tasklet, (unsigned long)engine); > + i915_tasklet_init(&engine->execlists.tasklet, > + execlists_submission_tasklet, (unsigned long)engine); > > logical_ring_default_vfuncs(engine); > logical_ring_default_irqs(engine); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 010750e8ee44..f6ba354faf89 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -11,6 +11,7 @@ > #include "i915_pmu.h" > #include "i915_request.h" > #include "i915_selftest.h" > +#include "i915_tasklet.h" > #include "i915_timeline.h" > #include "intel_gpu_commands.h" > > @@ -202,7 +203,7 @@ struct intel_engine_execlists { > /** > * @tasklet: softirq tasklet for bottom handler > */ > - struct tasklet_struct tasklet; > + struct i915_tasklet tasklet; > > /** > * @default_priolist: priority list for I915_PRIORITY_NORMAL > -- > 2.17.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2018-05-09 14:44:30) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > In the next few patches, we want to abuse tasklet to avoid ksoftirqd > > latency along critical paths. To make that abuse easily to swallow, > > first coat the tasklet in a little syntactic sugar. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > --- > > drivers/gpu/drm/i915/i915_gem.c | 8 +-- > > drivers/gpu/drm/i915/i915_irq.c | 2 +- > > drivers/gpu/drm/i915/i915_tasklet.h | 78 +++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_engine_cs.c | 11 ++- > > drivers/gpu/drm/i915/intel_guc_submission.c | 8 +-- > > drivers/gpu/drm/i915/intel_lrc.c | 18 ++--- > > drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +- > > 7 files changed, 104 insertions(+), 24 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/i915_tasklet.h > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index 89bf5d67cb74..98481e150e61 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -3043,9 +3043,9 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) > > * common case of recursively being called from set-wedged from inside > > * i915_reset. > > */ > > - if (!atomic_read(&engine->execlists.tasklet.count)) > > - tasklet_kill(&engine->execlists.tasklet); > > - tasklet_disable(&engine->execlists.tasklet); > > + if (i915_tasklet_is_enabled(&engine->execlists.tasklet)) > > + i915_tasklet_flush(&engine->execlists.tasklet); > > + i915_tasklet_disable(&engine->execlists.tasklet); > > > > /* > > * We're using worker to queue preemption requests from the tasklet in > > @@ -3265,7 +3265,7 @@ void i915_gem_reset(struct drm_i915_private *dev_priv, > > > > void i915_gem_reset_finish_engine(struct intel_engine_cs *engine) > > { > > - tasklet_enable(&engine->execlists.tasklet); > > + i915_tasklet_enable(&engine->execlists.tasklet); > > kthread_unpark(engine->breadcrumbs.signaler); > > > > intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL); > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index f9bc3aaa90d0..f8aff5a5aa83 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -1477,7 +1477,7 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir) > > } > > > > if (tasklet) > > - tasklet_hi_schedule(&execlists->tasklet); > > + i915_tasklet_schedule(&execlists->tasklet); > > } > > > > static void gen8_gt_irq_ack(struct drm_i915_private *i915, > > diff --git a/drivers/gpu/drm/i915/i915_tasklet.h b/drivers/gpu/drm/i915/i915_tasklet.h > > new file mode 100644 > > index 000000000000..c9f41a5ebb91 > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/i915_tasklet.h > > @@ -0,0 +1,78 @@ > > +/* > > + * SPDX-License-Identifier: GPL-2.0 > > + * > > + * Copyright © 2018 Intel Corporation > > + */ > > + > > +#ifndef _I915_TASKLET_H_ > > +#define _I915_TASKLET_H_ > > + > > +#include <linux/atomic.h> > > +#include <linux/interrupt.h> > > + > > +/** > > + * struct i915_tasklet - wrapper around tasklet_struct > > + * > > + * We want to abuse tasklets slightly, such as calling them directly. In some > > + * cases, this requires some auxiliary tracking so subclass the tasklet_struct > > + * so that we have a central place and helpers. > > + */ > > +struct i915_tasklet { > > + struct tasklet_struct base; > > +}; > > + > > +static inline void i915_tasklet_init(struct i915_tasklet *t, > > + void (*func)(unsigned long), > > + unsigned long data) > > +{ > > + tasklet_init(&t->base, func, data); > > +} > > + > > +static inline bool i915_tasklet_is_scheduled(const struct i915_tasklet *t) > > +{ > > + return test_bit(TASKLET_STATE_SCHED, &t->base.state); > > +} > > + > > +static inline bool i915_tasklet_is_locked(const struct i915_tasklet *t) > > why not is_running() ? Because I didn't want to resend the series immediately. It's confusing because it's set by tasklet_trylock and cleared by tasklet_unlock. > > + return test_bit(TASKLET_STATE_RUN, &t->base.state); > > +} > > + > > +static inline bool i915_tasklet_is_enabled(const struct i915_tasklet *t) > > +{ > > + return likely(!atomic_read(&t->base.count)); > > I am concerned that we bury the sign of racy nature out of sight and > then it comes and bites us. Oh, I wouldn't worry about that, just wait until you see what comes later :-p > > +static inline void i915_tasklet_set_func(struct i915_tasklet *t, > > + void (*func)(unsigned long data), > > + unsigned long data) > > +{ > > + tasklet_disable(&t->base); > > What does the disable/enable pair buy us here? > I was thinking about being clear about the ordering. > It looks that you want trylock and unlock > so that you can safely change the func/data. As you point out, the lock is tasklet_disable, the trylock is just a trylock. Embrace the insanity, let it flow through you. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 89bf5d67cb74..98481e150e61 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3043,9 +3043,9 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) * common case of recursively being called from set-wedged from inside * i915_reset. */ - if (!atomic_read(&engine->execlists.tasklet.count)) - tasklet_kill(&engine->execlists.tasklet); - tasklet_disable(&engine->execlists.tasklet); + if (i915_tasklet_is_enabled(&engine->execlists.tasklet)) + i915_tasklet_flush(&engine->execlists.tasklet); + i915_tasklet_disable(&engine->execlists.tasklet); /* * We're using worker to queue preemption requests from the tasklet in @@ -3265,7 +3265,7 @@ void i915_gem_reset(struct drm_i915_private *dev_priv, void i915_gem_reset_finish_engine(struct intel_engine_cs *engine) { - tasklet_enable(&engine->execlists.tasklet); + i915_tasklet_enable(&engine->execlists.tasklet); kthread_unpark(engine->breadcrumbs.signaler); intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index f9bc3aaa90d0..f8aff5a5aa83 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1477,7 +1477,7 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir) } if (tasklet) - tasklet_hi_schedule(&execlists->tasklet); + i915_tasklet_schedule(&execlists->tasklet); } static void gen8_gt_irq_ack(struct drm_i915_private *i915, diff --git a/drivers/gpu/drm/i915/i915_tasklet.h b/drivers/gpu/drm/i915/i915_tasklet.h new file mode 100644 index 000000000000..c9f41a5ebb91 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_tasklet.h @@ -0,0 +1,78 @@ +/* + * SPDX-License-Identifier: GPL-2.0 + * + * Copyright © 2018 Intel Corporation + */ + +#ifndef _I915_TASKLET_H_ +#define _I915_TASKLET_H_ + +#include <linux/atomic.h> +#include <linux/interrupt.h> + +/** + * struct i915_tasklet - wrapper around tasklet_struct + * + * We want to abuse tasklets slightly, such as calling them directly. In some + * cases, this requires some auxiliary tracking so subclass the tasklet_struct + * so that we have a central place and helpers. + */ +struct i915_tasklet { + struct tasklet_struct base; +}; + +static inline void i915_tasklet_init(struct i915_tasklet *t, + void (*func)(unsigned long), + unsigned long data) +{ + tasklet_init(&t->base, func, data); +} + +static inline bool i915_tasklet_is_scheduled(const struct i915_tasklet *t) +{ + return test_bit(TASKLET_STATE_SCHED, &t->base.state); +} + +static inline bool i915_tasklet_is_locked(const struct i915_tasklet *t) +{ + return test_bit(TASKLET_STATE_RUN, &t->base.state); +} + +static inline bool i915_tasklet_is_enabled(const struct i915_tasklet *t) +{ + return likely(!atomic_read(&t->base.count)); +} + +static inline void i915_tasklet_schedule(struct i915_tasklet *t) +{ + tasklet_hi_schedule(&t->base); +} + +static inline void i915_tasklet_flush(struct i915_tasklet *t) +{ + tasklet_kill(&t->base); +} + +static inline void i915_tasklet_enable(struct i915_tasklet *t) +{ + tasklet_enable(&t->base); +} + +static inline void i915_tasklet_disable(struct i915_tasklet *t) +{ + tasklet_disable(&t->base); +} + +static inline void i915_tasklet_set_func(struct i915_tasklet *t, + void (*func)(unsigned long data), + unsigned long data) +{ + tasklet_disable(&t->base); + + t->base.func = func; + t->base.data = data; + + tasklet_enable(&t->base); +} + +#endif /* _I915_TASKLET_H_ */ diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 70325e0824e3..5f1118ea96d8 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1032,7 +1032,7 @@ void intel_engines_park(struct drm_i915_private *i915) for_each_engine(engine, i915, id) { /* Flush the residual irq tasklets first. */ intel_engine_disarm_breadcrumbs(engine); - tasklet_kill(&engine->execlists.tasklet); + i915_tasklet_flush(&engine->execlists.tasklet); /* * We are committed now to parking the engines, make sure there @@ -1249,9 +1249,8 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine, intel_read_status_page(engine, intel_hws_csb_write_index(engine->i915)), yesno(test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)), - yesno(test_bit(TASKLET_STATE_SCHED, - &engine->execlists.tasklet.state)), - enableddisabled(!atomic_read(&engine->execlists.tasklet.count))); + yesno(i915_tasklet_is_scheduled(&engine->execlists.tasklet)), + enableddisabled(i915_tasklet_is_enabled(&engine->execlists.tasklet))); if (read >= GEN8_CSB_ENTRIES) read = 0; if (write >= GEN8_CSB_ENTRIES) @@ -1479,7 +1478,7 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine) if (!intel_engine_supports_stats(engine)) return -ENODEV; - tasklet_disable(&execlists->tasklet); + i915_tasklet_disable(&execlists->tasklet); write_seqlock_irqsave(&engine->stats.lock, flags); if (unlikely(engine->stats.enabled == ~0)) { @@ -1505,7 +1504,7 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine) unlock: write_sequnlock_irqrestore(&engine->stats.lock, flags); - tasklet_enable(&execlists->tasklet); + i915_tasklet_enable(&execlists->tasklet); return err; } diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 2feb65096966..a7afc976c3b9 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -591,7 +591,7 @@ static void inject_preempt_context(struct work_struct *work) if (WARN_ON(intel_guc_send(guc, data, ARRAY_SIZE(data)))) { execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_PREEMPT); - tasklet_schedule(&engine->execlists.tasklet); + i915_tasklet_schedule(&engine->execlists.tasklet); } } @@ -1263,10 +1263,10 @@ int intel_guc_submission_enable(struct intel_guc *guc) guc_interrupts_capture(dev_priv); for_each_engine(engine, dev_priv, id) { - struct intel_engine_execlists * const execlists = - &engine->execlists; + i915_tasklet_set_func(&engine->execlists.tasklet, + guc_submission_tasklet, + (unsigned long)engine); - execlists->tasklet.func = guc_submission_tasklet; engine->park = guc_submission_park; engine->unpark = guc_submission_unpark; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 7f98dda3c929..539fa03d7600 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1165,7 +1165,7 @@ static void queue_request(struct intel_engine_cs *engine, static void __submit_queue(struct intel_engine_cs *engine, int prio) { engine->execlists.queue_priority = prio; - tasklet_hi_schedule(&engine->execlists.tasklet); + i915_tasklet_schedule(&engine->execlists.tasklet); } static void submit_queue(struct intel_engine_cs *engine, int prio) @@ -1778,7 +1778,7 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine) /* After a GPU reset, we may have requests to replay */ if (execlists->first) - tasklet_schedule(&execlists->tasklet); + i915_tasklet_schedule(&execlists->tasklet); return 0; } @@ -2182,9 +2182,8 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *engine) * Tasklet cannot be active at this point due intel_mark_active/idle * so this is just for documentation. */ - if (WARN_ON(test_bit(TASKLET_STATE_SCHED, - &engine->execlists.tasklet.state))) - tasklet_kill(&engine->execlists.tasklet); + if (WARN_ON(i915_tasklet_is_scheduled(&engine->execlists.tasklet))) + i915_tasklet_flush(&engine->execlists.tasklet); dev_priv = engine->i915; @@ -2209,7 +2208,10 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine) engine->submit_request = execlists_submit_request; engine->cancel_requests = execlists_cancel_requests; engine->schedule = execlists_schedule; - engine->execlists.tasklet.func = execlists_submission_tasklet; + + i915_tasklet_set_func(&engine->execlists.tasklet, + execlists_submission_tasklet, + (unsigned long)engine); engine->park = NULL; engine->unpark = NULL; @@ -2303,8 +2305,8 @@ logical_ring_setup(struct intel_engine_cs *engine) engine->execlists.fw_domains = fw_domains; - tasklet_init(&engine->execlists.tasklet, - execlists_submission_tasklet, (unsigned long)engine); + i915_tasklet_init(&engine->execlists.tasklet, + execlists_submission_tasklet, (unsigned long)engine); logical_ring_default_vfuncs(engine); logical_ring_default_irqs(engine); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 010750e8ee44..f6ba354faf89 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -11,6 +11,7 @@ #include "i915_pmu.h" #include "i915_request.h" #include "i915_selftest.h" +#include "i915_tasklet.h" #include "i915_timeline.h" #include "intel_gpu_commands.h" @@ -202,7 +203,7 @@ struct intel_engine_execlists { /** * @tasklet: softirq tasklet for bottom handler */ - struct tasklet_struct tasklet; + struct i915_tasklet tasklet; /** * @default_priolist: priority list for I915_PRIORITY_NORMAL
In the next few patches, we want to abuse tasklet to avoid ksoftirqd latency along critical paths. To make that abuse easily to swallow, first coat the tasklet in a little syntactic sugar. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 8 +-- drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/i915_tasklet.h | 78 +++++++++++++++++++++ drivers/gpu/drm/i915/intel_engine_cs.c | 11 ++- drivers/gpu/drm/i915/intel_guc_submission.c | 8 +-- drivers/gpu/drm/i915/intel_lrc.c | 18 ++--- drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +- 7 files changed, 104 insertions(+), 24 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_tasklet.h