From patchwork Wed Jan 4 11:34:00 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 9496507 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4916B606A9 for ; Wed, 4 Jan 2017 11:34:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3ED7F27E33 for ; Wed, 4 Jan 2017 11:34:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3146B27E63; Wed, 4 Jan 2017 11:34:09 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 329DE27E33 for ; Wed, 4 Jan 2017 11:34:08 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B66276E228; Wed, 4 Jan 2017 11:34:04 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 66D406E236; Wed, 4 Jan 2017 11:34:03 +0000 (UTC) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 04 Jan 2017 03:34:01 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,459,1477983600"; d="scan'208";a="26004109" Received: from ajczyzyn-mobl.ger.corp.intel.com (HELO patser.lan) ([10.252.14.77]) by orsmga002.jf.intel.com with ESMTP; 04 Jan 2017 03:34:01 -0800 Subject: [PATCH v2] drm/atomic: Fix outdated comment. To: Daniel Vetter , Chris Wilson , dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org References: <1481812185-19098-1-git-send-email-maarten.lankhorst@linux.intel.com> <1481812185-19098-2-git-send-email-maarten.lankhorst@linux.intel.com> <20161215151913.3ey34pnci3wahwum@phenom.ffwll.local> <7f9a208d-519c-c392-e338-eac5f37d212a@linux.intel.com> <20170104112254.GF9639@nuc-i3427.alporthouse.com> <20170104112810.ozpfxohzvtynsap7@phenom.ffwll.local> From: Maarten Lankhorst Message-ID: <7acd1c78-ea86-7776-d98d-c846186e4b88@linux.intel.com> Date: Wed, 4 Jan 2017 12:34:00 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170104112810.ozpfxohzvtynsap7@phenom.ffwll.local> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 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-Virus-Scanned: ClamAV using ClamSMTP Op 04-01-17 om 12:28 schreef Daniel Vetter: > On Wed, Jan 04, 2017 at 11:22:54AM +0000, Chris Wilson wrote: >> On Wed, Jan 04, 2017 at 12:15:55PM +0100, Maarten Lankhorst wrote: >>> Op 15-12-16 om 16:19 schreef Daniel Vetter: >>>> On Thu, Dec 15, 2016 at 03:29:42PM +0100, Maarten Lankhorst wrote: >>>>> drm_atomic_state_put is called unconditionally, so TEST_ONLY is no >>>>> different from commit. >>>>> >>>>> Signed-off-by: Maarten Lankhorst >>>> I think it'd be good to update the kerneldoc for the atomic_commit >>>> callback to mention that drivers should grab their own references using >>>> drm_atomic_state_get() when they need it. >>>> >>>> Applied this one meanwhile, thanks. >>> --->8--- >>> Commit 0853695c3ba4 ("drm: Add reference counting to drm_atomic_state") >>> adds reference counting to atomic state, but didn't update the comments >>> in drm_atomic_(nonblocking_)commit. Clarify lifetime a bit more. >>> >>> Fixes: 0853695c3ba4 ("drm: Add reference counting to drm_atomic_state") >>> Cc: # v4.10-rc1+ >>> Cc: Chris Wilson >>> Signed-off-by: Maarten Lankhorst >>> --- >>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c >>> index 681d5f97639d..6492546476b4 100644 >>> --- a/drivers/gpu/drm/drm_atomic.c >>> +++ b/drivers/gpu/drm/drm_atomic.c >>> @@ -1599,10 +1599,9 @@ EXPORT_SYMBOL(drm_atomic_check_only); >>> * more locks but encountered a deadlock. The caller must then do the usual w/w >>> * backoff dance and restart. All other errors are fatal. >>> * >>> - * Also note that on successful execution ownership of @state is transferred >>> - * from the caller of this function to the function itself. The caller must not >>> - * free or in any other way access @state. If the function fails then the caller >>> - * must clean up @state itself. >>> + * In earlier versions of the atomic api, the caller was handing its reference >>> + * of @state over to this function on success. This is no longer the case, and >>> + * callers should always call drm_atomic_state_put(). >>> * >>> * Returns: >>> * 0 on success, negative error code on failure. >>> @@ -1630,10 +1629,9 @@ EXPORT_SYMBOL(drm_atomic_commit); >>> * more locks but encountered a deadlock. The caller must then do the usual w/w >>> * backoff dance and restart. All other errors are fatal. >>> * >>> - * Also note that on successful execution ownership of @state is transferred >>> - * from the caller of this function to the function itself. The caller must not >>> - * free or in any other way access @state. If the function fails then the caller >>> - * must clean up @state itself. >>> + * In earlier versions of the atomic api, the caller was handing its reference >>> + * of @state over to this function on success. This is no longer the case, and >>> + * callers should always call drm_atomic_state_put(). >> Reviewed-by: Chris Wilson >> >> But do we really want the confusion of mentioning what the api doesn't do >> any more? > Agreed, kerneldoc should state what is (and why), maybe with a FIXME if it > only makes sense with some hystorical context and what should be done > instead. git blame is for figuring out what was. > > Maarten, can you pls respin? > -Daniel ----8<--- Commit 0853695c3ba4 ("drm: Add reference counting to drm_atomic_state") adds reference counting to atomic state, but didn't update the comments in drm_atomic_(nonblocking_)commit. Clarify lifetime a bit more. Fixes: 0853695c3ba4 ("drm: Add reference counting to drm_atomic_state") Cc: # v4.10-rc1+ Cc: Chris Wilson Reviewed-by: Chris Wilson Signed-off-by: Maarten Lankhorst diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 681d5f97639d..672f1de84d6c 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1599,10 +1599,8 @@ EXPORT_SYMBOL(drm_atomic_check_only); * more locks but encountered a deadlock. The caller must then do the usual w/w * backoff dance and restart. All other errors are fatal. * - * Also note that on successful execution ownership of @state is transferred - * from the caller of this function to the function itself. The caller must not - * free or in any other way access @state. If the function fails then the caller - * must clean up @state itself. + * This function will take its own reference on @state. + * Callers should always release their reference with drm_atomic_state_put(). * * Returns: * 0 on success, negative error code on failure. @@ -1630,10 +1628,8 @@ EXPORT_SYMBOL(drm_atomic_commit); * more locks but encountered a deadlock. The caller must then do the usual w/w * backoff dance and restart. All other errors are fatal. * - * Also note that on successful execution ownership of @state is transferred - * from the caller of this function to the function itself. The caller must not - * free or in any other way access @state. If the function fails then the caller - * must clean up @state itself. + * This function will take its own reference on @state. + * Callers should always release their reference with drm_atomic_state_put(). * * Returns: * 0 on success, negative error code on failure.