[bug report] drm/vmwgfx: Cursor update fixes

From: Dan Carpenter <dan.carpenter@oracle.com>
To: thellstrom@vmware.com
Cc: dri-devel@lists.freedesktop.org
Subject: [bug report] drm/vmwgfx: Cursor update fixes
Date: Tue, 27 Mar 2018 12:05:38 +0300	[thread overview]
Message-ID: <20180327090538.GA23268@mwanda> (raw)

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.

   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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel