On Wed, Jan 04, 2017 at 12:34:00PM +0100, Maarten Lankhorst wrote:
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 maarten.lankhorst@linux.intel.com
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: drm-intel-fixes@lists.freedesktop.org # v4.10-rc1+ Cc: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
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 chris@chris-wilson.co.uk
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: drm-intel-fixes@lists.freedesktop.org # v4.10-rc1+ Cc: Chris Wilson chris@chris-wilson.co.uk Reviewed-by: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
Applied to drm-misc, thanks. -Daniel
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.