Re: [PATCH 4/4] drm/i915/gt: Distinguish the virtual breadcrumbs from the irq breadcrumbs

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

 



Hi Chris,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip next-20200717]
[cannot apply to linus/master v5.8-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-i915-Remove-requirement-for-holding-i915_request-lock-for-breadcrumbs/20200717-173754
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@xxxxxxxxx>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:71:6: warning: no previous prototype for 'intel_breadcrumbs_park' [-Wmissing-prototypes]
      71 | void intel_breadcrumbs_park(struct intel_breadcrumbs *b)
         |      ^~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:248:1: warning: no previous prototype for 'intel_breadcrumbs_create' [-Wmissing-prototypes]
     248 | intel_breadcrumbs_create(struct intel_engine_cs *irq_engine)
         | ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:269:6: warning: no previous prototype for 'intel_breadcrumbs_reset' [-Wmissing-prototypes]
     269 | void intel_breadcrumbs_reset(struct intel_breadcrumbs *b)
         |      ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:286:6: warning: no previous prototype for 'intel_breadcrumbs_free' [-Wmissing-prototypes]
     286 | void intel_breadcrumbs_free(struct intel_breadcrumbs *b)
         |      ^~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:344:6: warning: no previous prototype for 'i915_request_enable_breadcrumb' [-Wmissing-prototypes]
     344 | bool i915_request_enable_breadcrumb(struct i915_request *rq)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:400:6: warning: no previous prototype for 'i915_request_cancel_breadcrumb' [-Wmissing-prototypes]
     400 | void i915_request_cancel_breadcrumb(struct i915_request *rq)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:423:6: warning: no previous prototype for 'intel_engine_print_breadcrumbs' [-Wmissing-prototypes]
     423 | void intel_engine_print_breadcrumbs(struct intel_engine_cs *engine,
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/intel_breadcrumbs_park +71 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c

    70	
  > 71	void intel_breadcrumbs_park(struct intel_breadcrumbs *b)
    72	{
    73		unsigned long flags;
    74	
    75		if (!READ_ONCE(b->irq_armed))
    76			return;
    77	
    78		spin_lock_irqsave(&b->irq_lock, flags);
    79		if (b->irq_armed)
    80			__intel_breadcrumbs_disarm_irq(b);
    81		spin_unlock_irqrestore(&b->irq_lock, flags);
    82	}
    83	
    84	static inline bool __request_completed(const struct i915_request *rq)
    85	{
    86		return i915_seqno_passed(__hwsp_seqno(rq), rq->fence.seqno);
    87	}
    88	
    89	__maybe_unused static bool
    90	check_signal_order(struct intel_context *ce, struct i915_request *rq)
    91	{
    92		if (!list_is_last(&rq->signal_link, &ce->signals) &&
    93		    i915_seqno_passed(rq->fence.seqno,
    94				      list_next_entry(rq, signal_link)->fence.seqno))
    95			return false;
    96	
    97		if (!list_is_first(&rq->signal_link, &ce->signals) &&
    98		    i915_seqno_passed(list_prev_entry(rq, signal_link)->fence.seqno,
    99				      rq->fence.seqno))
   100			return false;
   101	
   102		return true;
   103	}
   104	
   105	static bool
   106	__dma_fence_signal(struct dma_fence *fence)
   107	{
   108		return !test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
   109	}
   110	
   111	static void
   112	__dma_fence_signal__timestamp(struct dma_fence *fence, ktime_t timestamp)
   113	{
   114		fence->timestamp = timestamp;
   115		set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
   116		trace_dma_fence_signaled(fence);
   117	}
   118	
   119	static void
   120	__dma_fence_signal__notify(struct dma_fence *fence,
   121				   const struct list_head *list)
   122	{
   123		struct dma_fence_cb *cur, *tmp;
   124	
   125		lockdep_assert_held(fence->lock);
   126	
   127		list_for_each_entry_safe(cur, tmp, list, node) {
   128			INIT_LIST_HEAD(&cur->node);
   129			cur->func(fence, cur);
   130		}
   131	}
   132	
   133	static void add_retire(struct intel_breadcrumbs *b, struct intel_timeline *tl)
   134	{
   135		if (b->irq_engine)
   136			intel_engine_add_retire(b->irq_engine, tl);
   137	}
   138	
   139	static void __signal_request(struct i915_request *rq, struct list_head *signals)
   140	{
   141		clear_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
   142	
   143		if (!__dma_fence_signal(&rq->fence))
   144			return;
   145	
   146		i915_request_get(rq);
   147		list_add_tail(&rq->signal_link, signals);
   148	}
   149	
   150	static void signal_irq_work(struct irq_work *work)
   151	{
   152		struct intel_breadcrumbs *b = container_of(work, typeof(*b), irq_work);
   153		const ktime_t timestamp = ktime_get();
   154		struct intel_context *ce, *cn;
   155		struct list_head *pos, *next;
   156		LIST_HEAD(signal);
   157	
   158		spin_lock(&b->irq_lock);
   159	
   160		if (b->irq_armed && list_empty(&b->signalers))
   161			__intel_breadcrumbs_disarm_irq(b);
   162	
   163		list_splice_init(&b->signaled_requests, &signal);
   164	
   165		list_for_each_entry_safe(ce, cn, &b->signalers, signal_link) {
   166			GEM_BUG_ON(list_empty(&ce->signals));
   167	
   168			list_for_each_safe(pos, next, &ce->signals) {
   169				struct i915_request *rq =
   170					list_entry(pos, typeof(*rq), signal_link);
   171	
   172				GEM_BUG_ON(!check_signal_order(ce, rq));
   173				if (!__request_completed(rq))
   174					break;
   175	
   176				/*
   177				 * Queue for execution after dropping the signaling
   178				 * spinlock as the callback chain may end up adding
   179				 * more signalers to the same context or engine.
   180				 */
   181				__signal_request(rq, &signal);
   182			}
   183	
   184			/*
   185			 * We process the list deletion in bulk, only using a list_add
   186			 * (not list_move) above but keeping the status of
   187			 * rq->signal_link known with the I915_FENCE_FLAG_SIGNAL bit.
   188			 */
   189			if (!list_is_first(pos, &ce->signals)) {
   190				/* Advance the list to the first incomplete request */
   191				__list_del_many(&ce->signals, pos);
   192				if (&ce->signals == pos) { /* now empty */
   193					list_del_init(&ce->signal_link);
   194					add_retire(b, ce->timeline);
   195				}
   196			}
   197		}
   198	
   199		spin_unlock(&b->irq_lock);
   200	
   201		list_for_each_safe(pos, next, &signal) {
   202			struct i915_request *rq =
   203				list_entry(pos, typeof(*rq), signal_link);
   204			struct list_head cb_list;
   205	
   206			spin_lock(&rq->lock);
   207			list_replace(&rq->fence.cb_list, &cb_list);
   208			__dma_fence_signal__timestamp(&rq->fence, timestamp);
   209			__dma_fence_signal__notify(&rq->fence, &cb_list);
   210			spin_unlock(&rq->lock);
   211	
   212			i915_request_put(rq);
   213		}
   214	}
   215	
   216	static void __intel_breadcrumbs_arm_irq(struct intel_breadcrumbs *b)
   217	{
   218		lockdep_assert_held(&b->irq_lock);
   219	
   220		if (b->irq_armed)
   221			return;
   222	
   223		GEM_BUG_ON(!b->irq_engine);
   224		if (!intel_gt_pm_get_if_awake(b->irq_engine->gt))
   225			return;
   226	
   227		/*
   228		 * The breadcrumb irq will be disarmed on the interrupt after the
   229		 * waiters are signaled. This gives us a single interrupt window in
   230		 * which we can add a new waiter and avoid the cost of re-enabling
   231		 * the irq.
   232		 */
   233		WRITE_ONCE(b->irq_armed, true);
   234	
   235		/*
   236		 * Since we are waiting on a request, the GPU should be busy
   237		 * and should have its own rpm reference. This is tracked
   238		 * by i915->gt.awake, we can forgo holding our own wakref
   239		 * for the interrupt as before i915->gt.awake is released (when
   240		 * the driver is idle) we disarm the breadcrumbs.
   241		 */
   242	
   243		if (!b->irq_enabled++)
   244			irq_enable(b->irq_engine);
   245	}
   246	
   247	struct intel_breadcrumbs *
 > 248	intel_breadcrumbs_create(struct intel_engine_cs *irq_engine)
   249	{
   250		struct intel_breadcrumbs *b;
   251	
   252		b = kzalloc(sizeof(*b), GFP_KERNEL);
   253		if (!b)
   254			return NULL;
   255	
   256		spin_lock_init(&b->irq_lock);
   257		INIT_LIST_HEAD(&b->signalers);
   258		INIT_LIST_HEAD(&b->signaled_requests);
   259	
   260		init_irq_work(&b->irq_work, signal_irq_work);
   261	
   262		b->irq_engine = irq_engine;
   263		if (!irq_engine)
   264			b->irq_armed = true; /* fake HW, used for irq_work */
   265	
   266		return b;
   267	}
   268	
 > 269	void intel_breadcrumbs_reset(struct intel_breadcrumbs *b)
   270	{
   271		unsigned long flags;
   272	
   273		if (!b->irq_engine)
   274			return;
   275	
   276		spin_lock_irqsave(&b->irq_lock, flags);
   277	
   278		if (b->irq_enabled)
   279			irq_enable(b->irq_engine);
   280		else
   281			irq_disable(b->irq_engine);
   282	
   283		spin_unlock_irqrestore(&b->irq_lock, flags);
   284	}
   285	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux