Hi Laurent,

On 13/01/2021 22:45, Laurent Pinchart wrote:

Hi Kieran,

Thank you for the patch.

On Wed, Jan 13, 2021 at 05:02:53PM +0000, Kieran Bingham wrote:

...

The encoder allocation was converted to a DRM managed resource at the same time as the addition of a new helper drmm_encoder_alloc() which simplifies the same process.

Convert the custom drm managed resource allocation of the encoder with the helper to simplify the implementation, and prevent hitting a WARN_ON() due to the handling the drm_encoder_init() call directly without registering a .destroy() function op.

Fixes: f5f16725edbc ("drm: rcar-du: Use DRM-managed allocation for encoders")

We could equally point to the patch that has added drmm_encoder_alloc(), but I'm fine taking the blame :-)

Perhaps, we could point there indeed, I'm surprised that patch/series didn't seem to add any users of drmm_encoder_alloc() as far as I can see.

I don't think this is a "blame" though. Just a reference to the most relevant change.

Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com

Thanks.

Kieran

...

Reported-by: Geert Uytterhoeven geert+renesas@glider.be Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com

---

drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 31 +++++------------------ 1 file changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c index ba8c6038cd63..ca3761772211 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c @@ -48,21 +48,12 @@ static unsigned int rcar_du_encoder_count_ports(struct device_node *node) static const struct drm_encoder_funcs rcar_du_encoder_funcs = { };

-static void rcar_du_encoder_release(struct drm_device *dev, void *res) -{

• struct rcar_du_encoder *renc = res;
• drm_encoder_cleanup(&renc->base);
• kfree(renc);

-}

int rcar_du_encoder_init(struct rcar_du_device *rcdu, enum rcar_du_output output, struct device_node *enc_node) { struct rcar_du_encoder *renc; struct drm_bridge *bridge;

• int ret;

  /*

  • Locate the DRM bridge from the DT node. For the DPAD outputs, if the

@@ -101,26 +92,16 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu, return -ENOLINK; }

• renc = kzalloc(sizeof(*renc), GFP_KERNEL);

• if (renc == NULL)

• return -ENOMEM;
  

• renc->output = output;

• dev_dbg(rcdu->dev, "initializing encoder %pOF for output %u\n", enc_node, output);

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

• 	       DRM_MODE_ENCODER_NONE, NULL);
  

• if (ret < 0) {

• kfree(renc);
  

• return ret;
  

• }

• renc = drmm_encoder_alloc(&rcdu->ddev, struct rcar_du_encoder, base,

• 		  &rcar_du_encoder_funcs, DRM_MODE_ENCODER_NONE,
  

• 		  NULL);
  

• if (!renc)

• return -ENOMEM;
  
  

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

• 		       renc);
  

• if (ret)

• return ret;
  

• renc->output = output;

  /*

  • Attach the bridge to the encoder. The bridge will create the