[PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()
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 > > --- > > 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(_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 >
[PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()
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 > --- > 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(_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
[PATCH 1/5] drm/imx: ipuv3 plane: Introduce ipu_plane_cleanup()
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 --- 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(_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