This patchset is part of the effort to remove tinydrm.ko. It removes struct tinydrm_device and tinydrm.h.
Only one change in this version and that is expanding the driver example.
The drm_dev_unplug() dependency series has been applied together with some patches from the previous version.
I've cc'ed intel-gfx so the Intel CI can verify the parent device ref patch.
Noralf.
Noralf Trønnes (7): drm/drv: Hold ref on parent device during drm_device lifetime drm: Add devm_drm_dev_init() drm/drv: DOC: Add driver example code drm/tinydrm/repaper: Drop using tinydrm_device drm/tinydrm: Drop using tinydrm_device drm/tinydrm: Remove tinydrm_device drm/tinydrm: Use drm_dev_enter/exit()
Documentation/driver-model/devres.txt | 3 + Documentation/gpu/tinydrm.rst | 32 +--- Documentation/gpu/todo.rst | 4 - drivers/gpu/drm/drm_drv.c | 176 +++++++++++++++++- drivers/gpu/drm/tinydrm/core/Makefile | 2 +- drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 169 ----------------- .../gpu/drm/tinydrm/core/tinydrm-helpers.c | 2 + drivers/gpu/drm/tinydrm/hx8357d.c | 49 ++++- drivers/gpu/drm/tinydrm/ili9225.c | 63 ++++++- drivers/gpu/drm/tinydrm/ili9341.c | 49 ++++- drivers/gpu/drm/tinydrm/mi0283qt.c | 49 ++++- drivers/gpu/drm/tinydrm/mipi-dbi.c | 109 ++++++++--- drivers/gpu/drm/tinydrm/repaper.c | 130 +++++++++---- drivers/gpu/drm/tinydrm/st7586.c | 129 ++++++++----- drivers/gpu/drm/tinydrm/st7735r.c | 49 ++++- include/drm/drm_drv.h | 3 + include/drm/tinydrm/mipi-dbi.h | 26 ++- include/drm/tinydrm/tinydrm.h | 42 ----- 18 files changed, 688 insertions(+), 398 deletions(-) delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c delete mode 100644 include/drm/tinydrm/tinydrm.h
This makes it safe to access drm_device->dev after the parent device has been removed/unplugged.
Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/drm_drv.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 53e8f4e103db..ce65f12db0fe 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -521,7 +521,7 @@ int drm_dev_init(struct drm_device *dev, BUG_ON(!parent);
kref_init(&dev->ref); - dev->dev = parent; + dev->dev = get_device(parent); dev->driver = driver;
/* no per-device feature limits by default */ @@ -591,6 +591,7 @@ int drm_dev_init(struct drm_device *dev, drm_minor_free(dev, DRM_MINOR_RENDER); drm_fs_inode_free(dev->anon_inode); err_free: + put_device(dev->dev); mutex_destroy(&dev->master_mutex); mutex_destroy(&dev->ctxlist_mutex); mutex_destroy(&dev->clientlist_mutex); @@ -626,6 +627,8 @@ void drm_dev_fini(struct drm_device *dev) drm_minor_free(dev, DRM_MINOR_PRIMARY); drm_minor_free(dev, DRM_MINOR_RENDER);
+ put_device(dev->dev); + mutex_destroy(&dev->master_mutex); mutex_destroy(&dev->ctxlist_mutex); mutex_destroy(&dev->clientlist_mutex);
On Mon, Feb 25, 2019 at 03:42:26PM +0100, Noralf Trønnes wrote:
This makes it safe to access drm_device->dev after the parent device has been removed/unplugged.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
Reviewed-by: Gerd Hoffmann kraxel@redhat.com
This adds a resource managed (devres) version of drm_dev_init().
v2: Remove devm_drm_dev_register() since we can't touch hw in devm release functions and drivers want to disable hw on driver module unload (Daniel Vetter, Greg KH)
Cc: Daniel Vetter daniel@ffwll.ch Cc: Greg KH gregkh@linuxfoundation.org Signed-off-by: Noralf Trønnes noralf@tronnes.org --- Documentation/driver-model/devres.txt | 3 +++ drivers/gpu/drm/drm_drv.c | 39 +++++++++++++++++++++++++++ include/drm/drm_drv.h | 3 +++ 3 files changed, 45 insertions(+)
diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt index b277cafce71e..351b7ac65a1e 100644 --- a/Documentation/driver-model/devres.txt +++ b/Documentation/driver-model/devres.txt @@ -254,6 +254,9 @@ DMA dmam_pool_create() dmam_pool_destroy()
+DRM + devm_drm_dev_init() + GPIO devm_gpiod_get() devm_gpiod_get_index() diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index ce65f12db0fe..934780a4c033 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -601,6 +601,45 @@ int drm_dev_init(struct drm_device *dev, } EXPORT_SYMBOL(drm_dev_init);
+static void devm_drm_dev_init_release(void *data) +{ + drm_dev_put(data); +} + +/** + * devm_drm_dev_init - Resource managed drm_dev_init() + * @parent: Parent device object + * @dev: DRM device + * @driver: DRM driver + * + * Managed drm_dev_init(). The DRM device initialized with this function is + * automatically put on driver detach using drm_dev_put(). You must supply a + * &drm_driver.release callback to control the finalization explicitly. + * + * RETURNS: + * 0 on success, or error code on failure. + */ +int devm_drm_dev_init(struct device *parent, + struct drm_device *dev, + struct drm_driver *driver) +{ + int ret; + + if (WARN_ON(!parent || !driver->release)) + return -EINVAL; + + ret = drm_dev_init(dev, driver, parent); + if (ret) + return ret; + + ret = devm_add_action(parent, devm_drm_dev_init_release, dev); + if (ret) + devm_drm_dev_init_release(dev); + + return ret; +} +EXPORT_SYMBOL(devm_drm_dev_init); + /** * drm_dev_fini - Finalize a dead DRM device * @dev: DRM device diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index ca46a45a9cce..e81bce2698e3 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -718,6 +718,9 @@ extern unsigned int drm_debug; int drm_dev_init(struct drm_device *dev, struct drm_driver *driver, struct device *parent); +int devm_drm_dev_init(struct device *parent, + struct drm_device *dev, + struct drm_driver *driver); void drm_dev_fini(struct drm_device *dev);
struct drm_device *drm_dev_alloc(struct drm_driver *driver,
On Mon, Feb 25, 2019 at 03:42:27PM +0100, Noralf Trønnes wrote:
This adds a resource managed (devres) version of drm_dev_init().
v2: Remove devm_drm_dev_register() since we can't touch hw in devm release functions and drivers want to disable hw on driver module unload (Daniel Vetter, Greg KH)
Acked-by: Gerd Hoffmann kraxel@redhat.com
Add driver example that shows how devm_drm_dev_init() can be used.
v2: Expand docs (Sam, Daniel)
Signed-off-by: Noralf Trønnes noralf@tronnes.org Acked-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_drv.c | 132 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 934780a4c033..50d849d1bc6e 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -286,6 +286,138 @@ void drm_minor_release(struct drm_minor *minor) * Note that the lifetime rules for &drm_device instance has still a lot of * historical baggage. Hence use the reference counting provided by * drm_dev_get() and drm_dev_put() only carefully. + * + * Display driver example + * ~~~~~~~~~~~~~~~~~~~~~~ + * + * The following example shows a typical structure of a DRM display driver. + * The example focus on the probe() function and the other functions that is + * almost always present and serves as a demonstration of devm_drm_dev_init() + * usage with its accompanying drm_driver->release callback. + * + * .. code-block:: c + * + * struct driver_device { + * struct drm_device drm; + * void *userspace_facing; + * struct clk *pclk; + * }; + * + * static void driver_drm_release(struct drm_device *drm) + * { + * struct driver_device *priv = container_of(...); + * + * drm_mode_config_cleanup(drm); + * drm_dev_fini(drm); + * kfree(priv->userspace_facing); + * kfree(priv); + * } + * + * static struct drm_driver driver_drm_driver = { + * [...] + * .release = driver_drm_release, + * }; + * + * static int driver_probe(struct platform_device *pdev) + * { + * struct driver_device *priv; + * struct drm_device *drm; + * int ret; + * + * [ + * devm_kzalloc() can't be used here because the drm_device + * lifetime can exceed the device lifetime if driver unbind + * happens when userspace still has open file descriptors. + * ] + * priv = kzalloc(sizeof(*priv), GFP_KERNEL); + * if (!priv) + * return -ENOMEM; + * + * drm = &priv->drm; + * + * ret = devm_drm_dev_init(&pdev->dev, drm, &driver_drm_driver); + * if (ret) { + * kfree(drm); + * return ret; + * } + * + * drm_mode_config_init(drm); + * + * priv->userspace_facing = kzalloc(..., GFP_KERNEL); + * if (!priv->userspace_facing) + * return -ENOMEM; + * + * priv->pclk = devm_clk_get(dev, "PCLK"); + * if (IS_ERR(priv->pclk)) + * return PTR_ERR(priv->pclk); + * + * [ Further setup, display pipeline etc ] + * + * platform_set_drvdata(pdev, drm); + * + * drm_mode_config_reset(drm); + * + * ret = drm_dev_register(drm); + * if (ret) + * return ret; + * + * drm_fbdev_generic_setup(drm, 32); + * + * return 0; + * } + * + * [ This function is called before the devm_ resources are released ] + * static int driver_remove(struct platform_device *pdev) + * { + * struct drm_device *drm = platform_get_drvdata(pdev); + * + * drm_dev_unregister(drm); + * drm_atomic_helper_shutdown(drm) + * + * return 0; + * } + * + * [ This function is called on kernel restart and shutdown ] + * static void driver_shutdown(struct platform_device *pdev) + * { + * drm_atomic_helper_shutdown(platform_get_drvdata(pdev)); + * } + * + * static int __maybe_unused driver_pm_suspend(struct device *dev) + * { + * return drm_mode_config_helper_suspend(dev_get_drvdata(dev)); + * } + * + * static int __maybe_unused driver_pm_resume(struct device *dev) + * { + * drm_mode_config_helper_resume(dev_get_drvdata(dev)); + * + * return 0; + * } + * + * static const struct dev_pm_ops driver_pm_ops = { + * SET_SYSTEM_SLEEP_PM_OPS(driver_pm_suspend, driver_pm_resume) + * }; + * + * static struct platform_driver driver_driver = { + * .driver = { + * [...] + * .pm = &driver_pm_ops, + * }, + * .probe = driver_probe, + * .remove = driver_remove, + * .shutdown = driver_shutdown, + * }; + * module_platform_driver(driver_driver); + * + * Drivers that want to support device unplugging (USB, DT overlay unload) should + * use drm_dev_unplug() instead of drm_dev_unregister(). The driver must protect + * regions that is accessing device resources to prevent use after they're + * released. This is done using drm_dev_enter() and drm_dev_exit(). There is one + * shortcoming however, drm_dev_unplug() marks the drm_device as unplugged before + * drm_atomic_helper_shutdown() is called. This means that if the disable code + * paths are protected, they will not run on regular driver module unload, + * possibily leaving the hardware enabled. */
/**
On Mon, Feb 25, 2019 at 03:42:28PM +0100, Noralf Trønnes wrote:
Add driver example that shows how devm_drm_dev_init() can be used.
v2: Expand docs (Sam, Daniel)
Acked-by: Gerd Hoffmann kraxel@redhat.com
Use devm_drm_dev_init() and drop using tinydrm_device.
v2: devm_drm_dev_register() was dropped so add a driver release callback.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/tinydrm/repaper.c | 84 ++++++++++++++++++++++--------- 1 file changed, 61 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c index a32dda09458a..b1acf5aebe32 100644 --- a/drivers/gpu/drm/tinydrm/repaper.c +++ b/drivers/gpu/drm/tinydrm/repaper.c @@ -30,11 +30,12 @@ #include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_fb_cma_helper.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_rect.h> #include <drm/drm_vblank.h> -#include <drm/tinydrm/tinydrm.h> +#include <drm/drm_simple_kms_helper.h> #include <drm/tinydrm/tinydrm-helpers.h>
#define REPAPER_RID_G2_COG_ID 0x12 @@ -60,7 +61,8 @@ enum repaper_epd_border_byte { };
struct repaper_epd { - struct tinydrm_device tinydrm; + struct drm_device drm; + struct drm_simple_display_pipe pipe; struct spi_device *spi;
struct gpio_desc *panel_on; @@ -89,10 +91,9 @@ struct repaper_epd { bool partial; };
-static inline struct repaper_epd * -epd_from_tinydrm(struct tinydrm_device *tdev) +static inline struct repaper_epd *drm_to_epd(struct drm_device *drm) { - return container_of(tdev, struct repaper_epd, tinydrm); + return container_of(drm, struct repaper_epd, drm); }
static int repaper_spi_transfer(struct spi_device *spi, u8 header, @@ -530,8 +531,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 repaper_epd *epd = epd_from_tinydrm(tdev); + struct repaper_epd *epd = drm_to_epd(fb->dev); struct drm_rect clip; u8 *buf = NULL; int ret = 0; @@ -646,8 +646,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe, struct drm_crtc_state *crtc_state, struct drm_plane_state *plane_state) { - struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); - struct repaper_epd *epd = epd_from_tinydrm(tdev); + struct repaper_epd *epd = drm_to_epd(pipe->crtc.dev); struct spi_device *spi = epd->spi; struct device *dev = &spi->dev; bool dc_ok = false; @@ -781,8 +780,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe) { - struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); - struct repaper_epd *epd = epd_from_tinydrm(tdev); + struct repaper_epd *epd = drm_to_epd(pipe->crtc.dev); struct spi_device *spi = epd->spi; unsigned int line;
@@ -856,6 +854,23 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = { .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, };
+static const struct drm_mode_config_funcs repaper_mode_config_funcs = { + .fb_create = drm_gem_fb_create_with_dirty, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, +}; + +static void repaper_release(struct drm_device *drm) +{ + struct repaper_epd *epd = drm_to_epd(drm); + + DRM_DEBUG_DRIVER("\n"); + + drm_mode_config_cleanup(drm); + drm_dev_fini(drm); + kfree(epd); +} + static const uint32_t repaper_formats[] = { DRM_FORMAT_XRGB8888, }; @@ -894,6 +909,7 @@ static struct drm_driver repaper_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC, .fops = &repaper_fops, + .release = repaper_release, DRM_GEM_CMA_VMAP_DRIVER_OPS, .name = "repaper", .desc = "Pervasive Displays RePaper e-ink panels", @@ -926,11 +942,11 @@ static int repaper_probe(struct spi_device *spi) const struct spi_device_id *spi_id; const struct of_device_id *match; struct device *dev = &spi->dev; - struct tinydrm_device *tdev; enum repaper_model model; const char *thermal_zone; struct repaper_epd *epd; size_t line_buffer_size; + struct drm_device *drm; int ret;
match = of_match_device(repaper_of_match, dev); @@ -950,10 +966,21 @@ 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;
+ drm = &epd->drm; + + ret = devm_drm_dev_init(dev, drm, &repaper_driver); + if (ret) { + kfree(epd); + return ret; + } + + drm_mode_config_init(drm); + drm->mode_config.funcs = &repaper_mode_config_funcs; + epd->spi = spi;
epd->panel_on = devm_gpiod_get(dev, "panel-on", GPIOD_OUT_LOW); @@ -1064,26 +1091,36 @@ static int repaper_probe(struct spi_device *spi) if (!epd->current_frame) return -ENOMEM;
- tdev = &epd->tinydrm; - - ret = devm_tinydrm_init(dev, tdev, &repaper_driver); - if (ret) - return ret; - - ret = tinydrm_display_pipe_init(tdev->drm, &tdev->pipe, &repaper_pipe_funcs, + ret = tinydrm_display_pipe_init(drm, &epd->pipe, &repaper_pipe_funcs, DRM_MODE_CONNECTOR_VIRTUAL, repaper_formats, ARRAY_SIZE(repaper_formats), mode, 0); if (ret) return ret;
- drm_mode_config_reset(tdev->drm); + drm_mode_config_reset(drm);
- spi_set_drvdata(spi, tdev->drm); + ret = drm_dev_register(drm, 0); + if (ret) + return ret; + + spi_set_drvdata(spi, drm);
DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
- return devm_tinydrm_register(tdev); + drm_fbdev_generic_setup(drm, 32); + + return 0; +} + +static int repaper_remove(struct spi_device *spi) +{ + struct drm_device *drm = spi_get_drvdata(spi); + + drm_dev_unplug(drm); + drm_atomic_helper_shutdown(drm); + + return 0; }
static void repaper_shutdown(struct spi_device *spi) @@ -1099,6 +1136,7 @@ static struct spi_driver repaper_spi_driver = { }, .id_table = repaper_id, .probe = repaper_probe, + .remove = repaper_remove, .shutdown = repaper_shutdown, }; module_spi_driver(repaper_spi_driver);
Use devm_drm_dev_init() and drop using tinydrm_device.
v2: devm_drm_dev_register() was dropped so add driver release callbacks.
Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/tinydrm/hx8357d.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/ili9225.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/ili9341.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/mi0283qt.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/mipi-dbi.c | 67 +++++++++++------- drivers/gpu/drm/tinydrm/st7586.c | 105 ++++++++++++++++------------- drivers/gpu/drm/tinydrm/st7735r.c | 40 +++++++++-- include/drm/tinydrm/mipi-dbi.h | 26 ++++--- 8 files changed, 294 insertions(+), 104 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c index 84dda622df85..e9b9e08fafc7 100644 --- a/drivers/gpu/drm/tinydrm/hx8357d.c +++ b/drivers/gpu/drm/tinydrm/hx8357d.c @@ -18,6 +18,7 @@
#include <drm/drm_atomic_helper.h> #include <drm/drm_drv.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_modeset_helper.h> @@ -189,6 +190,7 @@ DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops); static struct drm_driver hx8357d_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC, .fops = &hx8357d_fops, + .release = mipi_dbi_release, DRM_GEM_CMA_VMAP_DRIVER_OPS, .debugfs_init = mipi_dbi_debugfs_init, .name = "hx8357d", @@ -213,15 +215,25 @@ MODULE_DEVICE_TABLE(spi, hx8357d_id); static int hx8357d_probe(struct spi_device *spi) { struct device *dev = &spi->dev; + struct drm_device *drm; struct mipi_dbi *mipi; struct gpio_desc *dc; u32 rotation = 0; int ret;
- mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL); + mipi = kzalloc(sizeof(*mipi), GFP_KERNEL); if (!mipi) return -ENOMEM;
+ drm = &mipi->drm; + ret = devm_drm_dev_init(dev, drm, &hx8357d_driver); + if (ret) { + kfree(mipi); + return ret; + } + + drm_mode_config_init(drm); + dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW); if (IS_ERR(dc)) { DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n"); @@ -238,14 +250,31 @@ static int hx8357d_probe(struct spi_device *spi) if (ret) return ret;
- ret = mipi_dbi_init(&spi->dev, mipi, &hx8357d_pipe_funcs, - &hx8357d_driver, &yx350hv15_mode, rotation); + ret = mipi_dbi_init(mipi, &hx8357d_pipe_funcs, &yx350hv15_mode, rotation); if (ret) return ret;
- spi_set_drvdata(spi, mipi->tinydrm.drm); + drm_mode_config_reset(drm);
- return devm_tinydrm_register(&mipi->tinydrm); + ret = drm_dev_register(drm, 0); + if (ret) + return ret; + + spi_set_drvdata(spi, drm); + + drm_fbdev_generic_setup(drm, 32); + + return 0; +} + +static int hx8357d_remove(struct spi_device *spi) +{ + struct drm_device *drm = spi_get_drvdata(spi); + + drm_dev_unplug(drm); + drm_atomic_helper_shutdown(drm); + + return 0; }
static void hx8357d_shutdown(struct spi_device *spi) @@ -260,6 +289,7 @@ static struct spi_driver hx8357d_spi_driver = { }, .id_table = hx8357d_id, .probe = hx8357d_probe, + .remove = hx8357d_remove, .shutdown = hx8357d_shutdown, }; module_spi_driver(hx8357d_spi_driver); diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c index 3f59cfbd31ba..4d387a07c48b 100644 --- a/drivers/gpu/drm/tinydrm/ili9225.c +++ b/drivers/gpu/drm/tinydrm/ili9225.c @@ -24,6 +24,7 @@ #include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_fb_cma_helper.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_fourcc.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> @@ -339,6 +340,7 @@ static struct drm_driver ili9225_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC, .fops = &ili9225_fops, + .release = mipi_dbi_release, DRM_GEM_CMA_VMAP_DRIVER_OPS, .name = "ili9225", .desc = "Ilitek ILI9225", @@ -362,15 +364,25 @@ MODULE_DEVICE_TABLE(spi, ili9225_id); static int ili9225_probe(struct spi_device *spi) { struct device *dev = &spi->dev; + struct drm_device *drm; struct mipi_dbi *mipi; struct gpio_desc *rs; u32 rotation = 0; int ret;
- mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL); + mipi = kzalloc(sizeof(*mipi), GFP_KERNEL); if (!mipi) return -ENOMEM;
+ drm = &mipi->drm; + ret = devm_drm_dev_init(dev, drm, &ili9225_driver); + if (ret) { + kfree(mipi); + return ret; + } + + drm_mode_config_init(drm); + mipi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(mipi->reset)) { DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n"); @@ -392,14 +404,31 @@ static int ili9225_probe(struct spi_device *spi) /* override the command function set in mipi_dbi_spi_init() */ mipi->command = ili9225_dbi_command;
- ret = mipi_dbi_init(&spi->dev, mipi, &ili9225_pipe_funcs, - &ili9225_driver, &ili9225_mode, rotation); + ret = mipi_dbi_init(mipi, &ili9225_pipe_funcs, &ili9225_mode, rotation); if (ret) return ret;
- spi_set_drvdata(spi, mipi->tinydrm.drm); + drm_mode_config_reset(drm);
- return devm_tinydrm_register(&mipi->tinydrm); + ret = drm_dev_register(drm, 0); + if (ret) + return ret; + + spi_set_drvdata(spi, drm); + + drm_fbdev_generic_setup(drm, 32); + + return 0; +} + +static int ili9225_remove(struct spi_device *spi) +{ + struct drm_device *drm = spi_get_drvdata(spi); + + drm_dev_unplug(drm); + drm_atomic_helper_shutdown(drm); + + return 0; }
static void ili9225_shutdown(struct spi_device *spi) @@ -415,6 +444,7 @@ static struct spi_driver ili9225_spi_driver = { }, .id_table = ili9225_id, .probe = ili9225_probe, + .remove = ili9225_remove, .shutdown = ili9225_shutdown, }; module_spi_driver(ili9225_spi_driver); diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c index 3b7565a6311e..850ce9ed6dd2 100644 --- a/drivers/gpu/drm/tinydrm/ili9341.c +++ b/drivers/gpu/drm/tinydrm/ili9341.c @@ -17,6 +17,7 @@
#include <drm/drm_atomic_helper.h> #include <drm/drm_drv.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_modeset_helper.h> @@ -145,6 +146,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops); static struct drm_driver ili9341_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC, .fops = &ili9341_fops, + .release = mipi_dbi_release, DRM_GEM_CMA_VMAP_DRIVER_OPS, .debugfs_init = mipi_dbi_debugfs_init, .name = "ili9341", @@ -169,15 +171,25 @@ MODULE_DEVICE_TABLE(spi, ili9341_id); static int ili9341_probe(struct spi_device *spi) { struct device *dev = &spi->dev; + struct drm_device *drm; struct mipi_dbi *mipi; struct gpio_desc *dc; u32 rotation = 0; int ret;
- mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL); + mipi = kzalloc(sizeof(*mipi), GFP_KERNEL); if (!mipi) return -ENOMEM;
+ drm = &mipi->drm; + ret = devm_drm_dev_init(dev, drm, &ili9341_driver); + if (ret) { + kfree(mipi); + return ret; + } + + drm_mode_config_init(drm); + mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(mipi->reset)) { DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n"); @@ -200,14 +212,31 @@ static int ili9341_probe(struct spi_device *spi) if (ret) return ret;
- ret = mipi_dbi_init(&spi->dev, mipi, &ili9341_pipe_funcs, - &ili9341_driver, &yx240qv29_mode, rotation); + ret = mipi_dbi_init(mipi, &ili9341_pipe_funcs, &yx240qv29_mode, rotation); if (ret) return ret;
- spi_set_drvdata(spi, mipi->tinydrm.drm); + drm_mode_config_reset(drm);
- return devm_tinydrm_register(&mipi->tinydrm); + ret = drm_dev_register(drm, 0); + if (ret) + return ret; + + spi_set_drvdata(spi, drm); + + drm_fbdev_generic_setup(drm, 32); + + return 0; +} + +static int ili9341_remove(struct spi_device *spi) +{ + struct drm_device *drm = spi_get_drvdata(spi); + + drm_dev_unplug(drm); + drm_atomic_helper_shutdown(drm); + + return 0; }
static void ili9341_shutdown(struct spi_device *spi) @@ -222,6 +251,7 @@ static struct spi_driver ili9341_spi_driver = { }, .id_table = ili9341_id, .probe = ili9341_probe, + .remove = ili9341_remove, .shutdown = ili9341_shutdown, }; module_spi_driver(ili9341_spi_driver); diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index da326fd1450d..7aee05586e92 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -19,6 +19,7 @@
#include <drm/drm_atomic_helper.h> #include <drm/drm_drv.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_modeset_helper.h> @@ -154,6 +155,7 @@ static struct drm_driver mi0283qt_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC, .fops = &mi0283qt_fops, + .release = mipi_dbi_release, DRM_GEM_CMA_VMAP_DRIVER_OPS, .debugfs_init = mipi_dbi_debugfs_init, .name = "mi0283qt", @@ -178,15 +180,25 @@ MODULE_DEVICE_TABLE(spi, mi0283qt_id); static int mi0283qt_probe(struct spi_device *spi) { struct device *dev = &spi->dev; + struct drm_device *drm; struct mipi_dbi *mipi; struct gpio_desc *dc; u32 rotation = 0; int ret;
- mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL); + mipi = kzalloc(sizeof(*mipi), GFP_KERNEL); if (!mipi) return -ENOMEM;
+ drm = &mipi->drm; + ret = devm_drm_dev_init(dev, drm, &mi0283qt_driver); + if (ret) { + kfree(mipi); + return ret; + } + + drm_mode_config_init(drm); + mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(mipi->reset)) { DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n"); @@ -213,14 +225,31 @@ static int mi0283qt_probe(struct spi_device *spi) if (ret) return ret;
- ret = mipi_dbi_init(&spi->dev, mipi, &mi0283qt_pipe_funcs, - &mi0283qt_driver, &mi0283qt_mode, rotation); + ret = mipi_dbi_init(mipi, &mi0283qt_pipe_funcs, &mi0283qt_mode, rotation); if (ret) return ret;
- spi_set_drvdata(spi, mipi->tinydrm.drm); + drm_mode_config_reset(drm);
- return devm_tinydrm_register(&mipi->tinydrm); + ret = drm_dev_register(drm, 0); + if (ret) + return ret; + + spi_set_drvdata(spi, drm); + + drm_fbdev_generic_setup(drm, 32); + + return 0; +} + +static int mi0283qt_remove(struct spi_device *spi) +{ + struct drm_device *drm = spi_get_drvdata(spi); + + drm_dev_unplug(drm); + drm_atomic_helper_shutdown(drm); + + return 0; }
static void mi0283qt_shutdown(struct spi_device *spi) @@ -253,6 +282,7 @@ static struct spi_driver mi0283qt_spi_driver = { }, .id_table = mi0283qt_id, .probe = mi0283qt_probe, + .remove = mi0283qt_remove, .shutdown = mi0283qt_shutdown, }; module_spi_driver(mi0283qt_spi_driver); diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 246e708a9ff7..5c848f975ebe 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -316,7 +316,7 @@ EXPORT_SYMBOL(mipi_dbi_enable_flush);
static void mipi_dbi_blank(struct mipi_dbi *mipi) { - struct drm_device *drm = mipi->tinydrm.drm; + struct drm_device *drm = &mipi->drm; u16 height = drm->mode_config.min_height; u16 width = drm->mode_config.min_width; size_t len = width * height * 2; @@ -357,6 +357,12 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe) } EXPORT_SYMBOL(mipi_dbi_pipe_disable);
+static const struct drm_mode_config_funcs mipi_dbi_mode_config_funcs = { + .fb_create = drm_gem_fb_create_with_dirty, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, +}; + static const uint32_t mipi_dbi_formats[] = { DRM_FORMAT_RGB565, DRM_FORMAT_XRGB8888, @@ -364,31 +370,27 @@ static const uint32_t mipi_dbi_formats[] = {
/** * mipi_dbi_init - MIPI DBI initialization - * @dev: Parent device * @mipi: &mipi_dbi structure to initialize - * @pipe_funcs: Display pipe functions - * @driver: DRM driver + * @funcs: Display pipe functions * @mode: Display mode * @rotation: Initial rotation in degrees Counter Clock Wise * - * This function initializes a &mipi_dbi structure and it's underlying - * @tinydrm_device. It also sets up the display pipeline. + * This function sets up a &drm_simple_display_pipe with a &drm_connector that + * has one fixed &drm_display_mode which is rotated according to @rotation. + * This mode is used to set the mode config min/max width/height properties. + * Additionally &mipi_dbi.tx_buf is allocated. * * Supported formats: Native RGB565 and emulated XRGB8888. * - * Objects created by this function will be automatically freed on driver - * detach (devres). - * * Returns: * Zero on success, negative error code on failure. */ -int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi, - const struct drm_simple_display_pipe_funcs *pipe_funcs, - struct drm_driver *driver, +int mipi_dbi_init(struct mipi_dbi *mipi, + const struct drm_simple_display_pipe_funcs *funcs, const struct drm_display_mode *mode, unsigned int rotation) { size_t bufsize = mode->vdisplay * mode->hdisplay * sizeof(u16); - struct tinydrm_device *tdev = &mipi->tinydrm; + struct drm_device *drm = &mipi->drm; int ret;
if (!mipi->command) @@ -396,16 +398,12 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
mutex_init(&mipi->cmdlock);
- mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL); + mipi->tx_buf = devm_kmalloc(drm->dev, bufsize, GFP_KERNEL); if (!mipi->tx_buf) return -ENOMEM;
- ret = devm_tinydrm_init(dev, tdev, driver); - if (ret) - return ret; - /* TODO: Maybe add DRM_MODE_CONNECTOR_SPI */ - ret = tinydrm_display_pipe_init(tdev->drm, &tdev->pipe, pipe_funcs, + ret = tinydrm_display_pipe_init(drm, &mipi->pipe, funcs, DRM_MODE_CONNECTOR_VIRTUAL, mipi_dbi_formats, ARRAY_SIZE(mipi_dbi_formats), mode, @@ -413,20 +411,39 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi, if (ret) return ret;
- drm_plane_enable_fb_damage_clips(&tdev->pipe.plane); + drm_plane_enable_fb_damage_clips(&mipi->pipe.plane);
- tdev->drm->mode_config.preferred_depth = 16; + drm->mode_config.funcs = &mipi_dbi_mode_config_funcs; + drm->mode_config.preferred_depth = 16; mipi->rotation = rotation;
- drm_mode_config_reset(tdev->drm); - DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n", - tdev->drm->mode_config.preferred_depth, rotation); + drm->mode_config.preferred_depth, rotation);
return 0; } EXPORT_SYMBOL(mipi_dbi_init);
+/** + * mipi_dbi_release - DRM driver release helper + * @drm: DRM device + * + * This function finalizes and frees &mipi_dbi. + * + * Drivers can use this as their &drm_driver->release callback. + */ +void mipi_dbi_release(struct drm_device *drm) +{ + struct mipi_dbi *dbi = drm_to_mipi_dbi(drm); + + DRM_DEBUG_DRIVER("\n"); + + drm_mode_config_cleanup(drm); + drm_dev_fini(drm); + kfree(dbi); +} +EXPORT_SYMBOL(mipi_dbi_release); + /** * mipi_dbi_hw_reset - Hardware reset of controller * @mipi: MIPI DBI structure @@ -479,7 +496,7 @@ EXPORT_SYMBOL(mipi_dbi_display_is_on);
static int mipi_dbi_poweron_reset_conditional(struct mipi_dbi *mipi, bool cond) { - struct device *dev = mipi->tinydrm.drm->dev; + struct device *dev = mipi->drm.dev; int ret;
if (mipi->regulator) { diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index 6097585913f2..6e92de809b63 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -21,6 +21,7 @@ #include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_fb_cma_helper.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_rect.h> @@ -262,46 +263,6 @@ static const u32 st7586_formats[] = { DRM_FORMAT_XRGB8888, };
-static int st7586_init(struct device *dev, struct mipi_dbi *mipi, - const struct drm_simple_display_pipe_funcs *pipe_funcs, - struct drm_driver *driver, const struct drm_display_mode *mode, - unsigned int rotation) -{ - size_t bufsize = (mode->vdisplay + 2) / 3 * mode->hdisplay; - struct tinydrm_device *tdev = &mipi->tinydrm; - int ret; - - mutex_init(&mipi->cmdlock); - - mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL); - if (!mipi->tx_buf) - return -ENOMEM; - - ret = devm_tinydrm_init(dev, tdev, driver); - if (ret) - return ret; - - ret = tinydrm_display_pipe_init(tdev->drm, &tdev->pipe, pipe_funcs, - DRM_MODE_CONNECTOR_VIRTUAL, - st7586_formats, - ARRAY_SIZE(st7586_formats), - mode, rotation); - if (ret) - return ret; - - drm_plane_enable_fb_damage_clips(&tdev->pipe.plane); - - tdev->drm->mode_config.preferred_depth = 32; - mipi->rotation = rotation; - - drm_mode_config_reset(tdev->drm); - - DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n", - tdev->drm->mode_config.preferred_depth, rotation); - - return 0; -} - static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = { .enable = st7586_pipe_enable, .disable = st7586_pipe_disable, @@ -309,6 +270,12 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = { .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, };
+static const struct drm_mode_config_funcs st7586_mode_config_funcs = { + .fb_create = drm_gem_fb_create_with_dirty, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, +}; + static const struct drm_display_mode st7586_mode = { DRM_SIMPLE_MODE(178, 128, 37, 27), }; @@ -319,6 +286,7 @@ static struct drm_driver st7586_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC, .fops = &st7586_fops, + .release = mipi_dbi_release, DRM_GEM_CMA_VMAP_DRIVER_OPS, .debugfs_init = mipi_dbi_debugfs_init, .name = "st7586", @@ -343,15 +311,35 @@ MODULE_DEVICE_TABLE(spi, st7586_id); static int st7586_probe(struct spi_device *spi) { struct device *dev = &spi->dev; + struct drm_device *drm; struct mipi_dbi *mipi; struct gpio_desc *a0; u32 rotation = 0; + size_t bufsize; int ret;
- mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL); + mipi = kzalloc(sizeof(*mipi), GFP_KERNEL); if (!mipi) return -ENOMEM;
+ drm = &mipi->drm; + ret = devm_drm_dev_init(dev, drm, &st7586_driver); + if (ret) { + kfree(mipi); + return ret; + } + + drm_mode_config_init(drm); + drm->mode_config.preferred_depth = 32; + drm->mode_config.funcs = &st7586_mode_config_funcs; + + mutex_init(&mipi->cmdlock); + + bufsize = (st7586_mode.vdisplay + 2) / 3 * st7586_mode.hdisplay; + mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL); + if (!mipi->tx_buf) + return -ENOMEM; + mipi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(mipi->reset)) { DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n"); @@ -365,6 +353,7 @@ static int st7586_probe(struct spi_device *spi) }
device_property_read_u32(dev, "rotation", &rotation); + mipi->rotation = rotation;
ret = mipi_dbi_spi_init(spi, mipi, a0); if (ret) @@ -382,14 +371,39 @@ static int st7586_probe(struct spi_device *spi) */ mipi->swap_bytes = true;
- ret = st7586_init(&spi->dev, mipi, &st7586_pipe_funcs, &st7586_driver, - &st7586_mode, rotation); + ret = tinydrm_display_pipe_init(drm, &mipi->pipe, &st7586_pipe_funcs, + DRM_MODE_CONNECTOR_VIRTUAL, + st7586_formats, ARRAY_SIZE(st7586_formats), + &st7586_mode, rotation); if (ret) return ret;
- spi_set_drvdata(spi, mipi->tinydrm.drm); + drm_plane_enable_fb_damage_clips(&mipi->pipe.plane);
- return devm_tinydrm_register(&mipi->tinydrm); + drm_mode_config_reset(drm); + + ret = drm_dev_register(drm, 0); + if (ret) + return ret; + + spi_set_drvdata(spi, drm); + + DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n", + drm->mode_config.preferred_depth, rotation); + + drm_fbdev_generic_setup(drm, 32); + + return 0; +} + +static int st7586_remove(struct spi_device *spi) +{ + struct drm_device *drm = spi_get_drvdata(spi); + + drm_dev_unplug(drm); + drm_atomic_helper_shutdown(drm); + + return 0; }
static void st7586_shutdown(struct spi_device *spi) @@ -405,6 +419,7 @@ static struct spi_driver st7586_spi_driver = { }, .id_table = st7586_id, .probe = st7586_probe, + .remove = st7586_remove, .shutdown = st7586_shutdown, }; module_spi_driver(st7586_spi_driver); diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c index bfa7e2221540..0f8a346026ac 100644 --- a/drivers/gpu/drm/tinydrm/st7735r.c +++ b/drivers/gpu/drm/tinydrm/st7735r.c @@ -16,6 +16,7 @@
#include <drm/drm_atomic_helper.h> #include <drm/drm_drv.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/tinydrm/mipi-dbi.h> @@ -120,6 +121,7 @@ static struct drm_driver st7735r_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC, .fops = &st7735r_fops, + .release = mipi_dbi_release, DRM_GEM_CMA_VMAP_DRIVER_OPS, .debugfs_init = mipi_dbi_debugfs_init, .name = "st7735r", @@ -144,15 +146,25 @@ MODULE_DEVICE_TABLE(spi, st7735r_id); static int st7735r_probe(struct spi_device *spi) { struct device *dev = &spi->dev; + struct drm_device *drm; struct mipi_dbi *mipi; struct gpio_desc *dc; u32 rotation = 0; int ret;
- mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL); + mipi = kzalloc(sizeof(*mipi), GFP_KERNEL); if (!mipi) return -ENOMEM;
+ drm = &mipi->drm; + ret = devm_drm_dev_init(dev, drm, &st7735r_driver); + if (ret) { + kfree(mipi); + return ret; + } + + drm_mode_config_init(drm); + mipi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(mipi->reset)) { DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n"); @@ -178,14 +190,31 @@ static int st7735r_probe(struct spi_device *spi) /* Cannot read from Adafruit 1.8" display via SPI */ mipi->read_commands = NULL;
- ret = mipi_dbi_init(&spi->dev, mipi, &jd_t18003_t01_pipe_funcs, - &st7735r_driver, &jd_t18003_t01_mode, rotation); + ret = mipi_dbi_init(mipi, &jd_t18003_t01_pipe_funcs, &jd_t18003_t01_mode, rotation); if (ret) return ret;
- spi_set_drvdata(spi, mipi->tinydrm.drm); + drm_mode_config_reset(drm);
- return devm_tinydrm_register(&mipi->tinydrm); + ret = drm_dev_register(drm, 0); + if (ret) + return ret; + + spi_set_drvdata(spi, drm); + + drm_fbdev_generic_setup(drm, 32); + + return 0; +} + +static int st7735r_remove(struct spi_device *spi) +{ + struct drm_device *drm = spi_get_drvdata(spi); + + drm_dev_unplug(drm); + drm_atomic_helper_shutdown(drm); + + return 0; }
static void st7735r_shutdown(struct spi_device *spi) @@ -201,6 +230,7 @@ static struct spi_driver st7735r_spi_driver = { }, .id_table = st7735r_id, .probe = st7735r_probe, + .remove = st7735r_remove, .shutdown = st7735r_shutdown, }; module_spi_driver(st7735r_spi_driver); diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h index ad7e6bedab5f..301b2c34cd08 100644 --- a/include/drm/tinydrm/mipi-dbi.h +++ b/include/drm/tinydrm/mipi-dbi.h @@ -12,7 +12,9 @@ #ifndef __LINUX_MIPI_DBI_H #define __LINUX_MIPI_DBI_H
-#include <drm/tinydrm/tinydrm.h> +#include <linux/mutex.h> +#include <drm/drm_device.h> +#include <drm/drm_simple_kms_helper.h>
struct drm_rect; struct spi_device; @@ -21,7 +23,6 @@ struct regulator;
/** * struct mipi_dbi - MIPI DBI controller - * @tinydrm: tinydrm base * @spi: SPI device * @enabled: Pipeline is enabled * @cmdlock: Command lock @@ -39,7 +40,16 @@ struct regulator; * @regulator: power regulator (optional) */ struct mipi_dbi { - struct tinydrm_device tinydrm; + /** + * @drm: DRM device + */ + struct drm_device drm; + + /** + * @pipe: Display pipe structure + */ + struct drm_simple_display_pipe pipe; + struct spi_device *spi; bool enabled; struct mutex cmdlock; @@ -58,17 +68,15 @@ struct mipi_dbi {
static inline struct mipi_dbi *drm_to_mipi_dbi(struct drm_device *drm) { - struct tinydrm_device *tdev = drm->dev_private; - - return container_of(tdev, struct mipi_dbi, tinydrm); + return container_of(drm, struct mipi_dbi, drm); }
int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi, struct gpio_desc *dc); -int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi, - const struct drm_simple_display_pipe_funcs *pipe_funcs, - struct drm_driver *driver, +int mipi_dbi_init(struct mipi_dbi *mipi, + const struct drm_simple_display_pipe_funcs *funcs, const struct drm_display_mode *mode, unsigned int rotation); +void mipi_dbi_release(struct drm_device *drm); void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_plane_state *old_state); void mipi_dbi_enable_flush(struct mipi_dbi *mipi,
On Mon, Feb 25, 2019 at 03:42:30PM +0100, Noralf Trønnes wrote:
Use devm_drm_dev_init() and drop using tinydrm_device.
v2: devm_drm_dev_register() was dropped so add driver release callbacks.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/tinydrm/hx8357d.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/ili9225.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/ili9341.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/mi0283qt.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/mipi-dbi.c | 67 +++++++++++------- drivers/gpu/drm/tinydrm/st7586.c | 105 ++++++++++++++++------------- drivers/gpu/drm/tinydrm/st7735r.c | 40 +++++++++-- include/drm/tinydrm/mipi-dbi.h | 26 ++++---
Hmm, repaper got its own patch and these are all bundled. Slightly confusing, but at the end of the day it doesn't matter much.
Acked-by: Gerd Hoffmann kraxel@redhat.com
Den 27.02.2019 15.27, skrev Gerd Hoffmann:
On Mon, Feb 25, 2019 at 03:42:30PM +0100, Noralf Trønnes wrote:
Use devm_drm_dev_init() and drop using tinydrm_device.
v2: devm_drm_dev_register() was dropped so add driver release callbacks.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/tinydrm/hx8357d.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/ili9225.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/ili9341.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/mi0283qt.c | 40 +++++++++-- drivers/gpu/drm/tinydrm/mipi-dbi.c | 67 +++++++++++------- drivers/gpu/drm/tinydrm/st7586.c | 105 ++++++++++++++++------------- drivers/gpu/drm/tinydrm/st7735r.c | 40 +++++++++-- include/drm/tinydrm/mipi-dbi.h | 26 ++++---
Hmm, repaper got its own patch and these are all bundled. Slightly confusing, but at the end of the day it doesn't matter much.
They had to since they all use the mipi-dbi helper which had to be changed. The repaper driver is the only one that didn't use that helper.
thanks, Noralf.
Acked-by: Gerd Hoffmann kraxel@redhat.com
No more users left so it can go alongside its helpers. Update the tinydrm docs description and remove todo entry.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Sam Ravnborg sam@ravnborg.org Acked-by: Daniel Vetter daniel.vetter@ffwll.ch --- Documentation/gpu/tinydrm.rst | 32 ++-- Documentation/gpu/todo.rst | 4 - drivers/gpu/drm/tinydrm/core/Makefile | 2 +- drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 169 ------------------ .../gpu/drm/tinydrm/core/tinydrm-helpers.c | 2 + include/drm/tinydrm/tinydrm.h | 42 ----- 6 files changed, 13 insertions(+), 238 deletions(-) delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c delete mode 100644 include/drm/tinydrm/tinydrm.h
diff --git a/Documentation/gpu/tinydrm.rst b/Documentation/gpu/tinydrm.rst index a913644bfc19..33a41544f659 100644 --- a/Documentation/gpu/tinydrm.rst +++ b/Documentation/gpu/tinydrm.rst @@ -1,27 +1,12 @@ -========================== -drm/tinydrm Driver library -========================== +============================ +drm/tinydrm Tiny DRM drivers +============================
-.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c - :doc: overview +tinydrm is a collection of DRM drivers that are so small they can fit in a +single source file.
-Core functionality -================== - -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c - :doc: core - -.. kernel-doc:: include/drm/tinydrm/tinydrm.h - :internal: - -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c - :export: - -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c - :export: - -Additional helpers -================== +Helpers +=======
.. kernel-doc:: include/drm/tinydrm/tinydrm-helpers.h :internal: @@ -29,6 +14,9 @@ Additional helpers .. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c :export:
+.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c + :export: + MIPI DBI Compatible Controllers ===============================
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 4afbc186f119..1528ad2d598b 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -469,10 +469,6 @@ those drivers as simple as possible, so lots of room for refactoring: one of the ideas for having a shared dsi/dbi helper, abstracting away the transport details more.
-- Quick aside: The unregister devm stuff is kinda getting the lifetimes of - a drm_device wrong. Doesn't matter, since everyone else gets it wrong - too :-) - Contact: Noralf Trønnes, Daniel Vetter
AMD DC Display Driver diff --git a/drivers/gpu/drm/tinydrm/core/Makefile b/drivers/gpu/drm/tinydrm/core/Makefile index fb221e6f8885..6f8f764560e0 100644 --- a/drivers/gpu/drm/tinydrm/core/Makefile +++ b/drivers/gpu/drm/tinydrm/core/Makefile @@ -1,3 +1,3 @@ -tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-helpers.o +tinydrm-y := tinydrm-pipe.o tinydrm-helpers.o
obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c deleted file mode 100644 index 2366a33fd62f..000000000000 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c +++ /dev/null @@ -1,169 +0,0 @@ -/* - * Copyright (C) 2016 Noralf Trønnes - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - */ - -#include <drm/drm_atomic.h> -#include <drm/drm_atomic_helper.h> -#include <drm/drm_drv.h> -#include <drm/drm_fb_helper.h> -#include <drm/drm_gem_framebuffer_helper.h> -#include <drm/drm_probe_helper.h> -#include <drm/drm_print.h> -#include <drm/tinydrm/tinydrm.h> -#include <linux/device.h> -#include <linux/dma-buf.h> -#include <linux/module.h> - -/** - * DOC: overview - * - * This library provides driver helpers for very simple display hardware. - * - * It is based on &drm_simple_display_pipe coupled with a &drm_connector which - * has only one fixed &drm_display_mode. The framebuffers are backed by the - * cma helper and have support for framebuffer flushing (dirty). - * fbdev support is also included. - * - */ - -/** - * DOC: core - * - * The driver allocates &tinydrm_device, initializes it using - * devm_tinydrm_init(), sets up the pipeline using tinydrm_display_pipe_init() - * and registers the DRM device using devm_tinydrm_register(). - */ - -static const struct drm_mode_config_funcs tinydrm_mode_config_funcs = { - .fb_create = drm_gem_fb_create_with_dirty, - .atomic_check = drm_atomic_helper_check, - .atomic_commit = drm_atomic_helper_commit, -}; - -static int tinydrm_init(struct device *parent, struct tinydrm_device *tdev, - struct drm_driver *driver) -{ - struct drm_device *drm; - - /* - * We don't embed drm_device, because that prevent us from using - * devm_kzalloc() to allocate tinydrm_device in the driver since - * drm_dev_put() 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; - drm_mode_config_init(drm); - drm->mode_config.funcs = &tinydrm_mode_config_funcs; - drm->mode_config.allow_fb_modifiers = true; - - return 0; -} - -static void tinydrm_fini(struct tinydrm_device *tdev) -{ - drm_mode_config_cleanup(tdev->drm); - tdev->drm->dev_private = NULL; - drm_dev_put(tdev->drm); -} - -static void devm_tinydrm_release(void *data) -{ - tinydrm_fini(data); -} - -/** - * devm_tinydrm_init - Initialize tinydrm device - * @parent: Parent device object - * @tdev: tinydrm device - * @driver: DRM driver - * - * This function initializes @tdev, the underlying DRM device and it's - * mode_config. Resources will be automatically freed on driver detach (devres) - * using drm_mode_config_cleanup() and drm_dev_put(). - * - * Returns: - * Zero on success, negative error code on failure. - */ -int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev, - struct drm_driver *driver) -{ - int ret; - - ret = tinydrm_init(parent, tdev, driver); - if (ret) - return ret; - - ret = devm_add_action(parent, devm_tinydrm_release, tdev); - if (ret) - tinydrm_fini(tdev); - - return ret; -} -EXPORT_SYMBOL(devm_tinydrm_init); - -static int tinydrm_register(struct tinydrm_device *tdev) -{ - struct drm_device *drm = tdev->drm; - int ret; - - ret = drm_dev_register(tdev->drm, 0); - if (ret) - return ret; - - ret = drm_fbdev_generic_setup(drm, 0); - if (ret) - DRM_ERROR("Failed to initialize fbdev: %d\n", ret); - - return 0; -} - -static void tinydrm_unregister(struct tinydrm_device *tdev) -{ - drm_atomic_helper_shutdown(tdev->drm); - drm_dev_unregister(tdev->drm); -} - -static void devm_tinydrm_register_release(void *data) -{ - tinydrm_unregister(data); -} - -/** - * devm_tinydrm_register - Register tinydrm device - * @tdev: tinydrm device - * - * This function registers the underlying DRM device and fbdev. - * These resources will be automatically unregistered on driver detach (devres) - * and the display pipeline will be disabled. - * - * Returns: - * Zero on success, negative error code on failure. - */ -int devm_tinydrm_register(struct tinydrm_device *tdev) -{ - struct device *dev = tdev->drm->dev; - int ret; - - ret = tinydrm_register(tdev); - if (ret) - return ret; - - ret = devm_add_action(dev, devm_tinydrm_register_release, tdev); - if (ret) - tinydrm_unregister(tdev); - - return ret; -} -EXPORT_SYMBOL(devm_tinydrm_register); - -MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c index 2737b6fdadc8..d7b38dfb6438 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c @@ -365,3 +365,5 @@ int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, EXPORT_SYMBOL(tinydrm_spi_transfer);
#endif /* CONFIG_SPI */ + +MODULE_LICENSE("GPL"); diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h deleted file mode 100644 index ee9b17759391..000000000000 --- a/include/drm/tinydrm/tinydrm.h +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright (C) 2016 Noralf Trønnes - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - */ - -#ifndef __LINUX_TINYDRM_H -#define __LINUX_TINYDRM_H - -#include <drm/drm_simple_kms_helper.h> - -struct drm_driver; - -/** - * struct tinydrm_device - tinydrm device - */ -struct tinydrm_device { - /** - * @drm: DRM device - */ - struct drm_device *drm; - - /** - * @pipe: Display pipe structure - */ - struct drm_simple_display_pipe pipe; -}; - -static inline struct tinydrm_device * -pipe_to_tinydrm(struct drm_simple_display_pipe *pipe) -{ - return container_of(pipe, struct tinydrm_device, pipe); -} - -int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev, - struct drm_driver *driver); -int devm_tinydrm_register(struct tinydrm_device *tdev); - -#endif /* __LINUX_TINYDRM_H */
This protects device resources from use after device removal.
There are 3 ways for driver-device unbinding to happen: - The driver module is unloaded causing the driver to be unregistered. This can't happen as long as there are open file handles because a reference is taken on the module. - The device is removed (Device Tree overlay unloading). This can happen at any time. - The driver sysfs unbind file can be used to unbind the driver from the device. This can happen any time.
v2: Since drm_atomic_helper_shutdown() has to be called after drm_dev_unplug() we don't want do block ->disable after unplug.
Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/tinydrm/hx8357d.c | 9 ++++-- drivers/gpu/drm/tinydrm/ili9225.c | 23 +++++++++++++-- drivers/gpu/drm/tinydrm/ili9341.c | 9 ++++-- drivers/gpu/drm/tinydrm/mi0283qt.c | 9 ++++-- drivers/gpu/drm/tinydrm/mipi-dbi.c | 42 +++++++++++++++++++++++---- drivers/gpu/drm/tinydrm/repaper.c | 46 ++++++++++++++++++++++-------- drivers/gpu/drm/tinydrm/st7586.c | 24 +++++++++++++--- drivers/gpu/drm/tinydrm/st7735r.c | 9 ++++-- 8 files changed, 139 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c index e9b9e08fafc7..fab961dded87 100644 --- a/drivers/gpu/drm/tinydrm/hx8357d.c +++ b/drivers/gpu/drm/tinydrm/hx8357d.c @@ -50,13 +50,16 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe, { struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev); u8 addr_mode; - int ret; + int ret, idx; + + if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return;
DRM_DEBUG_KMS("\n");
ret = mipi_dbi_poweron_conditional_reset(mipi); if (ret < 0) - return; + goto out_exit; if (ret == 1) goto out_enable;
@@ -172,6 +175,8 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe, } mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode); mipi_dbi_enable_flush(mipi, crtc_state, plane_state); +out_exit: + drm_dev_exit(idx); }
static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = { diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c index 4d387a07c48b..0e9fde47b53b 100644 --- a/drivers/gpu/drm/tinydrm/ili9225.c +++ b/drivers/gpu/drm/tinydrm/ili9225.c @@ -89,13 +89,16 @@ static void ili9225_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect) bool swap = mipi->swap_bytes; u16 x_start, y_start; u16 x1, x2, y1, y2; - int ret = 0; + int idx, ret = 0; bool full; void *tr;
if (!mipi->enabled) return;
+ if (!drm_dev_enter(fb->dev, &idx)) + return; + full = width == fb->width && height == fb->height;
DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect)); @@ -158,6 +161,8 @@ static void ili9225_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect) err_msg: if (ret) dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret); + + drm_dev_exit(idx); }
static void ili9225_pipe_update(struct drm_simple_display_pipe *pipe, @@ -191,9 +196,12 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe, .y1 = 0, .y2 = fb->height, }; - int ret; + int ret, idx; u8 am_id;
+ if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return; + DRM_DEBUG_KMS("\n");
mipi_dbi_hw_reset(mipi); @@ -207,7 +215,7 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe, ret = ili9225_command(mipi, ILI9225_POWER_CONTROL_1, 0x0000); if (ret) { DRM_DEV_ERROR(dev, "Error sending command %d\n", ret); - return; + goto out_exit; } ili9225_command(mipi, ILI9225_POWER_CONTROL_2, 0x0000); ili9225_command(mipi, ILI9225_POWER_CONTROL_3, 0x0000); @@ -280,6 +288,8 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
mipi->enabled = true; ili9225_fb_dirty(fb, &rect); +out_exit: + drm_dev_exit(idx); }
static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe) @@ -288,6 +298,13 @@ static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
DRM_DEBUG_KMS("\n");
+ /* + * This callback is not protected by drm_dev_enter/exit since we want to + * turn off the display on regular driver unload. It's highly unlikely + * that the underlying SPI controller is gone should this be called after + * unplug. + */ + if (!mipi->enabled) return;
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c index 850ce9ed6dd2..d15f85e837ae 100644 --- a/drivers/gpu/drm/tinydrm/ili9341.c +++ b/drivers/gpu/drm/tinydrm/ili9341.c @@ -56,13 +56,16 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe, { struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev); u8 addr_mode; - int ret; + int ret, idx; + + if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return;
DRM_DEBUG_KMS("\n");
ret = mipi_dbi_poweron_conditional_reset(mipi); if (ret < 0) - return; + goto out_exit; if (ret == 1) goto out_enable;
@@ -128,6 +131,8 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe, addr_mode |= ILI9341_MADCTL_BGR; mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode); mipi_dbi_enable_flush(mipi, crtc_state, plane_state); +out_exit: + drm_dev_exit(idx); }
static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = { diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index 7aee05586e92..c6dc31084a4e 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -58,13 +58,16 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe, { struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev); u8 addr_mode; - int ret; + int ret, idx; + + if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return;
DRM_DEBUG_KMS("\n");
ret = mipi_dbi_poweron_conditional_reset(mipi); if (ret < 0) - return; + goto out_exit; if (ret == 1) goto out_enable;
@@ -136,6 +139,8 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe, addr_mode |= ILI9341_MADCTL_BGR; mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode); mipi_dbi_enable_flush(mipi, crtc_state, plane_state); +out_exit: + drm_dev_exit(idx); }
static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = { diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 5c848f975ebe..34d544f6e52d 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -220,13 +220,16 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect) unsigned int height = rect->y2 - rect->y1; unsigned int width = rect->x2 - rect->x1; bool swap = mipi->swap_bytes; - int ret = 0; + int idx, ret = 0; bool full; void *tr;
if (!mipi->enabled) return;
+ if (!drm_dev_enter(fb->dev, &idx)) + return; + full = width == fb->width && height == fb->height;
DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect)); @@ -253,6 +256,8 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect) err_msg: if (ret) dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret); + + drm_dev_exit(idx); }
/** @@ -307,10 +312,16 @@ void mipi_dbi_enable_flush(struct mipi_dbi *mipi, .y1 = 0, .y2 = fb->height, }; + int idx; + + if (!drm_dev_enter(&mipi->drm, &idx)) + return;
mipi->enabled = true; mipi_dbi_fb_dirty(fb, &rect); backlight_enable(mipi->backlight); + + drm_dev_exit(idx); } EXPORT_SYMBOL(mipi_dbi_enable_flush);
@@ -320,6 +331,10 @@ static void mipi_dbi_blank(struct mipi_dbi *mipi) u16 height = drm->mode_config.min_height; u16 width = drm->mode_config.min_width; size_t len = width * height * 2; + int idx; + + if (!drm_dev_enter(drm, &idx)) + return;
memset(mipi->tx_buf, 0, len);
@@ -329,6 +344,8 @@ static void mipi_dbi_blank(struct mipi_dbi *mipi) (height >> 8) & 0xFF, (height - 1) & 0xFF); mipi_dbi_command_buf(mipi, MIPI_DCS_WRITE_MEMORY_START, (u8 *)mipi->tx_buf, len); + + drm_dev_exit(idx); }
/** @@ -343,6 +360,9 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe) { struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
+ if (!mipi->enabled) + return; + DRM_DEBUG_KMS("\n");
mipi->enabled = false; @@ -991,11 +1011,16 @@ static ssize_t mipi_dbi_debugfs_command_write(struct file *file, u8 val, cmd = 0, parameters[64]; char *buf, *pos, *token; unsigned int i; - int ret; + int ret, idx; + + if (!drm_dev_enter(&mipi->drm, &idx)) + return -ENODEV;
buf = memdup_user_nul(ubuf, count); - if (IS_ERR(buf)) - return PTR_ERR(buf); + if (IS_ERR(buf)) { + ret = PTR_ERR(buf); + goto err_exit; + }
/* strip trailing whitespace */ for (i = count - 1; i > 0; i--) @@ -1031,6 +1056,8 @@ static ssize_t mipi_dbi_debugfs_command_write(struct file *file,
err_free: kfree(buf); +err_exit: + drm_dev_exit(idx);
return ret < 0 ? ret : count; } @@ -1039,8 +1066,11 @@ static int mipi_dbi_debugfs_command_show(struct seq_file *m, void *unused) { struct mipi_dbi *mipi = m->private; u8 cmd, val[4]; + int ret, idx; size_t len; - int ret; + + if (!drm_dev_enter(&mipi->drm, &idx)) + return -ENODEV;
for (cmd = 0; cmd < 255; cmd++) { if (!mipi_dbi_command_is_read(mipi, cmd)) @@ -1071,6 +1101,8 @@ static int mipi_dbi_debugfs_command_show(struct seq_file *m, void *unused) seq_printf(m, "%*phN\n", (int)len, val); }
+ drm_dev_exit(idx); + return 0; }
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c index b1acf5aebe32..3f3632457079 100644 --- a/drivers/gpu/drm/tinydrm/repaper.c +++ b/drivers/gpu/drm/tinydrm/repaper.c @@ -533,8 +533,14 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb) struct dma_buf_attachment *import_attach = cma_obj->base.import_attach; struct repaper_epd *epd = drm_to_epd(fb->dev); struct drm_rect clip; + int idx, ret = 0; u8 *buf = NULL; - int ret = 0; + + if (!epd->enabled) + return 0; + + if (!drm_dev_enter(fb->dev, &idx)) + return -ENODEV;
/* repaper can't do partial updates */ clip.x1 = 0; @@ -542,17 +548,16 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb) clip.y1 = 0; clip.y2 = fb->height;
- if (!epd->enabled) - return 0; - repaper_get_temperature(epd);
DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id, epd->factored_stage_time);
buf = kmalloc_array(fb->width, fb->height, GFP_KERNEL); - if (!buf) - return -ENOMEM; + if (!buf) { + ret = -ENOMEM; + goto out_exit; + }
if (import_attach) { ret = dma_buf_begin_cpu_access(import_attach->dmabuf, @@ -621,6 +626,8 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
out_free: kfree(buf); +out_exit: + drm_dev_exit(idx);
return ret; } @@ -650,7 +657,10 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe, struct spi_device *spi = epd->spi; struct device *dev = &spi->dev; bool dc_ok = false; - int i, ret; + int i, ret, idx; + + if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return;
DRM_DEBUG_DRIVER("\n");
@@ -689,7 +699,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe, if (!i) { DRM_DEV_ERROR(dev, "timeout waiting for panel to become ready.\n"); power_off(epd); - return; + goto out_exit; }
repaper_read_id(spi); @@ -700,7 +710,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe, else dev_err(dev, "wrong COG ID 0x%02x\n", ret); power_off(epd); - return; + goto out_exit; }
/* Disable OE */ @@ -713,7 +723,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe, else DRM_DEV_ERROR(dev, "panel is reported broken\n"); power_off(epd); - return; + goto out_exit; }
/* Power saving mode */ @@ -753,7 +763,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe, if (ret < 0) { DRM_DEV_ERROR(dev, "failed to read chip (%d)\n", ret); power_off(epd); - return; + goto out_exit; }
if (ret & 0x40) { @@ -765,7 +775,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe, if (!dc_ok) { DRM_DEV_ERROR(dev, "dc/dc failed\n"); power_off(epd); - return; + goto out_exit; }
/* @@ -776,6 +786,8 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
epd->enabled = true; epd->partial = false; +out_exit: + drm_dev_exit(idx); }
static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe) @@ -784,6 +796,16 @@ static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe) struct spi_device *spi = epd->spi; unsigned int line;
+ /* + * This callback is not protected by drm_dev_enter/exit since we want to + * turn off the display on regular driver unload. It's highly unlikely + * that the underlying SPI controller is gone should this be called after + * unplug. + */ + + if (!epd->enabled) + return; + DRM_DEBUG_DRIVER("\n");
epd->enabled = false; diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index 6e92de809b63..d99957bac532 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -119,12 +119,14 @@ static int st7586_buf_copy(void *dst, struct drm_framebuffer *fb, static void st7586_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect) { struct mipi_dbi *mipi = drm_to_mipi_dbi(fb->dev); - int start, end; - int ret = 0; + int start, end, idx, ret = 0;
if (!mipi->enabled) return;
+ if (!drm_dev_enter(fb->dev, &idx)) + return; + /* 3 pixels per byte, so grow clip to nearest multiple of 3 */ rect->x1 = rounddown(rect->x1, 3); rect->x2 = roundup(rect->x2, 3); @@ -152,6 +154,8 @@ static void st7586_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect) err_msg: if (ret) dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret); + + drm_dev_exit(idx); }
static void st7586_pipe_update(struct drm_simple_display_pipe *pipe, @@ -184,14 +188,17 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe, .y1 = 0, .y2 = fb->height, }; - int ret; + int idx, ret; u8 addr_mode;
+ if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return; + DRM_DEBUG_KMS("\n");
ret = mipi_dbi_poweron_reset(mipi); if (ret) - return; + goto out_exit;
mipi_dbi_command(mipi, ST7586_AUTO_READ_CTRL, 0x9f); mipi_dbi_command(mipi, ST7586_OTP_RW_CTRL, 0x00); @@ -244,12 +251,21 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe, st7586_fb_dirty(fb, &rect);
mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON); +out_exit: + drm_dev_exit(idx); }
static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe) { struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
+ /* + * This callback is not protected by drm_dev_enter/exit since we want to + * turn off the display on regular driver unload. It's highly unlikely + * that the underlying SPI controller is gone should this be called after + * unplug. + */ + DRM_DEBUG_KMS("\n");
if (!mipi->enabled) diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c index 0f8a346026ac..022e9849b95b 100644 --- a/drivers/gpu/drm/tinydrm/st7735r.c +++ b/drivers/gpu/drm/tinydrm/st7735r.c @@ -44,14 +44,17 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe, struct drm_plane_state *plane_state) { struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev); - int ret; + int ret, idx; u8 addr_mode;
+ if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return; + DRM_DEBUG_KMS("\n");
ret = mipi_dbi_poweron_reset(mipi); if (ret) - return; + goto out_exit;
msleep(150);
@@ -102,6 +105,8 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe, msleep(20);
mipi_dbi_enable_flush(mipi, crtc_state, plane_state); +out_exit: + drm_dev_exit(idx); }
static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = {
On Mon, Feb 25, 2019 at 03:42:32PM +0100, Noralf Trønnes wrote:
This protects device resources from use after device removal.
There are 3 ways for driver-device unbinding to happen:
- The driver module is unloaded causing the driver to be unregistered. This can't happen as long as there are open file handles because a reference is taken on the module.
- The device is removed (Device Tree overlay unloading). This can happen at any time.
- The driver sysfs unbind file can be used to unbind the driver from the device. This can happen any time.
v2: Since drm_atomic_helper_shutdown() has to be called after drm_dev_unplug() we don't want do block ->disable after unplug.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
Acked-by: Gerd Hoffmann kraxel@redhat.com
Den 25.02.2019 15.42, skrev Noralf Trønnes:
This patchset is part of the effort to remove tinydrm.ko. It removes struct tinydrm_device and tinydrm.h.
Only one change in this version and that is expanding the driver example.
The drm_dev_unplug() dependency series has been applied together with some patches from the previous version.
I've cc'ed intel-gfx so the Intel CI can verify the parent device ref patch.
Noralf.
Noralf Trønnes (7): drm/drv: Hold ref on parent device during drm_device lifetime drm: Add devm_drm_dev_init() drm/drv: DOC: Add driver example code drm/tinydrm/repaper: Drop using tinydrm_device drm/tinydrm: Drop using tinydrm_device drm/tinydrm: Remove tinydrm_device drm/tinydrm: Use drm_dev_enter/exit()
Series is applied to drm-misc-next, thanks for reviewing!
Noralf.
Documentation/driver-model/devres.txt | 3 + Documentation/gpu/tinydrm.rst | 32 +--- Documentation/gpu/todo.rst | 4 - drivers/gpu/drm/drm_drv.c | 176 +++++++++++++++++- drivers/gpu/drm/tinydrm/core/Makefile | 2 +- drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 169 ----------------- .../gpu/drm/tinydrm/core/tinydrm-helpers.c | 2 + drivers/gpu/drm/tinydrm/hx8357d.c | 49 ++++- drivers/gpu/drm/tinydrm/ili9225.c | 63 ++++++- drivers/gpu/drm/tinydrm/ili9341.c | 49 ++++- drivers/gpu/drm/tinydrm/mi0283qt.c | 49 ++++- drivers/gpu/drm/tinydrm/mipi-dbi.c | 109 ++++++++--- drivers/gpu/drm/tinydrm/repaper.c | 130 +++++++++---- drivers/gpu/drm/tinydrm/st7586.c | 129 ++++++++----- drivers/gpu/drm/tinydrm/st7735r.c | 49 ++++- include/drm/drm_drv.h | 3 + include/drm/tinydrm/mipi-dbi.h | 26 ++- include/drm/tinydrm/tinydrm.h | 42 ----- 18 files changed, 688 insertions(+), 398 deletions(-) delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c delete mode 100644 include/drm/tinydrm/tinydrm.h
dri-devel@lists.freedesktop.org
-
Gerd Hoffmann -
Noralf Trønnes