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).
-Daniel
>
> 515
> 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://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
