[PATCH 1/6] drm/vc4: bo: Add a managed action to cleanup the cache

[PATCH 1/6] drm/vc4: bo: Add a managed action to cleanup the cache

[Maxime Ripard]

The BO cache needs to be cleaned up using vc4_bo_cache_destroy, but it's
not used consistently (vc4_drv's bind calls it in its error path, but
doesn't in unbind), and we can make that automatic through a managed
action. Let's remove the requirement to call vc4_bo_cache_destroy.

Fixes: c826a6e10644 ("drm/vc4: Add a BO cache.")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_bo.c  | 5 +++--
 drivers/gpu/drm/vc4/vc4_drv.c | 1 -
 drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index f432278173cd..730d5775ab42 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -1024,6 +1024,7 @@ int vc4_get_tiling_ioctl(struct drm_device *dev, void *data,
 	return 0;
 }
 
+static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused);
 int vc4_bo_cache_init(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -1052,10 +1053,10 @@ int vc4_bo_cache_init(struct drm_device *dev)
 	INIT_WORK(&vc4->bo_cache.time_work, vc4_bo_cache_time_work);
 	timer_setup(&vc4->bo_cache.time_timer, vc4_bo_cache_time_timer, 0);
 
-	return 0;
+	return drmm_add_action(dev, vc4_bo_cache_destroy, NULL);
 }
 
-void vc4_bo_cache_destroy(struct drm_device *dev)
+static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	int i;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 8f10f609e4f8..eb3fcd8232b5 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -303,7 +303,6 @@ static int vc4_drm_bind(struct device *dev)
 gem_destroy:
 	vc4_gem_destroy(drm);
 	drm_mode_config_cleanup(drm);
-	vc4_bo_cache_destroy(drm);
 dev_put:
 	drm_dev_put(drm);
 	return ret;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 836fdca5e643..3a989e8b39a2 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -14,6 +14,7 @@
 #include <drm/drm_device.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_mm.h>
 #include <drm/drm_modeset_lock.h>
 
@@ -808,7 +809,6 @@ struct drm_gem_object *vc4_prime_import_sg_table(struct drm_device *dev,
 						 struct sg_table *sgt);
 void *vc4_prime_vmap(struct drm_gem_object *obj);
 int vc4_bo_cache_init(struct drm_device *dev);
-void vc4_bo_cache_destroy(struct drm_device *dev);
 int vc4_bo_inc_usecnt(struct vc4_bo *bo);
 void vc4_bo_dec_usecnt(struct vc4_bo *bo);
 void vc4_bo_add_to_purgeable_pool(struct vc4_bo *bo);
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/6] drm/vc4: bo: Add a managed action to cleanup the cache

[Maxime Ripard]

The BO cache needs to be cleaned up using vc4_bo_cache_destroy, but it's
not used consistently (vc4_drv's bind calls it in its error path, but
doesn't in unbind), and we can make that automatic through a managed
action. Let's remove the requirement to call vc4_bo_cache_destroy.

Fixes: c826a6e10644 ("drm/vc4: Add a BO cache.")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_bo.c  | 5 +++--
 drivers/gpu/drm/vc4/vc4_drv.c | 1 -
 drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index f432278173cd..730d5775ab42 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -1024,6 +1024,7 @@ int vc4_get_tiling_ioctl(struct drm_device *dev, void *data,
 	return 0;
 }
 
+static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused);
 int vc4_bo_cache_init(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -1052,10 +1053,10 @@ int vc4_bo_cache_init(struct drm_device *dev)
 	INIT_WORK(&vc4->bo_cache.time_work, vc4_bo_cache_time_work);
 	timer_setup(&vc4->bo_cache.time_timer, vc4_bo_cache_time_timer, 0);
 
-	return 0;
+	return drmm_add_action(dev, vc4_bo_cache_destroy, NULL);
 }
 
-void vc4_bo_cache_destroy(struct drm_device *dev)
+static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	int i;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 8f10f609e4f8..eb3fcd8232b5 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -303,7 +303,6 @@ static int vc4_drm_bind(struct device *dev)
 gem_destroy:
 	vc4_gem_destroy(drm);
 	drm_mode_config_cleanup(drm);
-	vc4_bo_cache_destroy(drm);
 dev_put:
 	drm_dev_put(drm);
 	return ret;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 836fdca5e643..3a989e8b39a2 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -14,6 +14,7 @@
 #include <drm/drm_device.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_mm.h>
 #include <drm/drm_modeset_lock.h>
 
@@ -808,7 +809,6 @@ struct drm_gem_object *vc4_prime_import_sg_table(struct drm_device *dev,
 						 struct sg_table *sgt);
 void *vc4_prime_vmap(struct drm_gem_object *obj);
 int vc4_bo_cache_init(struct drm_device *dev);
-void vc4_bo_cache_destroy(struct drm_device *dev);
 int vc4_bo_inc_usecnt(struct vc4_bo *bo);
 void vc4_bo_dec_usecnt(struct vc4_bo *bo);
 void vc4_bo_add_to_purgeable_pool(struct vc4_bo *bo);
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/6] drm/vc4: drv: Use managed drm_mode_config_init

[Maxime Ripard]

Using drmm_mode_config_init instead of drm_mode_config_init allows us to
cleanup a bit the error path.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index eb3fcd8232b5..0bcc541486a9 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -269,7 +269,9 @@ static int vc4_drm_bind(struct device *dev)
 	if (ret)
 		goto dev_put;
 
-	drm_mode_config_init(drm);
+	ret = drmm_mode_config_init(drm);
+	if (ret)
+		goto dev_put;
 
 	vc4_gem_init(drm);
 
@@ -302,7 +304,6 @@ static int vc4_drm_bind(struct device *dev)
 	component_unbind_all(dev, drm);
 gem_destroy:
 	vc4_gem_destroy(drm);
-	drm_mode_config_cleanup(drm);
 dev_put:
 	drm_dev_put(drm);
 	return ret;
@@ -317,8 +318,6 @@ static void vc4_drm_unbind(struct device *dev)
 
 	drm_atomic_helper_shutdown(drm);
 
-	drm_mode_config_cleanup(drm);
-
 	drm_atomic_private_obj_fini(&vc4->load_tracker);
 	drm_atomic_private_obj_fini(&vc4->ctm_manager);
 
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/6] drm/vc4: drv: Use managed drm_mode_config_init

[Maxime Ripard]

Using drmm_mode_config_init instead of drm_mode_config_init allows us to
cleanup a bit the error path.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index eb3fcd8232b5..0bcc541486a9 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -269,7 +269,9 @@ static int vc4_drm_bind(struct device *dev)
 	if (ret)
 		goto dev_put;
 
-	drm_mode_config_init(drm);
+	ret = drmm_mode_config_init(drm);
+	if (ret)
+		goto dev_put;
 
 	vc4_gem_init(drm);
 
@@ -302,7 +304,6 @@ static int vc4_drm_bind(struct device *dev)
 	component_unbind_all(dev, drm);
 gem_destroy:
 	vc4_gem_destroy(drm);
-	drm_mode_config_cleanup(drm);
 dev_put:
 	drm_dev_put(drm);
 	return ret;
@@ -317,8 +318,6 @@ static void vc4_drm_unbind(struct device *dev)
 
 	drm_atomic_helper_shutdown(drm);
 
-	drm_mode_config_cleanup(drm);
-
 	drm_atomic_private_obj_fini(&vc4->load_tracker);
 	drm_atomic_private_obj_fini(&vc4->ctm_manager);
 
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/6] drm/vc4: gem: Add a managed action to cleanup the job queue

[Maxime Ripard]

The job queue needs to be cleaned up using vc4_gem_destroy, but it's
not used consistently (vc4_drv's bind calls it in its error path, but
doesn't in unbind), and we can make that automatic through a managed
action. Let's remove the requirement to call vc4_gem_destroy.

Fixes: d5b1a78a772f ("drm/vc4: Add support for drawing 3D frames.")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 8 ++++----
 drivers/gpu/drm/vc4/vc4_drv.h | 3 +--
 drivers/gpu/drm/vc4/vc4_gem.c | 9 +++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 0bcc541486a9..ab873e27aa0e 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -273,11 +273,13 @@ static int vc4_drm_bind(struct device *dev)
 	if (ret)
 		goto dev_put;
 
-	vc4_gem_init(drm);
+	ret = vc4_gem_init(drm);
+	if (ret)
+		goto dev_put;
 
 	ret = component_bind_all(dev, drm);
 	if (ret)
-		goto gem_destroy;
+		goto dev_put;
 
 	ret = vc4_plane_create_additional_planes(drm);
 	if (ret)
@@ -302,8 +304,6 @@ static int vc4_drm_bind(struct device *dev)
 
 unbind_all:
 	component_unbind_all(dev, drm);
-gem_destroy:
-	vc4_gem_destroy(drm);
 dev_put:
 	drm_dev_put(drm);
 	return ret;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 3a989e8b39a2..b1a038df00f3 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -873,8 +873,7 @@ extern struct platform_driver vc4_dsi_driver;
 extern const struct dma_fence_ops vc4_fence_ops;
 
 /* vc4_gem.c */
-void vc4_gem_init(struct drm_device *dev);
-void vc4_gem_destroy(struct drm_device *dev);
+int vc4_gem_init(struct drm_device *dev);
 int vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file_priv);
 int vc4_wait_seqno_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 9f01ddd5b932..071626ad8551 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -1263,8 +1263,8 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
 	return ret;
 }
 
-void
-vc4_gem_init(struct drm_device *dev)
+static void vc4_gem_destroy(struct drm_device *dev, void *unused);
+int vc4_gem_init(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 
@@ -1285,10 +1285,11 @@ vc4_gem_init(struct drm_device *dev)
 
 	INIT_LIST_HEAD(&vc4->purgeable.list);
 	mutex_init(&vc4->purgeable.lock);
+
+	return drmm_add_action(dev, vc4_gem_destroy, NULL);
 }
 
-void
-vc4_gem_destroy(struct drm_device *dev)
+static void vc4_gem_destroy(struct drm_device *dev, void *unused)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/6] drm/vc4: gem: Add a managed action to cleanup the job queue

[Maxime Ripard]

The job queue needs to be cleaned up using vc4_gem_destroy, but it's
not used consistently (vc4_drv's bind calls it in its error path, but
doesn't in unbind), and we can make that automatic through a managed
action. Let's remove the requirement to call vc4_gem_destroy.

Fixes: d5b1a78a772f ("drm/vc4: Add support for drawing 3D frames.")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 8 ++++----
 drivers/gpu/drm/vc4/vc4_drv.h | 3 +--
 drivers/gpu/drm/vc4/vc4_gem.c | 9 +++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 0bcc541486a9..ab873e27aa0e 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -273,11 +273,13 @@ static int vc4_drm_bind(struct device *dev)
 	if (ret)
 		goto dev_put;
 
-	vc4_gem_init(drm);
+	ret = vc4_gem_init(drm);
+	if (ret)
+		goto dev_put;
 
 	ret = component_bind_all(dev, drm);
 	if (ret)
-		goto gem_destroy;
+		goto dev_put;
 
 	ret = vc4_plane_create_additional_planes(drm);
 	if (ret)
@@ -302,8 +304,6 @@ static int vc4_drm_bind(struct device *dev)
 
 unbind_all:
 	component_unbind_all(dev, drm);
-gem_destroy:
-	vc4_gem_destroy(drm);
 dev_put:
 	drm_dev_put(drm);
 	return ret;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 3a989e8b39a2..b1a038df00f3 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -873,8 +873,7 @@ extern struct platform_driver vc4_dsi_driver;
 extern const struct dma_fence_ops vc4_fence_ops;
 
 /* vc4_gem.c */
-void vc4_gem_init(struct drm_device *dev);
-void vc4_gem_destroy(struct drm_device *dev);
+int vc4_gem_init(struct drm_device *dev);
 int vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file_priv);
 int vc4_wait_seqno_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 9f01ddd5b932..071626ad8551 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -1263,8 +1263,8 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
 	return ret;
 }
 
-void
-vc4_gem_init(struct drm_device *dev)
+static void vc4_gem_destroy(struct drm_device *dev, void *unused);
+int vc4_gem_init(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 
@@ -1285,10 +1285,11 @@ vc4_gem_init(struct drm_device *dev)
 
 	INIT_LIST_HEAD(&vc4->purgeable.list);
 	mutex_init(&vc4->purgeable.lock);
+
+	return drmm_add_action(dev, vc4_gem_destroy, NULL);
 }
 
-void
-vc4_gem_destroy(struct drm_device *dev)
+static void vc4_gem_destroy(struct drm_device *dev, void *unused)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/6] drm/vc4: Use the helper to retrieve vc4_dev when needed

[Maxime Ripard]

We have a helper to retrieve the vc4_dev structure from the drm_device one
when needed, so let's use it consistently.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hvs.c | 4 ++--
 drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 4d0a833366ce..b72b2bd05a81 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -560,7 +560,7 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct drm_device *drm = dev_get_drvdata(master);
-	struct vc4_dev *vc4 = drm->dev_private;
+	struct vc4_dev *vc4 = to_vc4_dev(drm);
 	struct vc4_hvs *hvs = NULL;
 	int ret;
 	u32 dispctrl;
@@ -679,7 +679,7 @@ static void vc4_hvs_unbind(struct device *dev, struct device *master,
 			   void *data)
 {
 	struct drm_device *drm = dev_get_drvdata(master);
-	struct vc4_dev *vc4 = drm->dev_private;
+	struct vc4_dev *vc4 = to_vc4_dev(drm);
 	struct vc4_hvs *hvs = vc4->hvs;
 
 	if (drm_mm_node_allocated(&vc4->hvs->mitchell_netravali_filter))
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 149825ff5df8..708099a24406 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -51,7 +51,7 @@ static struct vc4_ctm_state *vc4_get_ctm_state(struct drm_atomic_state *state,
 					       struct drm_private_obj *manager)
 {
 	struct drm_device *dev = state->dev;
-	struct vc4_dev *vc4 = dev->dev_private;
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct drm_private_state *priv_state;
 	int ret;
 
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] drm/vc4: Use the helper to retrieve vc4_dev when needed

[Maxime Ripard]

We have a helper to retrieve the vc4_dev structure from the drm_device one
when needed, so let's use it consistently.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hvs.c | 4 ++--
 drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 4d0a833366ce..b72b2bd05a81 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -560,7 +560,7 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct drm_device *drm = dev_get_drvdata(master);
-	struct vc4_dev *vc4 = drm->dev_private;
+	struct vc4_dev *vc4 = to_vc4_dev(drm);
 	struct vc4_hvs *hvs = NULL;
 	int ret;
 	u32 dispctrl;
@@ -679,7 +679,7 @@ static void vc4_hvs_unbind(struct device *dev, struct device *master,
 			   void *data)
 {
 	struct drm_device *drm = dev_get_drvdata(master);
-	struct vc4_dev *vc4 = drm->dev_private;
+	struct vc4_dev *vc4 = to_vc4_dev(drm);
 	struct vc4_hvs *hvs = vc4->hvs;
 
 	if (drm_mm_node_allocated(&vc4->hvs->mitchell_netravali_filter))
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 149825ff5df8..708099a24406 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -51,7 +51,7 @@ static struct vc4_ctm_state *vc4_get_ctm_state(struct drm_atomic_state *state,
 					       struct drm_private_obj *manager)
 {
 	struct drm_device *dev = state->dev;
-	struct vc4_dev *vc4 = dev->dev_private;
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct drm_private_state *priv_state;
 	int ret;
 
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/6] drm/vc4: Use devm_drm_dev_alloc

[Maxime Ripard]

We can simplify a bit the bind code, its error path and unbind by using
the managed devm_drm_dev_alloc function.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_bo.c  |  4 ++--
 drivers/gpu/drm/vc4/vc4_drv.c | 27 ++++++++++-----------------
 drivers/gpu/drm/vc4/vc4_drv.h |  4 ++--
 drivers/gpu/drm/vc4/vc4_gem.c | 10 +++++-----
 drivers/gpu/drm/vc4/vc4_v3d.c | 12 ++++++------
 5 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 730d5775ab42..74129dc78875 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -468,7 +468,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size,
 	}
 
 	if (IS_ERR(cma_obj)) {
-		struct drm_printer p = drm_info_printer(vc4->dev->dev);
+		struct drm_printer p = drm_info_printer(vc4->base.dev);
 		DRM_ERROR("Failed to allocate from CMA:\n");
 		vc4_bo_stats_print(&p, vc4);
 		return ERR_PTR(-ENOMEM);
@@ -609,7 +609,7 @@ static void vc4_bo_cache_time_work(struct work_struct *work)
 {
 	struct vc4_dev *vc4 =
 		container_of(work, struct vc4_dev, bo_cache.time_work);
-	struct drm_device *dev = vc4->dev;
+	struct drm_device *dev = &vc4->base;
 
 	mutex_lock(&vc4->bo_lock);
 	vc4_bo_cache_free_old(dev);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index ab873e27aa0e..6e037fbaa090 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -245,41 +245,37 @@ static int vc4_drm_bind(struct device *dev)
 
 	dev->coherent_dma_mask = DMA_BIT_MASK(32);
 
-	vc4 = devm_kzalloc(dev, sizeof(*vc4), GFP_KERNEL);
-	if (!vc4)
-		return -ENOMEM;
-
 	/* If VC4 V3D is missing, don't advertise render nodes. */
 	node = of_find_matching_node_and_match(NULL, vc4_v3d_dt_match, NULL);
 	if (!node || !of_device_is_available(node))
 		vc4_drm_driver.driver_features &= ~DRIVER_RENDER;
 	of_node_put(node);
 
-	drm = drm_dev_alloc(&vc4_drm_driver, dev);
-	if (IS_ERR(drm))
-		return PTR_ERR(drm);
+	vc4 = devm_drm_dev_alloc(dev, &vc4_drm_driver, struct vc4_dev, base);
+	if (IS_ERR(vc4))
+		return PTR_ERR(vc4);
+
+	drm = &vc4->base;
 	platform_set_drvdata(pdev, drm);
-	vc4->dev = drm;
-	drm->dev_private = vc4;
 	INIT_LIST_HEAD(&vc4->debugfs_list);
 
 	mutex_init(&vc4->bin_bo_lock);
 
 	ret = vc4_bo_cache_init(drm);
 	if (ret)
-		goto dev_put;
+		return ret;
 
 	ret = drmm_mode_config_init(drm);
 	if (ret)
-		goto dev_put;
+		return ret;
 
 	ret = vc4_gem_init(drm);
 	if (ret)
-		goto dev_put;
+		return ret;
 
 	ret = component_bind_all(dev, drm);
 	if (ret)
-		goto dev_put;
+		return ret;
 
 	ret = vc4_plane_create_additional_planes(drm);
 	if (ret)
@@ -304,8 +300,7 @@ static int vc4_drm_bind(struct device *dev)
 
 unbind_all:
 	component_unbind_all(dev, drm);
-dev_put:
-	drm_dev_put(drm);
+
 	return ret;
 }
 
@@ -320,8 +315,6 @@ static void vc4_drm_unbind(struct device *dev)
 
 	drm_atomic_private_obj_fini(&vc4->load_tracker);
 	drm_atomic_private_obj_fini(&vc4->ctm_manager);
-
-	drm_dev_put(drm);
 }
 
 static const struct component_master_ops vc4_drm_ops = {
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index b1a038df00f3..f8a1426be14a 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -72,7 +72,7 @@ struct vc4_perfmon {
 };
 
 struct vc4_dev {
-	struct drm_device *dev;
+	struct drm_device base;
 
 	struct vc4_hvs *hvs;
 	struct vc4_v3d *v3d;
@@ -235,7 +235,7 @@ struct vc4_dev {
 static inline struct vc4_dev *
 to_vc4_dev(struct drm_device *dev)
 {
-	return (struct vc4_dev *)dev->dev_private;
+	return container_of(dev, struct vc4_dev, base);
 }
 
 struct vc4_bo {
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 071626ad8551..c4bf96c8cef6 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -314,16 +314,16 @@ vc4_reset_work(struct work_struct *work)
 	struct vc4_dev *vc4 =
 		container_of(work, struct vc4_dev, hangcheck.reset_work);
 
-	vc4_save_hang_state(vc4->dev);
+	vc4_save_hang_state(&vc4->base);
 
-	vc4_reset(vc4->dev);
+	vc4_reset(&vc4->base);
 }
 
 static void
 vc4_hangcheck_elapsed(struct timer_list *t)
 {
 	struct vc4_dev *vc4 = from_timer(vc4, t, hangcheck.timer);
-	struct drm_device *dev = vc4->dev;
+	struct drm_device *dev = &vc4->base;
 	uint32_t ct0ca, ct1ca;
 	unsigned long irqflags;
 	struct vc4_exec_info *bin_exec, *render_exec;
@@ -1000,7 +1000,7 @@ vc4_job_handle_completed(struct vc4_dev *vc4)
 		list_del(&exec->head);
 
 		spin_unlock_irqrestore(&vc4->job_lock, irqflags);
-		vc4_complete_exec(vc4->dev, exec);
+		vc4_complete_exec(&vc4->base, exec);
 		spin_lock_irqsave(&vc4->job_lock, irqflags);
 	}
 
@@ -1258,7 +1258,7 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
 	return 0;
 
 fail:
-	vc4_complete_exec(vc4->dev, exec);
+	vc4_complete_exec(&vc4->base, exec);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index f7ab979721b3..65d0dac69b0b 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -168,7 +168,7 @@ static void vc4_v3d_init_hw(struct drm_device *dev)
 
 int vc4_v3d_get_bin_slot(struct vc4_dev *vc4)
 {
-	struct drm_device *dev = vc4->dev;
+	struct drm_device *dev = &vc4->base;
 	unsigned long irqflags;
 	int slot;
 	uint64_t seqno = 0;
@@ -246,7 +246,7 @@ static int bin_bo_alloc(struct vc4_dev *vc4)
 	INIT_LIST_HEAD(&list);
 
 	while (true) {
-		struct vc4_bo *bo = vc4_bo_create(vc4->dev, size, true,
+		struct vc4_bo *bo = vc4_bo_create(&vc4->base, size, true,
 						  VC4_BO_TYPE_BIN);
 
 		if (IS_ERR(bo)) {
@@ -361,7 +361,7 @@ static int vc4_v3d_runtime_suspend(struct device *dev)
 	struct vc4_v3d *v3d = dev_get_drvdata(dev);
 	struct vc4_dev *vc4 = v3d->vc4;
 
-	vc4_irq_uninstall(vc4->dev);
+	vc4_irq_uninstall(&vc4->base);
 
 	clk_disable_unprepare(v3d->clk);
 
@@ -378,11 +378,11 @@ static int vc4_v3d_runtime_resume(struct device *dev)
 	if (ret != 0)
 		return ret;
 
-	vc4_v3d_init_hw(vc4->dev);
+	vc4_v3d_init_hw(&vc4->base);
 
 	/* We disabled the IRQ as part of vc4_irq_uninstall in suspend. */
-	enable_irq(vc4->dev->irq);
-	vc4_irq_postinstall(vc4->dev);
+	enable_irq(vc4->base.irq);
+	vc4_irq_postinstall(&vc4->base);
 
 	return 0;
 }
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/6] drm/vc4: Use devm_drm_dev_alloc

[Maxime Ripard]

We can simplify a bit the bind code, its error path and unbind by using
the managed devm_drm_dev_alloc function.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_bo.c  |  4 ++--
 drivers/gpu/drm/vc4/vc4_drv.c | 27 ++++++++++-----------------
 drivers/gpu/drm/vc4/vc4_drv.h |  4 ++--
 drivers/gpu/drm/vc4/vc4_gem.c | 10 +++++-----
 drivers/gpu/drm/vc4/vc4_v3d.c | 12 ++++++------
 5 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 730d5775ab42..74129dc78875 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -468,7 +468,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size,
 	}
 
 	if (IS_ERR(cma_obj)) {
-		struct drm_printer p = drm_info_printer(vc4->dev->dev);
+		struct drm_printer p = drm_info_printer(vc4->base.dev);
 		DRM_ERROR("Failed to allocate from CMA:\n");
 		vc4_bo_stats_print(&p, vc4);
 		return ERR_PTR(-ENOMEM);
@@ -609,7 +609,7 @@ static void vc4_bo_cache_time_work(struct work_struct *work)
 {
 	struct vc4_dev *vc4 =
 		container_of(work, struct vc4_dev, bo_cache.time_work);
-	struct drm_device *dev = vc4->dev;
+	struct drm_device *dev = &vc4->base;
 
 	mutex_lock(&vc4->bo_lock);
 	vc4_bo_cache_free_old(dev);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index ab873e27aa0e..6e037fbaa090 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -245,41 +245,37 @@ static int vc4_drm_bind(struct device *dev)
 
 	dev->coherent_dma_mask = DMA_BIT_MASK(32);
 
-	vc4 = devm_kzalloc(dev, sizeof(*vc4), GFP_KERNEL);
-	if (!vc4)
-		return -ENOMEM;
-
 	/* If VC4 V3D is missing, don't advertise render nodes. */
 	node = of_find_matching_node_and_match(NULL, vc4_v3d_dt_match, NULL);
 	if (!node || !of_device_is_available(node))
 		vc4_drm_driver.driver_features &= ~DRIVER_RENDER;
 	of_node_put(node);
 
-	drm = drm_dev_alloc(&vc4_drm_driver, dev);
-	if (IS_ERR(drm))
-		return PTR_ERR(drm);
+	vc4 = devm_drm_dev_alloc(dev, &vc4_drm_driver, struct vc4_dev, base);
+	if (IS_ERR(vc4))
+		return PTR_ERR(vc4);
+
+	drm = &vc4->base;
 	platform_set_drvdata(pdev, drm);
-	vc4->dev = drm;
-	drm->dev_private = vc4;
 	INIT_LIST_HEAD(&vc4->debugfs_list);
 
 	mutex_init(&vc4->bin_bo_lock);
 
 	ret = vc4_bo_cache_init(drm);
 	if (ret)
-		goto dev_put;
+		return ret;
 
 	ret = drmm_mode_config_init(drm);
 	if (ret)
-		goto dev_put;
+		return ret;
 
 	ret = vc4_gem_init(drm);
 	if (ret)
-		goto dev_put;
+		return ret;
 
 	ret = component_bind_all(dev, drm);
 	if (ret)
-		goto dev_put;
+		return ret;
 
 	ret = vc4_plane_create_additional_planes(drm);
 	if (ret)
@@ -304,8 +300,7 @@ static int vc4_drm_bind(struct device *dev)
 
 unbind_all:
 	component_unbind_all(dev, drm);
-dev_put:
-	drm_dev_put(drm);
+
 	return ret;
 }
 
@@ -320,8 +315,6 @@ static void vc4_drm_unbind(struct device *dev)
 
 	drm_atomic_private_obj_fini(&vc4->load_tracker);
 	drm_atomic_private_obj_fini(&vc4->ctm_manager);
-
-	drm_dev_put(drm);
 }
 
 static const struct component_master_ops vc4_drm_ops = {
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index b1a038df00f3..f8a1426be14a 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -72,7 +72,7 @@ struct vc4_perfmon {
 };
 
 struct vc4_dev {
-	struct drm_device *dev;
+	struct drm_device base;
 
 	struct vc4_hvs *hvs;
 	struct vc4_v3d *v3d;
@@ -235,7 +235,7 @@ struct vc4_dev {
 static inline struct vc4_dev *
 to_vc4_dev(struct drm_device *dev)
 {
-	return (struct vc4_dev *)dev->dev_private;
+	return container_of(dev, struct vc4_dev, base);
 }
 
 struct vc4_bo {
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 071626ad8551..c4bf96c8cef6 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -314,16 +314,16 @@ vc4_reset_work(struct work_struct *work)
 	struct vc4_dev *vc4 =
 		container_of(work, struct vc4_dev, hangcheck.reset_work);
 
-	vc4_save_hang_state(vc4->dev);
+	vc4_save_hang_state(&vc4->base);
 
-	vc4_reset(vc4->dev);
+	vc4_reset(&vc4->base);
 }
 
 static void
 vc4_hangcheck_elapsed(struct timer_list *t)
 {
 	struct vc4_dev *vc4 = from_timer(vc4, t, hangcheck.timer);
-	struct drm_device *dev = vc4->dev;
+	struct drm_device *dev = &vc4->base;
 	uint32_t ct0ca, ct1ca;
 	unsigned long irqflags;
 	struct vc4_exec_info *bin_exec, *render_exec;
@@ -1000,7 +1000,7 @@ vc4_job_handle_completed(struct vc4_dev *vc4)
 		list_del(&exec->head);
 
 		spin_unlock_irqrestore(&vc4->job_lock, irqflags);
-		vc4_complete_exec(vc4->dev, exec);
+		vc4_complete_exec(&vc4->base, exec);
 		spin_lock_irqsave(&vc4->job_lock, irqflags);
 	}
 
@@ -1258,7 +1258,7 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
 	return 0;
 
 fail:
-	vc4_complete_exec(vc4->dev, exec);
+	vc4_complete_exec(&vc4->base, exec);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index f7ab979721b3..65d0dac69b0b 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -168,7 +168,7 @@ static void vc4_v3d_init_hw(struct drm_device *dev)
 
 int vc4_v3d_get_bin_slot(struct vc4_dev *vc4)
 {
-	struct drm_device *dev = vc4->dev;
+	struct drm_device *dev = &vc4->base;
 	unsigned long irqflags;
 	int slot;
 	uint64_t seqno = 0;
@@ -246,7 +246,7 @@ static int bin_bo_alloc(struct vc4_dev *vc4)
 	INIT_LIST_HEAD(&list);
 
 	while (true) {
-		struct vc4_bo *bo = vc4_bo_create(vc4->dev, size, true,
+		struct vc4_bo *bo = vc4_bo_create(&vc4->base, size, true,
 						  VC4_BO_TYPE_BIN);
 
 		if (IS_ERR(bo)) {
@@ -361,7 +361,7 @@ static int vc4_v3d_runtime_suspend(struct device *dev)
 	struct vc4_v3d *v3d = dev_get_drvdata(dev);
 	struct vc4_dev *vc4 = v3d->vc4;
 
-	vc4_irq_uninstall(vc4->dev);
+	vc4_irq_uninstall(&vc4->base);
 
 	clk_disable_unprepare(v3d->clk);
 
@@ -378,11 +378,11 @@ static int vc4_v3d_runtime_resume(struct device *dev)
 	if (ret != 0)
 		return ret;
 
-	vc4_v3d_init_hw(vc4->dev);
+	vc4_v3d_init_hw(&vc4->base);
 
 	/* We disabled the IRQ as part of vc4_irq_uninstall in suspend. */
-	enable_irq(vc4->dev->irq);
-	vc4_irq_postinstall(vc4->dev);
+	enable_irq(vc4->base.irq);
+	vc4_irq_postinstall(&vc4->base);
 
 	return 0;
 }
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/6] drm/vc4: kms: Add functions to create the state objects

[Maxime Ripard]

In order to make the vc4_kms_load code and error path a bit easier to
read and extend, add functions to create state objects, and use managed
actions to cleanup if needed.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_drv.c |  3 --
 drivers/gpu/drm/vc4/vc4_kms.c | 78 +++++++++++++++++++++++++----------
 2 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 6e037fbaa090..08c1cc225045 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -312,9 +312,6 @@ static void vc4_drm_unbind(struct device *dev)
 	drm_dev_unregister(drm);
 
 	drm_atomic_helper_shutdown(drm);
-
-	drm_atomic_private_obj_fini(&vc4->load_tracker);
-	drm_atomic_private_obj_fini(&vc4->ctm_manager);
 }
 
 static const struct component_master_ops vc4_drm_ops = {
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 708099a24406..fbfb0698073e 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -93,6 +93,29 @@ static const struct drm_private_state_funcs vc4_ctm_state_funcs = {
 	.atomic_destroy_state = vc4_ctm_destroy_state,
 };
 
+static void vc4_ctm_obj_fini(struct drm_device *dev, void *unused)
+{
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+
+	drm_atomic_private_obj_fini(&vc4->ctm_manager);
+}
+
+static int vc4_ctm_obj_init(struct vc4_dev *vc4)
+{
+	struct vc4_ctm_state *ctm_state;
+
+	drm_modeset_lock_init(&vc4->ctm_state_lock);
+
+	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
+	if (!ctm_state)
+		return -ENOMEM;
+
+	drm_atomic_private_obj_init(vc4->dev, &vc4->ctm_manager, &ctm_state->base,
+				    &vc4_ctm_state_funcs);
+
+	return drmm_add_action(&vc4->base, vc4_ctm_obj_fini, NULL);
+}
+
 /* Converts a DRM S31.32 value to the HW S0.9 format. */
 static u16 vc4_ctm_s31_32_to_s0_9(u64 in)
 {
@@ -609,6 +632,34 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = {
 	.atomic_destroy_state = vc4_load_tracker_destroy_state,
 };
 
+static void vc4_load_tracker_obj_fini(struct drm_device *dev, void *unused)
+{
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+
+	if (!vc4->load_tracker_available)
+		return 0;
+
+	drm_atomic_private_obj_fini(&vc4->load_tracker);
+}
+
+static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
+{
+	struct vc4_load_tracker_state *load_state;
+
+	if (!vc4->load_tracker_available)
+		return 0;
+
+	load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
+	if (!load_state)
+		return -ENOMEM;
+
+	drm_atomic_private_obj_init(vc4->dev, &vc4->load_tracker,
+				    &load_state->base,
+				    &vc4_load_tracker_state_funcs);
+
+	return drmm_add_action(&vc4->base, vc4_load_tracker_obj_fini, NULL);
+}
+
 #define NUM_OUTPUTS  6
 #define NUM_CHANNELS 3
 
@@ -711,8 +762,6 @@ static const struct drm_mode_config_funcs vc4_mode_funcs = {
 int vc4_kms_load(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	struct vc4_ctm_state *ctm_state;
-	struct vc4_load_tracker_state *load_state;
 	bool is_vc5 = of_device_is_compatible(dev->dev->of_node,
 					      "brcm,bcm2711-vc5");
 	int ret;
@@ -751,26 +800,13 @@ int vc4_kms_load(struct drm_device *dev)
 	dev->mode_config.async_page_flip = true;
 	dev->mode_config.allow_fb_modifiers = true;
 
-	drm_modeset_lock_init(&vc4->ctm_state_lock);
+	ret = vc4_ctm_obj_init(vc4);
+	if (ret)
+		return ret;
 
-	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
-	if (!ctm_state)
-		return -ENOMEM;
-
-	drm_atomic_private_obj_init(dev, &vc4->ctm_manager, &ctm_state->base,
-				    &vc4_ctm_state_funcs);
-
-	if (vc4->load_tracker_available) {
-		load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
-		if (!load_state) {
-			drm_atomic_private_obj_fini(&vc4->ctm_manager);
-			return -ENOMEM;
-		}
-
-		drm_atomic_private_obj_init(dev, &vc4->load_tracker,
-					    &load_state->base,
-					    &vc4_load_tracker_state_funcs);
-	}
+	ret = vc4_load_tracker_obj_init(vc4);
+	if (ret)
+		return ret;
 
 	drm_mode_config_reset(dev);
 
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/6] drm/vc4: kms: Add functions to create the state objects

[Maxime Ripard]

In order to make the vc4_kms_load code and error path a bit easier to
read and extend, add functions to create state objects, and use managed
actions to cleanup if needed.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_drv.c |  3 --
 drivers/gpu/drm/vc4/vc4_kms.c | 78 +++++++++++++++++++++++++----------
 2 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 6e037fbaa090..08c1cc225045 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -312,9 +312,6 @@ static void vc4_drm_unbind(struct device *dev)
 	drm_dev_unregister(drm);
 
 	drm_atomic_helper_shutdown(drm);
-
-	drm_atomic_private_obj_fini(&vc4->load_tracker);
-	drm_atomic_private_obj_fini(&vc4->ctm_manager);
 }
 
 static const struct component_master_ops vc4_drm_ops = {
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 708099a24406..fbfb0698073e 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -93,6 +93,29 @@ static const struct drm_private_state_funcs vc4_ctm_state_funcs = {
 	.atomic_destroy_state = vc4_ctm_destroy_state,
 };
 
+static void vc4_ctm_obj_fini(struct drm_device *dev, void *unused)
+{
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+
+	drm_atomic_private_obj_fini(&vc4->ctm_manager);
+}
+
+static int vc4_ctm_obj_init(struct vc4_dev *vc4)
+{
+	struct vc4_ctm_state *ctm_state;
+
+	drm_modeset_lock_init(&vc4->ctm_state_lock);
+
+	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
+	if (!ctm_state)
+		return -ENOMEM;
+
+	drm_atomic_private_obj_init(vc4->dev, &vc4->ctm_manager, &ctm_state->base,
+				    &vc4_ctm_state_funcs);
+
+	return drmm_add_action(&vc4->base, vc4_ctm_obj_fini, NULL);
+}
+
 /* Converts a DRM S31.32 value to the HW S0.9 format. */
 static u16 vc4_ctm_s31_32_to_s0_9(u64 in)
 {
@@ -609,6 +632,34 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = {
 	.atomic_destroy_state = vc4_load_tracker_destroy_state,
 };
 
+static void vc4_load_tracker_obj_fini(struct drm_device *dev, void *unused)
+{
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+
+	if (!vc4->load_tracker_available)
+		return 0;
+
+	drm_atomic_private_obj_fini(&vc4->load_tracker);
+}
+
+static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
+{
+	struct vc4_load_tracker_state *load_state;
+
+	if (!vc4->load_tracker_available)
+		return 0;
+
+	load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
+	if (!load_state)
+		return -ENOMEM;
+
+	drm_atomic_private_obj_init(vc4->dev, &vc4->load_tracker,
+				    &load_state->base,
+				    &vc4_load_tracker_state_funcs);
+
+	return drmm_add_action(&vc4->base, vc4_load_tracker_obj_fini, NULL);
+}
+
 #define NUM_OUTPUTS  6
 #define NUM_CHANNELS 3
 
@@ -711,8 +762,6 @@ static const struct drm_mode_config_funcs vc4_mode_funcs = {
 int vc4_kms_load(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	struct vc4_ctm_state *ctm_state;
-	struct vc4_load_tracker_state *load_state;
 	bool is_vc5 = of_device_is_compatible(dev->dev->of_node,
 					      "brcm,bcm2711-vc5");
 	int ret;
@@ -751,26 +800,13 @@ int vc4_kms_load(struct drm_device *dev)
 	dev->mode_config.async_page_flip = true;
 	dev->mode_config.allow_fb_modifiers = true;
 
-	drm_modeset_lock_init(&vc4->ctm_state_lock);
+	ret = vc4_ctm_obj_init(vc4);
+	if (ret)
+		return ret;
 
-	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
-	if (!ctm_state)
-		return -ENOMEM;
-
-	drm_atomic_private_obj_init(dev, &vc4->ctm_manager, &ctm_state->base,
-				    &vc4_ctm_state_funcs);
-
-	if (vc4->load_tracker_available) {
-		load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
-		if (!load_state) {
-			drm_atomic_private_obj_fini(&vc4->ctm_manager);
-			return -ENOMEM;
-		}
-
-		drm_atomic_private_obj_init(dev, &vc4->load_tracker,
-					    &load_state->base,
-					    &vc4_load_tracker_state_funcs);
-	}
+	ret = vc4_load_tracker_obj_init(vc4);
+	if (ret)
+		return ret;
 
 	drm_mode_config_reset(dev);
 
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/6] drm/vc4: bo: Add a managed action to cleanup the cache

[Daniel Vetter]

On Thu, Oct 29, 2020 at 08:00:59PM +0100, Maxime Ripard wrote:
> The BO cache needs to be cleaned up using vc4_bo_cache_destroy, but it's
> not used consistently (vc4_drv's bind calls it in its error path, but
> doesn't in unbind), and we can make that automatic through a managed
> action. Let's remove the requirement to call vc4_bo_cache_destroy.
> 
> Fixes: c826a6e10644 ("drm/vc4: Add a BO cache.")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/vc4/vc4_bo.c  | 5 +++--
>  drivers/gpu/drm/vc4/vc4_drv.c | 1 -
>  drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
> index f432278173cd..730d5775ab42 100644
> --- a/drivers/gpu/drm/vc4/vc4_bo.c
> +++ b/drivers/gpu/drm/vc4/vc4_bo.c
> @@ -1024,6 +1024,7 @@ int vc4_get_tiling_ioctl(struct drm_device *dev, void *data,
>  	return 0;
>  }
>  
> +static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused);
>  int vc4_bo_cache_init(struct drm_device *dev)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> @@ -1052,10 +1053,10 @@ int vc4_bo_cache_init(struct drm_device *dev)
>  	INIT_WORK(&vc4->bo_cache.time_work, vc4_bo_cache_time_work);
>  	timer_setup(&vc4->bo_cache.time_timer, vc4_bo_cache_time_timer, 0);
>  
> -	return 0;
> +	return drmm_add_action(dev, vc4_bo_cache_destroy, NULL);

I think you want drmm_add_action_or_reset here, same in the other patches.

Otherwise if drmm_add_action fails, you leak the thing you just set up.
-Daniel

>  }
>  
> -void vc4_bo_cache_destroy(struct drm_device *dev)
> +static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  	int i;
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 8f10f609e4f8..eb3fcd8232b5 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -303,7 +303,6 @@ static int vc4_drm_bind(struct device *dev)
>  gem_destroy:
>  	vc4_gem_destroy(drm);
>  	drm_mode_config_cleanup(drm);
> -	vc4_bo_cache_destroy(drm);
>  dev_put:
>  	drm_dev_put(drm);
>  	return ret;
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 836fdca5e643..3a989e8b39a2 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -14,6 +14,7 @@
>  #include <drm/drm_device.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_managed.h>
>  #include <drm/drm_mm.h>
>  #include <drm/drm_modeset_lock.h>
>  
> @@ -808,7 +809,6 @@ struct drm_gem_object *vc4_prime_import_sg_table(struct drm_device *dev,
>  						 struct sg_table *sgt);
>  void *vc4_prime_vmap(struct drm_gem_object *obj);
>  int vc4_bo_cache_init(struct drm_device *dev);
> -void vc4_bo_cache_destroy(struct drm_device *dev);
>  int vc4_bo_inc_usecnt(struct vc4_bo *bo);
>  void vc4_bo_dec_usecnt(struct vc4_bo *bo);
>  void vc4_bo_add_to_purgeable_pool(struct vc4_bo *bo);
> -- 
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] drm/vc4: bo: Add a managed action to cleanup the cache

[Daniel Vetter]

On Thu, Oct 29, 2020 at 08:00:59PM +0100, Maxime Ripard wrote:
> The BO cache needs to be cleaned up using vc4_bo_cache_destroy, but it's
> not used consistently (vc4_drv's bind calls it in its error path, but
> doesn't in unbind), and we can make that automatic through a managed
> action. Let's remove the requirement to call vc4_bo_cache_destroy.
> 
> Fixes: c826a6e10644 ("drm/vc4: Add a BO cache.")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/vc4/vc4_bo.c  | 5 +++--
>  drivers/gpu/drm/vc4/vc4_drv.c | 1 -
>  drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
> index f432278173cd..730d5775ab42 100644
> --- a/drivers/gpu/drm/vc4/vc4_bo.c
> +++ b/drivers/gpu/drm/vc4/vc4_bo.c
> @@ -1024,6 +1024,7 @@ int vc4_get_tiling_ioctl(struct drm_device *dev, void *data,
>  	return 0;
>  }
>  
> +static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused);
>  int vc4_bo_cache_init(struct drm_device *dev)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> @@ -1052,10 +1053,10 @@ int vc4_bo_cache_init(struct drm_device *dev)
>  	INIT_WORK(&vc4->bo_cache.time_work, vc4_bo_cache_time_work);
>  	timer_setup(&vc4->bo_cache.time_timer, vc4_bo_cache_time_timer, 0);
>  
> -	return 0;
> +	return drmm_add_action(dev, vc4_bo_cache_destroy, NULL);

I think you want drmm_add_action_or_reset here, same in the other patches.

Otherwise if drmm_add_action fails, you leak the thing you just set up.
-Daniel

>  }
>  
> -void vc4_bo_cache_destroy(struct drm_device *dev)
> +static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  	int i;
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 8f10f609e4f8..eb3fcd8232b5 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -303,7 +303,6 @@ static int vc4_drm_bind(struct device *dev)
>  gem_destroy:
>  	vc4_gem_destroy(drm);
>  	drm_mode_config_cleanup(drm);
> -	vc4_bo_cache_destroy(drm);
>  dev_put:
>  	drm_dev_put(drm);
>  	return ret;
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 836fdca5e643..3a989e8b39a2 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -14,6 +14,7 @@
>  #include <drm/drm_device.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_managed.h>
>  #include <drm/drm_mm.h>
>  #include <drm/drm_modeset_lock.h>
>  
> @@ -808,7 +809,6 @@ struct drm_gem_object *vc4_prime_import_sg_table(struct drm_device *dev,
>  						 struct sg_table *sgt);
>  void *vc4_prime_vmap(struct drm_gem_object *obj);
>  int vc4_bo_cache_init(struct drm_device *dev);
> -void vc4_bo_cache_destroy(struct drm_device *dev);
>  int vc4_bo_inc_usecnt(struct vc4_bo *bo);
>  void vc4_bo_dec_usecnt(struct vc4_bo *bo);
>  void vc4_bo_add_to_purgeable_pool(struct vc4_bo *bo);
> -- 
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/6] drm/vc4: kms: Add functions to create the state objects

[Daniel Vetter]

On Thu, Oct 29, 2020 at 08:01:04PM +0100, Maxime Ripard wrote:
> In order to make the vc4_kms_load code and error path a bit easier to
> read and extend, add functions to create state objects, and use managed
> actions to cleanup if needed.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Nice. With an s/drmm_add_action/drmm_add_action_or_reset/ over the entire
series:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

btw there's a series pending from imx people (Philip Zabel iirc) to add
drmm support for modeset objects. I think that would really help clean up
vc4. Or well make it slightly less buggy, since atm you're using
devm_kzalloc, which strictly speaking, frees the memory too early.

Anyway this here looks all nice.
-Daniel

> ---
>  drivers/gpu/drm/vc4/vc4_drv.c |  3 --
>  drivers/gpu/drm/vc4/vc4_kms.c | 78 +++++++++++++++++++++++++----------
>  2 files changed, 57 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 6e037fbaa090..08c1cc225045 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -312,9 +312,6 @@ static void vc4_drm_unbind(struct device *dev)
>  	drm_dev_unregister(drm);
>  
>  	drm_atomic_helper_shutdown(drm);
> -
> -	drm_atomic_private_obj_fini(&vc4->load_tracker);
> -	drm_atomic_private_obj_fini(&vc4->ctm_manager);
>  }
>  
>  static const struct component_master_ops vc4_drm_ops = {
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 708099a24406..fbfb0698073e 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -93,6 +93,29 @@ static const struct drm_private_state_funcs vc4_ctm_state_funcs = {
>  	.atomic_destroy_state = vc4_ctm_destroy_state,
>  };
>  
> +static void vc4_ctm_obj_fini(struct drm_device *dev, void *unused)
> +{
> +	struct vc4_dev *vc4 = to_vc4_dev(dev);
> +
> +	drm_atomic_private_obj_fini(&vc4->ctm_manager);
> +}
> +
> +static int vc4_ctm_obj_init(struct vc4_dev *vc4)
> +{
> +	struct vc4_ctm_state *ctm_state;
> +
> +	drm_modeset_lock_init(&vc4->ctm_state_lock);
> +
> +	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
> +	if (!ctm_state)
> +		return -ENOMEM;
> +
> +	drm_atomic_private_obj_init(vc4->dev, &vc4->ctm_manager, &ctm_state->base,
> +				    &vc4_ctm_state_funcs);
> +
> +	return drmm_add_action(&vc4->base, vc4_ctm_obj_fini, NULL);
> +}
> +
>  /* Converts a DRM S31.32 value to the HW S0.9 format. */
>  static u16 vc4_ctm_s31_32_to_s0_9(u64 in)
>  {
> @@ -609,6 +632,34 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = {
>  	.atomic_destroy_state = vc4_load_tracker_destroy_state,
>  };
>  
> +static void vc4_load_tracker_obj_fini(struct drm_device *dev, void *unused)
> +{
> +	struct vc4_dev *vc4 = to_vc4_dev(dev);
> +
> +	if (!vc4->load_tracker_available)
> +		return 0;
> +
> +	drm_atomic_private_obj_fini(&vc4->load_tracker);
> +}
> +
> +static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
> +{
> +	struct vc4_load_tracker_state *load_state;
> +
> +	if (!vc4->load_tracker_available)
> +		return 0;
> +
> +	load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
> +	if (!load_state)
> +		return -ENOMEM;
> +
> +	drm_atomic_private_obj_init(vc4->dev, &vc4->load_tracker,
> +				    &load_state->base,
> +				    &vc4_load_tracker_state_funcs);
> +
> +	return drmm_add_action(&vc4->base, vc4_load_tracker_obj_fini, NULL);
> +}
> +
>  #define NUM_OUTPUTS  6
>  #define NUM_CHANNELS 3
>  
> @@ -711,8 +762,6 @@ static const struct drm_mode_config_funcs vc4_mode_funcs = {
>  int vc4_kms_load(struct drm_device *dev)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> -	struct vc4_ctm_state *ctm_state;
> -	struct vc4_load_tracker_state *load_state;
>  	bool is_vc5 = of_device_is_compatible(dev->dev->of_node,
>  					      "brcm,bcm2711-vc5");
>  	int ret;
> @@ -751,26 +800,13 @@ int vc4_kms_load(struct drm_device *dev)
>  	dev->mode_config.async_page_flip = true;
>  	dev->mode_config.allow_fb_modifiers = true;
>  
> -	drm_modeset_lock_init(&vc4->ctm_state_lock);
> +	ret = vc4_ctm_obj_init(vc4);
> +	if (ret)
> +		return ret;
>  
> -	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
> -	if (!ctm_state)
> -		return -ENOMEM;
> -
> -	drm_atomic_private_obj_init(dev, &vc4->ctm_manager, &ctm_state->base,
> -				    &vc4_ctm_state_funcs);
> -
> -	if (vc4->load_tracker_available) {
> -		load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
> -		if (!load_state) {
> -			drm_atomic_private_obj_fini(&vc4->ctm_manager);
> -			return -ENOMEM;
> -		}
> -
> -		drm_atomic_private_obj_init(dev, &vc4->load_tracker,
> -					    &load_state->base,
> -					    &vc4_load_tracker_state_funcs);
> -	}
> +	ret = vc4_load_tracker_obj_init(vc4);
> +	if (ret)
> +		return ret;
>  
>  	drm_mode_config_reset(dev);
>  
> -- 
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] drm/vc4: kms: Add functions to create the state objects

[Daniel Vetter]

On Thu, Oct 29, 2020 at 08:01:04PM +0100, Maxime Ripard wrote:
> In order to make the vc4_kms_load code and error path a bit easier to
> read and extend, add functions to create state objects, and use managed
> actions to cleanup if needed.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Nice. With an s/drmm_add_action/drmm_add_action_or_reset/ over the entire
series:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

btw there's a series pending from imx people (Philip Zabel iirc) to add
drmm support for modeset objects. I think that would really help clean up
vc4. Or well make it slightly less buggy, since atm you're using
devm_kzalloc, which strictly speaking, frees the memory too early.

Anyway this here looks all nice.
-Daniel

> ---
>  drivers/gpu/drm/vc4/vc4_drv.c |  3 --
>  drivers/gpu/drm/vc4/vc4_kms.c | 78 +++++++++++++++++++++++++----------
>  2 files changed, 57 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 6e037fbaa090..08c1cc225045 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -312,9 +312,6 @@ static void vc4_drm_unbind(struct device *dev)
>  	drm_dev_unregister(drm);
>  
>  	drm_atomic_helper_shutdown(drm);
> -
> -	drm_atomic_private_obj_fini(&vc4->load_tracker);
> -	drm_atomic_private_obj_fini(&vc4->ctm_manager);
>  }
>  
>  static const struct component_master_ops vc4_drm_ops = {
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 708099a24406..fbfb0698073e 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -93,6 +93,29 @@ static const struct drm_private_state_funcs vc4_ctm_state_funcs = {
>  	.atomic_destroy_state = vc4_ctm_destroy_state,
>  };
>  
> +static void vc4_ctm_obj_fini(struct drm_device *dev, void *unused)
> +{
> +	struct vc4_dev *vc4 = to_vc4_dev(dev);
> +
> +	drm_atomic_private_obj_fini(&vc4->ctm_manager);
> +}
> +
> +static int vc4_ctm_obj_init(struct vc4_dev *vc4)
> +{
> +	struct vc4_ctm_state *ctm_state;
> +
> +	drm_modeset_lock_init(&vc4->ctm_state_lock);
> +
> +	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
> +	if (!ctm_state)
> +		return -ENOMEM;
> +
> +	drm_atomic_private_obj_init(vc4->dev, &vc4->ctm_manager, &ctm_state->base,
> +				    &vc4_ctm_state_funcs);
> +
> +	return drmm_add_action(&vc4->base, vc4_ctm_obj_fini, NULL);
> +}
> +
>  /* Converts a DRM S31.32 value to the HW S0.9 format. */
>  static u16 vc4_ctm_s31_32_to_s0_9(u64 in)
>  {
> @@ -609,6 +632,34 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = {
>  	.atomic_destroy_state = vc4_load_tracker_destroy_state,
>  };
>  
> +static void vc4_load_tracker_obj_fini(struct drm_device *dev, void *unused)
> +{
> +	struct vc4_dev *vc4 = to_vc4_dev(dev);
> +
> +	if (!vc4->load_tracker_available)
> +		return 0;
> +
> +	drm_atomic_private_obj_fini(&vc4->load_tracker);
> +}
> +
> +static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
> +{
> +	struct vc4_load_tracker_state *load_state;
> +
> +	if (!vc4->load_tracker_available)
> +		return 0;
> +
> +	load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
> +	if (!load_state)
> +		return -ENOMEM;
> +
> +	drm_atomic_private_obj_init(vc4->dev, &vc4->load_tracker,
> +				    &load_state->base,
> +				    &vc4_load_tracker_state_funcs);
> +
> +	return drmm_add_action(&vc4->base, vc4_load_tracker_obj_fini, NULL);
> +}
> +
>  #define NUM_OUTPUTS  6
>  #define NUM_CHANNELS 3
>  
> @@ -711,8 +762,6 @@ static const struct drm_mode_config_funcs vc4_mode_funcs = {
>  int vc4_kms_load(struct drm_device *dev)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> -	struct vc4_ctm_state *ctm_state;
> -	struct vc4_load_tracker_state *load_state;
>  	bool is_vc5 = of_device_is_compatible(dev->dev->of_node,
>  					      "brcm,bcm2711-vc5");
>  	int ret;
> @@ -751,26 +800,13 @@ int vc4_kms_load(struct drm_device *dev)
>  	dev->mode_config.async_page_flip = true;
>  	dev->mode_config.allow_fb_modifiers = true;
>  
> -	drm_modeset_lock_init(&vc4->ctm_state_lock);
> +	ret = vc4_ctm_obj_init(vc4);
> +	if (ret)
> +		return ret;
>  
> -	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
> -	if (!ctm_state)
> -		return -ENOMEM;
> -
> -	drm_atomic_private_obj_init(dev, &vc4->ctm_manager, &ctm_state->base,
> -				    &vc4_ctm_state_funcs);
> -
> -	if (vc4->load_tracker_available) {
> -		load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
> -		if (!load_state) {
> -			drm_atomic_private_obj_fini(&vc4->ctm_manager);
> -			return -ENOMEM;
> -		}
> -
> -		drm_atomic_private_obj_init(dev, &vc4->load_tracker,
> -					    &load_state->base,
> -					    &vc4_load_tracker_state_funcs);
> -	}
> +	ret = vc4_load_tracker_obj_init(vc4);
> +	if (ret)
> +		return ret;
>  
>  	drm_mode_config_reset(dev);
>  
> -- 
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/6] drm/vc4: kms: Add functions to create the state objects

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 895 bytes --]

On Fri, Oct 30, 2020 at 09:59:31AM +0100, Daniel Vetter wrote:
> On Thu, Oct 29, 2020 at 08:01:04PM +0100, Maxime Ripard wrote:
> > In order to make the vc4_kms_load code and error path a bit easier to
> > read and extend, add functions to create state objects, and use managed
> > actions to cleanup if needed.
> > 
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> 
> Nice. With an s/drmm_add_action/drmm_add_action_or_reset/ over the entire
> series:
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Applied with drmm_add_action_or_reset

> btw there's a series pending from imx people (Philip Zabel iirc) to add
> drmm support for modeset objects. I think that would really help clean up
> vc4. Or well make it slightly less buggy, since atm you're using
> devm_kzalloc, which strictly speaking, frees the memory too early.

I'll have a look, thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] drm/vc4: kms: Add functions to create the state objects

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 895 bytes --]

On Fri, Oct 30, 2020 at 09:59:31AM +0100, Daniel Vetter wrote:
> On Thu, Oct 29, 2020 at 08:01:04PM +0100, Maxime Ripard wrote:
> > In order to make the vc4_kms_load code and error path a bit easier to
> > read and extend, add functions to create state objects, and use managed
> > actions to cleanup if needed.
> > 
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> 
> Nice. With an s/drmm_add_action/drmm_add_action_or_reset/ over the entire
> series:
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Applied with drmm_add_action_or_reset

> btw there's a series pending from imx people (Philip Zabel iirc) to add
> drmm support for modeset objects. I think that would really help clean up
> vc4. Or well make it slightly less buggy, since atm you're using
> devm_kzalloc, which strictly speaking, frees the memory too early.

I'll have a look, thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel