Hi Noralf,
On Thursday, 31 August 2017 20:16:42 EEST Noralf Trønnes wrote:
> Den 31.08.2017 12.18, skrev Laurent Pinchart:
> > On Monday, 28 August 2017 20:17:46 EEST Noralf Trønnes wrote:
> >> Might as well embed drm_device since tinydrm_device (embeds pipe struct
> >> and fbdev pointer) needs to stick around after driver-device unbind to
> >> handle open fd's after device removal.
> >>
> >> Cc: David Lechner <da...@lechnology.com>
> >> Signed-off-by: Noralf Trønnes <nor...@tronnes.org>
> >> ---
> >>
> >> drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 44 +++++++++++-----------
> >> drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 2 +-
> >> drivers/gpu/drm/tinydrm/mi0283qt.c | 8 +++---
> >> drivers/gpu/drm/tinydrm/mipi-dbi.c | 12 ++++----
> >> drivers/gpu/drm/tinydrm/repaper.c | 10 +++----
> >> drivers/gpu/drm/tinydrm/st7586.c | 16 +++++------
> >> include/drm/tinydrm/tinydrm.h | 9 +++++-
> >> 7 files changed, 50 insertions(+), 51 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
> >> b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c index 551709e..f11f4cd
> >> 100644
> >> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
> >> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
> >
> > [snip]
> >
> >> @@ -142,23 +142,16 @@ static int tinydrm_init(struct device *parent,
> >> struct
> >> tinydrm_device *tdev, const struct drm_framebuffer_funcs *fb_funcs,
> >>
> >> struct drm_driver *driver)
> >>
> >> {
> >>
> >> - struct drm_device *drm;
> >> + struct drm_device *drm = &tdev->drm;
> >> + int ret;
> >>
> >> mutex_init(&tdev->dirty_lock);
> >> tdev->fb_funcs = fb_funcs;
> >>
> >> - /*
> >> - * We don't embed drm_device, because that prevent us from using
> >> - * devm_kzalloc() to allocate tinydrm_device in the driver since
> >> - * drm_dev_unref() frees the structure. The devm_ functions provide
> >> - * for easy error handling.
> >
> > Don't you then need a custom drm_driver.release handler to free the parent
> > structure ?
>
> I rely on the fact that drm_device is the first member in the driver
> structure and thus it will be freed in drm_dev_release(). A later patch
> adds a drm_driver.release function though.
That's a bit hackish. As a later patch changes this I'd be OK with this one,
but you should mention that you rely on the structure layout in the commit
message.
> >> - */
> >> - drm = drm_dev_alloc(driver, parent);
> >> - if (IS_ERR(drm))
> >> - return PTR_ERR(drm);
> >> -
> >> - tdev->drm = drm;
> >> - drm->dev_private = tdev;
> >> + ret = drm_dev_init(drm, driver, parent);
> >> + if (ret)
> >> + return ret;
> >> +
> >>
> >> drm_mode_config_init(drm);
> >> drm->mode_config.funcs = &tinydrm_mode_config_funcs;
> >
> > [snip]
> >
> >> diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c
> >> b/drivers/gpu/drm/tinydrm/mi0283qt.c index 7e5bb7d..77d40c9 100644
> >> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> >> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
> >
> > [snip]
> >
> >> @@ -169,7 +169,7 @@ static int mi0283qt_probe(struct spi_device *spi)
> >>
> >> u32 rotation = 0;
> >> int ret;
> >>
> >> - mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
> >> + mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
> >
> > Where's the related kfree() ?
> >
> >> if (!mipi)
> >>
> >> return -ENOMEM;
> >
> > [snip]
> >
> >> diff --git a/drivers/gpu/drm/tinydrm/repaper.c
> >> b/drivers/gpu/drm/tinydrm/repaper.c index 30dc97b..b8fc8eb 100644
> >> --- a/drivers/gpu/drm/tinydrm/repaper.c
> >> +++ b/drivers/gpu/drm/tinydrm/repaper.c
> >
> > [snip]
> >
> >> @@ -949,7 +949,7 @@ static int repaper_probe(struct spi_device *spi)
> >>
> >> }
> >>
> >> }
> >>
> >> - epd = devm_kzalloc(dev, sizeof(*epd), GFP_KERNEL);
> >> + epd = kzalloc(sizeof(*epd), GFP_KERNEL);
> >
> > Ditto.
> >
> >> if (!epd)
> >>
> >> return -ENOMEM;
> >
> > [snip]
> >
> >> diff --git a/drivers/gpu/drm/tinydrm/st7586.c
> >> b/drivers/gpu/drm/tinydrm/st7586.c index b439956..bc2b905 100644
> >> --- a/drivers/gpu/drm/tinydrm/st7586.c
> >> +++ b/drivers/gpu/drm/tinydrm/st7586.c
> >
> > [snip]
> >
> >> @@ -349,7 +349,7 @@ static int st7586_probe(struct spi_device *spi)
> >>
> >> u32 rotation = 0;
> >> int ret;
> >>
> >> - mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
> >> + mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
> >
> > Ang here again.
> >
> >> if (!mipi)
> >>
> >> return -ENOMEM;
> >
> > [snip]
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
