| Message ID | 20210113170253.443820-1-kieran.bingham+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: rcar-du: Use drmm_encoder_alloc() to manage encoder | expand |
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 :-) Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > 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
On Wed, Jan 13, 2021 at 6:02 PM Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> 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") > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> As in "the WARNING from drm_encoder_init() is gone". Gr{oetje,eeting}s, Geert
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 >
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
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") 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(-)