Hi! Thanks for the comments, I'll take a closer look. /Thomas On 03/27/2018 11:16 AM, Daniel Vetter wrote:
On Tue, Mar 27, 2018 at 12:05:38PM +0300, Dan Carpenter wrote:Hello Thomas Hellstrom, The patch 25db875401c8: "drm/vmwgfx: Cursor update fixes" from Jan 16, 2018, leads to the following static checker warning: drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:513 vmw_du_cursor_plane_atomic_check() info: return a literal instead of 'ret' drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 493 int vmw_du_cursor_plane_atomic_check(struct drm_plane *plane, 494 struct drm_plane_state *new_state) 495 { 496 int ret = 0; 497 struct vmw_surface *surface = NULL; 498 struct drm_framebuffer *fb = new_state->fb; 499 500 struct drm_rect src = drm_plane_state_src(new_state); 501 struct drm_rect dest = drm_plane_state_dest(new_state); 502 503 /* Turning off */ 504 if (!fb) 505 return ret; ^^^^^^^^^^ This would be more clear if it were "return 0;" 506 507 ret = drm_plane_helper_check_update(plane, new_state->crtc, fb, 508 &src, &dest, 509 DRM_MODE_ROTATE_0, 510 DRM_PLANE_HELPER_NO_SCALING, 511 DRM_PLANE_HELPER_NO_SCALING, 512 true, true, &new_state->visible); 513 if (!ret) 514 return ret; Same here. With the current code, it almost looks like that the if statement is reversed. As a newbie to this subsystem but with a bit of kernel experience, it would take me a while to figure out if the if statement is deliberate or a bug.It is a bug, drm_plane_helper_check_update returns negative errno's on failure. Also, this code should use drm_atomic_helper_check_plane_state, not the legacy wrapper, since vmwgfx is an atomic driver (but maybe that's fixed in -next already). -Daniel515 516 /* A lot of the code assumes this */ 517 if (new_state->crtc_w != 64 || new_state->crtc_h != 64) { 518 DRM_ERROR("Invalid cursor dimensions (%d, %d)\n", 519 new_state->crtc_w, new_state->crtc_h); 520 ret = -EINVAL; 521 } 522 523 if (!vmw_framebuffer_to_vfb(fb)->dmabuf) 524 surface = vmw_framebuffer_to_vfbs(fb)->surface; 525 526 if (surface && !surface->snooper.image) { 527 DRM_ERROR("surface not suitable for cursor\n"); 528 ret = -EINVAL; 529 } 530 531 return ret; 532 } regards, dan carpenter _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ&m=EmKCi9L_95tppMT-q6ky7JkUBYYdk22nnaoiRUQRZ3c&s=acr_OiqUZeMipWTzfnf3FAppMYvjv-tetZPy2bYXVxg&e=
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
