* [PATCH 1/6] drm/vc4: bo: Add a managed action to cleanup the cache
@ 2020-10-29 19:00 ` Maxime Ripard
0 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:00 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 2/6] drm/vc4: drv: Use managed drm_mode_config_init
2020-10-29 19:00 ` Maxime Ripard
@ 2020-10-29 19:01 ` Maxime Ripard
-1 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/6] drm/vc4: drv: Use managed drm_mode_config_init
@ 2020-10-29 19:01 ` Maxime Ripard
0 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/6] drm/vc4: gem: Add a managed action to cleanup the job queue
2020-10-29 19:00 ` Maxime Ripard
@ 2020-10-29 19:01 ` Maxime Ripard
-1 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 3/6] drm/vc4: gem: Add a managed action to cleanup the job queue
@ 2020-10-29 19:01 ` Maxime Ripard
0 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/6] drm/vc4: Use the helper to retrieve vc4_dev when needed
2020-10-29 19:00 ` Maxime Ripard
@ 2020-10-29 19:01 ` Maxime Ripard
-1 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 4/6] drm/vc4: Use the helper to retrieve vc4_dev when needed
@ 2020-10-29 19:01 ` Maxime Ripard
0 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/6] drm/vc4: Use devm_drm_dev_alloc
2020-10-29 19:00 ` Maxime Ripard
@ 2020-10-29 19:01 ` Maxime Ripard
-1 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 5/6] drm/vc4: Use devm_drm_dev_alloc
@ 2020-10-29 19:01 ` Maxime Ripard
0 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/6] drm/vc4: kms: Add functions to create the state objects
2020-10-29 19:00 ` Maxime Ripard
@ 2020-10-29 19:01 ` Maxime Ripard
-1 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 6/6] drm/vc4: kms: Add functions to create the state objects
@ 2020-10-29 19:01 ` Maxime Ripard
0 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-10-29 19:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Eric Anholt
Cc: linux-arm-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
dri-devel, Dave Stevenson
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 6/6] drm/vc4: kms: Add functions to create the state objects
2020-10-29 19:01 ` Maxime Ripard
@ 2020-10-30 8:59 ` Daniel Vetter
-1 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2020-10-30 8:59 UTC (permalink / raw)
To: Maxime Ripard
Cc: Dave Stevenson, David Airlie, Maarten Lankhorst, dri-devel,
Eric Anholt, bcm-kernel-feedback-list, linux-rpi-kernel,
Thomas Zimmermann, Daniel Vetter, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 6/6] drm/vc4: kms: Add functions to create the state objects
@ 2020-10-30 8:59 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2020-10-30 8:59 UTC (permalink / raw)
To: Maxime Ripard
Cc: Dave Stevenson, David Airlie, dri-devel,
bcm-kernel-feedback-list, linux-rpi-kernel, Thomas Zimmermann,
Daniel Vetter, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 6/6] drm/vc4: kms: Add functions to create the state objects
2020-10-30 8:59 ` Daniel Vetter
@ 2020-11-02 11:17 ` Maxime Ripard
-1 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-11-02 11:17 UTC (permalink / raw)
To: Daniel Vetter
Cc: Dave Stevenson, David Airlie, Maarten Lankhorst, dri-devel,
Eric Anholt, bcm-kernel-feedback-list, linux-rpi-kernel,
Thomas Zimmermann, Daniel Vetter, linux-arm-kernel
[-- 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] drm/vc4: kms: Add functions to create the state objects
@ 2020-11-02 11:17 ` Maxime Ripard
0 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2020-11-02 11:17 UTC (permalink / raw)
To: Daniel Vetter
Cc: Dave Stevenson, David Airlie, dri-devel,
bcm-kernel-feedback-list, linux-rpi-kernel, Thomas Zimmermann,
Daniel Vetter, linux-arm-kernel
[-- 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/6] drm/vc4: bo: Add a managed action to cleanup the cache
2020-10-29 19:00 ` Maxime Ripard
@ 2020-10-30 8:56 ` Daniel Vetter
-1 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2020-10-30 8:56 UTC (permalink / raw)
To: Maxime Ripard
Cc: Dave Stevenson, David Airlie, Maarten Lankhorst, dri-devel,
Eric Anholt, bcm-kernel-feedback-list, linux-rpi-kernel,
Thomas Zimmermann, Daniel Vetter, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 1/6] drm/vc4: bo: Add a managed action to cleanup the cache
@ 2020-10-30 8:56 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2020-10-30 8:56 UTC (permalink / raw)
To: Maxime Ripard
Cc: Dave Stevenson, David Airlie, dri-devel,
bcm-kernel-feedback-list, linux-rpi-kernel, Thomas Zimmermann,
Daniel Vetter, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 18+ messages in thread