Re: [PATCH RESEND drm-misc-next 4/7] drm/arm/hdlcd: plane: use drm managed resources

[Danilo Krummrich]

Hi Liviu,

Thanks for having a look!
This is not about this patch, it's about patch 3/7 "drm/arm/hdlcd: crtc: 
use drmm_crtc_init_with_planes()".

And there it's the other way around. When using 
drmm_crtc_init_with_planes() we shouldn't have a destroy hook in place, 
that's the whole purpose of drmm_crtc_init_with_planes().

We should just drop patch 3/7 "drm/arm/hdlcd: crtc: use 
drmm_crtc_init_with_planes()", it's wrong.

Do you want me to send a v2 for that?

- Danilo



On 9/12/22 19:36, Liviu Dudau wrote:
Hi Danilo,

I have applied your patch series for HDLCD on top of drm-next (commit 
213cb76ddc8b)
and on start up I get a warning:

[   12.882554] hdlcd 7ff50000.hdlcd: drm_WARN_ON(funcs && funcs->destroy)
[   12.882596] WARNING: CPU: 1 PID: 211 at drivers/gpu/drm/drm_crtc.c:393 
__drmm_crtc_init_with_planes+0x70/0xf0 [drm]

It looks like the .destroy hook is still required or I'm missing some other 
required
series where the WARN has been removed?

Best regards,
Liviu

On Mon, Sep 05, 2022 at 05:27:16PM +0200, Danilo Krummrich wrote:
Use drm managed resource allocation (drmm_universal_plane_alloc()) in
order to get rid of the explicit destroy hook in struct drm_plane_funcs.

Signed-off-by: Danilo Krummrich <d...@redhat.com>
---
  drivers/gpu/drm/arm/hdlcd_crtc.c | 20 +++++++-------------
  1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index c0a5ca7f578a..17d3ccf12245 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -289,7 +289,6 @@ static const struct drm_plane_helper_funcs 
hdlcd_plane_helper_funcs = {
  static const struct drm_plane_funcs hdlcd_plane_funcs = {
        .update_plane           = drm_atomic_helper_update_plane,
        .disable_plane          = drm_atomic_helper_disable_plane,
-       .destroy                = drm_plane_cleanup,
        .reset                  = drm_atomic_helper_plane_reset,
        .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
        .atomic_destroy_state   = drm_atomic_helper_plane_destroy_state,
@@ -297,24 +296,19 @@ static const struct drm_plane_funcs hdlcd_plane_funcs = {
  
  static struct drm_plane *hdlcd_plane_init(struct drm_device *drm)
  {
-       struct hdlcd_drm_private *hdlcd = drm->dev_private;
+       struct hdlcd_drm_private *hdlcd = drm_to_hdlcd_priv(drm);
        struct drm_plane *plane = NULL;
        u32 formats[ARRAY_SIZE(supported_formats)], i;
-       int ret;
-
-       plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);
-       if (!plane)
-               return ERR_PTR(-ENOMEM);
  
  	for (i = 0; i < ARRAY_SIZE(supported_formats); i++)
                formats[i] = supported_formats[i].fourcc;
  
-	ret = drm_universal_plane_init(drm, plane, 0xff, &hdlcd_plane_funcs,
-                                      formats, ARRAY_SIZE(formats),
-                                      NULL,
-                                      DRM_PLANE_TYPE_PRIMARY, NULL);
-       if (ret)
-               return ERR_PTR(ret);
+       plane = drmm_universal_plane_alloc(drm, struct drm_plane, dev, 0xff,
+                                          &hdlcd_plane_funcs,
+                                          formats, ARRAY_SIZE(formats),
+                                          NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+       if (IS_ERR(plane))
+               return plane;
  
  	drm_plane_helper_add(plane, &hdlcd_plane_helper_funcs);
        hdlcd->plane = plane;
--
2.37.2