| Message ID | 9af0b0e18c6f3ce3348cc728f177bf466e30e66a.1687423204.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: renesas: shmobile: Atomic conversion + DT support | expand |
Hi Geert, Thank you for the patch. On Thu, Jun 22, 2023 at 11:21:29AM +0200, Geert Uytterhoeven wrote: > According to the comments for drm_universal_plane_init(), the plane > structure should not be allocated with devm_kzalloc(). > > Fix lifetime issues by using drmm_universal_plane_alloc() instead. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Plane (and connector) structures are still allocated with devm_kzalloc() > in several other drivers... > --- > .../drm/renesas/shmobile/shmob_drm_plane.c | 24 ++++++------------- > 1 file changed, 7 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c > index 0b2ab153e9ae76df..1fb68b5fe915b8dc 100644 > --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c > +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c > @@ -176,16 +176,9 @@ static int shmob_drm_plane_disable(struct drm_plane *plane, > return 0; > } > > -static void shmob_drm_plane_destroy(struct drm_plane *plane) > -{ > - drm_plane_force_disable(plane); > - drm_plane_cleanup(plane); drm_plane_cleanup() will still be called from drmm_universal_plane_alloc_release(), but drm_plane_force_disable() won't. Is this an issue ? This should be documented in the commit message. > -} > - > static const struct drm_plane_funcs shmob_drm_plane_funcs = { > .update_plane = shmob_drm_plane_update, > .disable_plane = shmob_drm_plane_disable, > - .destroy = shmob_drm_plane_destroy, > }; > > static const uint32_t formats[] = { > @@ -204,19 +197,16 @@ static const uint32_t formats[] = { > int shmob_drm_plane_create(struct shmob_drm_device *sdev, unsigned int index) > { > struct shmob_drm_plane *splane; > - int ret; > > - splane = devm_kzalloc(sdev->dev, sizeof(*splane), GFP_KERNEL); > - if (splane == NULL) > - return -ENOMEM; > + splane = drmm_universal_plane_alloc(sdev->ddev, struct shmob_drm_plane, > + plane, 1, &shmob_drm_plane_funcs, > + formats, ARRAY_SIZE(formats), NULL, > + DRM_PLANE_TYPE_OVERLAY, NULL); > + if (IS_ERR(splane)) > + return PTR_ERR(splane); > > splane->index = index; > splane->alpha = 255; > > - ret = drm_universal_plane_init(sdev->ddev, &splane->plane, 1, > - &shmob_drm_plane_funcs, > - formats, ARRAY_SIZE(formats), NULL, > - DRM_PLANE_TYPE_OVERLAY, NULL); > - > - return ret; > + return 0; > }
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c index 0b2ab153e9ae76df..1fb68b5fe915b8dc 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c @@ -176,16 +176,9 @@ static int shmob_drm_plane_disable(struct drm_plane *plane, return 0; } -static void shmob_drm_plane_destroy(struct drm_plane *plane) -{ - drm_plane_force_disable(plane); - drm_plane_cleanup(plane); -} - static const struct drm_plane_funcs shmob_drm_plane_funcs = { .update_plane = shmob_drm_plane_update, .disable_plane = shmob_drm_plane_disable, - .destroy = shmob_drm_plane_destroy, }; static const uint32_t formats[] = { @@ -204,19 +197,16 @@ static const uint32_t formats[] = { int shmob_drm_plane_create(struct shmob_drm_device *sdev, unsigned int index) { struct shmob_drm_plane *splane; - int ret; - splane = devm_kzalloc(sdev->dev, sizeof(*splane), GFP_KERNEL); - if (splane == NULL) - return -ENOMEM; + splane = drmm_universal_plane_alloc(sdev->ddev, struct shmob_drm_plane, + plane, 1, &shmob_drm_plane_funcs, + formats, ARRAY_SIZE(formats), NULL, + DRM_PLANE_TYPE_OVERLAY, NULL); + if (IS_ERR(splane)) + return PTR_ERR(splane); splane->index = index; splane->alpha = 255; - ret = drm_universal_plane_init(sdev->ddev, &splane->plane, 1, - &shmob_drm_plane_funcs, - formats, ARRAY_SIZE(formats), NULL, - DRM_PLANE_TYPE_OVERLAY, NULL); - - return ret; + return 0; }
According to the comments for drm_universal_plane_init(), the plane structure should not be allocated with devm_kzalloc(). Fix lifetime issues by using drmm_universal_plane_alloc() instead. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Plane (and connector) structures are still allocated with devm_kzalloc() in several other drivers... --- .../drm/renesas/shmobile/shmob_drm_plane.c | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-)