| Message ID | 20180327142617.6471-1-thellstrom@vmware.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 27, 2018 at 04:26:17PM +0200, Thomas Hellstrom wrote: > Use the correct helper and also return early on helper > success rather than on helper failure. > > Also explicitly return 0 in the case of no fb. > > Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Reported-by: Daniel Vetter <daniel@ffwll.ch> > --- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > index 3628a9fe705f..0f7dc9ea2657 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > @@ -494,23 +494,23 @@ int vmw_du_cursor_plane_atomic_check(struct drm_plane *plane, > struct drm_plane_state *new_state) > { > int ret = 0; > + struct drm_crtc_state *crtc_state = NULL; > struct vmw_surface *surface = NULL; > struct drm_framebuffer *fb = new_state->fb; > > - struct drm_rect src = drm_plane_state_src(new_state); > - struct drm_rect dest = drm_plane_state_dest(new_state); > - > /* Turning off */ > if (!fb) > - return ret; > + return 0; This should probably be checked after drm_atomic_helper_check_plane_state() has been called. Otherwise new_state->visible may be left with a stale value. > > - ret = drm_plane_helper_check_update(plane, new_state->crtc, fb, > - &src, &dest, > - DRM_MODE_ROTATE_0, > - DRM_PLANE_HELPER_NO_SCALING, > - DRM_PLANE_HELPER_NO_SCALING, > - true, true, &new_state->visible); > - if (!ret) > + if (new_state->crtc) > + crtc_state = drm_atomic_get_new_crtc_state(new_state->state, > + new_state->crtc); > + > + ret = drm_atomic_helper_check_plane_state(new_state, crtc_state, > + DRM_PLANE_HELPER_NO_SCALING, > + DRM_PLANE_HELPER_NO_SCALING, > + true, true); > + if (ret) > return ret; > > /* A lot of the code assumes this */ > -- > 2.14.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 03/27/2018 05:08 PM, Ville Syrjälä wrote: > On Tue, Mar 27, 2018 at 04:26:17PM +0200, Thomas Hellstrom wrote: >> Use the correct helper and also return early on helper >> success rather than on helper failure. >> >> Also explicitly return 0 in the case of no fb. >> >> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> >> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> >> Reported-by: Daniel Vetter <daniel@ffwll.ch> >> --- >> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +++++++++++----------- >> 1 file changed, 11 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> index 3628a9fe705f..0f7dc9ea2657 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> @@ -494,23 +494,23 @@ int vmw_du_cursor_plane_atomic_check(struct drm_plane *plane, >> struct drm_plane_state *new_state) >> { >> int ret = 0; >> + struct drm_crtc_state *crtc_state = NULL; >> struct vmw_surface *surface = NULL; >> struct drm_framebuffer *fb = new_state->fb; >> >> - struct drm_rect src = drm_plane_state_src(new_state); >> - struct drm_rect dest = drm_plane_state_dest(new_state); >> - >> /* Turning off */ >> if (!fb) >> - return ret; >> + return 0; > This should probably be checked after > drm_atomic_helper_check_plane_state() has been called. Otherwise > new_state->visible may be left with a stale value. > Thanks. I'll respin. /Thomas
On Tue, Mar 27, 2018 at 5:15 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote: > On 03/27/2018 05:08 PM, Ville Syrjälä wrote: >> >> On Tue, Mar 27, 2018 at 04:26:17PM +0200, Thomas Hellstrom wrote: >>> >>> Use the correct helper and also return early on helper >>> success rather than on helper failure. >>> >>> Also explicitly return 0 in the case of no fb. >>> >>> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> >>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> >>> Reported-by: Daniel Vetter <daniel@ffwll.ch> >>> --- >>> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +++++++++++----------- >>> 1 file changed, 11 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >>> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >>> index 3628a9fe705f..0f7dc9ea2657 100644 >>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >>> @@ -494,23 +494,23 @@ int vmw_du_cursor_plane_atomic_check(struct >>> drm_plane *plane, >>> struct drm_plane_state *new_state) >>> { >>> int ret = 0; >>> + struct drm_crtc_state *crtc_state = NULL; >>> struct vmw_surface *surface = NULL; >>> struct drm_framebuffer *fb = new_state->fb; >>> - struct drm_rect src = drm_plane_state_src(new_state); >>> - struct drm_rect dest = drm_plane_state_dest(new_state); >>> - >>> /* Turning off */ >>> if (!fb) >>> - return ret; >>> + return 0; >> >> This should probably be checked after >> drm_atomic_helper_check_plane_state() has been called. Otherwise >> new_state->visible may be left with a stale value. >> > > Thanks. I'll respin. I just wanted to nuke drm_plane_helper_check_update and noticed that vmwgfx is still using it. Will you respin, or did I catch it in-flight, or should I respin to get this going? Thanks, Daniel
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 3628a9fe705f..0f7dc9ea2657 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -494,23 +494,23 @@ int vmw_du_cursor_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *new_state) { int ret = 0; + struct drm_crtc_state *crtc_state = NULL; struct vmw_surface *surface = NULL; struct drm_framebuffer *fb = new_state->fb; - struct drm_rect src = drm_plane_state_src(new_state); - struct drm_rect dest = drm_plane_state_dest(new_state); - /* Turning off */ if (!fb) - return ret; + return 0; - ret = drm_plane_helper_check_update(plane, new_state->crtc, fb, - &src, &dest, - DRM_MODE_ROTATE_0, - DRM_PLANE_HELPER_NO_SCALING, - DRM_PLANE_HELPER_NO_SCALING, - true, true, &new_state->visible); - if (!ret) + if (new_state->crtc) + crtc_state = drm_atomic_get_new_crtc_state(new_state->state, + new_state->crtc); + + ret = drm_atomic_helper_check_plane_state(new_state, crtc_state, + DRM_PLANE_HELPER_NO_SCALING, + DRM_PLANE_HELPER_NO_SCALING, + true, true); + if (ret) return ret; /* A lot of the code assumes this */
Use the correct helper and also return early on helper success rather than on helper failure. Also explicitly return 0 in the case of no fb. Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Reported-by: Daniel Vetter <daniel@ffwll.ch> --- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)