[PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()

[PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()

[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);
+}
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

[PATCH 2/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_crtc_init() when necessary

[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);
 
-- 
2.5.0

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

Re: [PATCH 2/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_crtc_init() when necessary

[Philipp Zabel]

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

[PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind()

[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);
 }
 
-- 
2.5.0

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

Re: [PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind()

[Philipp Zabel]

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

Re: [PATCH 3/5] drm/imx: ipuv3 crtc: Cleanup ipu planes in ipu_drm_unbind()

[Liu Ying]

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

[PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy()

[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 = {
-- 
2.5.0

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

Re: [PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy()

[Philipp Zabel]

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

Re: [PATCH 4/5] drm/imx: ipuv3 plane: Use the helper ipu_plane_cleanup() in ipu_plane_destroy()

[Liu Ying]

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

[PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes

[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;
 
-- 
2.5.0

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

Re: [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes

[Philipp Zabel]

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

Re: [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes

[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?

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

Re: [PATCH 5/5] drm/imx: ipuv3 plane: Replace dev_info with dev_dbg if a plane's CRTC changes

[Philipp Zabel]

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

Re: [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()

[Philipp Zabel]

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

Re: [PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()

[Liu Ying]

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