Re: [PATCH v2 2/5] drm/vkms: map/unmap buffers in [prepare/cleanup]_fb hooks

On Sat, Jul 14, 2018 at 03:20:21PM +0300, Haneen Mohammed wrote:

This patch map/unmap GEM backing memory to kernel address space in prepare/cleanup_fb respectively and cache the virtual address for later use.

Signed-off-by: Haneen Mohammed hamohammed.sa@gmail.com

Changes in v2:

• use vkms_gem_vunmap

drivers/gpu/drm/vkms/vkms_plane.c | 36 ++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 9f75b1e2c1c4..877483984897 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -7,8 +7,9 @@ */

#include "vkms_drv.h" -#include <drm/drm_plane_helper.h>

nit: This seems unrelated to the patch

#include <drm/drm_atomic_helper.h> +#include <drm/drm_gem_framebuffer_helper.h> +#include <drm/drm_plane_helper.h>

static const struct drm_plane_funcs vkms_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, @@ -24,8 +25,41 @@ static void vkms_primary_plane_update(struct drm_plane *plane, { }

+static int vkms_prepare_fb(struct drm_plane *plane,

• 	   struct drm_plane_state *state)
  

+{

• struct drm_gem_object *gem_obj;
• struct vkms_gem_object *vkms_obj;
• if (!state->fb)

• return 0;
  

• gem_obj = drm_gem_fb_get_obj(state->fb, 0);
• vkms_obj = drm_gem_to_vkms_gem(gem_obj);
• vkms_obj->vaddr = vkms_gem_vmap(gem_obj);
• if (!vkms_obj->vaddr)

- vkms_gem_vmap() returns ERR_PTR(), not NULL. So IS_ERR_OR_NULL would be more appropriate - You shouldn't be assigning the return of vkms_gem_vmap() to vkms_obj->vaddr since vkms_gem_vmap() already does this internally (with proper locks and checks)

So you should probably change vkms_gem_vmap() to return int and accept a vkms_obj upon which it'll populate vaddr for you.

Sean

• DRM_ERROR("vmap failed\n");
  

• return drm_gem_fb_prepare_fb(plane, state);

+}

+static void vkms_cleanup_fb(struct drm_plane *plane,

• 	    struct drm_plane_state *old_state)
  

+{

• struct drm_gem_object *gem_obj;
• if (!old_state->fb)

• return;
  

• gem_obj = drm_gem_fb_get_obj(old_state->fb, 0);
• vkms_gem_vunmap(gem_obj);

+}

static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = { .atomic_update = vkms_primary_plane_update,

• .prepare_fb = vkms_prepare_fb,
• .cleanup_fb = vkms_cleanup_fb,

};

struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)

2.17.1