All of lore.kernel.org
 help / color / mirror / Atom feed
* convert dc to using krefs for object reference counts
@ 2017-10-03  2:38 Dave Airlie
       [not found] ` <20171003023902.5036-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Dave Airlie @ 2017-10-03  2:38 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

This series might be a bit of a too big step :-)

So in the kernel we use krefs for reference counts for lots of good
reasons, the main one, is no bugs with roll your own atomic reference
counting for structs, which lots of this code is.

I've ported all the structs I could find using hand rolled atomic
reference counters to use krefs.

Dave.

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/6] amdgpu/dc: convert dc_transfer to use a kref.
       [not found] ` <20171003023902.5036-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-10-03  2:38   ` Dave Airlie
       [not found]     ` <20171003023902.5036-2-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2017-10-03  2:38   ` [PATCH 2/6] amdgpu/dc: convert dc_gamma to kref reference counting Dave Airlie
                     ` (6 subsequent siblings)
  7 siblings, 1 reply; 10+ messages in thread
From: Dave Airlie @ 2017-10-03  2:38 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Dave Airlie <airlied@redhat.com>

Rolling your own atomic ref counts is frowned upon.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 17 +++++++++--------
 drivers/gpu/drm/amd/display/dc/dc.h              |  2 +-
 drivers/gpu/drm/amd/display/dc/os_types.h        |  2 ++
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
index ff07105..c2168df 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
@@ -161,17 +161,18 @@ struct dc_gamma *dc_create_gamma()
 
 void dc_transfer_func_retain(struct dc_transfer_func *tf)
 {
-	ASSERT(atomic_read(&tf->ref_count) > 0);
-	atomic_inc(&tf->ref_count);
+	kref_get(&tf->refcount);
 }
 
-void dc_transfer_func_release(struct dc_transfer_func *tf)
+static void dc_transfer_func_free(struct kref *kref)
 {
-	ASSERT(atomic_read(&tf->ref_count) > 0);
-	atomic_dec(&tf->ref_count);
+	struct dc_transfer_func *tf = container_of(kref, struct dc_transfer_func, refcount);
+	kfree(tf);
+}
 
-	if (atomic_read(&tf->ref_count) == 0)
-		kfree(tf);
+void dc_transfer_func_release(struct dc_transfer_func *tf)
+{
+	kref_put(&tf->refcount, dc_transfer_func_free);
 }
 
 struct dc_transfer_func *dc_create_transfer_func()
@@ -181,7 +182,7 @@ struct dc_transfer_func *dc_create_transfer_func()
 	if (tf == NULL)
 		goto alloc_fail;
 
-	atomic_inc(&tf->ref_count);
+	kref_init(&tf->refcount);
 
 	return tf;
 
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index 9c0e000..17f0c73 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -334,11 +334,11 @@ enum dc_transfer_func_predefined {
 };
 
 struct dc_transfer_func {
+	struct kref refcount;
 	struct dc_transfer_func_distributed_points tf_pts;
 	enum dc_transfer_func_type type;
 	enum dc_transfer_func_predefined tf;
 	struct dc_context *ctx;
-	atomic_t ref_count;
 };
 
 /*
diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h b/drivers/gpu/drm/amd/display/dc/os_types.h
index 27ed2a6..e0cd527 100644
--- a/drivers/gpu/drm/amd/display/dc/os_types.h
+++ b/drivers/gpu/drm/amd/display/dc/os_types.h
@@ -32,6 +32,8 @@
 #include <linux/types.h>
 #include <drm/drmP.h>
 
+#include <linux/kref.h>
+
 #include "cgs_linux.h"
 
 #if defined(__BIG_ENDIAN) && !defined(BIGENDIAN_CPU)
-- 
2.9.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/6] amdgpu/dc: convert dc_gamma to kref reference counting.
       [not found] ` <20171003023902.5036-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2017-10-03  2:38   ` [PATCH 1/6] amdgpu/dc: convert dc_transfer to use a kref Dave Airlie
@ 2017-10-03  2:38   ` Dave Airlie
  2017-10-03  2:38   ` [PATCH 3/6] amdgpu/dc: use kref for dc_plane_state Dave Airlie
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Dave Airlie @ 2017-10-03  2:38 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Dave Airlie <airlied@redhat.com>

Rolling your own reference counting is frowned upon.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 19 +++++++++----------
 drivers/gpu/drm/amd/display/dc/dc_hw_types.h     |  4 +---
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
index c2168df..0950075 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
@@ -129,18 +129,18 @@ void dc_plane_state_release(struct dc_plane_state *plane_state)
 
 void dc_gamma_retain(struct dc_gamma *gamma)
 {
-	ASSERT(atomic_read(&gamma->ref_count) > 0);
-	atomic_inc(&gamma->ref_count);
+	kref_get(&gamma->refcount);
 }
 
-void dc_gamma_release(struct dc_gamma **gamma)
+static void dc_gamma_free(struct kref *kref)
 {
-	ASSERT(atomic_read(&(*gamma)->ref_count) > 0);
-	atomic_dec(&(*gamma)->ref_count);
-
-	if (atomic_read(&(*gamma)->ref_count) == 0)
-		kfree((*gamma));
+	struct dc_gamma *gamma = container_of(kref, struct dc_gamma, refcount);
+	kfree(gamma);
+}
 
+void dc_gamma_release(struct dc_gamma **gamma)
+{
+	kref_put(&(*gamma)->refcount, dc_gamma_free);
 	*gamma = NULL;
 }
 
@@ -151,8 +151,7 @@ struct dc_gamma *dc_create_gamma()
 	if (gamma == NULL)
 		goto alloc_fail;
 
-	atomic_inc(&gamma->ref_count);
-
+	kref_init(&gamma->refcount);
 	return gamma;
 
 alloc_fail:
diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
index 94f83cd..4ab1093 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
@@ -420,6 +420,7 @@ enum dc_gamma_type {
 };
 
 struct dc_gamma {
+	struct kref refcount;
 	enum dc_gamma_type type;
 	unsigned int num_entries;
 
@@ -431,9 +432,6 @@ struct dc_gamma {
 
 	/* private to DC core */
 	struct dc_context *ctx;
-
-	/* private to dc_surface.c */
-	atomic_t ref_count;
 };
 
 /* Used by both ipp amd opp functions*/
-- 
2.9.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/6] amdgpu/dc: use kref for dc_plane_state.
       [not found] ` <20171003023902.5036-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2017-10-03  2:38   ` [PATCH 1/6] amdgpu/dc: convert dc_transfer to use a kref Dave Airlie
  2017-10-03  2:38   ` [PATCH 2/6] amdgpu/dc: convert dc_gamma to kref reference counting Dave Airlie
@ 2017-10-03  2:38   ` Dave Airlie
  2017-10-03  2:39   ` [PATCH 4/6] amdgpu/dc: convert dc_stream_state to kref Dave Airlie
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Dave Airlie @ 2017-10-03  2:38 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 20 ++++++++++----------
 drivers/gpu/drm/amd/display/dc/dc.h              |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
index 0950075..d43783a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
@@ -71,8 +71,8 @@ struct dc_plane_state *dc_create_plane_state(struct dc *dc)
 	if (NULL == plane_state)
 		return NULL;
 
+	kref_init(&plane_state->refcount);
 	construct(core_dc->ctx, plane_state);
-	atomic_inc(&plane_state->ref_count);
 
 	return plane_state;
 }
@@ -112,19 +112,19 @@ const struct dc_plane_status *dc_plane_get_status(
 
 void dc_plane_state_retain(struct dc_plane_state *plane_state)
 {
-	ASSERT(atomic_read(&plane_state->ref_count) > 0);
-	atomic_inc(&plane_state->ref_count);
+	kref_get(&plane_state->refcount);
 }
 
-void dc_plane_state_release(struct dc_plane_state *plane_state)
+static void dc_plane_state_free(struct kref *kref)
 {
-	ASSERT(atomic_read(&plane_state->ref_count) > 0);
-	atomic_dec(&plane_state->ref_count);
+	struct dc_plane_state *plane_state = container_of(kref, struct dc_plane_state, refcount);
+	destruct(plane_state);
+	kfree(plane_state);
+}
 
-	if (atomic_read(&plane_state->ref_count) == 0) {
-		destruct(plane_state);
-		kfree(plane_state);
-	}
+void dc_plane_state_release(struct dc_plane_state *plane_state)
+{
+	kref_put(&plane_state->refcount, dc_plane_state_free);
 }
 
 void dc_gamma_retain(struct dc_gamma *gamma)
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index 17f0c73..c4d89d2 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -385,7 +385,7 @@ struct dc_plane_state {
 
 	/* private to dc_surface.c */
 	enum dc_irq_source irq_source;
-	atomic_t ref_count;
+	struct kref refcount;
 };
 
 struct dc_plane_info {
-- 
2.9.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/6] amdgpu/dc: convert dc_stream_state to kref.
       [not found] ` <20171003023902.5036-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (2 preceding siblings ...)
  2017-10-03  2:38   ` [PATCH 3/6] amdgpu/dc: use kref for dc_plane_state Dave Airlie
@ 2017-10-03  2:39   ` Dave Airlie
  2017-10-03  2:39   ` [PATCH 5/6] amdgpu/dc: convert dc_sink " Dave Airlie
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Dave Airlie @ 2017-10-03  2:39 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 22 +++++++++++-----------
 drivers/gpu/drm/amd/display/dc/dc.h             |  2 +-
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
index a9919641..23df7bc 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -118,20 +118,21 @@ static void destruct(struct dc_stream_state *stream)
 
 void dc_stream_retain(struct dc_stream_state *stream)
 {
-	ASSERT(atomic_read(&stream->ref_count) > 0);
-	atomic_inc(&stream->ref_count);
+	kref_get(&stream->refcount);
+}
+
+static void dc_stream_free(struct kref *kref)
+{
+	struct dc_stream_state *stream = container_of(kref, struct dc_stream_state, refcount);
+
+	destruct(stream);
+	kfree(stream);
 }
 
 void dc_stream_release(struct dc_stream_state *stream)
 {
 	if (stream != NULL) {
-		ASSERT(atomic_read(&stream->ref_count) > 0);
-		atomic_dec(&stream->ref_count);
-
-		if (atomic_read(&stream->ref_count) == 0) {
-			destruct(stream);
-			kfree(stream);
-		}
+		kref_put(&stream->refcount, dc_stream_free);
 	}
 }
 
@@ -149,10 +150,9 @@ struct dc_stream_state *dc_create_stream_for_sink(
 
 	construct(stream, sink);
 
-	atomic_inc(&stream->ref_count);
+	kref_init(&stream->refcount);
 
 	return stream;
-
 }
 
 struct dc_stream_status *dc_stream_get_status(
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index c4d89d2..abba134 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -562,7 +562,7 @@ struct dc_stream_state {
 	struct dc_stream_status status;
 
 	/* from stream struct */
-	atomic_t ref_count;
+	struct kref refcount;
 };
 
 struct dc_stream_update {
-- 
2.9.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/6] amdgpu/dc: convert dc_sink to kref.
       [not found] ` <20171003023902.5036-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (3 preceding siblings ...)
  2017-10-03  2:39   ` [PATCH 4/6] amdgpu/dc: convert dc_stream_state to kref Dave Airlie
@ 2017-10-03  2:39   ` Dave Airlie
  2017-10-03  2:39   ` [PATCH 6/6] amdgpu/dc: use kref for dc_state Dave Airlie
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Dave Airlie @ 2017-10-03  2:39 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Dave Airlie <airlied@redhat.com>

Refcounts use krefs.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_sink.c | 20 ++++++++++----------
 drivers/gpu/drm/amd/display/dc/dc.h           |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_sink.c b/drivers/gpu/drm/amd/display/dc/core/dc_sink.c
index b3bbafc..f2b2e82 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_sink.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_sink.c
@@ -63,19 +63,19 @@ static bool construct(struct dc_sink *sink, const struct dc_sink_init_data *init
 
 void dc_sink_retain(struct dc_sink *sink)
 {
-	ASSERT(atomic_read(&sink->ref_count) > 0);
-	atomic_inc(&sink->ref_count);
+	kref_get(&sink->refcount);
 }
 
-void dc_sink_release(struct dc_sink *sink)
+static void dc_sink_free(struct kref *kref)
 {
-	ASSERT(atomic_read(&sink->ref_count) > 0);
-	atomic_dec(&sink->ref_count);
+	struct dc_sink *sink = container_of(kref, struct dc_sink, refcount);
+	destruct(sink);
+	kfree(sink);
+}
 
-	if (atomic_read(&sink->ref_count) == 0) {
-		destruct(sink);
-		kfree(sink);
-	}
+void dc_sink_release(struct dc_sink *sink)
+{
+	kref_put(&sink->refcount, dc_sink_free);
 }
 
 struct dc_sink *dc_sink_create(const struct dc_sink_init_data *init_params)
@@ -88,7 +88,7 @@ struct dc_sink *dc_sink_create(const struct dc_sink_init_data *init_params)
 	if (false == construct(sink, init_params))
 		goto construct_fail;
 
-	atomic_inc(&sink->ref_count);
+	kref_init(&sink->refcount);
 
 	return sink;
 
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index abba134..31952b9 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -972,7 +972,7 @@ struct dc_sink {
 	struct dc_context *ctx;
 
 	/* private to dc_sink.c */
-	atomic_t ref_count;
+	struct kref refcount;
 };
 
 void dc_sink_retain(struct dc_sink *sink);
-- 
2.9.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/6] amdgpu/dc: use kref for dc_state.
       [not found] ` <20171003023902.5036-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (4 preceding siblings ...)
  2017-10-03  2:39   ` [PATCH 5/6] amdgpu/dc: convert dc_sink " Dave Airlie
@ 2017-10-03  2:39   ` Dave Airlie
  2017-10-03  7:50   ` convert dc to using krefs for object reference counts Christian König
  2017-10-03 21:14   ` Harry Wentland
  7 siblings, 0 replies; 10+ messages in thread
From: Dave Airlie @ 2017-10-03  2:39 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Dave Airlie <airlied@redhat.com>

I'm not a huge fan of those copying around refcounts bits, might
want to consider alternates, but this should work for now.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc.c          | 26 +++++++++++------------
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c |  4 ++--
 drivers/gpu/drm/amd/display/dc/inc/core_types.h   |  2 +-
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index c8235b0..e8634b8 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -964,25 +964,25 @@ struct dc_state *dc_create_state(void)
 	if (!context)
 		return NULL;
 
-	atomic_inc(&context->ref_count);
+	kref_init(&context->refcount);
 	return context;
 }
 
 void dc_retain_state(struct dc_state *context)
 {
-	ASSERT(atomic_read(&context->ref_count) > 0);
-	atomic_inc(&context->ref_count);
+	kref_get(&context->refcount);
 }
 
-void dc_release_state(struct dc_state *context)
+static void dc_state_free(struct kref *kref)
 {
-	ASSERT(atomic_read(&context->ref_count) > 0);
-	atomic_dec(&context->ref_count);
+	struct dc_state *context = container_of(kref, struct dc_state, refcount);
+	dc_resource_state_destruct(context);
+	kfree(context);
+}
 
-	if (atomic_read(&context->ref_count) == 0) {
-		dc_resource_state_destruct(context);
-		kfree(context);
-	}
+void dc_release_state(struct dc_state *context)
+{
+	kref_put(&context->refcount, dc_state_free);
 }
 
 static bool is_surface_in_context(
@@ -1566,7 +1566,7 @@ void dc_set_power_state(
 	struct dc *dc,
 	enum dc_acpi_cm_power_state power_state)
 {
-	atomic_t ref_count;
+	struct kref refcount;
 
 	switch (power_state) {
 	case DC_ACPI_CM_POWER_STATE_D0:
@@ -1584,12 +1584,12 @@ void dc_set_power_state(
 		 */
 
 		/* Preserve refcount */
-		ref_count = dc->current_state->ref_count;
+		refcount = dc->current_state->refcount;
 		dc_resource_state_destruct(dc->current_state);
 		memset(dc->current_state, 0,
 				sizeof(*dc->current_state));
 
-		dc->current_state->ref_count = ref_count;
+		dc->current_state->refcount = refcount;
 
 		break;
 	}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index 1832f25..057bfe6 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -2433,7 +2433,7 @@ void dc_resource_state_copy_construct(
 		struct dc_state *dst_ctx)
 {
 	int i, j;
-	atomic_t ref_count = dst_ctx->ref_count;
+	struct kref refcount = dst_ctx->refcount;
 
 	*dst_ctx = *src_ctx;
 
@@ -2456,7 +2456,7 @@ void dc_resource_state_copy_construct(
 	}
 
 	/* context refcount should not be overridden */
-	dst_ctx->ref_count = ref_count;
+	dst_ctx->refcount = refcount;
 
 }
 
diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_types.h b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
index 1a1d322..d786ec4 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/core_types.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
@@ -268,7 +268,7 @@ struct dc_state {
 
 	struct display_clock *dis_clk;
 
-	atomic_t ref_count;
+	struct kref refcount;
 };
 
 #endif /* _CORE_TYPES_H_ */
-- 
2.9.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] amdgpu/dc: convert dc_transfer to use a kref.
       [not found]     ` <20171003023902.5036-2-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-10-03  7:48       ` Christian König
  0 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2017-10-03  7:48 UTC (permalink / raw)
  To: Dave Airlie, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 03.10.2017 um 04:38 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Rolling your own atomic ref counts is frowned upon.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>   drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 17 +++++++++--------
>   drivers/gpu/drm/amd/display/dc/dc.h              |  2 +-
>   drivers/gpu/drm/amd/display/dc/os_types.h        |  2 ++
>   3 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> index ff07105..c2168df 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> @@ -161,17 +161,18 @@ struct dc_gamma *dc_create_gamma()
>   
>   void dc_transfer_func_retain(struct dc_transfer_func *tf)
>   {
> -	ASSERT(atomic_read(&tf->ref_count) > 0);
> -	atomic_inc(&tf->ref_count);
> +	kref_get(&tf->refcount);
>   }
>   
> -void dc_transfer_func_release(struct dc_transfer_func *tf)
> +static void dc_transfer_func_free(struct kref *kref)
>   {
> -	ASSERT(atomic_read(&tf->ref_count) > 0);
> -	atomic_dec(&tf->ref_count);
> +	struct dc_transfer_func *tf = container_of(kref, struct dc_transfer_func, refcount);
> +	kfree(tf);

Minor style nit pick, but we should have a new line between declaration 
and code. Alternative I could also like with "kfree(container....)".

Not that I care to much, but I had to spend a good part of my time 
beating people at AMD to come up with good kernel coding style :)

Christian.

> +}
>   
> -	if (atomic_read(&tf->ref_count) == 0)
> -		kfree(tf);
> +void dc_transfer_func_release(struct dc_transfer_func *tf)
> +{
> +	kref_put(&tf->refcount, dc_transfer_func_free);
>   }
>   
>   struct dc_transfer_func *dc_create_transfer_func()
> @@ -181,7 +182,7 @@ struct dc_transfer_func *dc_create_transfer_func()
>   	if (tf == NULL)
>   		goto alloc_fail;
>   
> -	atomic_inc(&tf->ref_count);
> +	kref_init(&tf->refcount);
>   
>   	return tf;
>   
> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
> index 9c0e000..17f0c73 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -334,11 +334,11 @@ enum dc_transfer_func_predefined {
>   };
>   
>   struct dc_transfer_func {
> +	struct kref refcount;
>   	struct dc_transfer_func_distributed_points tf_pts;
>   	enum dc_transfer_func_type type;
>   	enum dc_transfer_func_predefined tf;
>   	struct dc_context *ctx;
> -	atomic_t ref_count;
>   };
>   
>   /*
> diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h b/drivers/gpu/drm/amd/display/dc/os_types.h
> index 27ed2a6..e0cd527 100644
> --- a/drivers/gpu/drm/amd/display/dc/os_types.h
> +++ b/drivers/gpu/drm/amd/display/dc/os_types.h
> @@ -32,6 +32,8 @@
>   #include <linux/types.h>
>   #include <drm/drmP.h>
>   
> +#include <linux/kref.h>
> +
>   #include "cgs_linux.h"
>   
>   #if defined(__BIG_ENDIAN) && !defined(BIGENDIAN_CPU)


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: convert dc to using krefs for object reference counts
       [not found] ` <20171003023902.5036-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (5 preceding siblings ...)
  2017-10-03  2:39   ` [PATCH 6/6] amdgpu/dc: use kref for dc_state Dave Airlie
@ 2017-10-03  7:50   ` Christian König
  2017-10-03 21:14   ` Harry Wentland
  7 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2017-10-03  7:50 UTC (permalink / raw)
  To: Dave Airlie, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Apart from a minor style nit pick which was repeated a few times the 
series is Reviewed-by: Christian König <christian.koenig@amd.com>.

Regards,
Christian.

Am 03.10.2017 um 04:38 schrieb Dave Airlie:
> This series might be a bit of a too big step :-)
>
> So in the kernel we use krefs for reference counts for lots of good
> reasons, the main one, is no bugs with roll your own atomic reference
> counting for structs, which lots of this code is.
>
> I've ported all the structs I could find using hand rolled atomic
> reference counters to use krefs.
>
> Dave.
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: convert dc to using krefs for object reference counts
       [not found] ` <20171003023902.5036-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (6 preceding siblings ...)
  2017-10-03  7:50   ` convert dc to using krefs for object reference counts Christian König
@ 2017-10-03 21:14   ` Harry Wentland
  7 siblings, 0 replies; 10+ messages in thread
From: Harry Wentland @ 2017-10-03 21:14 UTC (permalink / raw)
  To: Dave Airlie, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On 2017-10-02 10:38 PM, Dave Airlie wrote:
> This series might be a bit of a too big step :-)
> 

Should be okay since ref counts are pretty self-contained.

> So in the kernel we use krefs for reference counts for lots of good
> reasons, the main one, is no bugs with roll your own atomic reference
> counting for structs, which lots of this code is.
> 
> I've ported all the structs I could find using hand rolled atomic
> reference counters to use krefs.
> 

I'd still echo Christian's nitpick but knowing the state of DC code
style this series is
Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> Dave.
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-10-03 21:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-03  2:38 convert dc to using krefs for object reference counts Dave Airlie
     [not found] ` <20171003023902.5036-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-10-03  2:38   ` [PATCH 1/6] amdgpu/dc: convert dc_transfer to use a kref Dave Airlie
     [not found]     ` <20171003023902.5036-2-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-10-03  7:48       ` Christian König
2017-10-03  2:38   ` [PATCH 2/6] amdgpu/dc: convert dc_gamma to kref reference counting Dave Airlie
2017-10-03  2:38   ` [PATCH 3/6] amdgpu/dc: use kref for dc_plane_state Dave Airlie
2017-10-03  2:39   ` [PATCH 4/6] amdgpu/dc: convert dc_stream_state to kref Dave Airlie
2017-10-03  2:39   ` [PATCH 5/6] amdgpu/dc: convert dc_sink " Dave Airlie
2017-10-03  2:39   ` [PATCH 6/6] amdgpu/dc: use kref for dc_state Dave Airlie
2017-10-03  7:50   ` convert dc to using krefs for object reference counts Christian König
2017-10-03 21:14   ` Harry Wentland

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.