From: Boris Brezillon <boris.brezillon@collabora.com>
To: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: kernel@collabora.com, "Thomas Zimmermann" <tzimmermann@suse.de>,
"Emma Anholt" <emma@anholt.net>,
"Christian König" <christian.koenig@amd.com>,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
"Maxime Ripard" <mripard@kernel.org>,
"Gurchetan Singh" <gurchetansingh@chromium.org>,
"Melissa Wen" <mwen@igalia.com>,
"Gerd Hoffmann" <kraxel@redhat.com>,
"Steven Price" <steven.price@arm.com>,
virtualization@lists.linux-foundation.org,
"Qiang Yu" <yuq825@gmail.com>
Subject: Re: [PATCH v14 12/12] drm/gem: Add _unlocked postfix to drm_gem_pin/unpin()
Date: Tue, 25 Jul 2023 09:53:03 +0200 [thread overview]
Message-ID: <20230725095303.07611834@collabora.com> (raw)
In-Reply-To: <20230722234746.205949-13-dmitry.osipenko@collabora.com>
On Sun, 23 Jul 2023 02:47:46 +0300
Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote:
> Make clear that drm_gem_pin/unpin() functions take reservation lock by
> adding _unlocked postfix to the function names.
>
> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
I'm still a bit confused by the fact we sometimes use the
xxx[_locked]() pattern (version without the _locked suffix takes the
lock) and other times the xxx[_unlocked]() pattern (version with the
_unlocked suffix takes the lock). It'd be good to chose one pattern and
stick to it, at least for all core functions...
> ---
> drivers/gpu/drm/drm_gem.c | 4 ++--
> drivers/gpu/drm/drm_internal.h | 4 ++--
> drivers/gpu/drm/drm_prime.c | 4 ++--
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index c18686f434d4..805eb0d85297 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1146,7 +1146,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
> obj->funcs->print_info(p, indent, obj);
> }
>
> -int drm_gem_pin(struct drm_gem_object *obj)
> +int drm_gem_pin_unlocked(struct drm_gem_object *obj)
> {
> if (obj->funcs->pin)
> return obj->funcs->pin(obj);
> @@ -1154,7 +1154,7 @@ int drm_gem_pin(struct drm_gem_object *obj)
> return 0;
> }
>
> -void drm_gem_unpin(struct drm_gem_object *obj)
> +void drm_gem_unpin_unlocked(struct drm_gem_object *obj)
> {
> if (obj->funcs->unpin)
> obj->funcs->unpin(obj);
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index d7e023bbb0d5..80f5bd1da8fd 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -173,8 +173,8 @@ void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
> void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
> const struct drm_gem_object *obj);
>
> -int drm_gem_pin(struct drm_gem_object *obj);
> -void drm_gem_unpin(struct drm_gem_object *obj);
> +int drm_gem_pin_unlocked(struct drm_gem_object *obj);
> +void drm_gem_unpin_unlocked(struct drm_gem_object *obj);
> int drm_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map);
> void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map);
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 63b709a67471..8145b49e95ff 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -583,7 +583,7 @@ int drm_gem_map_attach(struct dma_buf *dma_buf,
> if (!obj->funcs->get_sg_table)
> return -ENOSYS;
>
> - return drm_gem_pin(obj);
> + return drm_gem_pin_unlocked(obj);
> }
> EXPORT_SYMBOL(drm_gem_map_attach);
>
> @@ -601,7 +601,7 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
> {
> struct drm_gem_object *obj = dma_buf->priv;
>
> - drm_gem_unpin(obj);
> + drm_gem_unpin_unlocked(obj);
> }
> EXPORT_SYMBOL(drm_gem_map_detach);
>
WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: "David Airlie" <airlied@gmail.com>,
"Gerd Hoffmann" <kraxel@redhat.com>,
"Gurchetan Singh" <gurchetansingh@chromium.org>,
"Chia-I Wu" <olvaffe@gmail.com>,
"Daniel Vetter" <daniel@ffwll.ch>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"Christian König" <christian.koenig@amd.com>,
"Qiang Yu" <yuq825@gmail.com>,
"Steven Price" <steven.price@arm.com>,
"Emma Anholt" <emma@anholt.net>, "Melissa Wen" <mwen@igalia.com>,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
kernel@collabora.com, virtualization@lists.linux-foundation.org
Subject: Re: [PATCH v14 12/12] drm/gem: Add _unlocked postfix to drm_gem_pin/unpin()
Date: Tue, 25 Jul 2023 09:53:03 +0200 [thread overview]
Message-ID: <20230725095303.07611834@collabora.com> (raw)
In-Reply-To: <20230722234746.205949-13-dmitry.osipenko@collabora.com>
On Sun, 23 Jul 2023 02:47:46 +0300
Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote:
> Make clear that drm_gem_pin/unpin() functions take reservation lock by
> adding _unlocked postfix to the function names.
>
> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
I'm still a bit confused by the fact we sometimes use the
xxx[_locked]() pattern (version without the _locked suffix takes the
lock) and other times the xxx[_unlocked]() pattern (version with the
_unlocked suffix takes the lock). It'd be good to chose one pattern and
stick to it, at least for all core functions...
> ---
> drivers/gpu/drm/drm_gem.c | 4 ++--
> drivers/gpu/drm/drm_internal.h | 4 ++--
> drivers/gpu/drm/drm_prime.c | 4 ++--
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index c18686f434d4..805eb0d85297 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1146,7 +1146,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
> obj->funcs->print_info(p, indent, obj);
> }
>
> -int drm_gem_pin(struct drm_gem_object *obj)
> +int drm_gem_pin_unlocked(struct drm_gem_object *obj)
> {
> if (obj->funcs->pin)
> return obj->funcs->pin(obj);
> @@ -1154,7 +1154,7 @@ int drm_gem_pin(struct drm_gem_object *obj)
> return 0;
> }
>
> -void drm_gem_unpin(struct drm_gem_object *obj)
> +void drm_gem_unpin_unlocked(struct drm_gem_object *obj)
> {
> if (obj->funcs->unpin)
> obj->funcs->unpin(obj);
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index d7e023bbb0d5..80f5bd1da8fd 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -173,8 +173,8 @@ void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
> void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
> const struct drm_gem_object *obj);
>
> -int drm_gem_pin(struct drm_gem_object *obj);
> -void drm_gem_unpin(struct drm_gem_object *obj);
> +int drm_gem_pin_unlocked(struct drm_gem_object *obj);
> +void drm_gem_unpin_unlocked(struct drm_gem_object *obj);
> int drm_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map);
> void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map);
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 63b709a67471..8145b49e95ff 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -583,7 +583,7 @@ int drm_gem_map_attach(struct dma_buf *dma_buf,
> if (!obj->funcs->get_sg_table)
> return -ENOSYS;
>
> - return drm_gem_pin(obj);
> + return drm_gem_pin_unlocked(obj);
> }
> EXPORT_SYMBOL(drm_gem_map_attach);
>
> @@ -601,7 +601,7 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
> {
> struct drm_gem_object *obj = dma_buf->priv;
>
> - drm_gem_unpin(obj);
> + drm_gem_unpin_unlocked(obj);
> }
> EXPORT_SYMBOL(drm_gem_map_detach);
>
next prev parent reply other threads:[~2023-07-25 7:53 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-22 23:47 [PATCH v14 00/12] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-22 23:47 ` [PATCH v14 01/12] drm/shmem-helper: Factor out pages alloc/release from drm_gem_shmem_get/put_pages() Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-25 7:14 ` Boris Brezillon
2023-07-25 7:14 ` Boris Brezillon
2023-07-22 23:47 ` [PATCH v14 02/12] drm/shmem-helper: Add pages_pin_count field Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-25 7:27 ` Boris Brezillon
2023-07-25 7:27 ` Boris Brezillon
2023-07-25 8:32 ` Boris Brezillon
2023-07-25 8:32 ` Boris Brezillon
2023-07-31 12:27 ` Dmitry Osipenko
2023-07-31 12:27 ` Dmitry Osipenko
2023-07-31 12:31 ` Dmitry Osipenko
2023-07-31 12:31 ` Dmitry Osipenko
2023-07-31 13:35 ` Boris Brezillon
2023-07-31 13:35 ` Boris Brezillon
2023-08-02 2:31 ` Danilo Krummrich
2023-08-02 2:31 ` Danilo Krummrich
2023-08-02 9:06 ` Boris Brezillon
2023-08-02 9:06 ` Boris Brezillon
2023-07-22 23:47 ` [PATCH v14 03/12] drm/shmem-helper: Switch drm_gem_shmem_vmap/vunmap to use pin/unpin Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-22 23:47 ` [PATCH v14 04/12] drm/shmem-helper: Factor out unpinning part from drm_gem_shmem_purge() Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-22 23:47 ` [PATCH v14 05/12] drm/shmem-helper: Add memory shrinker Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-22 23:47 ` [PATCH v14 06/12] drm/shmem-helper: Remove obsoleted is_iomem test Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-22 23:47 ` [PATCH v14 07/12] drm/shmem-helper: Export drm_gem_shmem_get_pages_sgt_locked() Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-22 23:47 ` [PATCH v14 08/12] drm/virtio: Support memory shrinking Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-22 23:47 ` [PATCH v14 09/12] drm/panfrost: Switch to generic memory shrinker Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-22 23:47 ` [PATCH v14 10/12] drm/shmem-helper: Refactor locked/unlocked functions Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-25 7:47 ` Boris Brezillon
2023-07-25 7:47 ` Boris Brezillon
2023-07-25 7:58 ` Boris Brezillon
2023-07-25 7:58 ` Boris Brezillon
2023-07-22 23:47 ` [PATCH v14 11/12] drm/shmem-helper: Make drm_gem_shmem_print_info() symbol GPL Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-22 23:47 ` [PATCH v14 12/12] drm/gem: Add _unlocked postfix to drm_gem_pin/unpin() Dmitry Osipenko
2023-07-22 23:47 ` Dmitry Osipenko
2023-07-25 7:53 ` Boris Brezillon [this message]
2023-07-25 7:53 ` Boris Brezillon
2023-07-31 13:04 ` Dmitry Osipenko
2023-07-31 13:04 ` Dmitry Osipenko
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=20230725095303.07611834@collabora.com \
--to=boris.brezillon@collabora.com \
--cc=christian.koenig@amd.com \
--cc=dmitry.osipenko@collabora.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=emma@anholt.net \
--cc=gurchetansingh@chromium.org \
--cc=kernel@collabora.com \
--cc=kraxel@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mripard@kernel.org \
--cc=mwen@igalia.com \
--cc=steven.price@arm.com \
--cc=tzimmermann@suse.de \
--cc=virtualization@lists.linux-foundation.org \
--cc=yuq825@gmail.com \
/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.