| Message ID | 1503940668-25883-5-git-send-email-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 28, 2017 at 07:17:46PM +0200, 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 <david@lechnology.com> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> I think you should be able to merge this right away, and it's definitely a nice cleanup. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > 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 > @@ -44,7 +44,7 @@ > */ > void tinydrm_lastclose(struct drm_device *drm) > { > - struct tinydrm_device *tdev = drm->dev_private; > + struct tinydrm_device *tdev = drm_to_tinydrm(drm); > > DRM_DEBUG_KMS("\n"); > drm_fbdev_cma_restore_mode(tdev->fbdev_cma); > @@ -126,7 +126,7 @@ static struct drm_framebuffer * > tinydrm_fb_create(struct drm_device *drm, struct drm_file *file_priv, > const struct drm_mode_fb_cmd2 *mode_cmd) > { > - struct tinydrm_device *tdev = drm->dev_private; > + struct tinydrm_device *tdev = drm_to_tinydrm(drm); > > return drm_fb_cma_create_with_funcs(drm, file_priv, mode_cmd, > tdev->fb_funcs); > @@ -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. > - */ > - 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; > > @@ -167,10 +160,9 @@ static int tinydrm_init(struct device *parent, struct tinydrm_device *tdev, > > static void tinydrm_fini(struct tinydrm_device *tdev) > { > - drm_mode_config_cleanup(tdev->drm); > + drm_mode_config_cleanup(&tdev->drm); > mutex_destroy(&tdev->dirty_lock); > - tdev->drm->dev_private = NULL; > - drm_dev_unref(tdev->drm); > + drm_dev_unref(&tdev->drm); > } > > static void devm_tinydrm_release(void *data) > @@ -212,12 +204,12 @@ EXPORT_SYMBOL(devm_tinydrm_init); > > static int tinydrm_register(struct tinydrm_device *tdev) > { > - struct drm_device *drm = tdev->drm; > + struct drm_device *drm = &tdev->drm; > int bpp = drm->mode_config.preferred_depth; > struct drm_fbdev_cma *fbdev; > int ret; > > - ret = drm_dev_register(tdev->drm, 0); > + ret = drm_dev_register(drm, 0); > if (ret) > return ret; > > @@ -236,10 +228,10 @@ static void tinydrm_unregister(struct tinydrm_device *tdev) > { > struct drm_fbdev_cma *fbdev_cma = tdev->fbdev_cma; > > - drm_atomic_helper_shutdown(tdev->drm); > + drm_atomic_helper_shutdown(&tdev->drm); > /* don't restore fbdev in lastclose, keep pipeline disabled */ > tdev->fbdev_cma = NULL; > - drm_dev_unregister(tdev->drm); > + drm_dev_unregister(&tdev->drm); > if (fbdev_cma) > drm_fbdev_cma_fini(fbdev_cma); > } > @@ -262,7 +254,7 @@ static void devm_tinydrm_register_release(void *data) > */ > int devm_tinydrm_register(struct tinydrm_device *tdev) > { > - struct device *dev = tdev->drm->dev; > + struct device *dev = tdev->drm.dev; > int ret; > > ret = tinydrm_register(tdev); > @@ -287,7 +279,7 @@ EXPORT_SYMBOL(devm_tinydrm_register); > */ > void tinydrm_shutdown(struct tinydrm_device *tdev) > { > - drm_atomic_helper_shutdown(tdev->drm); > + drm_atomic_helper_shutdown(&tdev->drm); > } > EXPORT_SYMBOL(tinydrm_shutdown); > > @@ -312,7 +304,7 @@ int tinydrm_suspend(struct tinydrm_device *tdev) > } > > drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 1); > - state = drm_atomic_helper_suspend(tdev->drm); > + state = drm_atomic_helper_suspend(&tdev->drm); > if (IS_ERR(state)) { > drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 0); > return PTR_ERR(state); > @@ -346,7 +338,7 @@ int tinydrm_resume(struct tinydrm_device *tdev) > > tdev->suspend_state = NULL; > > - ret = drm_atomic_helper_resume(tdev->drm, state); > + ret = drm_atomic_helper_resume(&tdev->drm, state); > if (ret) { > DRM_ERROR("Error resuming state: %d\n", ret); > return ret; > diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c > index 177e9d8..1bcb43a 100644 > --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c > +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c > @@ -198,7 +198,7 @@ tinydrm_display_pipe_init(struct tinydrm_device *tdev, > const struct drm_display_mode *mode, > unsigned int rotation) > { > - struct drm_device *drm = tdev->drm; > + struct drm_device *drm = &tdev->drm; > struct drm_display_mode *mode_copy; > struct drm_connector *connector; > int ret; > 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 > @@ -23,7 +23,7 @@ > static int mi0283qt_init(struct mipi_dbi *mipi) > { > struct tinydrm_device *tdev = &mipi->tinydrm; > - struct device *dev = tdev->drm->dev; > + struct device *dev = tdev->drm.dev; > u8 addr_mode; > int ret; > > @@ -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); > if (!mipi) > return -ENOMEM; > > @@ -224,9 +224,9 @@ static int mi0283qt_probe(struct spi_device *spi) > spi_set_drvdata(spi, mipi); > > DRM_DEBUG_DRIVER("Initialized %s:%s @%uMHz on minor %d\n", > - tdev->drm->driver->name, dev_name(dev), > + tdev->drm.driver->name, dev_name(dev), > spi->max_speed_hz / 1000000, > - tdev->drm->primary->index); > + tdev->drm.primary->index); > > return 0; > } > diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c > index 2caeabc..c22e352 100644 > --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c > +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c > @@ -199,7 +199,7 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb, > unsigned int num_clips) > { > struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0); > - struct tinydrm_device *tdev = fb->dev->dev_private; > + struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev); > struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); > bool swap = mipi->swap_bytes; > struct drm_clip_rect clip; > @@ -285,7 +285,7 @@ EXPORT_SYMBOL(mipi_dbi_pipe_enable); > > static void mipi_dbi_blank(struct mipi_dbi *mipi) > { > - struct drm_device *drm = mipi->tinydrm.drm; > + struct drm_device *drm = &mipi->tinydrm.drm; > u16 height = drm->mode_config.min_height; > u16 width = drm->mode_config.min_width; > size_t len = width * height * 2; > @@ -380,13 +380,13 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi, > if (ret) > return ret; > > - tdev->drm->mode_config.preferred_depth = 16; > + tdev->drm.mode_config.preferred_depth = 16; > mipi->rotation = rotation; > > - drm_mode_config_reset(tdev->drm); > + drm_mode_config_reset(&tdev->drm); > > DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n", > - tdev->drm->mode_config.preferred_depth, rotation); > + tdev->drm.mode_config.preferred_depth, rotation); > > return 0; > } > @@ -975,7 +975,7 @@ static const struct drm_info_list mipi_dbi_debugfs_list[] = { > */ > int mipi_dbi_debugfs_init(struct drm_minor *minor) > { > - struct tinydrm_device *tdev = minor->dev->dev_private; > + struct tinydrm_device *tdev = drm_to_tinydrm(minor->dev); > struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); > umode_t mode = S_IFREG | S_IWUSR; > > 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 > @@ -528,7 +528,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, > { > struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0); > struct dma_buf_attachment *import_attach = cma_obj->base.import_attach; > - struct tinydrm_device *tdev = fb->dev->dev_private; > + struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev); > struct repaper_epd *epd = epd_from_tinydrm(tdev); > struct drm_clip_rect clip; > u8 *buf = NULL; > @@ -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); > if (!epd) > return -ENOMEM; > > @@ -1077,7 +1077,7 @@ static int repaper_probe(struct spi_device *spi) > if (ret) > return ret; > > - drm_mode_config_reset(tdev->drm); > + drm_mode_config_reset(&tdev->drm); > > ret = devm_tinydrm_register(tdev); > if (ret) > @@ -1086,9 +1086,9 @@ static int repaper_probe(struct spi_device *spi) > spi_set_drvdata(spi, tdev); > > DRM_DEBUG_DRIVER("Initialized %s:%s @%uMHz on minor %d\n", > - tdev->drm->driver->name, dev_name(dev), > + tdev->drm.driver->name, dev_name(dev), > spi->max_speed_hz / 1000000, > - tdev->drm->primary->index); > + tdev->drm.primary->index); > > return 0; > } > 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 > @@ -112,7 +112,7 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb, > unsigned int color, struct drm_clip_rect *clips, > unsigned int num_clips) > { > - struct tinydrm_device *tdev = fb->dev->dev_private; > + struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev); > struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); > struct drm_clip_rect clip; > int start, end; > @@ -178,7 +178,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe, > struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); > struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); > struct drm_framebuffer *fb = pipe->plane.fb; > - struct device *dev = tdev->drm->dev; > + struct device *dev = tdev->drm.dev; > int ret; > u8 addr_mode; > > @@ -290,13 +290,13 @@ static int st7586_init(struct device *dev, struct mipi_dbi *mipi, > if (ret) > return ret; > > - tdev->drm->mode_config.preferred_depth = 32; > + tdev->drm.mode_config.preferred_depth = 32; > mipi->rotation = rotation; > > - drm_mode_config_reset(tdev->drm); > + drm_mode_config_reset(&tdev->drm); > > DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n", > - tdev->drm->mode_config.preferred_depth, rotation); > + tdev->drm.mode_config.preferred_depth, rotation); > > return 0; > } > @@ -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); > if (!mipi) > return -ENOMEM; > > @@ -397,9 +397,9 @@ static int st7586_probe(struct spi_device *spi) > spi_set_drvdata(spi, mipi); > > DRM_DEBUG_DRIVER("Initialized %s:%s @%uMHz on minor %d\n", > - tdev->drm->driver->name, dev_name(dev), > + tdev->drm.driver->name, dev_name(dev), > spi->max_speed_hz / 1000000, > - tdev->drm->primary->index); > + tdev->drm.primary->index); > > return 0; > } > diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h > index 4774fe3..ded9817 100644 > --- a/include/drm/tinydrm/tinydrm.h > +++ b/include/drm/tinydrm/tinydrm.h > @@ -10,6 +10,7 @@ > #ifndef __LINUX_TINYDRM_H > #define __LINUX_TINYDRM_H > > +#include <drm/drmP.h> > #include <drm/drm_gem_cma_helper.h> > #include <drm/drm_fb_cma_helper.h> > #include <drm/drm_simple_kms_helper.h> > @@ -24,7 +25,7 @@ > * @fb_funcs: Framebuffer functions used when creating framebuffers > */ > struct tinydrm_device { > - struct drm_device *drm; > + struct drm_device drm; > struct drm_simple_display_pipe pipe; > struct mutex dirty_lock; > struct drm_fbdev_cma *fbdev_cma; > @@ -33,6 +34,12 @@ struct tinydrm_device { > }; > > static inline struct tinydrm_device * > +drm_to_tinydrm(struct drm_device *drm) > +{ > + return container_of(drm, struct tinydrm_device, drm); > +} > + > +static inline struct tinydrm_device * > pipe_to_tinydrm(struct drm_simple_display_pipe *pipe) > { > return container_of(pipe, struct tinydrm_device, pipe); > -- > 2.7.4 >
On 08/28/2017 12:17 PM, 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 <david@lechnology.com> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> Acked-By: David Lechner <david@lechnology.com>
Hi Noralf, Thank you for the patch. 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 <david@lechnology.com> > Signed-off-by: Noralf Trønnes <noralf@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 ? > - */ > - 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]
Den 31.08.2017 12.18, skrev Laurent Pinchart: > Hi Noralf, > > Thank you for the patch. > > 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 <david@lechnology.com> >> Signed-off-by: Noralf Trønnes <noralf@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. Noralf. >> - */ >> - 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] >
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 <david@lechnology.com> > >> Signed-off-by: Noralf Trønnes <noralf@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]
Den 01.09.2017 09.28, skrev Laurent Pinchart: > 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 <david@lechnology.com> >>>> Signed-off-by: Noralf Trønnes <noralf@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. I've seen it around, so I figured it was a common pattern. Looking at it now, I see that it adds obscurity: - Where is that driver struct freed? - In tinydrm_release(). - How can it do that when it doesn't have a pointer to it? - It does this indirectly since tinydrm_device is the first member. - Ooh... I remember now that I hit the same kind of obscurity in the vbox driver where it assigns a private structure to fb_info.par and I know that drm_fb_helper expects this to be struct drm_fb_helper. So how could this work? After some digging I realised that it worked because drm_fb_helper was the first member of the private struct. It made the code difficult to read. So yeah, better stay away from things like this when possible. In patch 6 I did document this freeing behaviour in the struct tinydrm_device docs, but that probably wouldn't have helped me much if I was new to tinydrm. It would have taken me some time to pick up that piece of information. I think I'll add drm_driver.release functions to this patch and explicitly free the driver structure, since it logically belongs here. An upside of having the drm_driver.release callback in the driver is the educational/awareness part, it acts as a reminder that the driver structure can live on even after driver.remove has finished. This argument in itself is probably strong enough to warrant one release function per driver structure, since this lifetime issue is so easy to get wrong. I've also realised that the driver struct is leaked if devm_tinydrm_init() fails, so I need to fix that as well. Noralf. >>>> - */ >>>> - 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]
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 @@ -44,7 +44,7 @@ */ void tinydrm_lastclose(struct drm_device *drm) { - struct tinydrm_device *tdev = drm->dev_private; + struct tinydrm_device *tdev = drm_to_tinydrm(drm); DRM_DEBUG_KMS("\n"); drm_fbdev_cma_restore_mode(tdev->fbdev_cma); @@ -126,7 +126,7 @@ static struct drm_framebuffer * tinydrm_fb_create(struct drm_device *drm, struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd) { - struct tinydrm_device *tdev = drm->dev_private; + struct tinydrm_device *tdev = drm_to_tinydrm(drm); return drm_fb_cma_create_with_funcs(drm, file_priv, mode_cmd, tdev->fb_funcs); @@ -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. - */ - 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; @@ -167,10 +160,9 @@ static int tinydrm_init(struct device *parent, struct tinydrm_device *tdev, static void tinydrm_fini(struct tinydrm_device *tdev) { - drm_mode_config_cleanup(tdev->drm); + drm_mode_config_cleanup(&tdev->drm); mutex_destroy(&tdev->dirty_lock); - tdev->drm->dev_private = NULL; - drm_dev_unref(tdev->drm); + drm_dev_unref(&tdev->drm); } static void devm_tinydrm_release(void *data) @@ -212,12 +204,12 @@ EXPORT_SYMBOL(devm_tinydrm_init); static int tinydrm_register(struct tinydrm_device *tdev) { - struct drm_device *drm = tdev->drm; + struct drm_device *drm = &tdev->drm; int bpp = drm->mode_config.preferred_depth; struct drm_fbdev_cma *fbdev; int ret; - ret = drm_dev_register(tdev->drm, 0); + ret = drm_dev_register(drm, 0); if (ret) return ret; @@ -236,10 +228,10 @@ static void tinydrm_unregister(struct tinydrm_device *tdev) { struct drm_fbdev_cma *fbdev_cma = tdev->fbdev_cma; - drm_atomic_helper_shutdown(tdev->drm); + drm_atomic_helper_shutdown(&tdev->drm); /* don't restore fbdev in lastclose, keep pipeline disabled */ tdev->fbdev_cma = NULL; - drm_dev_unregister(tdev->drm); + drm_dev_unregister(&tdev->drm); if (fbdev_cma) drm_fbdev_cma_fini(fbdev_cma); } @@ -262,7 +254,7 @@ static void devm_tinydrm_register_release(void *data) */ int devm_tinydrm_register(struct tinydrm_device *tdev) { - struct device *dev = tdev->drm->dev; + struct device *dev = tdev->drm.dev; int ret; ret = tinydrm_register(tdev); @@ -287,7 +279,7 @@ EXPORT_SYMBOL(devm_tinydrm_register); */ void tinydrm_shutdown(struct tinydrm_device *tdev) { - drm_atomic_helper_shutdown(tdev->drm); + drm_atomic_helper_shutdown(&tdev->drm); } EXPORT_SYMBOL(tinydrm_shutdown); @@ -312,7 +304,7 @@ int tinydrm_suspend(struct tinydrm_device *tdev) } drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 1); - state = drm_atomic_helper_suspend(tdev->drm); + state = drm_atomic_helper_suspend(&tdev->drm); if (IS_ERR(state)) { drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 0); return PTR_ERR(state); @@ -346,7 +338,7 @@ int tinydrm_resume(struct tinydrm_device *tdev) tdev->suspend_state = NULL; - ret = drm_atomic_helper_resume(tdev->drm, state); + ret = drm_atomic_helper_resume(&tdev->drm, state); if (ret) { DRM_ERROR("Error resuming state: %d\n", ret); return ret; diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c index 177e9d8..1bcb43a 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c @@ -198,7 +198,7 @@ tinydrm_display_pipe_init(struct tinydrm_device *tdev, const struct drm_display_mode *mode, unsigned int rotation) { - struct drm_device *drm = tdev->drm; + struct drm_device *drm = &tdev->drm; struct drm_display_mode *mode_copy; struct drm_connector *connector; int ret; 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 @@ -23,7 +23,7 @@ static int mi0283qt_init(struct mipi_dbi *mipi) { struct tinydrm_device *tdev = &mipi->tinydrm; - struct device *dev = tdev->drm->dev; + struct device *dev = tdev->drm.dev; u8 addr_mode; int ret; @@ -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); if (!mipi) return -ENOMEM; @@ -224,9 +224,9 @@ static int mi0283qt_probe(struct spi_device *spi) spi_set_drvdata(spi, mipi); DRM_DEBUG_DRIVER("Initialized %s:%s @%uMHz on minor %d\n", - tdev->drm->driver->name, dev_name(dev), + tdev->drm.driver->name, dev_name(dev), spi->max_speed_hz / 1000000, - tdev->drm->primary->index); + tdev->drm.primary->index); return 0; } diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 2caeabc..c22e352 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -199,7 +199,7 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb, unsigned int num_clips) { struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0); - struct tinydrm_device *tdev = fb->dev->dev_private; + struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); bool swap = mipi->swap_bytes; struct drm_clip_rect clip; @@ -285,7 +285,7 @@ EXPORT_SYMBOL(mipi_dbi_pipe_enable); static void mipi_dbi_blank(struct mipi_dbi *mipi) { - struct drm_device *drm = mipi->tinydrm.drm; + struct drm_device *drm = &mipi->tinydrm.drm; u16 height = drm->mode_config.min_height; u16 width = drm->mode_config.min_width; size_t len = width * height * 2; @@ -380,13 +380,13 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi, if (ret) return ret; - tdev->drm->mode_config.preferred_depth = 16; + tdev->drm.mode_config.preferred_depth = 16; mipi->rotation = rotation; - drm_mode_config_reset(tdev->drm); + drm_mode_config_reset(&tdev->drm); DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n", - tdev->drm->mode_config.preferred_depth, rotation); + tdev->drm.mode_config.preferred_depth, rotation); return 0; } @@ -975,7 +975,7 @@ static const struct drm_info_list mipi_dbi_debugfs_list[] = { */ int mipi_dbi_debugfs_init(struct drm_minor *minor) { - struct tinydrm_device *tdev = minor->dev->dev_private; + struct tinydrm_device *tdev = drm_to_tinydrm(minor->dev); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); umode_t mode = S_IFREG | S_IWUSR; 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 @@ -528,7 +528,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, { struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0); struct dma_buf_attachment *import_attach = cma_obj->base.import_attach; - struct tinydrm_device *tdev = fb->dev->dev_private; + struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev); struct repaper_epd *epd = epd_from_tinydrm(tdev); struct drm_clip_rect clip; u8 *buf = NULL; @@ -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); if (!epd) return -ENOMEM; @@ -1077,7 +1077,7 @@ static int repaper_probe(struct spi_device *spi) if (ret) return ret; - drm_mode_config_reset(tdev->drm); + drm_mode_config_reset(&tdev->drm); ret = devm_tinydrm_register(tdev); if (ret) @@ -1086,9 +1086,9 @@ static int repaper_probe(struct spi_device *spi) spi_set_drvdata(spi, tdev); DRM_DEBUG_DRIVER("Initialized %s:%s @%uMHz on minor %d\n", - tdev->drm->driver->name, dev_name(dev), + tdev->drm.driver->name, dev_name(dev), spi->max_speed_hz / 1000000, - tdev->drm->primary->index); + tdev->drm.primary->index); return 0; } 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 @@ -112,7 +112,7 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb, unsigned int color, struct drm_clip_rect *clips, unsigned int num_clips) { - struct tinydrm_device *tdev = fb->dev->dev_private; + struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); struct drm_clip_rect clip; int start, end; @@ -178,7 +178,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe, struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); struct drm_framebuffer *fb = pipe->plane.fb; - struct device *dev = tdev->drm->dev; + struct device *dev = tdev->drm.dev; int ret; u8 addr_mode; @@ -290,13 +290,13 @@ static int st7586_init(struct device *dev, struct mipi_dbi *mipi, if (ret) return ret; - tdev->drm->mode_config.preferred_depth = 32; + tdev->drm.mode_config.preferred_depth = 32; mipi->rotation = rotation; - drm_mode_config_reset(tdev->drm); + drm_mode_config_reset(&tdev->drm); DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n", - tdev->drm->mode_config.preferred_depth, rotation); + tdev->drm.mode_config.preferred_depth, rotation); return 0; } @@ -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); if (!mipi) return -ENOMEM; @@ -397,9 +397,9 @@ static int st7586_probe(struct spi_device *spi) spi_set_drvdata(spi, mipi); DRM_DEBUG_DRIVER("Initialized %s:%s @%uMHz on minor %d\n", - tdev->drm->driver->name, dev_name(dev), + tdev->drm.driver->name, dev_name(dev), spi->max_speed_hz / 1000000, - tdev->drm->primary->index); + tdev->drm.primary->index); return 0; } diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h index 4774fe3..ded9817 100644 --- a/include/drm/tinydrm/tinydrm.h +++ b/include/drm/tinydrm/tinydrm.h @@ -10,6 +10,7 @@ #ifndef __LINUX_TINYDRM_H #define __LINUX_TINYDRM_H +#include <drm/drmP.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_fb_cma_helper.h> #include <drm/drm_simple_kms_helper.h> @@ -24,7 +25,7 @@ * @fb_funcs: Framebuffer functions used when creating framebuffers */ struct tinydrm_device { - struct drm_device *drm; + struct drm_device drm; struct drm_simple_display_pipe pipe; struct mutex dirty_lock; struct drm_fbdev_cma *fbdev_cma; @@ -33,6 +34,12 @@ struct tinydrm_device { }; static inline struct tinydrm_device * +drm_to_tinydrm(struct drm_device *drm) +{ + return container_of(drm, struct tinydrm_device, drm); +} + +static inline struct tinydrm_device * pipe_to_tinydrm(struct drm_simple_display_pipe *pipe) { return container_of(pipe, struct tinydrm_device, pipe);
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 <david@lechnology.com> Signed-off-by: Noralf Trønnes <noralf@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(-)