diff mbox series

[v2,11/12] drm/tinydrm: Use drm_dev_enter/exit()

Message ID 20190210131039.52664-12-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show
Series drm/tinydrm: Remove tinydrm_device | expand

Commit Message

Noralf Trønnes Feb. 10, 2019, 1:10 p.m. UTC 
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 mbox series

Patch

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 ecfee357436d..ac4baa5fcc81 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 = {