* [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()
@ 2015-11-20 8:14 Liu Ying
2015-11-20 8:14 ` [PATCH 2/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_crtc_init() when necessary Liu Ying
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Liu Ying @ 2015-11-20 8:14 UTC (permalink / raw)
To: dri-devel
This patch adds a helper ipu_plane_cleanup() to cleanup a IPU plane.
It can be used in the bailout path of ipu_crtc_init(), for instance.
Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
drivers/gpu/drm/imx/ipuv3-plane.c | 6 ++++++
drivers/gpu/drm/imx/ipuv3-plane.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index e2ff410..e60b382 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -410,3 +410,9 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
return ipu_plane;
}
+
+void ipu_plane_cleanup(struct ipu_plane *ipu_plane)
+{
+ drm_plane_cleanup(&ipu_plane->base);
+ kfree(ipu_plane);
+}
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.h b/drivers/gpu/drm/imx/ipuv3-plane.h
index 3a443b4..dd2239c 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.h
+++ b/drivers/gpu/drm/imx/ipuv3-plane.h
@@ -36,6 +36,8 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
int dma, int dp, unsigned int possible_crtcs,
enum drm_plane_type type);
+void ipu_plane_cleanup(struct ipu_plane *ipu_plane);
+
/* Init IDMAC, DMFC, DP */
int ipu_plane_mode_set(struct ipu_plane *plane, struct drm_crtc *crtc,
struct drm_display_mode *mode,
--
2.5.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_crtc_init() when necessary
2015-11-20 8:14 [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup() Liu Ying
@ 2015-11-20 8:14 ` Liu Ying
2015-11-23 11:48 ` Philipp Zabel
2015-11-20 8:14 ` [PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind() Liu Ying
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Liu Ying @ 2015-11-20 8:14 UTC (permalink / raw)
To: dri-devel
To avoid memory leakage, we need to cleanup the initialized ipu planes in
the bailout path of ipu_crtc_init().
Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
drivers/gpu/drm/imx/ipuv3-crtc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 67813ca..59f44df 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -371,7 +371,7 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
ipu_crtc->dev->of_node);
if (ret) {
dev_err(ipu_crtc->dev, "adding crtc failed with %d.\n", ret);
- goto err_put_resources;
+ goto err_cleanup_plane0;
}
ret = ipu_plane_get_resources(ipu_crtc->plane[0]);
@@ -402,9 +402,14 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
return 0;
err_put_plane_res:
+ if (ipu_crtc->plane[1])
+ ipu_plane_cleanup(ipu_crtc->plane[1]);
+
ipu_plane_put_resources(ipu_crtc->plane[0]);
err_remove_crtc:
imx_drm_remove_crtc(ipu_crtc->imx_crtc);
+err_cleanup_plane0:
+ ipu_plane_cleanup(ipu_crtc->plane[0]);
err_put_resources:
ipu_put_resources(ipu_crtc);
--
2.5.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind()
2015-11-20 8:14 [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup() Liu Ying
2015-11-20 8:14 ` [PATCH 2/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_crtc_init() when necessary Liu Ying
@ 2015-11-20 8:14 ` Liu Ying
2015-11-23 11:48 ` Philipp Zabel
2015-11-20 8:14 ` [PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy() Liu Ying
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Liu Ying @ 2015-11-20 8:14 UTC (permalink / raw)
To: dri-devel
To avoid memory leakage, we need to cleanup ipu planes in ipu_drm_unbind().
Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
drivers/gpu/drm/imx/ipuv3-crtc.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 59f44df..467905c 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -446,6 +446,11 @@ static void ipu_drm_unbind(struct device *dev, struct device *master,
imx_drm_remove_crtc(ipu_crtc->imx_crtc);
ipu_plane_put_resources(ipu_crtc->plane[0]);
+
+ if (ipu_crtc->plane[1])
+ ipu_plane_cleanup(ipu_crtc->plane[1]);
+ ipu_plane_cleanup(ipu_crtc->plane[0]);
+
ipu_put_resources(ipu_crtc);
}
--
2.5.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy()
2015-11-20 8:14 [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup() Liu Ying
2015-11-20 8:14 ` [PATCH 2/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_crtc_init() when necessary Liu Ying
2015-11-20 8:14 ` [PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind() Liu Ying
@ 2015-11-20 8:14 ` Liu Ying
2015-11-23 11:48 ` Philipp Zabel
2015-11-20 8:14 ` [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes Liu Ying
2015-11-23 11:48 ` [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup() Philipp Zabel
4 siblings, 1 reply; 15+ messages in thread
From: Liu Ying @ 2015-11-20 8:14 UTC (permalink / raw)
To: dri-devel
To reduce code duplication, we can use the helper ipu_plane_cleanup() in
ipu_plane_destroy().
Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
drivers/gpu/drm/imx/ipuv3-plane.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index e60b382..b3ed207 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -369,8 +369,7 @@ static void ipu_plane_destroy(struct drm_plane *plane)
DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
ipu_disable_plane(plane);
- drm_plane_cleanup(plane);
- kfree(ipu_plane);
+ ipu_plane_cleanup(ipu_plane);
}
static struct drm_plane_funcs ipu_plane_funcs = {
--
2.5.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes
2015-11-20 8:14 [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup() Liu Ying
` (2 preceding siblings ...)
2015-11-20 8:14 ` [PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy() Liu Ying
@ 2015-11-20 8:14 ` Liu Ying
2015-11-23 11:48 ` Philipp Zabel
2015-11-23 11:48 ` [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup() Philipp Zabel
4 siblings, 1 reply; 15+ messages in thread
From: Liu Ying @ 2015-11-20 8:14 UTC (permalink / raw)
To: dri-devel
This patch changes the dev_info() call to dev_dbg() in ipu_plane_update()
to print out the information that a plane's CRTC is changed, because this
kind of information is only useful for debugging.
Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index b3ed207..b24bf94 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -338,7 +338,7 @@ static int ipu_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
}
if (crtc != plane->crtc)
- dev_info(plane->dev->dev, "crtc change: %p -> %p\n",
+ dev_dbg(plane->dev->dev, "crtc change: %p -> %p\n",
plane->crtc, crtc);
plane->crtc = crtc;
--
2.5.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_crtc_init() when necessary
2015-11-20 8:14 ` [PATCH 2/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_crtc_init() when necessary Liu Ying
@ 2015-11-23 11:48 ` Philipp Zabel
0 siblings, 0 replies; 15+ messages in thread
From: Philipp Zabel @ 2015-11-23 11:48 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel
Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> To avoid memory leakage, we need to cleanup the initialized ipu planes in
> the bailout path of ipu_crtc_init().
>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
>
> drivers/gpu/drm/imx/ipuv3-crtc.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 67813ca..59f44df 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -371,7 +371,7 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
> ipu_crtc->dev->of_node);
> if (ret) {
> dev_err(ipu_crtc->dev, "adding crtc failed with %d.\n", ret);
> - goto err_put_resources;
> + goto err_cleanup_plane0;
> }
>
> ret = ipu_plane_get_resources(ipu_crtc->plane[0]);
> @@ -402,9 +402,14 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
> return 0;
>
> err_put_plane_res:
> + if (ipu_crtc->plane[1])
> + ipu_plane_cleanup(ipu_crtc->plane[1]);
> +
> ipu_plane_put_resources(ipu_crtc->plane[0]);
> err_remove_crtc:
> imx_drm_remove_crtc(ipu_crtc->imx_crtc);
> +err_cleanup_plane0:
> + ipu_plane_cleanup(ipu_crtc->plane[0]);
> err_put_resources:
> ipu_put_resources(ipu_crtc);
I think we can use ipu_plane_destroy as-is instead of
ipu_plane_put_resources + ipu_plane_cleanup.
regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy()
2015-11-20 8:14 ` [PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy() Liu Ying
@ 2015-11-23 11:48 ` Philipp Zabel
2015-11-24 5:29 ` Liu Ying
0 siblings, 1 reply; 15+ messages in thread
From: Philipp Zabel @ 2015-11-23 11:48 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel
Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> To reduce code duplication, we can use the helper ipu_plane_cleanup() in
> ipu_plane_destroy().
>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
>
> drivers/gpu/drm/imx/ipuv3-plane.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index e60b382..b3ed207 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -369,8 +369,7 @@ static void ipu_plane_destroy(struct drm_plane *plane)
> DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
>
> ipu_disable_plane(plane);
> - drm_plane_cleanup(plane);
> - kfree(ipu_plane);
> + ipu_plane_cleanup(ipu_plane);
> }
>
> static struct drm_plane_funcs ipu_plane_funcs = {
This could be merged into the first patch, but I don't think
ipu_plane_cleanup is necessary at all.
regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()
2015-11-20 8:14 [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup() Liu Ying
` (3 preceding siblings ...)
2015-11-20 8:14 ` [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes Liu Ying
@ 2015-11-23 11:48 ` Philipp Zabel
2015-11-24 3:31 ` Liu Ying
4 siblings, 1 reply; 15+ messages in thread
From: Philipp Zabel @ 2015-11-23 11:48 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel
Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> This patch adds a helper ipu_plane_cleanup() to cleanup a IPU plane.
> It can be used in the bailout path of ipu_crtc_init(), for instance.
>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
>
> drivers/gpu/drm/imx/ipuv3-plane.c | 6 ++++++
> drivers/gpu/drm/imx/ipuv3-plane.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index e2ff410..e60b382 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -410,3 +410,9 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
>
> return ipu_plane;
> }
> +
> +void ipu_plane_cleanup(struct ipu_plane *ipu_plane)
> +{
> + drm_plane_cleanup(&ipu_plane->base);
> + kfree(ipu_plane);
> +}
The name says cleanup, but that's not what it does. This function should
be named ipu_plane_free, or ipu_plane_destroy. Actually, we have that
already.
regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind()
2015-11-20 8:14 ` [PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind() Liu Ying
@ 2015-11-23 11:48 ` Philipp Zabel
2015-11-24 5:23 ` Liu Ying
0 siblings, 1 reply; 15+ messages in thread
From: Philipp Zabel @ 2015-11-23 11:48 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel
Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> To avoid memory leakage, we need to cleanup ipu planes in ipu_drm_unbind().
>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
>
> drivers/gpu/drm/imx/ipuv3-crtc.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 59f44df..467905c 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -446,6 +446,11 @@ static void ipu_drm_unbind(struct device *dev, struct device *master,
> imx_drm_remove_crtc(ipu_crtc->imx_crtc);
>
> ipu_plane_put_resources(ipu_crtc->plane[0]);
> +
> + if (ipu_crtc->plane[1])
> + ipu_plane_cleanup(ipu_crtc->plane[1]);
> + ipu_plane_cleanup(ipu_crtc->plane[0]);
> +
> ipu_put_resources(ipu_crtc);
> }
>
Shouldn't this already be handled by the DRM core calling the
plane->destroy callbacks from drm_mode_config_cleanup (called by
imx_drm_driver_unload shortly after component_unbind_all)?
regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes
2015-11-20 8:14 ` [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes Liu Ying
@ 2015-11-23 11:48 ` Philipp Zabel
2016-01-22 2:29 ` Liu Ying
0 siblings, 1 reply; 15+ messages in thread
From: Philipp Zabel @ 2015-11-23 11:48 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel
Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> This patch changes the dev_info() call to dev_dbg() in ipu_plane_update()
> to print out the information that a plane's CRTC is changed, because this
> kind of information is only useful for debugging.
>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
>
> drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index b3ed207..b24bf94 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -338,7 +338,7 @@ static int ipu_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> }
>
> if (crtc != plane->crtc)
> - dev_info(plane->dev->dev, "crtc change: %p -> %p\n",
> + dev_dbg(plane->dev->dev, "crtc change: %p -> %p\n",
> plane->crtc, crtc);
> plane->crtc = crtc;
This change is separate from the others, I have applied it.
thanks
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()
2015-11-23 11:48 ` [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup() Philipp Zabel
@ 2015-11-24 3:31 ` Liu Ying
0 siblings, 0 replies; 15+ messages in thread
From: Liu Ying @ 2015-11-24 3:31 UTC (permalink / raw)
To: Philipp Zabel; +Cc: dri-devel
On Mon, Nov 23, 2015 at 12:48:13PM +0100, Philipp Zabel wrote:
> Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> > This patch adds a helper ipu_plane_cleanup() to cleanup a IPU plane.
> > It can be used in the bailout path of ipu_crtc_init(), for instance.
> >
> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> > ---
> > This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
> >
> > drivers/gpu/drm/imx/ipuv3-plane.c | 6 ++++++
> > drivers/gpu/drm/imx/ipuv3-plane.h | 2 ++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> > index e2ff410..e60b382 100644
> > --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> > @@ -410,3 +410,9 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
> >
> > return ipu_plane;
> > }
> > +
> > +void ipu_plane_cleanup(struct ipu_plane *ipu_plane)
> > +{
> > + drm_plane_cleanup(&ipu_plane->base);
> > + kfree(ipu_plane);
> > +}
>
> The name says cleanup, but that's not what it does. This function should
> be named ipu_plane_free, or ipu_plane_destroy. Actually, we have that
> already.
Since ipu_crtc_init() may call ipu_plane_init()/ipu_plane_get_resources()
for a plane, the bailout path deserves the same granularity to clarity the
logic. It looks somewhat awkward to use the callback plane->destroy() to
tear down the plane in the bailout path or to export the static function
ipu_plane_destroy() and use it directly.
I prefer to change ipu_plane_cleanup to ipu_plane_free and follow the fine
granularity way.
Regards,
Liu Ying
>
> regards
> Philipp
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind()
2015-11-23 11:48 ` Philipp Zabel
@ 2015-11-24 5:23 ` Liu Ying
0 siblings, 0 replies; 15+ messages in thread
From: Liu Ying @ 2015-11-24 5:23 UTC (permalink / raw)
To: Philipp Zabel; +Cc: dri-devel
On Mon, Nov 23, 2015 at 12:48:14PM +0100, Philipp Zabel wrote:
> Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> > To avoid memory leakage, we need to cleanup ipu planes in ipu_drm_unbind().
> >
> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> > ---
> > This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
> >
> > drivers/gpu/drm/imx/ipuv3-crtc.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > index 59f44df..467905c 100644
> > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > @@ -446,6 +446,11 @@ static void ipu_drm_unbind(struct device *dev, struct device *master,
> > imx_drm_remove_crtc(ipu_crtc->imx_crtc);
> >
> > ipu_plane_put_resources(ipu_crtc->plane[0]);
> > +
> > + if (ipu_crtc->plane[1])
> > + ipu_plane_cleanup(ipu_crtc->plane[1]);
> > + ipu_plane_cleanup(ipu_crtc->plane[0]);
> > +
> > ipu_put_resources(ipu_crtc);
> > }
> >
>
> Shouldn't this already be handled by the DRM core calling the
> plane->destroy callbacks from drm_mode_config_cleanup (called by
> imx_drm_driver_unload shortly after component_unbind_all)?
I take drm_mode_config_cleanup() as the final goal keeper. The component
->unbind() may clean things up by itself other than rely on the master's
behaviour. Otherwise, we even don't need to call ipu_plane_put_resources()
here.
Regards,
Liu Ying
> regards
> Philipp
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy()
2015-11-23 11:48 ` Philipp Zabel
@ 2015-11-24 5:29 ` Liu Ying
0 siblings, 0 replies; 15+ messages in thread
From: Liu Ying @ 2015-11-24 5:29 UTC (permalink / raw)
To: Philipp Zabel; +Cc: dri-devel
On Mon, Nov 23, 2015 at 12:48:12PM +0100, Philipp Zabel wrote:
> Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> > To reduce code duplication, we can use the helper ipu_plane_cleanup() in
> > ipu_plane_destroy().
> >
> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> > ---
> > This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
> >
> > drivers/gpu/drm/imx/ipuv3-plane.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> > index e60b382..b3ed207 100644
> > --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> > @@ -369,8 +369,7 @@ static void ipu_plane_destroy(struct drm_plane *plane)
> > DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
> >
> > ipu_disable_plane(plane);
> > - drm_plane_cleanup(plane);
> > - kfree(ipu_plane);
> > + ipu_plane_cleanup(ipu_plane);
> > }
> >
> > static struct drm_plane_funcs ipu_plane_funcs = {
>
> This could be merged into the first patch, but I don't think
> ipu_plane_cleanup is necessary at all.
IMHO, it doesn't hurt to split it up as two patches :)
Regards,
Liu Ying
>
> regards
> Philipp
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes
2015-11-23 11:48 ` Philipp Zabel
@ 2016-01-22 2:29 ` Liu Ying
2016-01-25 9:56 ` Philipp Zabel
0 siblings, 1 reply; 15+ messages in thread
From: Liu Ying @ 2016-01-22 2:29 UTC (permalink / raw)
To: Philipp Zabel; +Cc: dri-devel
Hi Philipp,
2015-11-23 19:48 GMT+08:00 Philipp Zabel <p.zabel@pengutronix.de>:
> Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
>> This patch changes the dev_info() call to dev_dbg() in ipu_plane_update()
>> to print out the information that a plane's CRTC is changed, because this
>> kind of information is only useful for debugging.
>>
>> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>> ---
>> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
>>
>> drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
>> index b3ed207..b24bf94 100644
>> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
>> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
>> @@ -338,7 +338,7 @@ static int ipu_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>> }
>>
>> if (crtc != plane->crtc)
>> - dev_info(plane->dev->dev, "crtc change: %p -> %p\n",
>> + dev_dbg(plane->dev->dev, "crtc change: %p -> %p\n",
>> plane->crtc, crtc);
>> plane->crtc = crtc;
>
> This change is separate from the others, I have applied it.
I don't find this patch on any imx-drm branch in your open git repository.
Do I miss anything?
Regards,
Liu Ying
>
> thanks
> Philipp
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes
2016-01-22 2:29 ` Liu Ying
@ 2016-01-25 9:56 ` Philipp Zabel
0 siblings, 0 replies; 15+ messages in thread
From: Philipp Zabel @ 2016-01-25 9:56 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel
Hi Liu,
Am Freitag, den 22.01.2016, 10:29 +0800 schrieb Liu Ying:
> Hi Philipp,
>
> 2015-11-23 19:48 GMT+08:00 Philipp Zabel <p.zabel@pengutronix.de>:
> > Am Freitag, den 20.11.2015, 16:14 +0800 schrieb Liu Ying:
> >> This patch changes the dev_info() call to dev_dbg() in ipu_plane_update()
> >> to print out the information that a plane's CRTC is changed, because this
> >> kind of information is only useful for debugging.
> >>
> >> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> >> ---
> >> This patch applies to the imx-drm/fixes branch of Philipp Zabel's open git.
> >>
> >> drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> >> index b3ed207..b24bf94 100644
> >> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> >> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> >> @@ -338,7 +338,7 @@ static int ipu_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> >> }
> >>
> >> if (crtc != plane->crtc)
> >> - dev_info(plane->dev->dev, "crtc change: %p -> %p\n",
> >> + dev_dbg(plane->dev->dev, "crtc change: %p -> %p\n",
> >> plane->crtc, crtc);
> >> plane->crtc = crtc;
> >
> > This change is separate from the others, I have applied it.
>
> I don't find this patch on any imx-drm branch in your open git repository.
> Do I miss anything?
It's in the imx-drm/next branch, I have waited for v4.5-rc1 before
pushing it.
regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-01-25 9:56 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-20 8:14 [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup() Liu Ying
2015-11-20 8:14 ` [PATCH 2/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_crtc_init() when necessary Liu Ying
2015-11-23 11:48 ` Philipp Zabel
2015-11-20 8:14 ` [PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind() Liu Ying
2015-11-23 11:48 ` Philipp Zabel
2015-11-24 5:23 ` Liu Ying
2015-11-20 8:14 ` [PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy() Liu Ying
2015-11-23 11:48 ` Philipp Zabel
2015-11-24 5:29 ` Liu Ying
2015-11-20 8:14 ` [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes Liu Ying
2015-11-23 11:48 ` Philipp Zabel
2016-01-22 2:29 ` Liu Ying
2016-01-25 9:56 ` Philipp Zabel
2015-11-23 11:48 ` [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup() Philipp Zabel
2015-11-24 3:31 ` Liu Ying
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).