Re: [Nouveau] [PATCH v3 5/7] drm/gem: Store client buffer mappings as struct dma_buf_map
On Tue, Sep 29, 2020 at 05:14:35PM +0200, Thomas Zimmermann wrote:
> Kernel DRM clients now store their framebuffer address in an instance
> of struct dma_buf_map. Depending on the buffer's location, the address
> refers to system or I/O memory.
>
> Callers of drm_client_buffer_vmap() receive a copy of the value in
> the call's supplied arguments. It can be accessed and modified with
> dma_buf_map interfaces.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_client.c| 34 +++--
> drivers/gpu/drm/drm_fb_helper.c | 23 +-
> include/drm/drm_client.h| 7 ---
> 3 files changed, 38 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index ac0082bed966..fe573acf1067 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
> drm_client_buffer *buffer)
> {
> struct drm_device *dev = buffer->client->dev;
>
> - drm_gem_vunmap(buffer->gem, buffer->vaddr);
> + drm_gem_vunmap(buffer->gem, >map);
>
> if (buffer->gem)
> drm_gem_object_put(buffer->gem);
> @@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev *client,
> u32 width, u32 height, u
> /**
> * drm_client_buffer_vmap - Map DRM client buffer into address space
> * @buffer: DRM client buffer
> + * @map_copy: Returns the mapped memory's address
> *
> * This function maps a client buffer into kernel address space. If the
> - * buffer is already mapped, it returns the mapping's address.
> + * buffer is already mapped, it returns the existing mapping's address.
> *
> * Client buffer mappings are not ref'counted. Each call to
> * drm_client_buffer_vmap() should be followed by a call to
> * drm_client_buffer_vunmap(); or the client buffer should be mapped
> * throughout its lifetime.
> *
> + * The returned address is a copy of the internal value. In contrast to
> + * other vmap interfaces, you don't need it for the client's vunmap
> + * function. So you can modify it at will during blit and draw operations.
> + *
> * Returns:
> - * The mapped memory's address
> + * 0 on success, or a negative errno code otherwise.
> */
> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> +int
> +drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map
> *map_copy)
> {
> - struct dma_buf_map map;
> + struct dma_buf_map *map = >map;
> int ret;
>
> - if (buffer->vaddr)
> - return buffer->vaddr;
> + if (dma_buf_map_is_set(map))
> + goto out;
>
> /*
>* FIXME: The dependency on GEM here isn't required, we could
> @@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
> *buffer)
>* fd_install step out of the driver backend hooks, to make that
>* final step optional for internal users.
>*/
> - ret = drm_gem_vmap(buffer->gem, );
> + ret = drm_gem_vmap(buffer->gem, map);
> if (ret)
> - return ERR_PTR(ret);
> + return ret;
>
> - buffer->vaddr = map.vaddr;
> +out:
> + *map_copy = *map;
>
> - return map.vaddr;
> + return 0;
> }
> EXPORT_SYMBOL(drm_client_buffer_vmap);
>
> @@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
> */
> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> {
> - struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
> + struct dma_buf_map *map = >map;
>
> - drm_gem_vunmap(buffer->gem, );
> - buffer->vaddr = NULL;
> + drm_gem_vunmap(buffer->gem, map);
> }
> EXPORT_SYMBOL(drm_client_buffer_vunmap);
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 8697554ccd41..343a292f2c7c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -394,7 +394,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> drm_fb_helper *fb_helper,
> unsigned int cpp = fb->format->cpp[0];
> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> void *src = fb_helper->fbdev->screen_buffer + offset;
> - void *dst = fb_helper->buffer->vaddr + offset;
> + void *dst = fb_helper->buffer->map.vaddr + offset;
> size_t len = (clip->x2 - clip->x1) * cpp;
> unsigned int y;
>
> @@ -416,7 +416,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
> struct drm_clip_rect *clip = >dirty_clip;
> struct drm_clip_rect clip_copy;
> unsigned long flags;
> - void *vaddr;
> + struct dma_buf_map map;
> + int ret;
>
> spin_lock_irqsave(>dirty_lock, flags);
> clip_copy = *clip;
> @@ -429,8 +430,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
>
> /* Generic fbdev uses a shadow buffer */
> if (helper->buffer) {
> -
[Nouveau] [PATCH v3 5/7] drm/gem: Store client buffer mappings as struct dma_buf_map
Kernel DRM clients now store their framebuffer address in an instance
of struct dma_buf_map. Depending on the buffer's location, the address
refers to system or I/O memory.
Callers of drm_client_buffer_vmap() receive a copy of the value in
the call's supplied arguments. It can be accessed and modified with
dma_buf_map interfaces.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_client.c| 34 +++--
drivers/gpu/drm/drm_fb_helper.c | 23 +-
include/drm/drm_client.h| 7 ---
3 files changed, 38 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index ac0082bed966..fe573acf1067 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
drm_client_buffer *buffer)
{
struct drm_device *dev = buffer->client->dev;
- drm_gem_vunmap(buffer->gem, buffer->vaddr);
+ drm_gem_vunmap(buffer->gem, >map);
if (buffer->gem)
drm_gem_object_put(buffer->gem);
@@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev *client,
u32 width, u32 height, u
/**
* drm_client_buffer_vmap - Map DRM client buffer into address space
* @buffer: DRM client buffer
+ * @map_copy: Returns the mapped memory's address
*
* This function maps a client buffer into kernel address space. If the
- * buffer is already mapped, it returns the mapping's address.
+ * buffer is already mapped, it returns the existing mapping's address.
*
* Client buffer mappings are not ref'counted. Each call to
* drm_client_buffer_vmap() should be followed by a call to
* drm_client_buffer_vunmap(); or the client buffer should be mapped
* throughout its lifetime.
*
+ * The returned address is a copy of the internal value. In contrast to
+ * other vmap interfaces, you don't need it for the client's vunmap
+ * function. So you can modify it at will during blit and draw operations.
+ *
* Returns:
- * The mapped memory's address
+ * 0 on success, or a negative errno code otherwise.
*/
-void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
+int
+drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map
*map_copy)
{
- struct dma_buf_map map;
+ struct dma_buf_map *map = >map;
int ret;
- if (buffer->vaddr)
- return buffer->vaddr;
+ if (dma_buf_map_is_set(map))
+ goto out;
/*
* FIXME: The dependency on GEM here isn't required, we could
@@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
*buffer)
* fd_install step out of the driver backend hooks, to make that
* final step optional for internal users.
*/
- ret = drm_gem_vmap(buffer->gem, );
+ ret = drm_gem_vmap(buffer->gem, map);
if (ret)
- return ERR_PTR(ret);
+ return ret;
- buffer->vaddr = map.vaddr;
+out:
+ *map_copy = *map;
- return map.vaddr;
+ return 0;
}
EXPORT_SYMBOL(drm_client_buffer_vmap);
@@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
*/
void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
{
- struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
+ struct dma_buf_map *map = >map;
- drm_gem_vunmap(buffer->gem, );
- buffer->vaddr = NULL;
+ drm_gem_vunmap(buffer->gem, map);
}
EXPORT_SYMBOL(drm_client_buffer_vunmap);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 8697554ccd41..343a292f2c7c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -394,7 +394,7 @@ static void drm_fb_helper_dirty_blit_real(struct
drm_fb_helper *fb_helper,
unsigned int cpp = fb->format->cpp[0];
size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
void *src = fb_helper->fbdev->screen_buffer + offset;
- void *dst = fb_helper->buffer->vaddr + offset;
+ void *dst = fb_helper->buffer->map.vaddr + offset;
size_t len = (clip->x2 - clip->x1) * cpp;
unsigned int y;
@@ -416,7 +416,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
*work)
struct drm_clip_rect *clip = >dirty_clip;
struct drm_clip_rect clip_copy;
unsigned long flags;
- void *vaddr;
+ struct dma_buf_map map;
+ int ret;
spin_lock_irqsave(>dirty_lock, flags);
clip_copy = *clip;
@@ -429,8 +430,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
*work)
/* Generic fbdev uses a shadow buffer */
if (helper->buffer) {
- vaddr = drm_client_buffer_vmap(helper->buffer);
- if (IS_ERR(vaddr))
+ ret = drm_client_buffer_vmap(helper->buffer, );
+ if (ret)
