All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Zimmermann <tzimmermann@suse.de>
To: alexander.deucher@amd.com, christian.koenig@amd.com,
	Xinhui.Pan@amd.com, airlied@gmail.com, daniel@ffwll.ch,
	maarten.lankhorst@linux.intel.com, mripard@kernel.org,
	kherbst@redhat.com, lyude@redhat.com, dakr@redhat.com
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	nouveau@lists.freedesktop.org,
	Thomas Zimmermann <tzimmermann@suse.de>
Subject: [PATCH 7/8] drm: Remove struct drm_driver.lastclose
Date: Wed,  7 Aug 2024 10:41:39 +0200	[thread overview]
Message-ID: <20240807084539.304014-8-tzimmermann@suse.de> (raw)
In-Reply-To: <20240807084539.304014-1-tzimmermann@suse.de>

The lastclose callback in struct drm_driver is unused. Remove it. Also
inline the now trivial drm_lastclose() and update documentation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_drv.c       |  5 ++---
 drivers/gpu/drm/drm_fb_helper.c | 18 +++++------------
 drivers/gpu/drm/drm_file.c      | 35 +++++++--------------------------
 drivers/gpu/drm/drm_internal.h  |  1 -
 include/drm/drm_client.h        |  7 ++++---
 include/drm/drm_drv.h           | 28 --------------------------
 6 files changed, 18 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 93543071a500..5859838b68cb 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -409,9 +409,8 @@ void drm_minor_release(struct drm_minor *minor)
  * drm_put_dev - Unregister and release a DRM device
  * @dev: DRM device
  *
- * Called at module unload time or when a PCI device is unplugged.
- *
- * Cleans up all DRM device, calling drm_lastclose().
+ * Called at module unload time or when a PCI device is unplugged. Cleans up all DRM
+ * device resrouces.
  *
  * Note: Use of this function is deprecated. It will eventually go away
  * completely.  Please use drm_dev_unregister() and drm_dev_put() explicitly
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 3cafb28236f7..36ac75a2ebb8 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -89,14 +89,6 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
  * interfaces. Drivers that use one of the shared memory managers, TTM, SHMEM,
  * DMA, should instead use the corresponding fbdev emulation.
  *
- * Existing fbdev implementations should restore the fbdev console by using
- * drm_fb_helper_lastclose() as their &drm_driver.lastclose callback.
- * They should also notify the fb helper code from updates to the output
- * configuration by using drm_fb_helper_output_poll_changed() as their
- * &drm_mode_config_funcs.output_poll_changed callback. New implementations
- * of fbdev should be build on top of struct &drm_client_funcs, which handles
- * this automatically. Setting the old callbacks should be avoided.
- *
  * For suspend/resume consider using drm_mode_config_helper_suspend() and
  * drm_mode_config_helper_resume() which takes care of fbdev as well.
  *
@@ -260,12 +252,12 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
  * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
  * @fb_helper: driver-allocated fbdev helper, can be NULL
  *
- * This should be called from driver's drm &drm_driver.lastclose callback
- * when implementing an fbcon on top of kms using this helper. This ensures that
- * the user isn't greeted with a black screen when e.g. X dies.
+ * This helper should be called from fbdev emulation's &drm_client_funcs.restore
+ * callback. It ensures that the user isn't greeted with a black screen when the
+ * userspace compositor releases the display device.
  *
- * RETURNS:
- * Zero if everything went ok, negative error code otherwise.
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
  */
 int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 {
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 714e42b05108..8fc664974e72 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -62,15 +62,6 @@ bool drm_dev_needs_global_mutex(struct drm_device *dev)
 	if (dev->driver->load || dev->driver->unload)
 		return true;
 
-	/*
-	 * Drivers with the lastclose callback assume that it's synchronized
-	 * against concurrent opens, which again needs the BKL. The proper fix
-	 * is to use the drm_client infrastructure with proper locking for each
-	 * client.
-	 */
-	if (dev->driver->lastclose)
-		return true;
-
 	return false;
 }
 
@@ -395,26 +386,15 @@ int drm_open(struct inode *inode, struct file *filp)
 }
 EXPORT_SYMBOL(drm_open);
 
-void drm_lastclose(struct drm_device * dev)
-{
-	drm_dbg_core(dev, "\n");
-
-	if (dev->driver->lastclose)
-		dev->driver->lastclose(dev);
-	drm_dbg_core(dev, "driver lastclose completed\n");
-
-	drm_client_dev_restore(dev);
-}
-
 /**
  * drm_release - release method for DRM file
  * @inode: device inode
  * @filp: file pointer.
  *
  * This function must be used by drivers as their &file_operations.release
- * method. It frees any resources associated with the open file, and calls the
- * &drm_driver.postclose driver callback. If this is the last open file for the
- * DRM device also proceeds to call the &drm_driver.lastclose driver callback.
+ * method. It frees any resources associated with the open file. If this
+ * is the last open file for the DRM device, it also restores the active
+ * in-kernel DRM client.
  *
  * RETURNS:
  *
@@ -434,7 +414,7 @@ int drm_release(struct inode *inode, struct file *filp)
 	drm_close_helper(filp);
 
 	if (atomic_dec_and_test(&dev->open_count))
-		drm_lastclose(dev);
+		drm_client_dev_restore(dev);
 
 	if (drm_dev_needs_global_mutex(dev))
 		mutex_unlock(&drm_global_mutex);
@@ -484,9 +464,8 @@ void drm_file_update_pid(struct drm_file *filp)
  *
  * This function may be used by drivers as their &file_operations.release
  * method. It frees any resources associated with the open file prior to taking
- * the drm_global_mutex, which then calls the &drm_driver.postclose driver
- * callback. If this is the last open file for the DRM device also proceeds to
- * call the &drm_driver.lastclose driver callback.
+ * the drm_global_mutex. If this is the last open file for the DRM device, it
+ * then restores the active in-kernel DRM client.
  *
  * RETURNS:
  *
@@ -501,7 +480,7 @@ int drm_release_noglobal(struct inode *inode, struct file *filp)
 	drm_close_helper(filp);
 
 	if (atomic_dec_and_mutex_lock(&dev->open_count, &drm_global_mutex)) {
-		drm_lastclose(dev);
+		drm_client_dev_restore(dev);
 		mutex_unlock(&drm_global_mutex);
 	}
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 690505a1f7a5..23c99803af44 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -53,7 +53,6 @@ extern struct mutex drm_global_mutex;
 bool drm_dev_needs_global_mutex(struct drm_device *dev);
 struct drm_file *drm_file_alloc(struct drm_minor *minor);
 void drm_file_free(struct drm_file *file);
-void drm_lastclose(struct drm_device *dev);
 
 #ifdef CONFIG_PCI
 
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index bc0e66f9c425..ddf38b210254 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -41,9 +41,10 @@ struct drm_client_funcs {
 	/**
 	 * @restore:
 	 *
-	 * Called on drm_lastclose(). The first client instance in the list that
-	 * returns zero gets the privilege to restore and no more clients are
-	 * called. This callback is not called after @unregister has been called.
+	 * Called userspace closes the final reference to the DRM device file. The
+	 * first client instance in the list that returns zero gets the privilege
+	 * to restore and no more clients are called. This callback is not called
+	 * after @unregister has been called.
 	 *
 	 * Note that the core does not guarantee exclusion against concurrent
 	 * drm_open(). Clients need to ensure this themselves, for example by
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index cd37936c3926..02ea4e3248fd 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -228,34 +228,6 @@ struct drm_driver {
 	 */
 	void (*postclose) (struct drm_device *, struct drm_file *);
 
-	/**
-	 * @lastclose:
-	 *
-	 * Called when the last &struct drm_file has been closed and there's
-	 * currently no userspace client for the &struct drm_device.
-	 *
-	 * Modern drivers should only use this to force-restore the fbdev
-	 * framebuffer using drm_fb_helper_restore_fbdev_mode_unlocked().
-	 * Anything else would indicate there's something seriously wrong.
-	 * Modern drivers can also use this to execute delayed power switching
-	 * state changes, e.g. in conjunction with the :ref:`vga_switcheroo`
-	 * infrastructure.
-	 *
-	 * This is called after @postclose hook has been called.
-	 *
-	 * NOTE:
-	 *
-	 * All legacy drivers use this callback to de-initialize the hardware.
-	 * This is purely because of the shadow-attach model, where the DRM
-	 * kernel driver does not really own the hardware. Instead ownershipe is
-	 * handled with the help of userspace through an inheritedly racy dance
-	 * to set/unset the VT into raw mode.
-	 *
-	 * Legacy drivers initialize the hardware in the @firstopen callback,
-	 * which isn't even called for modern drivers.
-	 */
-	void (*lastclose) (struct drm_device *);
-
 	/**
 	 * @unload:
 	 *
-- 
2.46.0


  parent reply	other threads:[~2024-08-07  8:45 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-07  8:41 [PATCH 0/8] drm/{amdgpu,nouveau}: Remove old fbdev hooks Thomas Zimmermann
2024-08-07  8:41 ` [PATCH 1/8] drm/fbdev-helper: Do delayed switcheroo in lastclose helper Thomas Zimmermann
2024-08-09  8:44   ` Daniel Vetter
2024-08-09  9:28     ` Thomas Zimmermann
2024-08-07  8:41 ` [PATCH 2/8] drm/amdgpu: Do not set struct drm_driver.lastclose Thomas Zimmermann
2024-08-07  8:41 ` [PATCH 3/8] drm/nouveau: " Thomas Zimmermann
2024-08-07  8:41 ` [PATCH 4/8] drm/nouveau: Do not set struct drm_mode_config_funcs.output_poll_changed Thomas Zimmermann
2024-08-09  8:45   ` Daniel Vetter
2024-08-07  8:41 ` [PATCH 5/8] drm/nouveau: Implement switcheroo reprobe with drm_client_dev_hotplug() Thomas Zimmermann
2024-08-09  8:47   ` Daniel Vetter
2024-08-07  8:41 ` [PATCH 6/8] drm/fbdev-helper: Remove drm_fb_helper_output_poll_changed() Thomas Zimmermann
2024-08-09  8:50   ` Daniel Vetter
2024-08-07  8:41 ` Thomas Zimmermann [this message]
2024-08-07  8:41 ` [PATCH 8/8] drm: Remove struct drm_mode_config_funcs.output_poll_changed Thomas Zimmermann
2024-08-09  8:51   ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240807084539.304014-8-tzimmermann@suse.de \
    --to=tzimmermann@suse.de \
    --cc=Xinhui.Pan@amd.com \
    --cc=airlied@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    --cc=dakr@redhat.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kherbst@redhat.com \
    --cc=lyude@redhat.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.