Hi Tomasz,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.16-rc7 next-20180329]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:    
https://github.com/0day-ci/linux/commits/Tomasz-Lis/drm-i915-Add-Exec-param-to-control-data-port-coherency/20180401-021313
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x010-201813 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu//drm/i915/i915_request.h:30:0,
                    from drivers/gpu//drm/i915/i915_gem_timeline.h:30,
                    from drivers/gpu//drm/i915/intel_ringbuffer.h:8,
                    from drivers/gpu//drm/i915/intel_lrc.h:27,
                    from drivers/gpu//drm/i915/i915_drv.h:63,
                    from drivers/gpu//drm/i915/i915_gem_execbuffer.c:38:
   drivers/gpu//drm/i915/i915_gem_execbuffer.c: In function 
'i915_gem_do_execbuffer':
   drivers/gpu//drm/i915/i915_gem.h:47:54: warning: statement with no effect 
[-Wunused-value]
    #define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> drivers/gpu//drm/i915/i915_gem_execbuffer.c:2389:2: note: in expansion of 
>> macro 'GEM_WARN_ON'
     GEM_WARN_ON(err);
     ^~~~~~~~~~~

vim +/GEM_WARN_ON +2389 drivers/gpu//drm/i915/i915_gem_execbuffer.c

  2182  
  2183  static int
  2184  i915_gem_do_execbuffer(struct drm_device *dev,
  2185                         struct drm_file *file,
  2186                         struct drm_i915_gem_execbuffer2 *args,
  2187                         struct drm_i915_gem_exec_object2 *exec,
  2188                         struct drm_syncobj **fences)
  2189  {
  2190          struct i915_execbuffer eb;
  2191          struct dma_fence *in_fence = NULL;
  2192          struct sync_file *out_fence = NULL;
  2193          int out_fence_fd = -1;
  2194          int err;
  2195  
  2196          BUILD_BUG_ON(__EXEC_INTERNAL_FLAGS & 
~__I915_EXEC_ILLEGAL_FLAGS);
  2197          BUILD_BUG_ON(__EXEC_OBJECT_INTERNAL_FLAGS &
  2198                       ~__EXEC_OBJECT_UNKNOWN_FLAGS);
  2199  
  2200          eb.i915 = to_i915(dev);
  2201          eb.file = file;
  2202          eb.args = args;
  2203          if (DBG_FORCE_RELOC || !(args->flags & I915_EXEC_NO_RELOC))
  2204                  args->flags |= __EXEC_HAS_RELOC;
  2205  
  2206          eb.exec = exec;
  2207          eb.vma = (struct i915_vma **)(exec + args->buffer_count + 1);
  2208          eb.vma[0] = NULL;
  2209          eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1);
  2210  
  2211          eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
  2212          if (USES_FULL_PPGTT(eb.i915))
  2213                  eb.invalid_flags |= EXEC_OBJECT_NEEDS_GTT;
  2214          reloc_cache_init(&eb.reloc_cache, eb.i915);
  2215  
  2216          eb.buffer_count = args->buffer_count;
  2217          eb.batch_start_offset = args->batch_start_offset;
  2218          eb.batch_len = args->batch_len;
  2219  
  2220          eb.batch_flags = 0;
  2221          if (args->flags & I915_EXEC_SECURE) {
  2222                  if (!drm_is_current_master(file) || 
!capable(CAP_SYS_ADMIN))
  2223                      return -EPERM;
  2224  
  2225                  eb.batch_flags |= I915_DISPATCH_SECURE;
  2226          }
  2227          if (args->flags & I915_EXEC_IS_PINNED)
  2228                  eb.batch_flags |= I915_DISPATCH_PINNED;
  2229  
  2230          eb.engine = eb_select_engine(eb.i915, file, args);
  2231          if (!eb.engine)
  2232                  return -EINVAL;
  2233  
  2234          if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
  2235                  if (!HAS_RESOURCE_STREAMER(eb.i915)) {
  2236                          DRM_DEBUG("RS is only allowed for Haswell, Gen8 
and above\n");
  2237                          return -EINVAL;
  2238                  }
  2239                  if (eb.engine->id != RCS) {
  2240                          DRM_DEBUG("RS is not available on %s\n",
  2241                                   eb.engine->name);
  2242                          return -EINVAL;
  2243                  }
  2244  
  2245                  eb.batch_flags |= I915_DISPATCH_RS;
  2246          }
  2247  
  2248          if (args->flags & I915_EXEC_DATA_PORT_COHERENT) {
  2249                  if (INTEL_GEN(eb.i915) < 9) {
  2250                          DRM_DEBUG("Data Port Coherency is only allowed 
for Gen9 and above\n");
  2251                          return -EINVAL;
  2252                  }
  2253                  if (eb.engine->class != RENDER_CLASS) {
  2254                          DRM_DEBUG("Data Port Coherency is not available 
on %s\n",
  2255                                   eb.engine->name);
  2256                          return -EINVAL;
  2257                  }
  2258          }
  2259  
  2260          if (args->flags & I915_EXEC_FENCE_IN) {
  2261                  in_fence = 
sync_file_get_fence(lower_32_bits(args->rsvd2));
  2262                  if (!in_fence)
  2263                          return -EINVAL;
  2264          }
  2265  
  2266          if (args->flags & I915_EXEC_FENCE_OUT) {
  2267                  out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
  2268                  if (out_fence_fd < 0) {
  2269                          err = out_fence_fd;
  2270                          goto err_in_fence;
  2271                  }
  2272          }
  2273  
  2274          err = eb_create(&eb);
  2275          if (err)
  2276                  goto err_out_fence;
  2277  
  2278          GEM_BUG_ON(!eb.lut_size);
  2279  
  2280          err = eb_select_context(&eb);
  2281          if (unlikely(err))
  2282                  goto err_destroy;
  2283  
  2284          /*
  2285           * Take a local wakeref for preparing to dispatch the execbuf as
  2286           * we expect to access the hardware fairly frequently in the
  2287           * process. Upon first dispatch, we acquire another prolonged
  2288           * wakeref that we hold until the GPU has been idle for at least
  2289           * 100ms.
  2290           */
  2291          intel_runtime_pm_get(eb.i915);
  2292  
  2293          err = i915_mutex_lock_interruptible(dev);
  2294          if (err)
  2295                  goto err_rpm;
  2296  
  2297          err = eb_relocate(&eb);
  2298          if (err) {
  2299                  /*
  2300                   * If the user expects the execobject.offset and
  2301                   * reloc.presumed_offset to be an exact match,
  2302                   * as for using NO_RELOC, then we cannot update
  2303                   * the execobject.offset until we have completed
  2304                   * relocation.
  2305                   */
  2306                  args->flags &= ~__EXEC_HAS_RELOC;
  2307                  goto err_vma;
  2308          }
  2309  
  2310          if (unlikely(*eb.batch->exec_flags & EXEC_OBJECT_WRITE)) {
  2311                  DRM_DEBUG("Attempting to use self-modifying batch 
buffer\n");
  2312                  err = -EINVAL;
  2313                  goto err_vma;
  2314          }
  2315          if (eb.batch_start_offset > eb.batch->size ||
  2316              eb.batch_len > eb.batch->size - eb.batch_start_offset) {
  2317                  DRM_DEBUG("Attempting to use out-of-bounds batch\n");
  2318                  err = -EINVAL;
  2319                  goto err_vma;
  2320          }
  2321  
  2322          if (eb_use_cmdparser(&eb)) {
  2323                  struct i915_vma *vma;
  2324  
  2325                  vma = eb_parse(&eb, drm_is_current_master(file));
  2326                  if (IS_ERR(vma)) {
  2327                          err = PTR_ERR(vma);
  2328                          goto err_vma;
  2329                  }
  2330  
  2331                  if (vma) {
  2332                          /*
  2333                           * Batch parsed and accepted:
  2334                           *
  2335                           * Set the DISPATCH_SECURE bit to remove the 
NON_SECURE
  2336                           * bit from MI_BATCH_BUFFER_START commands 
issued in
  2337                           * the dispatch_execbuffer implementations. We
  2338                           * specifically don't want that set on batches 
the
  2339                           * command parser has accepted.
  2340                           */
  2341                          eb.batch_flags |= I915_DISPATCH_SECURE;
  2342                          eb.batch_start_offset = 0;
  2343                          eb.batch = vma;
  2344                  }
  2345          }
  2346  
  2347          if (eb.batch_len == 0)
  2348                  eb.batch_len = eb.batch->size - eb.batch_start_offset;
  2349  
  2350          /*
  2351           * snb/ivb/vlv conflate the "batch in ppgtt" bit with the 
"non-secure
  2352           * batch" bit. Hence we need to pin secure batches into the 
global gtt.
  2353           * hsw should have this fixed, but bdw mucks it up again. */
  2354          if (eb.batch_flags & I915_DISPATCH_SECURE) {
  2355                  struct i915_vma *vma;
  2356  
  2357                  /*
  2358                   * So on first glance it looks freaky that we pin the 
batch here
  2359                   * outside of the reservation loop. But:
  2360                   * - The batch is already pinned into the relevant 
ppgtt, so we
  2361                   *   already have the backing storage fully allocated.
  2362                   * - No other BO uses the global gtt (well contexts, 
but meh),
  2363                   *   so we don't really have issues with multiple 
objects not
  2364                   *   fitting due to fragmentation.
  2365                   * So this is actually safe.
  2366                   */
  2367                  vma = i915_gem_object_ggtt_pin(eb.batch->obj, NULL, 0, 
0, 0);
  2368                  if (IS_ERR(vma)) {
  2369                          err = PTR_ERR(vma);
  2370                          goto err_vma;
  2371                  }
  2372  
  2373                  eb.batch = vma;
  2374          }
  2375  
  2376          /* All GPU relocation batches must be submitted prior to the 
user rq */
  2377          GEM_BUG_ON(eb.reloc_cache.rq);
  2378  
  2379          /* Allocate a request for this batch buffer nice and early. */
  2380          eb.request = i915_request_alloc(eb.engine, eb.ctx);
  2381          if (IS_ERR(eb.request)) {
  2382                  err = PTR_ERR(eb.request);
  2383                  goto err_batch_unpin;
  2384          }
  2385  
  2386          /* Emit the switch of data port coherency state if needed */
  2387          err = intel_lr_context_modify_data_port_coherency(eb.request,
  2388                          (args->flags & I915_EXEC_DATA_PORT_COHERENT) != 
0);
> 2389          GEM_WARN_ON(err);
  2390  
  2391          if (in_fence) {
  2392                  err = i915_request_await_dma_fence(eb.request, 
in_fence);
  2393                  if (err < 0)
  2394                          goto err_request;
  2395          }
  2396  
  2397          if (fences) {
  2398                  err = await_fence_array(&eb, fences);
  2399                  if (err)
  2400                          goto err_request;
  2401          }
  2402  
  2403          if (out_fence_fd != -1) {
  2404                  out_fence = sync_file_create(&eb.request->fence);
  2405                  if (!out_fence) {
  2406                          err = -ENOMEM;
  2407                          goto err_request;
  2408                  }
  2409          }
  2410  
  2411          /*
  2412           * Whilst this request exists, batch_obj will be on the
  2413           * active_list, and so will hold the active reference. Only 
when this
  2414           * request is retired will the the batch_obj be moved onto the
  2415           * inactive_list and lose its active reference. Hence we do not 
need
  2416           * to explicitly hold another reference here.
  2417           */
  2418          eb.request->batch = eb.batch;
  2419  
  2420          trace_i915_request_queue(eb.request, eb.batch_flags);
  2421          err = eb_submit(&eb);
  2422  err_request:
  2423          __i915_request_add(eb.request, err == 0);
  2424          add_to_client(eb.request, file);
  2425  
  2426          if (fences)
  2427                  signal_fence_array(&eb, fences);
  2428  
  2429          if (out_fence) {
  2430                  if (err == 0) {
  2431                          fd_install(out_fence_fd, out_fence->file);
  2432                          args->rsvd2 &= GENMASK_ULL(31, 0); /* keep 
in-fence */
  2433                          args->rsvd2 |= (u64)out_fence_fd << 32;
  2434                          out_fence_fd = -1;
  2435                  } else {
  2436                          fput(out_fence->file);
  2437                  }
  2438          }
  2439  
  2440  err_batch_unpin:
  2441          if (eb.batch_flags & I915_DISPATCH_SECURE)
  2442                  i915_vma_unpin(eb.batch);
  2443  err_vma:
  2444          if (eb.exec)
  2445                  eb_release_vmas(&eb);
  2446          mutex_unlock(&dev->struct_mutex);
  2447  err_rpm:
  2448          intel_runtime_pm_put(eb.i915);
  2449          i915_gem_context_put(eb.ctx);
  2450  err_destroy:
  2451          eb_destroy(&eb);
  2452  err_out_fence:
  2453          if (out_fence_fd != -1)
  2454                  put_unused_fd(out_fence_fd);
  2455  err_in_fence:
  2456          dma_fence_put(in_fence);
  2457          return err;
  2458  }
  2459  

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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