From patchwork Tue Apr 15 13:30:21 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 3993541 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 3E049BFF02 for ; Tue, 15 Apr 2014 14:56:52 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6BFBA20253 for ; Tue, 15 Apr 2014 14:56:51 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 945692021A for ; Tue, 15 Apr 2014 14:56:50 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 59F5E6E911; Tue, 15 Apr 2014 07:56:41 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from arroyo.ext.ti.com (arroyo.ext.ti.com [192.94.94.40]) by gabe.freedesktop.org (Postfix) with ESMTP id 7A1746E217 for ; Tue, 15 Apr 2014 06:30:25 -0700 (PDT) Received: from dflxv15.itg.ti.com ([128.247.5.124]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id s3FDUO4U012289; Tue, 15 Apr 2014 08:30:24 -0500 Received: from DFLE73.ent.ti.com (dfle73.ent.ti.com [128.247.5.110]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id s3FDUOBH020856; Tue, 15 Apr 2014 08:30:24 -0500 Received: from dflp33.itg.ti.com (10.64.6.16) by DFLE73.ent.ti.com (128.247.5.110) with Microsoft SMTP Server id 14.3.174.1; Tue, 15 Apr 2014 08:30:23 -0500 Received: from [192.168.2.6] (ileax41-snat.itg.ti.com [10.172.224.153]) by dflp33.itg.ti.com (8.14.3/8.13.8) with ESMTP id s3FDUMMF010126; Tue, 15 Apr 2014 08:30:23 -0500 Message-ID: <534D346D.4000001@ti.com> Date: Tue, 15 Apr 2014 16:30:21 +0300 From: Tomi Valkeinen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Rob Clark Subject: Re: Possible fb ref count issue with drm_plane_force_disable() References: <534684E8.9000203@ti.com> <20140411115054.GC18465@intel.com> <534B9FC0.2020700@ti.com> <534CF904.1070105@ti.com> <534D09F9.2030608@samsung.com> <534D0D8D.3070100@ti.com> In-Reply-To: X-Enigmail-Version: 1.6 X-Mailman-Approved-At: Tue, 15 Apr 2014 07:56:36 -0700 Cc: Andrzej Hajda , dri-devel X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 15/04/14 15:24, Rob Clark wrote: > probably split out omap_plane_mode_set_internal(), call that directly > from update_plane() for plane operations. And then do the refcnt > dance in the new omap_plane_mode_set() which calls _internal().. We don't actually need that. This did the trick: With quick tests, works fine so far. Still I have to say that the ref counting doesn't feel quite right (or I'm missing something). As far as I understand, the drm_mode_setplane() gets a ref to the fb, and stores it in plane->fb. Why do we take a new ref in omap_plane_update (or with the patch above, in omap_plane_mode_set), and also store it in plane->fb there? Feels like the same task is done in two places. It looks to me like drm_mode_setplane() doesn't really presume that the update_plane would set plane->fb or plane->crtc, as it does that itself (and only if update_plane has succeeded). Tomi diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index df1725247cca..3cf31ee59aac 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -225,6 +225,11 @@ int omap_plane_mode_set(struct drm_plane *plane, omap_plane->apply_done_cb.arg = arg; } + if (plane->fb) + drm_framebuffer_unreference(plane->fb); + + drm_framebuffer_reference(fb); + plane->fb = fb; plane->crtc = crtc; @@ -241,11 +246,6 @@ static int omap_plane_update(struct drm_plane *plane, struct omap_plane *omap_plane = to_omap_plane(plane); omap_plane->enabled = true; - if (plane->fb) - drm_framebuffer_unreference(plane->fb); - - drm_framebuffer_reference(fb); - /* omap_plane_mode_set() takes adjusted src */ switch (omap_plane->win.rotation & 0xf) { case BIT(DRM_ROTATE_90):