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 <l...@intel.com>

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-...@lists.01.org

Attachment: .config.gz
Description: application/gzip

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

Reply via email to