Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/plane: Make framebuffer refcounting the responsibility of setplane_internal callers

lock_all_ctx in setplane_internal may return -EINTR, and
__setplane_internal could return -EDEADLK. Making more
special cases for fb would make the code even harder to
read, so the easiest solution is not taking over the fb
refcount, and making callers responsible for dropping
the ref.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102707
Fixes: 13736ba3b38b ("drm/legacy: Convert setplane ioctl locking to interruptible.")
Testcase: kms_atomic_interruptible
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171220093545.613-2-maarten.lankhorst@linux.intel.com
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

+20 -22
+20 -22
drivers/gpu/drm/drm_plane.c
··· 558 558 } 559 559 560 560 /* 561 - * setplane_internal - setplane handler for internal callers 561 + * __setplane_internal - setplane handler for internal callers 562 562 * 563 - * Note that we assume an extra reference has already been taken on fb. If the 564 - * update fails, this reference will be dropped before return; if it succeeds, 565 - * the previous framebuffer (if any) will be unreferenced instead. 563 + * This function will take a reference on the new fb for the plane 564 + * on success. 566 565 * 567 566 * src_{x,y,w,h} are provided in 16.16 fixed point format 568 567 */ ··· 629 630 if (!ret) { 630 631 plane->crtc = crtc; 631 632 plane->fb = fb; 632 - fb = NULL; 633 + drm_framebuffer_get(plane->fb); 633 634 } else { 634 635 plane->old_fb = NULL; 635 636 } 636 637 637 638 out: 638 - if (fb) 639 - drm_framebuffer_put(fb); 640 639 if (plane->old_fb) 641 640 drm_framebuffer_put(plane->old_fb); 642 641 plane->old_fb = NULL; ··· 682 685 struct drm_plane *plane; 683 686 struct drm_crtc *crtc = NULL; 684 687 struct drm_framebuffer *fb = NULL; 688 + int ret; 685 689 686 690 if (!drm_core_check_feature(dev, DRIVER_MODESET)) 687 691 return -EINVAL; ··· 715 717 } 716 718 } 717 719 718 - /* 719 - * setplane_internal will take care of deref'ing either the old or new 720 - * framebuffer depending on success. 721 - */ 722 - return setplane_internal(plane, crtc, fb, 723 - plane_req->crtc_x, plane_req->crtc_y, 724 - plane_req->crtc_w, plane_req->crtc_h, 725 - plane_req->src_x, plane_req->src_y, 726 - plane_req->src_w, plane_req->src_h); 720 + ret = setplane_internal(plane, crtc, fb, 721 + plane_req->crtc_x, plane_req->crtc_y, 722 + plane_req->crtc_w, plane_req->crtc_h, 723 + plane_req->src_x, plane_req->src_y, 724 + plane_req->src_w, plane_req->src_h); 725 + 726 + if (fb) 727 + drm_framebuffer_put(fb); 728 + 729 + return ret; 727 730 } 728 731 729 732 static int drm_mode_cursor_universal(struct drm_crtc *crtc, ··· 787 788 src_h = fb->height << 16; 788 789 } 789 790 790 - /* 791 - * setplane_internal will take care of deref'ing either the old or new 792 - * framebuffer depending on success. 793 - */ 794 791 ret = __setplane_internal(crtc->cursor, crtc, fb, 795 - crtc_x, crtc_y, crtc_w, crtc_h, 796 - 0, 0, src_w, src_h, ctx); 792 + crtc_x, crtc_y, crtc_w, crtc_h, 793 + 0, 0, src_w, src_h, ctx); 794 + 795 + if (fb) 796 + drm_framebuffer_put(fb); 797 797 798 798 /* Update successful; save new cursor position, if necessary */ 799 799 if (ret == 0 && req->flags & DRM_MODE_CURSOR_MOVE) {