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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
