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

[Sean Paul <seanpaul@chromium.org>]

On Thu, Jul 19, 2018 at 03:18:55AM +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
> 
> Changes in v3:
> - return error number instead of vkms_obj->vaddr
> 
>  drivers/gpu/drm/vkms/vkms_plane.c | 35 +++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index 9f75b1e2c1c4..aa6cde34710c 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -9,6 +9,7 @@
>  #include "vkms_drv.h"
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
>  
>  static const struct drm_plane_funcs vkms_plane_funcs = {
>  	.update_plane		= drm_atomic_helper_update_plane,
> @@ -24,8 +25,42 @@ 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;
> +	int ret;
> +
> +	if (!state->fb)
> +		return 0;
> +
> +	gem_obj = drm_gem_fb_get_obj(state->fb, 0);
> +	vkms_obj = drm_gem_to_vkms_gem(gem_obj);
> +	ret = vkms_gem_vmap(gem_obj);
> +

nit: extra line

> +	if (ret)
> +		DRM_ERROR("vmap failed\n");

Can we actually do anything useful if the vmap fails? Asked another way, what's
the value in not returning the error here? (We should also print the value of
ret).

Sean

> +
> +	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
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel