Re: [PATCH 6/9] drm: rcar-du: Embed drm_device in rcar_du_device

Hi Laurent,

On 04/12/2020 22:01, Laurent Pinchart wrote:

Embedding drm_device in rcar_du_device allows usage of the DRM managed API to allocate both structures in one go, simplifying error handling.

No point making multiple allocations unnecessarily...

LGTM.

Reviewed-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com

Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com

drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 33 +++++++++------------ drivers/gpu/drm/rcar-du/rcar_du_drv.h | 5 ++-- drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 4 +-- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 4 +-- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++-- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 6 ++-- drivers/gpu/drm/rcar-du/rcar_du_writeback.c | 2 +- 8 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index e23b9c7b4afe..9a099c0fe1d4 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -1251,7 +1251,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex, else primary = &rgrp->planes[swindex % 2].plane;

• ret = drm_crtc_init_with_planes(rcdu->ddev, crtc, primary, NULL,

• ret = drm_crtc_init_with_planes(&rcdu->ddev, crtc, primary, NULL, rcdu->info->gen <= 2 ? &crtc_funcs_gen2 : &crtc_funcs_gen3, NULL);

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 1490ec182646..4ab99ac49891 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -18,10 +18,11 @@ #include <linux/wait.h>

#include <drm/drm_atomic_helper.h> +#include <drm/drm_drv.h> #include <drm/drm_fb_cma_helper.h> #include <drm/drm_fb_helper.h> -#include <drm/drm_drv.h> #include <drm/drm_gem_cma_helper.h> +#include <drm/drm_managed.h> #include <drm/drm_probe_helper.h>

#include "rcar_du_drv.h" @@ -529,14 +530,14 @@ static int rcar_du_pm_suspend(struct device *dev) { struct rcar_du_device *rcdu = dev_get_drvdata(dev);

• return drm_mode_config_helper_suspend(rcdu->ddev);

• return drm_mode_config_helper_suspend(&rcdu->ddev);

}

static int rcar_du_pm_resume(struct device *dev) { struct rcar_du_device *rcdu = dev_get_drvdata(dev);

• return drm_mode_config_helper_resume(rcdu->ddev);

• return drm_mode_config_helper_resume(&rcdu->ddev);

} #endif

@@ -551,7 +552,7 @@ static const struct dev_pm_ops rcar_du_pm_ops = { static int rcar_du_remove(struct platform_device *pdev) { struct rcar_du_device *rcdu = platform_get_drvdata(pdev);

• struct drm_device *ddev = rcdu->ddev;

• struct drm_device *ddev = &rcdu->ddev;

  drm_dev_unregister(ddev);

@@ -565,14 +566,14 @@ static int rcar_du_remove(struct platform_device *pdev) static int rcar_du_probe(struct platform_device *pdev) { struct rcar_du_device *rcdu;

• struct drm_device *ddev; struct resource *mem; int ret;

  /* Allocate and initialize the R-Car device structure. */

• rcdu = devm_kzalloc(&pdev->dev, sizeof(*rcdu), GFP_KERNEL);

• if (rcdu == NULL)

• return -ENOMEM;
  

• rcdu = devm_drm_dev_alloc(&pdev->dev, &rcar_du_driver,

• 		  struct rcar_du_device, ddev);
  

• if (IS_ERR(rcdu))

• return PTR_ERR(rcdu);
  
  

  rcdu->dev = &pdev->dev; rcdu->info = of_device_get_match_data(rcdu->dev);

@@ -586,12 +587,7 @@ static int rcar_du_probe(struct platform_device *pdev) return PTR_ERR(rcdu->mmio);

/* DRM/KMS objects */

• ddev = drm_dev_alloc(&rcar_du_driver, &pdev->dev);
• if (IS_ERR(ddev))

• return PTR_ERR(ddev);
  

• rcdu->ddev = ddev;
• ddev->dev_private = rcdu;

• rcdu->ddev.dev_private = rcdu;

  ret = rcar_du_modeset_init(rcdu); if (ret < 0) {

@@ -601,25 +597,24 @@ static int rcar_du_probe(struct platform_device *pdev) goto error; }

• ddev->irq_enabled = 1;

• rcdu->ddev.irq_enabled = 1;

  /*

  • Register the DRM device with the core and the connectors with
  • sysfs.

  */

• ret = drm_dev_register(ddev, 0);

• ret = drm_dev_register(&rcdu->ddev, 0); if (ret) goto error;

  DRM_INFO("Device %s probed\n", dev_name(&pdev->dev));

• drm_fbdev_generic_setup(ddev, 32);

• drm_fbdev_generic_setup(&rcdu->ddev, 32);

  return 0;

error:

• rcar_du_remove(pdev);

• drm_kms_helper_poll_fini(&rcdu->ddev); return ret;

}

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 71732fc5df8f..e5b6f456357e 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h @@ -13,6 +13,8 @@ #include <linux/kernel.h> #include <linux/wait.h>

+#include <drm/drm_device.h>

#include "rcar_cmm.h" #include "rcar_du_crtc.h" #include "rcar_du_group.h" @@ -21,7 +23,6 @@ struct clk; struct device; struct drm_bridge; -struct drm_device; struct drm_property; struct rcar_du_device; struct rcar_du_encoder; @@ -80,7 +81,7 @@ struct rcar_du_device {

void __iomem *mmio;

• struct drm_device *ddev;

• struct drm_device ddev;

  struct rcar_du_crtc crtcs[RCAR_DU_MAX_CRTCS]; unsigned int num_crtcs;

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c index 7c491ff72cd2..e4f35a88d00f 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c @@ -120,12 +120,12 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu, } }

• ret = drm_encoder_init(rcdu->ddev, encoder, &rcar_du_encoder_funcs,

• ret = drm_encoder_init(&rcdu->ddev, encoder, &rcar_du_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL); if (ret < 0) goto error;

• ret = drmm_add_action_or_reset(rcdu->ddev, rcar_du_encoder_release,

• ret = drmm_add_action_or_reset(&rcdu->ddev, rcar_du_encoder_release, renc); if (ret) return ret;

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 72dda446355f..57bb0dc22807 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -583,7 +583,7 @@ static int rcar_du_properties_init(struct rcar_du_device *rcdu) * or enable source color keying (1). */ rcdu->props.colorkey =

• drm_property_create_range(rcdu->ddev, 0, "colorkey",
  

• drm_property_create_range(&rcdu->ddev, 0, "colorkey",
  		  0, 0x01ffffff);
  

  if (rcdu->props.colorkey == NULL) return -ENOMEM;

@@ -752,7 +752,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) DU0_REG_OFFSET, DU2_REG_OFFSET };

• struct drm_device *dev = rcdu->ddev;

• struct drm_device *dev = &rcdu->ddev; struct drm_encoder *encoder; unsigned int dpad0_sources; unsigned int num_encoders;

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index a0021fc25b27..5f69ff4502c1 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -773,9 +773,9 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)

plane->group = rgrp;

• ret = drm_universal_plane_init(rcdu->ddev, &plane->plane, crtcs,
  

• 			       &rcar_du_plane_funcs, formats,
  

• 			       ARRAY_SIZE(formats),
  

• ret = drm_universal_plane_init(&rcdu->ddev, &plane->plane,
  

• 			       crtcs, &rcar_du_plane_funcs,
  

• 			       formats, ARRAY_SIZE(formats),
  		       NULL, type, NULL);
  

  if (ret < 0) return ret;

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 78a886651d9f..53221d8473c1 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -375,7 +375,7 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,

vsp->vsp = &pdev->dev;

• ret = drmm_add_action_or_reset(rcdu->ddev, rcar_du_vsp_cleanup, vsp);

• ret = drmm_add_action_or_reset(&rcdu->ddev, rcar_du_vsp_cleanup, vsp); if (ret < 0) return ret;

@@ -402,8 +402,8 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, plane->vsp = vsp; plane->index = i;

• ret = drm_universal_plane_init(rcdu->ddev, &plane->plane, crtcs,
  

• 			       &rcar_du_vsp_plane_funcs,
  

• ret = drm_universal_plane_init(&rcdu->ddev, &plane->plane,
  

• 			       crtcs, &rcar_du_vsp_plane_funcs,
  		       rcar_du_vsp_formats,
  		       ARRAY_SIZE(rcar_du_vsp_formats),
  		       NULL, type, NULL);
  

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c index 04efa78d70b6..c79d1259e49b 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c @@ -204,7 +204,7 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu, drm_connector_helper_add(&wb_conn->base, &rcar_du_wb_conn_helper_funcs);

• return drm_writeback_connector_init(rcdu->ddev, wb_conn,

• return drm_writeback_connector_init(&rcdu->ddev, wb_conn, &rcar_du_wb_conn_funcs, &rcar_du_wb_enc_helper_funcs, writeback_formats,