Re: [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 tzimmermann@suse.de

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, &buffer->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 = &buffer->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, &map);

• 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 = &buffer->map;

• drm_gem_vunmap(buffer->gem, &map);
• 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 = &helper->dirty_clip; struct drm_clip_rect clip_copy; unsigned long flags;

• void *vaddr;

• struct dma_buf_map map;

• int ret;

  spin_lock_irqsave(&helper->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, &map);
  

• 	if (ret)
  	return;
  drm_fb_helper_dirty_blit_real(helper, &clip_copy);
  

  }

@@ -2076,7 +2077,8 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper, struct drm_framebuffer *fb; struct fb_info *fbi; u32 format;

• void *vaddr;

• struct dma_buf_map map;

• int ret;

  drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n", sizes->surface_width, sizes->surface_height,

@@ -2112,11 +2114,14 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper, fb_deferred_io_init(fbi); } else { /* buffer is mapped for HW framebuffer */

• vaddr = drm_client_buffer_vmap(fb_helper->buffer);
  

• if (IS_ERR(vaddr))
  

• 	return PTR_ERR(vaddr);
  

• ret = drm_client_buffer_vmap(fb_helper->buffer, &map);
  

• if (ret)
  

• 	return ret;
  

• if (map.is_iomem)
  

• 	fbi->screen_base = map.vaddr_iomem;
  

• else
  

• 	fbi->screen_buffer = map.vaddr;
  
  

• fbi->screen_buffer = vaddr;
  

  /* Shamelessly leak the physical address to user-space */

#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) if (drm_leak_fbdev_smem && fbi->fix.smem_start == 0) diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h index 7aaea665bfc2..f07f2fb02e75 100644 --- a/include/drm/drm_client.h +++ b/include/drm/drm_client.h @@ -3,6 +3,7 @@ #ifndef _DRM_CLIENT_H_ #define _DRM_CLIENT_H_

+#include <linux/dma-buf-map.h> #include <linux/lockdep.h> #include <linux/mutex.h> #include <linux/types.h> @@ -141,9 +142,9 @@ struct drm_client_buffer { struct drm_gem_object *gem;

/**

• * @vaddr: Virtual address for the buffer
  

• * @map: Virtual address for the buffer
  

  */

• void *vaddr;

• struct dma_buf_map map;

  /**

  • @fb: DRM framebuffer

@@ -155,7 +156,7 @@ 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); -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); void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);

int drm_client_modeset_create(struct drm_client_dev *client);

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

-- 2.28.0