[PATCH 0/5] DRM deadcode

[PATCH 0/5] DRM deadcode

[linux]

From: "Dr. David Alan Gilbert" <linux@treblig.org>

Hi,
  This is a bunch of deadcode removals; they're all whole
function removals, no changing actual code or behaviour.

Note the last 3 delete functions that were each added (long ago)
by patches that explicitly added that individual function, but then
neve used them.

Dave

Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>


Dr. David Alan Gilbert (5):
  drm: Remove unused drm_atomic_helper_commit_planes_on_crtc
  drm/sysfs: Remove unused drm_class_device_(un)register
  drm/vblank: Remove unused drm_crtc_vblank_count_and_time
  drm/client: Remove unused drm_client_framebuffer_flush
  drm/client: Remove unused drm_client_modeset_check

 drivers/gpu/drm/drm_atomic_helper.c  | 72 ----------------------------
 drivers/gpu/drm/drm_client.c         | 33 -------------
 drivers/gpu/drm/drm_client_modeset.c | 24 ----------
 drivers/gpu/drm/drm_sysfs.c          | 32 -------------
 drivers/gpu/drm/drm_vblank.c         | 44 ++++-------------
 include/drm/drm_atomic_helper.h      |  1 -
 include/drm/drm_client.h             |  2 -
 include/drm/drm_sysfs.h              |  4 --
 include/drm/drm_vblank.h             | 10 ++--
 9 files changed, 12 insertions(+), 210 deletions(-)

-- 
2.47.0

[PATCH 1/5] drm: Remove unused drm_atomic_helper_commit_planes_on_crtc

[linux]

From: "Dr. David Alan Gilbert" <linux@treblig.org>

The last use of drm_atomic_helper_commit_planes_on_crtc() was removed
in 2018 by
commit 6c246b81f938 ("drm/i915: Replace call to commit_planes_on_crtc with
internal update, v2.")

Remove it.

Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
---
 drivers/gpu/drm/drm_atomic_helper.c | 72 -----------------------------
 include/drm/drm_atomic_helper.h     |  1 -
 2 files changed, 73 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 43cdf39019a4..dd8f7d6f5a36 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2705,8 +2705,6 @@ static bool plane_crtc_active(const struct drm_plane_state *state)
  * crtcs need to be updated though.
  *
  * Note that this function does all plane updates across all CRTCs in one step.
- * If the hardware can't support this approach look at
- * drm_atomic_helper_commit_planes_on_crtc() instead.
  *
  * Plane parameters can be updated by applications while the associated CRTC is
  * disabled. The DRM/KMS core will store the parameters in the plane state,
@@ -2833,76 +2831,6 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
 
-/**
- * drm_atomic_helper_commit_planes_on_crtc - commit plane state for a CRTC
- * @old_crtc_state: atomic state object with the old CRTC state
- *
- * This function commits the new plane state using the plane and atomic helper
- * functions for planes on the specific CRTC. It assumes that the atomic state
- * has already been pushed into the relevant object state pointers, since this
- * step can no longer fail.
- *
- * This function is useful when plane updates should be done CRTC-by-CRTC
- * instead of one global step like drm_atomic_helper_commit_planes() does.
- *
- * This function can only be savely used when planes are not allowed to move
- * between different CRTCs because this function doesn't handle inter-CRTC
- * dependencies. Callers need to ensure that either no such dependencies exist,
- * resolve them through ordering of commit calls or through some other means.
- */
-void
-drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state)
-{
-	const struct drm_crtc_helper_funcs *crtc_funcs;
-	struct drm_crtc *crtc = old_crtc_state->crtc;
-	struct drm_atomic_state *old_state = old_crtc_state->state;
-	struct drm_crtc_state *new_crtc_state =
-		drm_atomic_get_new_crtc_state(old_state, crtc);
-	struct drm_plane *plane;
-	unsigned int plane_mask;
-
-	plane_mask = old_crtc_state->plane_mask;
-	plane_mask |= new_crtc_state->plane_mask;
-
-	crtc_funcs = crtc->helper_private;
-	if (crtc_funcs && crtc_funcs->atomic_begin)
-		crtc_funcs->atomic_begin(crtc, old_state);
-
-	drm_for_each_plane_mask(plane, crtc->dev, plane_mask) {
-		struct drm_plane_state *old_plane_state =
-			drm_atomic_get_old_plane_state(old_state, plane);
-		struct drm_plane_state *new_plane_state =
-			drm_atomic_get_new_plane_state(old_state, plane);
-		const struct drm_plane_helper_funcs *plane_funcs;
-		bool disabling;
-
-		plane_funcs = plane->helper_private;
-
-		if (!old_plane_state || !plane_funcs)
-			continue;
-
-		WARN_ON(new_plane_state->crtc &&
-			new_plane_state->crtc != crtc);
-
-		disabling = drm_atomic_plane_disabling(old_plane_state, new_plane_state);
-
-		if (disabling && plane_funcs->atomic_disable) {
-			plane_funcs->atomic_disable(plane, old_state);
-		} else if (new_plane_state->crtc || disabling) {
-			plane_funcs->atomic_update(plane, old_state);
-
-			if (!disabling && plane_funcs->atomic_enable) {
-				if (drm_atomic_plane_enabling(old_plane_state, new_plane_state))
-					plane_funcs->atomic_enable(plane, old_state);
-			}
-		}
-	}
-
-	if (crtc_funcs && crtc_funcs->atomic_flush)
-		crtc_funcs->atomic_flush(crtc, old_state);
-}
-EXPORT_SYMBOL(drm_atomic_helper_commit_planes_on_crtc);
-
 /**
  * drm_atomic_helper_disable_planes_on_crtc - helper to disable CRTC's planes
  * @old_crtc_state: atomic state object with the old CRTC state
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 9aa0a05aa072..6ad50c531fc0 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -107,7 +107,6 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 				     uint32_t flags);
 void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 				      struct drm_atomic_state *old_state);
-void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state);
 void
 drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc_state *old_crtc_state,
 					 bool atomic);
-- 
2.47.0

[PATCH 2/5] drm/sysfs: Remove unused drm_class_device_(un)register

[linux]

From: "Dr. David Alan Gilbert" <linux@treblig.org>

drm_class_device_register() and drm_class_device_unregister() have been
unused since
commit ed89fff97382 ("drm/ttm: drop sysfs directory")

Remove them.

Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
---
 drivers/gpu/drm/drm_sysfs.c | 32 --------------------------------
 include/drm/drm_sysfs.h     |  4 ----
 2 files changed, 36 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index fb3bbb6adcd1..3bc90fe5cea3 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -548,35 +548,3 @@ struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
 	put_device(kdev);
 	return ERR_PTR(r);
 }
-
-/**
- * drm_class_device_register - register new device with the DRM sysfs class
- * @dev: device to register
- *
- * Registers a new &struct device within the DRM sysfs class. Essentially only
- * used by ttm to have a place for its global settings. Drivers should never use
- * this.
- */
-int drm_class_device_register(struct device *dev)
-{
-	if (!drm_class || IS_ERR(drm_class))
-		return -ENOENT;
-
-	dev->class = drm_class;
-	return device_register(dev);
-}
-EXPORT_SYMBOL_GPL(drm_class_device_register);
-
-/**
- * drm_class_device_unregister - unregister device with the DRM sysfs class
- * @dev: device to unregister
- *
- * Unregisters a &struct device from the DRM sysfs class. Essentially only used
- * by ttm to have a place for its global settings. Drivers should never use
- * this.
- */
-void drm_class_device_unregister(struct device *dev)
-{
-	return device_unregister(dev);
-}
-EXPORT_SYMBOL_GPL(drm_class_device_unregister);
diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
index 96a5d858404b..7695873a9456 100644
--- a/include/drm/drm_sysfs.h
+++ b/include/drm/drm_sysfs.h
@@ -3,13 +3,9 @@
 #define _DRM_SYSFS_H_
 
 struct drm_device;
-struct device;
 struct drm_connector;
 struct drm_property;
 
-int drm_class_device_register(struct device *dev);
-void drm_class_device_unregister(struct device *dev);
-
 void drm_sysfs_hotplug_event(struct drm_device *dev);
 void drm_sysfs_connector_hotplug_event(struct drm_connector *connector);
 void drm_sysfs_connector_property_event(struct drm_connector *connector,
-- 
2.47.0

[PATCH 3/5] drm/vblank: Remove unused drm_crtc_vblank_count_and_time

[linux]

From: "Dr. David Alan Gilbert" <linux@treblig.org>

drm_crtc_vblank_count_and_time() was explicitly added by
commit cf6483050e9b ("drm/irq: Add drm_crtc_vblank_count_and_time()")
in 2015, but never used.

Remove it, and rework comments that reference it.

Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
---
 drivers/gpu/drm/drm_vblank.c | 44 +++++++-----------------------------
 include/drm/drm_vblank.h     | 10 ++++----
 2 files changed, 12 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 94e45ed6869d..67d6367e9f4b 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -908,10 +908,10 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
  * drm_crtc_accurate_vblank_count() for such use-cases.
  *
  * Note that for a given vblank counter value drm_crtc_handle_vblank()
- * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
- * provide a barrier: Any writes done before calling
- * drm_crtc_handle_vblank() will be visible to callers of the later
- * functions, if the vblank count is the same or a later one.
+ * and drm_crtc_vblank_count() provide a barrier:
+ * Any writes done before calling drm_crtc_handle_vblank() will be
+ * visible to callers of the later functions, if the vblank count is
+ * the same or a later one.
  *
  * See also &drm_vblank_crtc.count.
  *
@@ -936,7 +936,6 @@ EXPORT_SYMBOL(drm_crtc_vblank_count);
  * modesetting activity. Returns corresponding system timestamp of the time
  * of the vblank interval that corresponds to the current vblank counter value.
  *
- * This is the legacy version of drm_crtc_vblank_count_and_time().
  */
 static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
 				     ktime_t *vblanktime)
@@ -959,33 +958,6 @@ static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
 	return vblank_count;
 }
 
-/**
- * drm_crtc_vblank_count_and_time - retrieve "cooked" vblank counter value
- *     and the system timestamp corresponding to that vblank counter value
- * @crtc: which counter to retrieve
- * @vblanktime: Pointer to time to receive the vblank timestamp.
- *
- * Fetches the "cooked" vblank count value that represents the number of
- * vblank events since the system was booted, including lost events due to
- * modesetting activity. Returns corresponding system timestamp of the time
- * of the vblank interval that corresponds to the current vblank counter value.
- *
- * Note that for a given vblank counter value drm_crtc_handle_vblank()
- * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
- * provide a barrier: Any writes done before calling
- * drm_crtc_handle_vblank() will be visible to callers of the later
- * functions, if the vblank count is the same or a later one.
- *
- * See also &drm_vblank_crtc.count.
- */
-u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
-				   ktime_t *vblanktime)
-{
-	return drm_vblank_count_and_time(crtc->dev, drm_crtc_index(crtc),
-					 vblanktime);
-}
-EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
-
 /**
  * drm_crtc_next_vblank_start - calculate the time of the next vblank
  * @crtc: the crtc for which to calculate next vblank time
@@ -1978,10 +1950,10 @@ EXPORT_SYMBOL(drm_handle_vblank);
  * This is the native KMS version of drm_handle_vblank().
  *
  * Note that for a given vblank counter value drm_crtc_handle_vblank()
- * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
- * provide a barrier: Any writes done before calling
- * drm_crtc_handle_vblank() will be visible to callers of the later
- * functions, if the vblank count is the same or a later one.
+ * and drm_crtc_vblank_count() * provide a barrier:
+ * Any writes done before calling * drm_crtc_handle_vblank() will be
+ * visible to callers of the later functions, if the vblank count is
+ * the same or a later one.
  *
  * See also &drm_vblank_crtc.count.
  *
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 151ab1e85b1b..572e54425970 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -141,10 +141,10 @@ struct drm_vblank_crtc {
 	 * Current software vblank counter.
 	 *
 	 * Note that for a given vblank counter value drm_crtc_handle_vblank()
-	 * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
-	 * provide a barrier: Any writes done before calling
-	 * drm_crtc_handle_vblank() will be visible to callers of the later
-	 * functions, iff the vblank count is the same or a later one.
+	 * and drm_crtc_vblank_count() provide a barrier:
+	 * Any writes done before calling drm_crtc_handle_vblank() will be
+	 * visible to callers of the later functions, iff the vblank count is
+	 * the same or a later one.
 	 *
 	 * IMPORTANT: This guarantee requires barriers, therefor never access
 	 * this field directly. Use drm_crtc_vblank_count() instead.
@@ -260,8 +260,6 @@ struct drm_vblank_crtc *drm_crtc_vblank_crtc(struct drm_crtc *crtc);
 int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs);
 bool drm_dev_has_vblank(const struct drm_device *dev);
 u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
-u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
-				   ktime_t *vblanktime);
 int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime);
 void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
 			       struct drm_pending_vblank_event *e);
-- 
2.47.0

[PATCH 4/5] drm/client: Remove unused drm_client_framebuffer_flush

[linux]

From: "Dr. David Alan Gilbert" <linux@treblig.org>

drm_client_framebuffer_flush() was explicitly added in 2020
by
commit c9c03e3cf072 ("drm/client: Add drm_client_framebuffer_flush()")
but has never been used.

Remove it.

Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
---
 drivers/gpu/drm/drm_client.c | 33 ---------------------------------
 include/drm/drm_client.h     |  1 -
 2 files changed, 34 deletions(-)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index bfedcbf516db..5d10ad3c2ca5 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -552,39 +552,6 @@ void drm_client_framebuffer_delete(struct drm_client_buffer *buffer)
 }
 EXPORT_SYMBOL(drm_client_framebuffer_delete);
 
-/**
- * drm_client_framebuffer_flush - Manually flush client framebuffer
- * @buffer: DRM client buffer (can be NULL)
- * @rect: Damage rectangle (if NULL flushes all)
- *
- * This calls &drm_framebuffer_funcs->dirty (if present) to flush buffer changes
- * for drivers that need it.
- *
- * Returns:
- * Zero on success or negative error code on failure.
- */
-int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect)
-{
-	if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
-		return 0;
-
-	if (rect) {
-		struct drm_clip_rect clip = {
-			.x1 = rect->x1,
-			.y1 = rect->y1,
-			.x2 = rect->x2,
-			.y2 = rect->y2,
-		};
-
-		return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
-						0, 0, &clip, 1);
-	}
-
-	return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
-					0, 0, NULL, 0);
-}
-EXPORT_SYMBOL(drm_client_framebuffer_flush);
-
 #ifdef CONFIG_DEBUG_FS
 static int drm_client_debugfs_internal_clients(struct seq_file *m, void *data)
 {
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index bc0e66f9c425..560aae47e06d 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -165,7 +165,6 @@ struct drm_client_buffer {
 struct drm_client_buffer *
 drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format);
 void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
-int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect);
 int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
 				 struct iosys_map *map_copy);
 void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer);
-- 
2.47.0

[PATCH 5/5] drm/client: Remove unused drm_client_modeset_check

[linux]

From: "Dr. David Alan Gilbert" <linux@treblig.org>

drm_client_modeset_check() was explicitly added in 2020 by
commit 64593f2a6fc9 ("drm/client: Add drm_client_modeset_check()")
but has never been used.

Remove it.

Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
---
 drivers/gpu/drm/drm_client_modeset.c | 24 ------------------------
 include/drm/drm_client.h             |  1 -
 2 files changed, 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index cee5eafbfb81..69e1ce4d18cd 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -1126,30 +1126,6 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
 	return ret;
 }
 
-/**
- * drm_client_modeset_check() - Check modeset configuration
- * @client: DRM client
- *
- * Check modeset configuration.
- *
- * Returns:
- * Zero on success or negative error code on failure.
- */
-int drm_client_modeset_check(struct drm_client_dev *client)
-{
-	int ret;
-
-	if (!drm_drv_uses_atomic_modeset(client->dev))
-		return 0;
-
-	mutex_lock(&client->modeset_mutex);
-	ret = drm_client_modeset_commit_atomic(client, true, true);
-	mutex_unlock(&client->modeset_mutex);
-
-	return ret;
-}
-EXPORT_SYMBOL(drm_client_modeset_check);
-
 /**
  * drm_client_modeset_commit_locked() - Force commit CRTC configuration
  * @client: DRM client
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 560aae47e06d..e1fd32adb3e9 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -176,7 +176,6 @@ int drm_client_modeset_create(struct drm_client_dev *client);
 void drm_client_modeset_free(struct drm_client_dev *client);
 int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width, unsigned int height);
 bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
-int drm_client_modeset_check(struct drm_client_dev *client);
 int drm_client_modeset_commit_locked(struct drm_client_dev *client);
 int drm_client_modeset_commit(struct drm_client_dev *client);
 int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
-- 
2.47.0

Re: [PATCH 4/5] drm/client: Remove unused drm_client_framebuffer_flush

[Thomas Zimmermann]

Hi

Am 23.10.24 um 01:29 schrieb linux@treblig.org:
> From: "Dr. David Alan Gilbert" <linux@treblig.org>
>
> drm_client_framebuffer_flush() was explicitly added in 2020
> by
> commit c9c03e3cf072 ("drm/client: Add drm_client_framebuffer_flush()")
> but has never been used.
>
> Remove it.

I had a patchset to use this helper for fbdev emulation. It just needs 
preparation in a number of drivers.

Best regards
Thomas

>
> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> ---
>   drivers/gpu/drm/drm_client.c | 33 ---------------------------------
>   include/drm/drm_client.h     |  1 -
>   2 files changed, 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index bfedcbf516db..5d10ad3c2ca5 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -552,39 +552,6 @@ void drm_client_framebuffer_delete(struct drm_client_buffer *buffer)
>   }
>   EXPORT_SYMBOL(drm_client_framebuffer_delete);
>   
> -/**
> - * drm_client_framebuffer_flush - Manually flush client framebuffer
> - * @buffer: DRM client buffer (can be NULL)
> - * @rect: Damage rectangle (if NULL flushes all)
> - *
> - * This calls &drm_framebuffer_funcs->dirty (if present) to flush buffer changes
> - * for drivers that need it.
> - *
> - * Returns:
> - * Zero on success or negative error code on failure.
> - */
> -int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect)
> -{
> -	if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
> -		return 0;
> -
> -	if (rect) {
> -		struct drm_clip_rect clip = {
> -			.x1 = rect->x1,
> -			.y1 = rect->y1,
> -			.x2 = rect->x2,
> -			.y2 = rect->y2,
> -		};
> -
> -		return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
> -						0, 0, &clip, 1);
> -	}
> -
> -	return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
> -					0, 0, NULL, 0);
> -}
> -EXPORT_SYMBOL(drm_client_framebuffer_flush);
> -
>   #ifdef CONFIG_DEBUG_FS
>   static int drm_client_debugfs_internal_clients(struct seq_file *m, void *data)
>   {
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index bc0e66f9c425..560aae47e06d 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -165,7 +165,6 @@ struct drm_client_buffer {
>   struct drm_client_buffer *
>   drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format);
>   void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
> -int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect);
>   int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
>   				 struct iosys_map *map_copy);
>   void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH 4/5] drm/client: Remove unused drm_client_framebuffer_flush

[Jocelyn Falempe]

On 23/10/2024 08:46, Thomas Zimmermann wrote:
> Hi
> 
> Am 23.10.24 um 01:29 schrieb linux@treblig.org:
>> From: "Dr. David Alan Gilbert" <linux@treblig.org>
>>
>> drm_client_framebuffer_flush() was explicitly added in 2020
>> by
>> commit c9c03e3cf072 ("drm/client: Add drm_client_framebuffer_flush()")
>> but has never been used.
>>
>> Remove it.
> 
> I had a patchset to use this helper for fbdev emulation. It just needs 
> preparation in a number of drivers.

It is used by drm_log, which is under review.
Please don't remove it.

https://patchwork.freedesktop.org/series/136789/

-- 

Jocelyn


> 
> Best regards
> Thomas
> 
>>
>> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
>> ---
>>   drivers/gpu/drm/drm_client.c | 33 ---------------------------------
>>   include/drm/drm_client.h     |  1 -
>>   2 files changed, 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
>> index bfedcbf516db..5d10ad3c2ca5 100644
>> --- a/drivers/gpu/drm/drm_client.c
>> +++ b/drivers/gpu/drm/drm_client.c
>> @@ -552,39 +552,6 @@ void drm_client_framebuffer_delete(struct 
>> drm_client_buffer *buffer)
>>   }
>>   EXPORT_SYMBOL(drm_client_framebuffer_delete);
>> -/**
>> - * drm_client_framebuffer_flush - Manually flush client framebuffer
>> - * @buffer: DRM client buffer (can be NULL)
>> - * @rect: Damage rectangle (if NULL flushes all)
>> - *
>> - * This calls &drm_framebuffer_funcs->dirty (if present) to flush 
>> buffer changes
>> - * for drivers that need it.
>> - *
>> - * Returns:
>> - * Zero on success or negative error code on failure.
>> - */
>> -int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, 
>> struct drm_rect *rect)
>> -{
>> -    if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
>> -        return 0;
>> -
>> -    if (rect) {
>> -        struct drm_clip_rect clip = {
>> -            .x1 = rect->x1,
>> -            .y1 = rect->y1,
>> -            .x2 = rect->x2,
>> -            .y2 = rect->y2,
>> -        };
>> -
>> -        return buffer->fb->funcs->dirty(buffer->fb, buffer->client- 
>> >file,
>> -                        0, 0, &clip, 1);
>> -    }
>> -
>> -    return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
>> -                    0, 0, NULL, 0);
>> -}
>> -EXPORT_SYMBOL(drm_client_framebuffer_flush);
>> -
>>   #ifdef CONFIG_DEBUG_FS
>>   static int drm_client_debugfs_internal_clients(struct seq_file *m, 
>> void *data)
>>   {
>> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
>> index bc0e66f9c425..560aae47e06d 100644
>> --- a/include/drm/drm_client.h
>> +++ b/include/drm/drm_client.h
>> @@ -165,7 +165,6 @@ struct drm_client_buffer {
>>   struct drm_client_buffer *
>>   drm_client_framebuffer_create(struct drm_client_dev *client, u32 
>> width, u32 height, u32 format);
>>   void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
>> -int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, 
>> struct drm_rect *rect);
>>   int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
>>                    struct iosys_map *map_copy);
>>   void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer);
> 

Re: [PATCH 4/5] drm/client: Remove unused drm_client_framebuffer_flush

[Dr. David Alan Gilbert]

* Jocelyn Falempe (jfalempe@redhat.com) wrote:
> On 23/10/2024 08:46, Thomas Zimmermann wrote:
> > Hi
> > 
> > Am 23.10.24 um 01:29 schrieb linux@treblig.org:
> > > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> > > 
> > > drm_client_framebuffer_flush() was explicitly added in 2020
> > > by
> > > commit c9c03e3cf072 ("drm/client: Add drm_client_framebuffer_flush()")
> > > but has never been used.
> > > 
> > > Remove it.
> > 
> > I had a patchset to use this helper for fbdev emulation. It just needs
> > preparation in a number of drivers.
> 
> It is used by drm_log, which is under review.
> Please don't remove it.

OK, no problem.

Reviews on the rest of the series would be great.

Dave

> https://patchwork.freedesktop.org/series/136789/
> 
> -- 
> 
> Jocelyn
> 
> 
> > 
> > Best regards
> > Thomas
> > 
> > > 
> > > Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> > > ---
> > >   drivers/gpu/drm/drm_client.c | 33 ---------------------------------
> > >   include/drm/drm_client.h     |  1 -
> > >   2 files changed, 34 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> > > index bfedcbf516db..5d10ad3c2ca5 100644
> > > --- a/drivers/gpu/drm/drm_client.c
> > > +++ b/drivers/gpu/drm/drm_client.c
> > > @@ -552,39 +552,6 @@ void drm_client_framebuffer_delete(struct
> > > drm_client_buffer *buffer)
> > >   }
> > >   EXPORT_SYMBOL(drm_client_framebuffer_delete);
> > > -/**
> > > - * drm_client_framebuffer_flush - Manually flush client framebuffer
> > > - * @buffer: DRM client buffer (can be NULL)
> > > - * @rect: Damage rectangle (if NULL flushes all)
> > > - *
> > > - * This calls &drm_framebuffer_funcs->dirty (if present) to flush
> > > buffer changes
> > > - * for drivers that need it.
> > > - *
> > > - * Returns:
> > > - * Zero on success or negative error code on failure.
> > > - */
> > > -int drm_client_framebuffer_flush(struct drm_client_buffer *buffer,
> > > struct drm_rect *rect)
> > > -{
> > > -    if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
> > > -        return 0;
> > > -
> > > -    if (rect) {
> > > -        struct drm_clip_rect clip = {
> > > -            .x1 = rect->x1,
> > > -            .y1 = rect->y1,
> > > -            .x2 = rect->x2,
> > > -            .y2 = rect->y2,
> > > -        };
> > > -
> > > -        return buffer->fb->funcs->dirty(buffer->fb, buffer->client-
> > > >file,
> > > -                        0, 0, &clip, 1);
> > > -    }
> > > -
> > > -    return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
> > > -                    0, 0, NULL, 0);
> > > -}
> > > -EXPORT_SYMBOL(drm_client_framebuffer_flush);
> > > -
> > >   #ifdef CONFIG_DEBUG_FS
> > >   static int drm_client_debugfs_internal_clients(struct seq_file *m,
> > > void *data)
> > >   {
> > > diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> > > index bc0e66f9c425..560aae47e06d 100644
> > > --- a/include/drm/drm_client.h
> > > +++ b/include/drm/drm_client.h
> > > @@ -165,7 +165,6 @@ struct drm_client_buffer {
> > >   struct drm_client_buffer *
> > >   drm_client_framebuffer_create(struct drm_client_dev *client, u32
> > > width, u32 height, u32 format);
> > >   void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
> > > -int drm_client_framebuffer_flush(struct drm_client_buffer *buffer,
> > > struct drm_rect *rect);
> > >   int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
> > >                    struct iosys_map *map_copy);
> > >   void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer);
> > 
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 1/5] drm: Remove unused drm_atomic_helper_commit_planes_on_crtc

[Dmitry Baryshkov]

On Wed, Oct 23, 2024 at 12:29:30AM +0100, linux@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <linux@treblig.org>
> 
> The last use of drm_atomic_helper_commit_planes_on_crtc() was removed
> in 2018 by
> commit 6c246b81f938 ("drm/i915: Replace call to commit_planes_on_crtc with
> internal update, v2.")
> 
> Remove it.
> 
> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 72 -----------------------------
>  include/drm/drm_atomic_helper.h     |  1 -
>  2 files changed, 73 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 2/5] drm/sysfs: Remove unused drm_class_device_(un)register

[Dmitry Baryshkov]

On Wed, Oct 23, 2024 at 12:29:31AM +0100, linux@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <linux@treblig.org>
> 
> drm_class_device_register() and drm_class_device_unregister() have been
> unused since
> commit ed89fff97382 ("drm/ttm: drop sysfs directory")
> 
> Remove them.
> 
> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> ---
>  drivers/gpu/drm/drm_sysfs.c | 32 --------------------------------
>  include/drm/drm_sysfs.h     |  4 ----
>  2 files changed, 36 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 5/5] drm/client: Remove unused drm_client_modeset_check

[Dmitry Baryshkov]

On Wed, Oct 23, 2024 at 12:29:34AM +0100, linux@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <linux@treblig.org>
> 
> drm_client_modeset_check() was explicitly added in 2020 by
> commit 64593f2a6fc9 ("drm/client: Add drm_client_modeset_check()")
> but has never been used.
> 
> Remove it.

If you are removing it, it makes more sense to revert the mentioned
commit completely, dropping the third argument of
drm_client_modeset_commit_atomic().

> 
> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> ---
>  drivers/gpu/drm/drm_client_modeset.c | 24 ------------------------
>  include/drm/drm_client.h             |  1 -
>  2 files changed, 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index cee5eafbfb81..69e1ce4d18cd 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -1126,30 +1126,6 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
>  	return ret;
>  }
>  
> -/**
> - * drm_client_modeset_check() - Check modeset configuration
> - * @client: DRM client
> - *
> - * Check modeset configuration.
> - *
> - * Returns:
> - * Zero on success or negative error code on failure.
> - */
> -int drm_client_modeset_check(struct drm_client_dev *client)
> -{
> -	int ret;
> -
> -	if (!drm_drv_uses_atomic_modeset(client->dev))
> -		return 0;
> -
> -	mutex_lock(&client->modeset_mutex);
> -	ret = drm_client_modeset_commit_atomic(client, true, true);
> -	mutex_unlock(&client->modeset_mutex);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(drm_client_modeset_check);
> -
>  /**
>   * drm_client_modeset_commit_locked() - Force commit CRTC configuration
>   * @client: DRM client
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 560aae47e06d..e1fd32adb3e9 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -176,7 +176,6 @@ int drm_client_modeset_create(struct drm_client_dev *client);
>  void drm_client_modeset_free(struct drm_client_dev *client);
>  int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width, unsigned int height);
>  bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
> -int drm_client_modeset_check(struct drm_client_dev *client);
>  int drm_client_modeset_commit_locked(struct drm_client_dev *client);
>  int drm_client_modeset_commit(struct drm_client_dev *client);
>  int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
> -- 
> 2.47.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH 5/5] drm/client: Remove unused drm_client_modeset_check

[Dr. David Alan Gilbert]

* Dmitry Baryshkov (dmitry.baryshkov@linaro.org) wrote:
> On Wed, Oct 23, 2024 at 12:29:34AM +0100, linux@treblig.org wrote:
> > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> > 
> > drm_client_modeset_check() was explicitly added in 2020 by
> > commit 64593f2a6fc9 ("drm/client: Add drm_client_modeset_check()")
> > but has never been used.
> > 
> > Remove it.
> 
> If you are removing it, it makes more sense to revert the mentioned
> commit completely, dropping the third argument of
> drm_client_modeset_commit_atomic().

Sure, I can look at that; and resend this series also without 4/5 which
people still wanted.
What about 3/5 - that's the only one that's not reviewed either way?

Dave

> > 
> > Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> > ---
> >  drivers/gpu/drm/drm_client_modeset.c | 24 ------------------------
> >  include/drm/drm_client.h             |  1 -
> >  2 files changed, 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> > index cee5eafbfb81..69e1ce4d18cd 100644
> > --- a/drivers/gpu/drm/drm_client_modeset.c
> > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > @@ -1126,30 +1126,6 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
> >  	return ret;
> >  }
> >  
> > -/**
> > - * drm_client_modeset_check() - Check modeset configuration
> > - * @client: DRM client
> > - *
> > - * Check modeset configuration.
> > - *
> > - * Returns:
> > - * Zero on success or negative error code on failure.
> > - */
> > -int drm_client_modeset_check(struct drm_client_dev *client)
> > -{
> > -	int ret;
> > -
> > -	if (!drm_drv_uses_atomic_modeset(client->dev))
> > -		return 0;
> > -
> > -	mutex_lock(&client->modeset_mutex);
> > -	ret = drm_client_modeset_commit_atomic(client, true, true);
> > -	mutex_unlock(&client->modeset_mutex);
> > -
> > -	return ret;
> > -}
> > -EXPORT_SYMBOL(drm_client_modeset_check);
> > -
> >  /**
> >   * drm_client_modeset_commit_locked() - Force commit CRTC configuration
> >   * @client: DRM client
> > diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> > index 560aae47e06d..e1fd32adb3e9 100644
> > --- a/include/drm/drm_client.h
> > +++ b/include/drm/drm_client.h
> > @@ -176,7 +176,6 @@ int drm_client_modeset_create(struct drm_client_dev *client);
> >  void drm_client_modeset_free(struct drm_client_dev *client);
> >  int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width, unsigned int height);
> >  bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
> > -int drm_client_modeset_check(struct drm_client_dev *client);
> >  int drm_client_modeset_commit_locked(struct drm_client_dev *client);
> >  int drm_client_modeset_commit(struct drm_client_dev *client);
> >  int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
> > -- 
> > 2.47.0
> > 
> 
> -- 
> With best wishes
> Dmitry
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 3/5] drm/vblank: Remove unused drm_crtc_vblank_count_and_time

[Dmitry Baryshkov]

On Wed, Oct 23, 2024 at 12:29:32AM +0100, linux@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <linux@treblig.org>
> 
> drm_crtc_vblank_count_and_time() was explicitly added by
> commit cf6483050e9b ("drm/irq: Add drm_crtc_vblank_count_and_time()")
> in 2015, but never used.
> 
> Remove it, and rework comments that reference it.

Not having the deep knowledge of the drm_vblank code, dropping the
function unused since 2015 should be fine.

Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> ---
>  drivers/gpu/drm/drm_vblank.c | 44 +++++++-----------------------------
>  include/drm/drm_vblank.h     | 10 ++++----
>  2 files changed, 12 insertions(+), 42 deletions(-)

-- 
With best wishes
Dmitry

Re: [PATCH 3/5] drm/vblank: Remove unused drm_crtc_vblank_count_and_time

[Ville Syrjälä]

On Wed, Oct 23, 2024 at 12:29:32AM +0100, linux@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <linux@treblig.org>
> 
> drm_crtc_vblank_count_and_time() was explicitly added by
> commit cf6483050e9b ("drm/irq: Add drm_crtc_vblank_count_and_time()")
> in 2015, but never used.

I see a bunch of places that could trivially use it.
That might be the more sensible thing to do so that
we keep moving towards using the crtc for everything.

The EXPORT_SYMBOL() looks completely pointless though.

> 
> Remove it, and rework comments that reference it.
> 
> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> ---
>  drivers/gpu/drm/drm_vblank.c | 44 +++++++-----------------------------
>  include/drm/drm_vblank.h     | 10 ++++----
>  2 files changed, 12 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 94e45ed6869d..67d6367e9f4b 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -908,10 +908,10 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
>   * drm_crtc_accurate_vblank_count() for such use-cases.
>   *
>   * Note that for a given vblank counter value drm_crtc_handle_vblank()
> - * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
> - * provide a barrier: Any writes done before calling
> - * drm_crtc_handle_vblank() will be visible to callers of the later
> - * functions, if the vblank count is the same or a later one.
> + * and drm_crtc_vblank_count() provide a barrier:
> + * Any writes done before calling drm_crtc_handle_vblank() will be
> + * visible to callers of the later functions, if the vblank count is
> + * the same or a later one.
>   *
>   * See also &drm_vblank_crtc.count.
>   *
> @@ -936,7 +936,6 @@ EXPORT_SYMBOL(drm_crtc_vblank_count);
>   * modesetting activity. Returns corresponding system timestamp of the time
>   * of the vblank interval that corresponds to the current vblank counter value.
>   *
> - * This is the legacy version of drm_crtc_vblank_count_and_time().
>   */
>  static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
>  				     ktime_t *vblanktime)
> @@ -959,33 +958,6 @@ static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
>  	return vblank_count;
>  }
>  
> -/**
> - * drm_crtc_vblank_count_and_time - retrieve "cooked" vblank counter value
> - *     and the system timestamp corresponding to that vblank counter value
> - * @crtc: which counter to retrieve
> - * @vblanktime: Pointer to time to receive the vblank timestamp.
> - *
> - * Fetches the "cooked" vblank count value that represents the number of
> - * vblank events since the system was booted, including lost events due to
> - * modesetting activity. Returns corresponding system timestamp of the time
> - * of the vblank interval that corresponds to the current vblank counter value.
> - *
> - * Note that for a given vblank counter value drm_crtc_handle_vblank()
> - * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
> - * provide a barrier: Any writes done before calling
> - * drm_crtc_handle_vblank() will be visible to callers of the later
> - * functions, if the vblank count is the same or a later one.
> - *
> - * See also &drm_vblank_crtc.count.
> - */
> -u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> -				   ktime_t *vblanktime)
> -{
> -	return drm_vblank_count_and_time(crtc->dev, drm_crtc_index(crtc),
> -					 vblanktime);
> -}
> -EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
> -
>  /**
>   * drm_crtc_next_vblank_start - calculate the time of the next vblank
>   * @crtc: the crtc for which to calculate next vblank time
> @@ -1978,10 +1950,10 @@ EXPORT_SYMBOL(drm_handle_vblank);
>   * This is the native KMS version of drm_handle_vblank().
>   *
>   * Note that for a given vblank counter value drm_crtc_handle_vblank()
> - * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
> - * provide a barrier: Any writes done before calling
> - * drm_crtc_handle_vblank() will be visible to callers of the later
> - * functions, if the vblank count is the same or a later one.
> + * and drm_crtc_vblank_count() * provide a barrier:
> + * Any writes done before calling * drm_crtc_handle_vblank() will be
> + * visible to callers of the later functions, if the vblank count is
> + * the same or a later one.
>   *
>   * See also &drm_vblank_crtc.count.
>   *
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index 151ab1e85b1b..572e54425970 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -141,10 +141,10 @@ struct drm_vblank_crtc {
>  	 * Current software vblank counter.
>  	 *
>  	 * Note that for a given vblank counter value drm_crtc_handle_vblank()
> -	 * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
> -	 * provide a barrier: Any writes done before calling
> -	 * drm_crtc_handle_vblank() will be visible to callers of the later
> -	 * functions, iff the vblank count is the same or a later one.
> +	 * and drm_crtc_vblank_count() provide a barrier:
> +	 * Any writes done before calling drm_crtc_handle_vblank() will be
> +	 * visible to callers of the later functions, iff the vblank count is
> +	 * the same or a later one.
>  	 *
>  	 * IMPORTANT: This guarantee requires barriers, therefor never access
>  	 * this field directly. Use drm_crtc_vblank_count() instead.
> @@ -260,8 +260,6 @@ struct drm_vblank_crtc *drm_crtc_vblank_crtc(struct drm_crtc *crtc);
>  int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs);
>  bool drm_dev_has_vblank(const struct drm_device *dev);
>  u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> -u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> -				   ktime_t *vblanktime);
>  int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime);
>  void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
>  			       struct drm_pending_vblank_event *e);
> -- 
> 2.47.0

-- 
Ville Syrjälä
Intel

Re: [PATCH 3/5] drm/vblank: Remove unused drm_crtc_vblank_count_and_time

[Dr. David Alan Gilbert]

* Ville Syrjälä (ville.syrjala@linux.intel.com) wrote:
> On Wed, Oct 23, 2024 at 12:29:32AM +0100, linux@treblig.org wrote:
> > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> > 
> > drm_crtc_vblank_count_and_time() was explicitly added by
> > commit cf6483050e9b ("drm/irq: Add drm_crtc_vblank_count_and_time()")
> > in 2015, but never used.
> 
> I see a bunch of places that could trivially use it.
> That might be the more sensible thing to do so that
> we keep moving towards using the crtc for everything.

Do you intend to send those since you understand it?

> The EXPORT_SYMBOL() looks completely pointless though.

Hmm, we probably shouldn't change that until something uses it?

Dave

> > 
> > Remove it, and rework comments that reference it.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> > ---
> >  drivers/gpu/drm/drm_vblank.c | 44 +++++++-----------------------------
> >  include/drm/drm_vblank.h     | 10 ++++----
> >  2 files changed, 12 insertions(+), 42 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index 94e45ed6869d..67d6367e9f4b 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -908,10 +908,10 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
> >   * drm_crtc_accurate_vblank_count() for such use-cases.
> >   *
> >   * Note that for a given vblank counter value drm_crtc_handle_vblank()
> > - * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
> > - * provide a barrier: Any writes done before calling
> > - * drm_crtc_handle_vblank() will be visible to callers of the later
> > - * functions, if the vblank count is the same or a later one.
> > + * and drm_crtc_vblank_count() provide a barrier:
> > + * Any writes done before calling drm_crtc_handle_vblank() will be
> > + * visible to callers of the later functions, if the vblank count is
> > + * the same or a later one.
> >   *
> >   * See also &drm_vblank_crtc.count.
> >   *
> > @@ -936,7 +936,6 @@ EXPORT_SYMBOL(drm_crtc_vblank_count);
> >   * modesetting activity. Returns corresponding system timestamp of the time
> >   * of the vblank interval that corresponds to the current vblank counter value.
> >   *
> > - * This is the legacy version of drm_crtc_vblank_count_and_time().
> >   */
> >  static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
> >  				     ktime_t *vblanktime)
> > @@ -959,33 +958,6 @@ static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
> >  	return vblank_count;
> >  }
> >  
> > -/**
> > - * drm_crtc_vblank_count_and_time - retrieve "cooked" vblank counter value
> > - *     and the system timestamp corresponding to that vblank counter value
> > - * @crtc: which counter to retrieve
> > - * @vblanktime: Pointer to time to receive the vblank timestamp.
> > - *
> > - * Fetches the "cooked" vblank count value that represents the number of
> > - * vblank events since the system was booted, including lost events due to
> > - * modesetting activity. Returns corresponding system timestamp of the time
> > - * of the vblank interval that corresponds to the current vblank counter value.
> > - *
> > - * Note that for a given vblank counter value drm_crtc_handle_vblank()
> > - * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
> > - * provide a barrier: Any writes done before calling
> > - * drm_crtc_handle_vblank() will be visible to callers of the later
> > - * functions, if the vblank count is the same or a later one.
> > - *
> > - * See also &drm_vblank_crtc.count.
> > - */
> > -u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > -				   ktime_t *vblanktime)
> > -{
> > -	return drm_vblank_count_and_time(crtc->dev, drm_crtc_index(crtc),
> > -					 vblanktime);
> > -}
> > -EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
> > -
> >  /**
> >   * drm_crtc_next_vblank_start - calculate the time of the next vblank
> >   * @crtc: the crtc for which to calculate next vblank time
> > @@ -1978,10 +1950,10 @@ EXPORT_SYMBOL(drm_handle_vblank);
> >   * This is the native KMS version of drm_handle_vblank().
> >   *
> >   * Note that for a given vblank counter value drm_crtc_handle_vblank()
> > - * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
> > - * provide a barrier: Any writes done before calling
> > - * drm_crtc_handle_vblank() will be visible to callers of the later
> > - * functions, if the vblank count is the same or a later one.
> > + * and drm_crtc_vblank_count() * provide a barrier:
> > + * Any writes done before calling * drm_crtc_handle_vblank() will be
> > + * visible to callers of the later functions, if the vblank count is
> > + * the same or a later one.
> >   *
> >   * See also &drm_vblank_crtc.count.
> >   *
> > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> > index 151ab1e85b1b..572e54425970 100644
> > --- a/include/drm/drm_vblank.h
> > +++ b/include/drm/drm_vblank.h
> > @@ -141,10 +141,10 @@ struct drm_vblank_crtc {
> >  	 * Current software vblank counter.
> >  	 *
> >  	 * Note that for a given vblank counter value drm_crtc_handle_vblank()
> > -	 * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time()
> > -	 * provide a barrier: Any writes done before calling
> > -	 * drm_crtc_handle_vblank() will be visible to callers of the later
> > -	 * functions, iff the vblank count is the same or a later one.
> > +	 * and drm_crtc_vblank_count() provide a barrier:
> > +	 * Any writes done before calling drm_crtc_handle_vblank() will be
> > +	 * visible to callers of the later functions, iff the vblank count is
> > +	 * the same or a later one.
> >  	 *
> >  	 * IMPORTANT: This guarantee requires barriers, therefor never access
> >  	 * this field directly. Use drm_crtc_vblank_count() instead.
> > @@ -260,8 +260,6 @@ struct drm_vblank_crtc *drm_crtc_vblank_crtc(struct drm_crtc *crtc);
> >  int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs);
> >  bool drm_dev_has_vblank(const struct drm_device *dev);
> >  u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> > -u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > -				   ktime_t *vblanktime);
> >  int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime);
> >  void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
> >  			       struct drm_pending_vblank_event *e);
> > -- 
> > 2.47.0
> 
> -- 
> Ville Syrjälä
> Intel
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 5/5] drm/client: Remove unused drm_client_modeset_check

[Dr. David Alan Gilbert]

* Dr. David Alan Gilbert (linux@treblig.org) wrote:
> * Dmitry Baryshkov (dmitry.baryshkov@linaro.org) wrote:
> > On Wed, Oct 23, 2024 at 12:29:34AM +0100, linux@treblig.org wrote:
> > > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> > > 
> > > drm_client_modeset_check() was explicitly added in 2020 by
> > > commit 64593f2a6fc9 ("drm/client: Add drm_client_modeset_check()")
> > > but has never been used.
> > > 
> > > Remove it.
> > 
> > If you are removing it, it makes more sense to revert the mentioned
> > commit completely, dropping the third argument of
> > drm_client_modeset_commit_atomic().
> 
> Sure, I can look at that; and resend this series also without 4/5 which
> people still wanted.
> What about 3/5 - that's the only one that's not reviewed either way?

I've just sent the v2 with this as a revert and the contentious pair dropped.

Dave

> Dave
> 
> > > 
> > > Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> > > ---
> > >  drivers/gpu/drm/drm_client_modeset.c | 24 ------------------------
> > >  include/drm/drm_client.h             |  1 -
> > >  2 files changed, 25 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> > > index cee5eafbfb81..69e1ce4d18cd 100644
> > > --- a/drivers/gpu/drm/drm_client_modeset.c
> > > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > > @@ -1126,30 +1126,6 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
> > >  	return ret;
> > >  }
> > >  
> > > -/**
> > > - * drm_client_modeset_check() - Check modeset configuration
> > > - * @client: DRM client
> > > - *
> > > - * Check modeset configuration.
> > > - *
> > > - * Returns:
> > > - * Zero on success or negative error code on failure.
> > > - */
> > > -int drm_client_modeset_check(struct drm_client_dev *client)
> > > -{
> > > -	int ret;
> > > -
> > > -	if (!drm_drv_uses_atomic_modeset(client->dev))
> > > -		return 0;
> > > -
> > > -	mutex_lock(&client->modeset_mutex);
> > > -	ret = drm_client_modeset_commit_atomic(client, true, true);
> > > -	mutex_unlock(&client->modeset_mutex);
> > > -
> > > -	return ret;
> > > -}
> > > -EXPORT_SYMBOL(drm_client_modeset_check);
> > > -
> > >  /**
> > >   * drm_client_modeset_commit_locked() - Force commit CRTC configuration
> > >   * @client: DRM client
> > > diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> > > index 560aae47e06d..e1fd32adb3e9 100644
> > > --- a/include/drm/drm_client.h
> > > +++ b/include/drm/drm_client.h
> > > @@ -176,7 +176,6 @@ int drm_client_modeset_create(struct drm_client_dev *client);
> > >  void drm_client_modeset_free(struct drm_client_dev *client);
> > >  int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width, unsigned int height);
> > >  bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
> > > -int drm_client_modeset_check(struct drm_client_dev *client);
> > >  int drm_client_modeset_commit_locked(struct drm_client_dev *client);
> > >  int drm_client_modeset_commit(struct drm_client_dev *client);
> > >  int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
> > > -- 
> > > 2.47.0
> > > 
> > 
> > -- 
> > With best wishes
> > Dmitry
> > 
> -- 
>  -----Open up your eyes, open up your mind, open up your code -------   
> / Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
> \        dave @ treblig.org |                               | In Hex /
>  \ _________________________|_____ http://www.treblig.org   |_______/
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/