diff mbox

[RFC,2/7] drm: Add GEM backed framebuffer library

Message ID 1499867165-60925-3-git-send-email-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show

Commit Message

Noralf Trønnes July 12, 2017, 1:46 p.m. UTC
Add a library for drivers that can use a simple representation
of a GEM backed framebuffer.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/Makefile            |   2 +-
 drivers/gpu/drm/drm_fb_gem_helper.c | 248 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_fb_gem_helper.h     |  64 ++++++++++
 3 files changed, 313 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_fb_gem_helper.c
 create mode 100644 include/drm/drm_fb_gem_helper.h

Comments

Noralf Trønnes July 18, 2017, 3:42 p.m. UTC | #1
Den 12.07.2017 15.46, skrev Noralf Trønnes:
> Add a library for drivers that can use a simple representation
> of a GEM backed framebuffer.
>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---

This patch adds a gem backed drm_framebuffer like this:

struct drm_fb_gem {
     /**
      * @base: Base DRM framebuffer
      */
     struct drm_framebuffer base;
     /**
      * @obj: GEM object array backing the framebuffer. One object per
      * plane.
      */
     struct drm_gem_object *obj[4];
};

Now I wonder if it would be better to extend drm_framebuffer instead:

  struct drm_framebuffer {
+    /**
+     * @obj: GEM objects backing the framebuffer, one per plane (optional).
+     */
+    struct drm_gem_object *obj[4];
  };

The reason for this, is that I have looked through all drivers that
subclass drm_framebuffer and found this:
- 11 drivers just adds drm_gem_object
- 2 drivers adds drm_gem_object and some more
- 6 drivers adds drm_gem_object indirectly through their subclassed
   drm_gem_object
- 3 drivers doesn't add drm_gem_object


Full details below:

Adds only drm_gem_object
------------------------

struct amdgpu_framebuffer {
     struct drm_framebuffer base;
     struct drm_gem_object *obj;
};

struct ast_framebuffer {
     struct drm_framebuffer base;
     struct drm_gem_object *obj;
};

struct bochs_framebuffer {
     struct drm_framebuffer base;
     struct drm_gem_object *obj;
};

struct cirrus_framebuffer {
     struct drm_framebuffer        base;
     struct drm_gem_object *obj;
};

struct drm_fb_cma {
     struct drm_framebuffer        fb;
     struct drm_gem_cma_object    *obj[4];
};

struct hibmc_framebuffer {
     struct drm_framebuffer fb;
     struct drm_gem_object *obj;
};

struct mga_framebuffer {
     struct drm_framebuffer base;
     struct drm_gem_object *obj;
};

struct mtk_drm_fb {
     struct drm_framebuffer    base;
     /* For now we only support a single plane */
     struct drm_gem_object    *gem_obj;
};

struct qxl_framebuffer {
     struct drm_framebuffer base;
     struct drm_gem_object *obj;
};

struct radeon_framebuffer {
     struct drm_framebuffer base;
     struct drm_gem_object *obj;
};

struct rockchip_drm_fb {
     struct drm_framebuffer fb;
     struct drm_gem_object *obj[ROCKCHIP_MAX_FB_BUFFER];
};
#define ROCKCHIP_MAX_FB_BUFFER    3


Adds drm_gem_object and some more
---------------------------------

struct msm_framebuffer {
     struct drm_framebuffer base;
     const struct msm_format *format;
     struct drm_gem_object *planes[MAX_PLANE];
};
#define MAX_PLANE    4

struct virtio_gpu_framebuffer {
     struct drm_framebuffer base;
     struct drm_gem_object *obj;
     int x1, y1, x2, y2; /* dirty rect */
     spinlock_t dirty_lock;
     uint32_t hw_res_handle;
};


Indirectly adds a drm_gem_object
--------------------------------

struct armada_framebuffer {
     struct drm_framebuffer    fb;
     struct armada_gem_object *obj;
     uint8_t            fmt;
     uint8_t            mod;
};

struct exynos_drm_fb {
     struct drm_framebuffer    fb;
     struct exynos_drm_gem    *exynos_gem[MAX_FB_BUFFER];
     dma_addr_t            dma_addr[MAX_FB_BUFFER];
};
#define MAX_FB_BUFFER    4

struct intel_framebuffer {
     struct drm_framebuffer base;
     struct drm_i915_gem_object *obj;
     struct intel_rotation_info rot_info;

     /* for each plane in the normal GTT view */
     struct {
         unsigned int x, y;
     } normal[2];
     /* for each plane in the rotated GTT view */
     struct {
         unsigned int x, y;
         unsigned int pitch; /* pixels */
     } rotated[2];
};

struct nouveau_framebuffer {
     struct drm_framebuffer base;
     struct nouveau_bo *nvbo;
     struct nvkm_vma vma;
     u32 r_handle;
     u32 r_format;
     u32 r_pitch;
     struct nvif_object h_base[4];
     struct nvif_object h_core;
};

struct tegra_fb {
     struct drm_framebuffer base;
     struct tegra_bo **planes;
     unsigned int num_planes;
};

struct udl_framebuffer {
     struct drm_framebuffer base;
     struct udl_gem_object *obj;
     bool active_16; /* active on the 16-bit channel */
};


Does not add drm_gem_object
---------------------------

struct omap_framebuffer {
     struct drm_framebuffer base;
     int pin_count;
     const struct drm_format_info *format;
     enum omap_color_mode dss_format;
     struct plane planes[2];
     /* lock for pinning (pin_count and planes.paddr) */
     struct mutex lock;
};

struct psb_framebuffer {
     struct drm_framebuffer base;
     struct address_space *addr_space;
     struct fb_info *fbdev;
     struct gtt_range *gtt;
};

struct vmw_framebuffer {
     struct drm_framebuffer base;
     int (*pin)(struct vmw_framebuffer *fb);
     int (*unpin)(struct vmw_framebuffer *fb);
     bool dmabuf;
     struct ttm_base_object *user_obj;
     uint32_t user_handle;
};


Noralf.


>   drivers/gpu/drm/Makefile            |   2 +-
>   drivers/gpu/drm/drm_fb_gem_helper.c | 248 ++++++++++++++++++++++++++++++++++++
>   include/drm/drm_fb_gem_helper.h     |  64 ++++++++++
>   3 files changed, 313 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/gpu/drm/drm_fb_gem_helper.c
>   create mode 100644 include/drm/drm_fb_gem_helper.h
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 24a066e..83d8b09 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -33,7 +33,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
>   		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
>   		drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
>   		drm_simple_kms_helper.o drm_modeset_helper.o \
> -		drm_scdc_helper.o
> +		drm_scdc_helper.o drm_fb_gem_helper.o
>   
>   drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
>   drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> diff --git a/drivers/gpu/drm/drm_fb_gem_helper.c b/drivers/gpu/drm/drm_fb_gem_helper.c
> new file mode 100644
> index 0000000..9a0da09
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_fb_gem_helper.c
> @@ -0,0 +1,248 @@
> +/*
> + * drm fb gem helper functions
> + *
> + * Copyright (C) 2017 Noralf Trønnes
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/reservation.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_fb_gem_helper.h>
> +#include <drm/drm_gem.h>
> +
> +/**
> + * drm_fb_gem_get_obj() - Get GEM object for framebuffer
> + * @fb: The framebuffer
> + * @plane: Which plane
> + *
> + * Returns the GEM object for given framebuffer.
> + */
> +struct drm_gem_object *drm_fb_gem_get_obj(struct drm_framebuffer *fb,
> +					  unsigned int plane)
> +{
> +	struct drm_fb_gem *fb_gem = to_fb_gem(fb);
> +
> +	if (plane >= 4)
> +		return NULL;
> +
> +	return fb_gem->obj[plane];
> +}
> +EXPORT_SYMBOL_GPL(drm_fb_gem_get_obj);
> +
> +/**
> + * drm_fb_gem_alloc - Allocate GEM backed framebuffer
> + * @dev: DRM device
> + * @mode_cmd: metadata from the userspace fb creation request
> + * @obj: GEM object nacking the framebuffer
> + * @num_planes: Number of planes
> + * @funcs: vtable to be used for the new framebuffer object
> + *
> + * Returns:
> + * Allocated struct drm_fb_gem * or error encoded pointer.
> + */
> +struct drm_fb_gem *
> +drm_fb_gem_alloc(struct drm_device *dev,
> +		 const struct drm_mode_fb_cmd2 *mode_cmd,
> +		 struct drm_gem_object **obj, unsigned int num_planes,
> +		 const struct drm_framebuffer_funcs *funcs)
> +{
> +	struct drm_fb_gem *fb_gem;
> +	int ret, i;
> +
> +	fb_gem = kzalloc(sizeof(*fb_gem), GFP_KERNEL);
> +	if (!fb_gem)
> +		return ERR_PTR(-ENOMEM);
> +
> +	drm_helper_mode_fill_fb_struct(dev, &fb_gem->base, mode_cmd);
> +
> +	for (i = 0; i < num_planes; i++)
> +		fb_gem->obj[i] = obj[i];
> +
> +	ret = drm_framebuffer_init(dev, &fb_gem->base, funcs);
> +	if (ret) {
> +		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret);
> +		kfree(fb_gem);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return fb_gem;
> +}
> +EXPORT_SYMBOL(drm_fb_gem_alloc);
> +
> +/**
> + * drm_fb_gem_destroy - Free GEM backed framebuffer
> + * @fb: DRM framebuffer
> + *
> + * Frees a GEM backed framebuffer with it's backing buffer(s) and the structure
> + * itself. Drivers can use this as their &drm_framebuffer_funcs->destroy
> + * callback.
> + */
> +void drm_fb_gem_destroy(struct drm_framebuffer *fb)
> +{
> +	struct drm_fb_gem *fb_gem = to_fb_gem(fb);
> +	int i;
> +
> +	for (i = 0; i < 4; i++) {
> +		if (fb_gem->obj[i])
> +			drm_gem_object_put_unlocked(fb_gem->obj[i]);
> +	}
> +
> +	drm_framebuffer_cleanup(fb);
> +	kfree(fb_gem);
> +}
> +EXPORT_SYMBOL(drm_fb_gem_destroy);
> +
> +/**
> + * drm_fb_gem_create_handle - Create handle for GEM backed framebuffer
> + * @fb: DRM framebuffer
> + * @file: drm file
> + * @handle: handle created
> + *
> + * Drivers can use this as their &drm_framebuffer_funcs->create_handle
> + * callback.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_fb_gem_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
> +			     unsigned int *handle)
> +{
> +	struct drm_fb_gem *fb_gem = to_fb_gem(fb);
> +
> +	return drm_gem_handle_create(file, fb_gem->obj[0], handle);
> +}
> +EXPORT_SYMBOL(drm_fb_gem_create_handle);
> +
> +/**
> + * drm_fb_gem_create_with_funcs() - helper function for the
> + *                                  &drm_mode_config_funcs.fb_create
> + *                                  callback
> + * @dev: DRM device
> + * @file: drm file for the ioctl call
> + * @mode_cmd: metadata from the userspace fb creation request
> + * @funcs: vtable to be used for the new framebuffer object
> + *
> + * This can be used to set &drm_framebuffer_funcs for drivers that need the
> + * &drm_framebuffer_funcs.dirty callback. Use drm_fb_gem_create() if you don't
> + * need to change &drm_framebuffer_funcs.
> + */
> +struct drm_framebuffer *
> +drm_fb_gem_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> +			     const struct drm_mode_fb_cmd2 *mode_cmd,
> +			     const struct drm_framebuffer_funcs *funcs)
> +{
> +	const struct drm_format_info *info;
> +	struct drm_gem_object *objs[4];
> +	struct drm_fb_gem *fb_gem;
> +	int ret, i;
> +
> +	info = drm_get_format_info(dev, mode_cmd);
> +	if (!info)
> +		return ERR_PTR(-EINVAL);
> +
> +	for (i = 0; i < info->num_planes; i++) {
> +		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> +		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> +		unsigned int min_size;
> +
> +		objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
> +		if (!objs[i]) {
> +			dev_err(dev->dev, "Failed to lookup GEM object\n");
> +			ret = -ENOENT;
> +			goto err_gem_object_put;
> +		}
> +
> +		min_size = (height - 1) * mode_cmd->pitches[i]
> +			 + width * info->cpp[i]
> +			 + mode_cmd->offsets[i];
> +
> +		if (objs[i]->size < min_size) {
> +			drm_gem_object_put_unlocked(objs[i]);
> +			ret = -EINVAL;
> +			goto err_gem_object_put;
> +		}
> +	}
> +
> +	fb_gem = drm_fb_gem_alloc(dev, mode_cmd, objs, i, funcs);
> +	if (IS_ERR(fb_gem)) {
> +		ret = PTR_ERR(fb_gem);
> +		goto err_gem_object_put;
> +	}
> +
> +	return &fb_gem->base;
> +
> +err_gem_object_put:
> +	for (i--; i >= 0; i--)
> +		drm_gem_object_put_unlocked(objs[i]);
> +
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(drm_fb_gem_create_with_funcs);
> +
> +static struct drm_framebuffer_funcs drm_fb_gem_fb_funcs = {
> +	.destroy	= drm_fb_gem_destroy,
> +	.create_handle	= drm_fb_gem_create_handle,
> +};
> +
> +/**
> + * drm_fb_gem_create() - &drm_mode_config_funcs.fb_create callback function
> + * @dev: DRM device
> + * @file: drm file for the ioctl call
> + * @mode_cmd: metadata from the userspace fb creation request
> + *
> + * If your hardware has special alignment or pitch requirements these should be
> + * checked before calling this function. Use drm_fb_gem_create_with_funcs() if
> + * you need to set &drm_framebuffer_funcs.dirty.
> + */
> +struct drm_framebuffer *
> +drm_fb_gem_create(struct drm_device *dev, struct drm_file *file,
> +		  const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> +	return drm_fb_gem_create_with_funcs(dev, file, mode_cmd,
> +					    &drm_fb_gem_fb_funcs);
> +}
> +EXPORT_SYMBOL_GPL(drm_fb_gem_create);
> +
> +/**
> + * drm_fb_gem_prepare_fb() - Prepare gem framebuffer
> + * @plane: Which plane
> + * @state: Plane state attach fence to
> + *
> + * This should be set as the &struct drm_plane_helper_funcs.prepare_fb hook.
> + *
> + * This function checks if the plane FB has an dma-buf attached, extracts
> + * the exclusive fence and attaches it to plane state for the atomic helper
> + * to wait on.
> + *
> + * There is no need for cleanup_fb for gem based framebuffer drivers.
> + */
> +int drm_fb_gem_prepare_fb(struct drm_plane *plane,
> +			  struct drm_plane_state *state)
> +{
> +	struct dma_buf *dma_buf;
> +	struct dma_fence *fence;
> +
> +	if ((plane->state->fb == state->fb) || !state->fb)
> +		return 0;
> +
> +	dma_buf = drm_fb_gem_get_obj(state->fb, 0)->dma_buf;
> +	if (dma_buf) {
> +		fence = reservation_object_get_excl_rcu(dma_buf->resv);
> +		drm_atomic_set_fence_for_plane(state, fence);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_fb_gem_prepare_fb);
> diff --git a/include/drm/drm_fb_gem_helper.h b/include/drm/drm_fb_gem_helper.h
> new file mode 100644
> index 0000000..405a1e1
> --- /dev/null
> +++ b/include/drm/drm_fb_gem_helper.h
> @@ -0,0 +1,64 @@
> +#ifndef __DRM_FB_GEM_HELPER_H__
> +#define __DRM_FB_GEM_HELPER_H__
> +
> +#include <drm/drm_framebuffer.h>
> +
> +struct drm_gem_shmem_object;
> +struct drm_mode_fb_cmd2;
> +struct drm_plane;
> +struct drm_plane_state;
> +
> +/**
> + * struct drm_fb_gem - GEM backed framebuffer
> + */
> +struct drm_fb_gem {
> +	/**
> +	 * @base: Base DRM framebuffer
> +	 */
> +	struct drm_framebuffer base;
> +	/**
> +	 * @obj: GEM object array backing the framebuffer. One object per
> +	 * plane.
> +	 */
> +	struct drm_gem_object *obj[4];
> +};
> +
> +static inline struct drm_fb_gem *to_fb_gem(struct drm_framebuffer *fb)
> +{
> +	return container_of(fb, struct drm_fb_gem, base);
> +}
> +
> +struct drm_gem_object *drm_fb_gem_get_obj(struct drm_framebuffer *fb,
> +					  unsigned int plane);
> +struct drm_fb_gem *
> +drm_fb_gem_alloc(struct drm_device *dev,
> +		 const struct drm_mode_fb_cmd2 *mode_cmd,
> +		 struct drm_gem_object **obj, unsigned int num_planes,
> +		 const struct drm_framebuffer_funcs *funcs);
> +void drm_fb_gem_destroy(struct drm_framebuffer *fb);
> +int drm_fb_gem_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
> +			     unsigned int *handle);
> +
> +struct drm_framebuffer *
> +drm_fb_gem_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> +			     const struct drm_mode_fb_cmd2 *mode_cmd,
> +			     const struct drm_framebuffer_funcs *funcs);
> +struct drm_framebuffer *
> +drm_fb_gem_create(struct drm_device *dev, struct drm_file *file,
> +		  const struct drm_mode_fb_cmd2 *mode_cmd);
> +
> +
> +int drm_fb_gem_prepare_fb(struct drm_plane *plane,
> +			  struct drm_plane_state *state);
> +
> +
> +
> +
> +#ifdef CONFIG_DEBUG_FS
> +struct seq_file;
> +
> +int drm_fb_gem_debugfs_show(struct seq_file *m, void *arg);
> +#endif
> +
> +#endif
> +
Eric Anholt July 19, 2017, 8:59 p.m. UTC | #2
Noralf Trønnes <noralf@tronnes.org> writes:

> Den 12.07.2017 15.46, skrev Noralf Trønnes:
>> Add a library for drivers that can use a simple representation
>> of a GEM backed framebuffer.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>
> This patch adds a gem backed drm_framebuffer like this:
>
> struct drm_fb_gem {
>      /**
>       * @base: Base DRM framebuffer
>       */
>      struct drm_framebuffer base;
>      /**
>       * @obj: GEM object array backing the framebuffer. One object per
>       * plane.
>       */
>      struct drm_gem_object *obj[4];
> };
>
> Now I wonder if it would be better to extend drm_framebuffer instead:
>
>   struct drm_framebuffer {
> +    /**
> +     * @obj: GEM objects backing the framebuffer, one per plane (optional).
> +     */
> +    struct drm_gem_object *obj[4];
>   };

FWIW, I would love to see this tried.  I think we would end up with some
nice cleanups if we did so.
Daniel Vetter July 20, 2017, 8:10 a.m. UTC | #3
On Tue, Jul 18, 2017 at 05:42:28PM +0200, Noralf Trønnes wrote:
> 
> Den 12.07.2017 15.46, skrev Noralf Trønnes:
> > Add a library for drivers that can use a simple representation
> > of a GEM backed framebuffer.
> > 
> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > ---
> 
> This patch adds a gem backed drm_framebuffer like this:
> 
> struct drm_fb_gem {
>     /**
>      * @base: Base DRM framebuffer
>      */
>     struct drm_framebuffer base;
>     /**
>      * @obj: GEM object array backing the framebuffer. One object per
>      * plane.
>      */
>     struct drm_gem_object *obj[4];
> };
> 
> Now I wonder if it would be better to extend drm_framebuffer instead:
> 
>  struct drm_framebuffer {
> +    /**
> +     * @obj: GEM objects backing the framebuffer, one per plane (optional).
> +     */
> +    struct drm_gem_object *obj[4];
>  };

I think we can directly embedd the gem obj pointers into the
drm_framebuffer. Again the idea that there would be anything else than gem
kinda didn't pan out, except for vmwgfx.

We could then have a gem version of drm_helper_mode_fill_fb_struct(),
which also does the gem bo lookups.

> The reason for this, is that I have looked through all drivers that
> subclass drm_framebuffer and found this:
> - 11 drivers just adds drm_gem_object

Usually that's because they don't support yuv, so only need one plane.

> - 2 drivers adds drm_gem_object and some more
> - 6 drivers adds drm_gem_object indirectly through their subclassed
>   drm_gem_object
> - 3 drivers doesn't add drm_gem_object

I think for easier conversion we can leave all the driver-specific stuff
intact, and just provide helpers that work on the core bits.

I guess this would also help a lot with unifying the cma fb helpers by
essentially making them fully generic gem fb helpers?
-Daniel

> 
> 
> Full details below:
> 
> Adds only drm_gem_object
> ------------------------
> 
> struct amdgpu_framebuffer {
>     struct drm_framebuffer base;
>     struct drm_gem_object *obj;
> };
> 
> struct ast_framebuffer {
>     struct drm_framebuffer base;
>     struct drm_gem_object *obj;
> };
> 
> struct bochs_framebuffer {
>     struct drm_framebuffer base;
>     struct drm_gem_object *obj;
> };
> 
> struct cirrus_framebuffer {
>     struct drm_framebuffer        base;
>     struct drm_gem_object *obj;
> };
> 
> struct drm_fb_cma {
>     struct drm_framebuffer        fb;
>     struct drm_gem_cma_object    *obj[4];
> };
> 
> struct hibmc_framebuffer {
>     struct drm_framebuffer fb;
>     struct drm_gem_object *obj;
> };
> 
> struct mga_framebuffer {
>     struct drm_framebuffer base;
>     struct drm_gem_object *obj;
> };
> 
> struct mtk_drm_fb {
>     struct drm_framebuffer    base;
>     /* For now we only support a single plane */
>     struct drm_gem_object    *gem_obj;
> };
> 
> struct qxl_framebuffer {
>     struct drm_framebuffer base;
>     struct drm_gem_object *obj;
> };
> 
> struct radeon_framebuffer {
>     struct drm_framebuffer base;
>     struct drm_gem_object *obj;
> };
> 
> struct rockchip_drm_fb {
>     struct drm_framebuffer fb;
>     struct drm_gem_object *obj[ROCKCHIP_MAX_FB_BUFFER];
> };
> #define ROCKCHIP_MAX_FB_BUFFER    3
> 
> 
> Adds drm_gem_object and some more
> ---------------------------------
> 
> struct msm_framebuffer {
>     struct drm_framebuffer base;
>     const struct msm_format *format;
>     struct drm_gem_object *planes[MAX_PLANE];
> };
> #define MAX_PLANE    4
> 
> struct virtio_gpu_framebuffer {
>     struct drm_framebuffer base;
>     struct drm_gem_object *obj;
>     int x1, y1, x2, y2; /* dirty rect */
>     spinlock_t dirty_lock;
>     uint32_t hw_res_handle;
> };
> 
> 
> Indirectly adds a drm_gem_object
> --------------------------------
> 
> struct armada_framebuffer {
>     struct drm_framebuffer    fb;
>     struct armada_gem_object *obj;
>     uint8_t            fmt;
>     uint8_t            mod;
> };
> 
> struct exynos_drm_fb {
>     struct drm_framebuffer    fb;
>     struct exynos_drm_gem    *exynos_gem[MAX_FB_BUFFER];
>     dma_addr_t            dma_addr[MAX_FB_BUFFER];
> };
> #define MAX_FB_BUFFER    4
> 
> struct intel_framebuffer {
>     struct drm_framebuffer base;
>     struct drm_i915_gem_object *obj;
>     struct intel_rotation_info rot_info;
> 
>     /* for each plane in the normal GTT view */
>     struct {
>         unsigned int x, y;
>     } normal[2];
>     /* for each plane in the rotated GTT view */
>     struct {
>         unsigned int x, y;
>         unsigned int pitch; /* pixels */
>     } rotated[2];
> };
> 
> struct nouveau_framebuffer {
>     struct drm_framebuffer base;
>     struct nouveau_bo *nvbo;
>     struct nvkm_vma vma;
>     u32 r_handle;
>     u32 r_format;
>     u32 r_pitch;
>     struct nvif_object h_base[4];
>     struct nvif_object h_core;
> };
> 
> struct tegra_fb {
>     struct drm_framebuffer base;
>     struct tegra_bo **planes;
>     unsigned int num_planes;
> };
> 
> struct udl_framebuffer {
>     struct drm_framebuffer base;
>     struct udl_gem_object *obj;
>     bool active_16; /* active on the 16-bit channel */
> };
> 
> 
> Does not add drm_gem_object
> ---------------------------
> 
> struct omap_framebuffer {
>     struct drm_framebuffer base;
>     int pin_count;
>     const struct drm_format_info *format;
>     enum omap_color_mode dss_format;
>     struct plane planes[2];
>     /* lock for pinning (pin_count and planes.paddr) */
>     struct mutex lock;
> };
> 
> struct psb_framebuffer {
>     struct drm_framebuffer base;
>     struct address_space *addr_space;
>     struct fb_info *fbdev;
>     struct gtt_range *gtt;
> };
> 
> struct vmw_framebuffer {
>     struct drm_framebuffer base;
>     int (*pin)(struct vmw_framebuffer *fb);
>     int (*unpin)(struct vmw_framebuffer *fb);
>     bool dmabuf;
>     struct ttm_base_object *user_obj;
>     uint32_t user_handle;
> };
> 
> 
> Noralf.
> 
> 
> >   drivers/gpu/drm/Makefile            |   2 +-
> >   drivers/gpu/drm/drm_fb_gem_helper.c | 248 ++++++++++++++++++++++++++++++++++++
> >   include/drm/drm_fb_gem_helper.h     |  64 ++++++++++
> >   3 files changed, 313 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/gpu/drm/drm_fb_gem_helper.c
> >   create mode 100644 include/drm/drm_fb_gem_helper.h
> > 
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 24a066e..83d8b09 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -33,7 +33,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
> >   		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
> >   		drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
> >   		drm_simple_kms_helper.o drm_modeset_helper.o \
> > -		drm_scdc_helper.o
> > +		drm_scdc_helper.o drm_fb_gem_helper.o
> >   drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
> >   drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> > diff --git a/drivers/gpu/drm/drm_fb_gem_helper.c b/drivers/gpu/drm/drm_fb_gem_helper.c
> > new file mode 100644
> > index 0000000..9a0da09
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_fb_gem_helper.c
> > @@ -0,0 +1,248 @@
> > +/*
> > + * drm fb gem helper functions
> > + *
> > + * Copyright (C) 2017 Noralf Trønnes
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include <linux/dma-buf.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/module.h>
> > +#include <linux/reservation.h>
> > +
> > +#include <drm/drmP.h>
> > +#include <drm/drm_atomic.h>
> > +#include <drm/drm_crtc.h>
> > +#include <drm/drm_crtc_helper.h>
> > +#include <drm/drm_fb_helper.h>
> > +#include <drm/drm_fb_gem_helper.h>
> > +#include <drm/drm_gem.h>
> > +
> > +/**
> > + * drm_fb_gem_get_obj() - Get GEM object for framebuffer
> > + * @fb: The framebuffer
> > + * @plane: Which plane
> > + *
> > + * Returns the GEM object for given framebuffer.
> > + */
> > +struct drm_gem_object *drm_fb_gem_get_obj(struct drm_framebuffer *fb,
> > +					  unsigned int plane)
> > +{
> > +	struct drm_fb_gem *fb_gem = to_fb_gem(fb);
> > +
> > +	if (plane >= 4)
> > +		return NULL;
> > +
> > +	return fb_gem->obj[plane];
> > +}
> > +EXPORT_SYMBOL_GPL(drm_fb_gem_get_obj);
> > +
> > +/**
> > + * drm_fb_gem_alloc - Allocate GEM backed framebuffer
> > + * @dev: DRM device
> > + * @mode_cmd: metadata from the userspace fb creation request
> > + * @obj: GEM object nacking the framebuffer
> > + * @num_planes: Number of planes
> > + * @funcs: vtable to be used for the new framebuffer object
> > + *
> > + * Returns:
> > + * Allocated struct drm_fb_gem * or error encoded pointer.
> > + */
> > +struct drm_fb_gem *
> > +drm_fb_gem_alloc(struct drm_device *dev,
> > +		 const struct drm_mode_fb_cmd2 *mode_cmd,
> > +		 struct drm_gem_object **obj, unsigned int num_planes,
> > +		 const struct drm_framebuffer_funcs *funcs)
> > +{
> > +	struct drm_fb_gem *fb_gem;
> > +	int ret, i;
> > +
> > +	fb_gem = kzalloc(sizeof(*fb_gem), GFP_KERNEL);
> > +	if (!fb_gem)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	drm_helper_mode_fill_fb_struct(dev, &fb_gem->base, mode_cmd);
> > +
> > +	for (i = 0; i < num_planes; i++)
> > +		fb_gem->obj[i] = obj[i];
> > +
> > +	ret = drm_framebuffer_init(dev, &fb_gem->base, funcs);
> > +	if (ret) {
> > +		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret);
> > +		kfree(fb_gem);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> > +	return fb_gem;
> > +}
> > +EXPORT_SYMBOL(drm_fb_gem_alloc);
> > +
> > +/**
> > + * drm_fb_gem_destroy - Free GEM backed framebuffer
> > + * @fb: DRM framebuffer
> > + *
> > + * Frees a GEM backed framebuffer with it's backing buffer(s) and the structure
> > + * itself. Drivers can use this as their &drm_framebuffer_funcs->destroy
> > + * callback.
> > + */
> > +void drm_fb_gem_destroy(struct drm_framebuffer *fb)
> > +{
> > +	struct drm_fb_gem *fb_gem = to_fb_gem(fb);
> > +	int i;
> > +
> > +	for (i = 0; i < 4; i++) {
> > +		if (fb_gem->obj[i])
> > +			drm_gem_object_put_unlocked(fb_gem->obj[i]);
> > +	}
> > +
> > +	drm_framebuffer_cleanup(fb);
> > +	kfree(fb_gem);
> > +}
> > +EXPORT_SYMBOL(drm_fb_gem_destroy);
> > +
> > +/**
> > + * drm_fb_gem_create_handle - Create handle for GEM backed framebuffer
> > + * @fb: DRM framebuffer
> > + * @file: drm file
> > + * @handle: handle created
> > + *
> > + * Drivers can use this as their &drm_framebuffer_funcs->create_handle
> > + * callback.
> > + *
> > + * Returns:
> > + * 0 on success or a negative error code on failure.
> > + */
> > +int drm_fb_gem_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
> > +			     unsigned int *handle)
> > +{
> > +	struct drm_fb_gem *fb_gem = to_fb_gem(fb);
> > +
> > +	return drm_gem_handle_create(file, fb_gem->obj[0], handle);
> > +}
> > +EXPORT_SYMBOL(drm_fb_gem_create_handle);
> > +
> > +/**
> > + * drm_fb_gem_create_with_funcs() - helper function for the
> > + *                                  &drm_mode_config_funcs.fb_create
> > + *                                  callback
> > + * @dev: DRM device
> > + * @file: drm file for the ioctl call
> > + * @mode_cmd: metadata from the userspace fb creation request
> > + * @funcs: vtable to be used for the new framebuffer object
> > + *
> > + * This can be used to set &drm_framebuffer_funcs for drivers that need the
> > + * &drm_framebuffer_funcs.dirty callback. Use drm_fb_gem_create() if you don't
> > + * need to change &drm_framebuffer_funcs.
> > + */
> > +struct drm_framebuffer *
> > +drm_fb_gem_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> > +			     const struct drm_mode_fb_cmd2 *mode_cmd,
> > +			     const struct drm_framebuffer_funcs *funcs)
> > +{
> > +	const struct drm_format_info *info;
> > +	struct drm_gem_object *objs[4];
> > +	struct drm_fb_gem *fb_gem;
> > +	int ret, i;
> > +
> > +	info = drm_get_format_info(dev, mode_cmd);
> > +	if (!info)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	for (i = 0; i < info->num_planes; i++) {
> > +		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> > +		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> > +		unsigned int min_size;
> > +
> > +		objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
> > +		if (!objs[i]) {
> > +			dev_err(dev->dev, "Failed to lookup GEM object\n");
> > +			ret = -ENOENT;
> > +			goto err_gem_object_put;
> > +		}
> > +
> > +		min_size = (height - 1) * mode_cmd->pitches[i]
> > +			 + width * info->cpp[i]
> > +			 + mode_cmd->offsets[i];
> > +
> > +		if (objs[i]->size < min_size) {
> > +			drm_gem_object_put_unlocked(objs[i]);
> > +			ret = -EINVAL;
> > +			goto err_gem_object_put;
> > +		}
> > +	}
> > +
> > +	fb_gem = drm_fb_gem_alloc(dev, mode_cmd, objs, i, funcs);
> > +	if (IS_ERR(fb_gem)) {
> > +		ret = PTR_ERR(fb_gem);
> > +		goto err_gem_object_put;
> > +	}
> > +
> > +	return &fb_gem->base;
> > +
> > +err_gem_object_put:
> > +	for (i--; i >= 0; i--)
> > +		drm_gem_object_put_unlocked(objs[i]);
> > +
> > +	return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL_GPL(drm_fb_gem_create_with_funcs);
> > +
> > +static struct drm_framebuffer_funcs drm_fb_gem_fb_funcs = {
> > +	.destroy	= drm_fb_gem_destroy,
> > +	.create_handle	= drm_fb_gem_create_handle,
> > +};
> > +
> > +/**
> > + * drm_fb_gem_create() - &drm_mode_config_funcs.fb_create callback function
> > + * @dev: DRM device
> > + * @file: drm file for the ioctl call
> > + * @mode_cmd: metadata from the userspace fb creation request
> > + *
> > + * If your hardware has special alignment or pitch requirements these should be
> > + * checked before calling this function. Use drm_fb_gem_create_with_funcs() if
> > + * you need to set &drm_framebuffer_funcs.dirty.
> > + */
> > +struct drm_framebuffer *
> > +drm_fb_gem_create(struct drm_device *dev, struct drm_file *file,
> > +		  const struct drm_mode_fb_cmd2 *mode_cmd)
> > +{
> > +	return drm_fb_gem_create_with_funcs(dev, file, mode_cmd,
> > +					    &drm_fb_gem_fb_funcs);
> > +}
> > +EXPORT_SYMBOL_GPL(drm_fb_gem_create);
> > +
> > +/**
> > + * drm_fb_gem_prepare_fb() - Prepare gem framebuffer
> > + * @plane: Which plane
> > + * @state: Plane state attach fence to
> > + *
> > + * This should be set as the &struct drm_plane_helper_funcs.prepare_fb hook.
> > + *
> > + * This function checks if the plane FB has an dma-buf attached, extracts
> > + * the exclusive fence and attaches it to plane state for the atomic helper
> > + * to wait on.
> > + *
> > + * There is no need for cleanup_fb for gem based framebuffer drivers.
> > + */
> > +int drm_fb_gem_prepare_fb(struct drm_plane *plane,
> > +			  struct drm_plane_state *state)
> > +{
> > +	struct dma_buf *dma_buf;
> > +	struct dma_fence *fence;
> > +
> > +	if ((plane->state->fb == state->fb) || !state->fb)
> > +		return 0;
> > +
> > +	dma_buf = drm_fb_gem_get_obj(state->fb, 0)->dma_buf;
> > +	if (dma_buf) {
> > +		fence = reservation_object_get_excl_rcu(dma_buf->resv);
> > +		drm_atomic_set_fence_for_plane(state, fence);
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(drm_fb_gem_prepare_fb);
> > diff --git a/include/drm/drm_fb_gem_helper.h b/include/drm/drm_fb_gem_helper.h
> > new file mode 100644
> > index 0000000..405a1e1
> > --- /dev/null
> > +++ b/include/drm/drm_fb_gem_helper.h
> > @@ -0,0 +1,64 @@
> > +#ifndef __DRM_FB_GEM_HELPER_H__
> > +#define __DRM_FB_GEM_HELPER_H__
> > +
> > +#include <drm/drm_framebuffer.h>
> > +
> > +struct drm_gem_shmem_object;
> > +struct drm_mode_fb_cmd2;
> > +struct drm_plane;
> > +struct drm_plane_state;
> > +
> > +/**
> > + * struct drm_fb_gem - GEM backed framebuffer
> > + */
> > +struct drm_fb_gem {
> > +	/**
> > +	 * @base: Base DRM framebuffer
> > +	 */
> > +	struct drm_framebuffer base;
> > +	/**
> > +	 * @obj: GEM object array backing the framebuffer. One object per
> > +	 * plane.
> > +	 */
> > +	struct drm_gem_object *obj[4];
> > +};
> > +
> > +static inline struct drm_fb_gem *to_fb_gem(struct drm_framebuffer *fb)
> > +{
> > +	return container_of(fb, struct drm_fb_gem, base);
> > +}
> > +
> > +struct drm_gem_object *drm_fb_gem_get_obj(struct drm_framebuffer *fb,
> > +					  unsigned int plane);
> > +struct drm_fb_gem *
> > +drm_fb_gem_alloc(struct drm_device *dev,
> > +		 const struct drm_mode_fb_cmd2 *mode_cmd,
> > +		 struct drm_gem_object **obj, unsigned int num_planes,
> > +		 const struct drm_framebuffer_funcs *funcs);
> > +void drm_fb_gem_destroy(struct drm_framebuffer *fb);
> > +int drm_fb_gem_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
> > +			     unsigned int *handle);
> > +
> > +struct drm_framebuffer *
> > +drm_fb_gem_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> > +			     const struct drm_mode_fb_cmd2 *mode_cmd,
> > +			     const struct drm_framebuffer_funcs *funcs);
> > +struct drm_framebuffer *
> > +drm_fb_gem_create(struct drm_device *dev, struct drm_file *file,
> > +		  const struct drm_mode_fb_cmd2 *mode_cmd);
> > +
> > +
> > +int drm_fb_gem_prepare_fb(struct drm_plane *plane,
> > +			  struct drm_plane_state *state);
> > +
> > +
> > +
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +struct seq_file;
> > +
> > +int drm_fb_gem_debugfs_show(struct seq_file *m, void *arg);
> > +#endif
> > +
> > +#endif
> > +
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Noralf Trønnes July 21, 2017, 6:39 p.m. UTC | #4
Den 20.07.2017 10.10, skrev Daniel Vetter:
> On Tue, Jul 18, 2017 at 05:42:28PM +0200, Noralf Trønnes wrote:
>> Den 12.07.2017 15.46, skrev Noralf Trønnes:
>>> Add a library for drivers that can use a simple representation
>>> of a GEM backed framebuffer.
>>>
>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>> ---
>> This patch adds a gem backed drm_framebuffer like this:
>>
>> struct drm_fb_gem {
>>      /**
>>       * @base: Base DRM framebuffer
>>       */
>>      struct drm_framebuffer base;
>>      /**
>>       * @obj: GEM object array backing the framebuffer. One object per
>>       * plane.
>>       */
>>      struct drm_gem_object *obj[4];
>> };
>>
>> Now I wonder if it would be better to extend drm_framebuffer instead:
>>
>>   struct drm_framebuffer {
>> +    /**
>> +     * @obj: GEM objects backing the framebuffer, one per plane (optional).
>> +     */
>> +    struct drm_gem_object *obj[4];
>>   };
> I think we can directly embedd the gem obj pointers into the
> drm_framebuffer. Again the idea that there would be anything else than gem
> kinda didn't pan out, except for vmwgfx.
>
> We could then have a gem version of drm_helper_mode_fill_fb_struct(),
> which also does the gem bo lookups.

How about this, I've copied from drm_fb_cma_create_with_funcs():

/**
  * drm_helper_mode_fill_fb_gem - fill out framebuffer metadata and 
lookup GEM
  * @dev: DRM device
  * @file_priv: DRM file to lookup buffers from
  * @fb: drm_framebuffer object to fill out
  * @mode_cmd: metadata from the userspace fb creation request
  *
  * This helper can be used in a drivers fb_create callback to pre-fill 
the fb's
  * metadata fields and lookup the GEM buffer object(s).
  *
  * Returns:
  * 0 on success or a negative error code on failure.
  */
int drm_helper_mode_fill_fb_gem(struct drm_device *dev,
                 struct drm_file *file_priv,
                 struct drm_framebuffer *fb,
                 const struct drm_mode_fb_cmd2 *mode_cmd)
{
     int i, ret;

     drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
     if (!fb->format)
         return -EINVAL;

     for (i = 0; i < fb->format->num_planes; i++) {
         unsigned int width = mode_cmd->width /
                      (i ? fb->format->hsub : 1);
         unsigned int height = mode_cmd->height /
                       (i ? fb->format->vsub : 1);
         unsigned int min_size;

         fb->objs[i] = drm_gem_object_lookup(file_priv,
                             mode_cmd->handles[i]);
         if (!fb->objs[i]) {
             ret = -ENOENT;
             goto err_gem_object_put;
         }

         min_size = (height - 1) * mode_cmd->pitches[i] +
                width * fb->format->cpp[i] + mode_cmd->offsets[i];

         if (fb->objs[i]->size < min_size) {
             drm_gem_object_put_unlocked(fb->objs[i]);
             fb->objs[i] = NULL;
             ret = -EINVAL;
             goto err_gem_object_put;
         }
     }

err_gem_object_put:
     for (i--; i >= 0; i--) {
         drm_gem_object_put_unlocked(fb->objs[i]);
         fb->objs[i] = NULL;
     }

     return ret;
}
EXPORT_SYMBOL(drm_helper_mode_fill_fb_gem);

>> The reason for this, is that I have looked through all drivers that
>> subclass drm_framebuffer and found this:
>> - 11 drivers just adds drm_gem_object
> Usually that's because they don't support yuv, so only need one plane.
>
>> - 2 drivers adds drm_gem_object and some more
>> - 6 drivers adds drm_gem_object indirectly through their subclassed
>>    drm_gem_object
>> - 3 drivers doesn't add drm_gem_object
> I think for easier conversion we can leave all the driver-specific stuff
> intact, and just provide helpers that work on the core bits.
>
> I guess this would also help a lot with unifying the cma fb helpers by
> essentially making them fully generic gem fb helpers?

Yes both cma and shmem library can now use these helpers. But pushing
the helpers all the way out to the cma drivers will be too much for me.

Any chance the fbdev helper could get a struct drm_file?
It would be nice to use drm_driver.dumb_create and
drm_mode_config_funcs.fb_create to get a
framebuffer for fbdev.

Noralf.
Daniel Vetter July 25, 2017, 7 a.m. UTC | #5
On Fri, Jul 21, 2017 at 08:39:17PM +0200, Noralf Trønnes wrote:
>
> Den 20.07.2017 10.10, skrev Daniel Vetter:
> > On Tue, Jul 18, 2017 at 05:42:28PM +0200, Noralf Trønnes wrote:
> > > Den 12.07.2017 15.46, skrev Noralf Trønnes:
> > > > Add a library for drivers that can use a simple representation
> > > > of a GEM backed framebuffer.
> > > >
> > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > > ---
> > > This patch adds a gem backed drm_framebuffer like this:
> > >
> > > struct drm_fb_gem {
> > >      /**
> > >       * @base: Base DRM framebuffer
> > >       */
> > >      struct drm_framebuffer base;
> > >      /**
> > >       * @obj: GEM object array backing the framebuffer. One object per
> > >       * plane.
> > >       */
> > >      struct drm_gem_object *obj[4];
> > > };
> > >
> > > Now I wonder if it would be better to extend drm_framebuffer instead:
> > >
> > >   struct drm_framebuffer {
> > > +    /**
> > > +     * @obj: GEM objects backing the framebuffer, one per plane (optional).
> > > +     */
> > > +    struct drm_gem_object *obj[4];
> > >   };
> > I think we can directly embedd the gem obj pointers into the
> > drm_framebuffer. Again the idea that there would be anything else than gem
> > kinda didn't pan out, except for vmwgfx.
> >
> > We could then have a gem version of drm_helper_mode_fill_fb_struct(),
> > which also does the gem bo lookups.
>
> How about this, I've copied from drm_fb_cma_create_with_funcs():

Hm, we might as well call it drm_fb_gem_create_with_funcs, to stay in line
with the naming scheme we have.

> /**
>  * drm_helper_mode_fill_fb_gem - fill out framebuffer metadata and lookup
> GEM
>  * @dev: DRM device
>  * @file_priv: DRM file to lookup buffers from
>  * @fb: drm_framebuffer object to fill out
>  * @mode_cmd: metadata from the userspace fb creation request
>  *
>  * This helper can be used in a drivers fb_create callback to pre-fill the
> fb's
>  * metadata fields and lookup the GEM buffer object(s).
>  *
>  * Returns:
>  * 0 on success or a negative error code on failure.
>  */
> int drm_helper_mode_fill_fb_gem(struct drm_device *dev,
>                 struct drm_file *file_priv,
>                 struct drm_framebuffer *fb,
>                 const struct drm_mode_fb_cmd2 *mode_cmd)
> {
>     int i, ret;
>
>     drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
>     if (!fb->format)
>         return -EINVAL;
>
>     for (i = 0; i < fb->format->num_planes; i++) {
>         unsigned int width = mode_cmd->width /
>                      (i ? fb->format->hsub : 1);
>         unsigned int height = mode_cmd->height /
>                       (i ? fb->format->vsub : 1);
>         unsigned int min_size;
>
>         fb->objs[i] = drm_gem_object_lookup(file_priv,
>                             mode_cmd->handles[i]);
>         if (!fb->objs[i]) {
>             ret = -ENOENT;
>             goto err_gem_object_put;
>         }
>
>         min_size = (height - 1) * mode_cmd->pitches[i] +
>                width * fb->format->cpp[i] + mode_cmd->offsets[i];
>
>         if (fb->objs[i]->size < min_size) {
>             drm_gem_object_put_unlocked(fb->objs[i]);
>             fb->objs[i] = NULL;
>             ret = -EINVAL;
>             goto err_gem_object_put;
>         }
>     }
>
> err_gem_object_put:
>     for (i--; i >= 0; i--) {
>         drm_gem_object_put_unlocked(fb->objs[i]);
>         fb->objs[i] = NULL;
>     }
>
>     return ret;
> }
> EXPORT_SYMBOL(drm_helper_mode_fill_fb_gem);
>
> > > The reason for this, is that I have looked through all drivers that
> > > subclass drm_framebuffer and found this:
> > > - 11 drivers just adds drm_gem_object
> > Usually that's because they don't support yuv, so only need one plane.
> >
> > > - 2 drivers adds drm_gem_object and some more
> > > - 6 drivers adds drm_gem_object indirectly through their subclassed
> > >    drm_gem_object
> > > - 3 drivers doesn't add drm_gem_object
> > I think for easier conversion we can leave all the driver-specific stuff
> > intact, and just provide helpers that work on the core bits.
> >
> > I guess this would also help a lot with unifying the cma fb helpers by
> > essentially making them fully generic gem fb helpers?
>
> Yes both cma and shmem library can now use these helpers. But pushing
> the helpers all the way out to the cma drivers will be too much for me.

No worries, we can do this one by one across different people.

> Any chance the fbdev helper could get a struct drm_file?
> It would be nice to use drm_driver.dumb_create and
> drm_mode_config_funcs.fb_create to get a
> framebuffer for fbdev.

David Herrmann had a patch series to allow the kernel to create a drm_file
without a real struct file attached behind it. I think we landed the prep
work, but not any of the work to move the fbdev helper over to it. I'll
ping him whether he still has some of those patches somewhere.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 24a066e..83d8b09 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -33,7 +33,7 @@  drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
 		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
 		drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
 		drm_simple_kms_helper.o drm_modeset_helper.o \
-		drm_scdc_helper.o
+		drm_scdc_helper.o drm_fb_gem_helper.o
 
 drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
diff --git a/drivers/gpu/drm/drm_fb_gem_helper.c b/drivers/gpu/drm/drm_fb_gem_helper.c
new file mode 100644
index 0000000..9a0da09
--- /dev/null
+++ b/drivers/gpu/drm/drm_fb_gem_helper.c
@@ -0,0 +1,248 @@ 
+/*
+ * drm fb gem helper functions
+ *
+ * Copyright (C) 2017 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/dma-buf.h>
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <linux/reservation.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fb_gem_helper.h>
+#include <drm/drm_gem.h>
+
+/**
+ * drm_fb_gem_get_obj() - Get GEM object for framebuffer
+ * @fb: The framebuffer
+ * @plane: Which plane
+ *
+ * Returns the GEM object for given framebuffer.
+ */
+struct drm_gem_object *drm_fb_gem_get_obj(struct drm_framebuffer *fb,
+					  unsigned int plane)
+{
+	struct drm_fb_gem *fb_gem = to_fb_gem(fb);
+
+	if (plane >= 4)
+		return NULL;
+
+	return fb_gem->obj[plane];
+}
+EXPORT_SYMBOL_GPL(drm_fb_gem_get_obj);
+
+/**
+ * drm_fb_gem_alloc - Allocate GEM backed framebuffer
+ * @dev: DRM device
+ * @mode_cmd: metadata from the userspace fb creation request
+ * @obj: GEM object nacking the framebuffer
+ * @num_planes: Number of planes
+ * @funcs: vtable to be used for the new framebuffer object
+ *
+ * Returns:
+ * Allocated struct drm_fb_gem * or error encoded pointer.
+ */
+struct drm_fb_gem *
+drm_fb_gem_alloc(struct drm_device *dev,
+		 const struct drm_mode_fb_cmd2 *mode_cmd,
+		 struct drm_gem_object **obj, unsigned int num_planes,
+		 const struct drm_framebuffer_funcs *funcs)
+{
+	struct drm_fb_gem *fb_gem;
+	int ret, i;
+
+	fb_gem = kzalloc(sizeof(*fb_gem), GFP_KERNEL);
+	if (!fb_gem)
+		return ERR_PTR(-ENOMEM);
+
+	drm_helper_mode_fill_fb_struct(dev, &fb_gem->base, mode_cmd);
+
+	for (i = 0; i < num_planes; i++)
+		fb_gem->obj[i] = obj[i];
+
+	ret = drm_framebuffer_init(dev, &fb_gem->base, funcs);
+	if (ret) {
+		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret);
+		kfree(fb_gem);
+		return ERR_PTR(ret);
+	}
+
+	return fb_gem;
+}
+EXPORT_SYMBOL(drm_fb_gem_alloc);
+
+/**
+ * drm_fb_gem_destroy - Free GEM backed framebuffer
+ * @fb: DRM framebuffer
+ *
+ * Frees a GEM backed framebuffer with it's backing buffer(s) and the structure
+ * itself. Drivers can use this as their &drm_framebuffer_funcs->destroy
+ * callback.
+ */
+void drm_fb_gem_destroy(struct drm_framebuffer *fb)
+{
+	struct drm_fb_gem *fb_gem = to_fb_gem(fb);
+	int i;
+
+	for (i = 0; i < 4; i++) {
+		if (fb_gem->obj[i])
+			drm_gem_object_put_unlocked(fb_gem->obj[i]);
+	}
+
+	drm_framebuffer_cleanup(fb);
+	kfree(fb_gem);
+}
+EXPORT_SYMBOL(drm_fb_gem_destroy);
+
+/**
+ * drm_fb_gem_create_handle - Create handle for GEM backed framebuffer
+ * @fb: DRM framebuffer
+ * @file: drm file
+ * @handle: handle created
+ *
+ * Drivers can use this as their &drm_framebuffer_funcs->create_handle
+ * callback.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_fb_gem_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
+			     unsigned int *handle)
+{
+	struct drm_fb_gem *fb_gem = to_fb_gem(fb);
+
+	return drm_gem_handle_create(file, fb_gem->obj[0], handle);
+}
+EXPORT_SYMBOL(drm_fb_gem_create_handle);
+
+/**
+ * drm_fb_gem_create_with_funcs() - helper function for the
+ *                                  &drm_mode_config_funcs.fb_create
+ *                                  callback
+ * @dev: DRM device
+ * @file: drm file for the ioctl call
+ * @mode_cmd: metadata from the userspace fb creation request
+ * @funcs: vtable to be used for the new framebuffer object
+ *
+ * This can be used to set &drm_framebuffer_funcs for drivers that need the
+ * &drm_framebuffer_funcs.dirty callback. Use drm_fb_gem_create() if you don't
+ * need to change &drm_framebuffer_funcs.
+ */
+struct drm_framebuffer *
+drm_fb_gem_create_with_funcs(struct drm_device *dev, struct drm_file *file,
+			     const struct drm_mode_fb_cmd2 *mode_cmd,
+			     const struct drm_framebuffer_funcs *funcs)
+{
+	const struct drm_format_info *info;
+	struct drm_gem_object *objs[4];
+	struct drm_fb_gem *fb_gem;
+	int ret, i;
+
+	info = drm_get_format_info(dev, mode_cmd);
+	if (!info)
+		return ERR_PTR(-EINVAL);
+
+	for (i = 0; i < info->num_planes; i++) {
+		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
+		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
+		unsigned int min_size;
+
+		objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
+		if (!objs[i]) {
+			dev_err(dev->dev, "Failed to lookup GEM object\n");
+			ret = -ENOENT;
+			goto err_gem_object_put;
+		}
+
+		min_size = (height - 1) * mode_cmd->pitches[i]
+			 + width * info->cpp[i]
+			 + mode_cmd->offsets[i];
+
+		if (objs[i]->size < min_size) {
+			drm_gem_object_put_unlocked(objs[i]);
+			ret = -EINVAL;
+			goto err_gem_object_put;
+		}
+	}
+
+	fb_gem = drm_fb_gem_alloc(dev, mode_cmd, objs, i, funcs);
+	if (IS_ERR(fb_gem)) {
+		ret = PTR_ERR(fb_gem);
+		goto err_gem_object_put;
+	}
+
+	return &fb_gem->base;
+
+err_gem_object_put:
+	for (i--; i >= 0; i--)
+		drm_gem_object_put_unlocked(objs[i]);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(drm_fb_gem_create_with_funcs);
+
+static struct drm_framebuffer_funcs drm_fb_gem_fb_funcs = {
+	.destroy	= drm_fb_gem_destroy,
+	.create_handle	= drm_fb_gem_create_handle,
+};
+
+/**
+ * drm_fb_gem_create() - &drm_mode_config_funcs.fb_create callback function
+ * @dev: DRM device
+ * @file: drm file for the ioctl call
+ * @mode_cmd: metadata from the userspace fb creation request
+ *
+ * If your hardware has special alignment or pitch requirements these should be
+ * checked before calling this function. Use drm_fb_gem_create_with_funcs() if
+ * you need to set &drm_framebuffer_funcs.dirty.
+ */
+struct drm_framebuffer *
+drm_fb_gem_create(struct drm_device *dev, struct drm_file *file,
+		  const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	return drm_fb_gem_create_with_funcs(dev, file, mode_cmd,
+					    &drm_fb_gem_fb_funcs);
+}
+EXPORT_SYMBOL_GPL(drm_fb_gem_create);
+
+/**
+ * drm_fb_gem_prepare_fb() - Prepare gem framebuffer
+ * @plane: Which plane
+ * @state: Plane state attach fence to
+ *
+ * This should be set as the &struct drm_plane_helper_funcs.prepare_fb hook.
+ *
+ * This function checks if the plane FB has an dma-buf attached, extracts
+ * the exclusive fence and attaches it to plane state for the atomic helper
+ * to wait on.
+ *
+ * There is no need for cleanup_fb for gem based framebuffer drivers.
+ */
+int drm_fb_gem_prepare_fb(struct drm_plane *plane,
+			  struct drm_plane_state *state)
+{
+	struct dma_buf *dma_buf;
+	struct dma_fence *fence;
+
+	if ((plane->state->fb == state->fb) || !state->fb)
+		return 0;
+
+	dma_buf = drm_fb_gem_get_obj(state->fb, 0)->dma_buf;
+	if (dma_buf) {
+		fence = reservation_object_get_excl_rcu(dma_buf->resv);
+		drm_atomic_set_fence_for_plane(state, fence);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_fb_gem_prepare_fb);
diff --git a/include/drm/drm_fb_gem_helper.h b/include/drm/drm_fb_gem_helper.h
new file mode 100644
index 0000000..405a1e1
--- /dev/null
+++ b/include/drm/drm_fb_gem_helper.h
@@ -0,0 +1,64 @@ 
+#ifndef __DRM_FB_GEM_HELPER_H__
+#define __DRM_FB_GEM_HELPER_H__
+
+#include <drm/drm_framebuffer.h>
+
+struct drm_gem_shmem_object;
+struct drm_mode_fb_cmd2;
+struct drm_plane;
+struct drm_plane_state;
+
+/**
+ * struct drm_fb_gem - GEM backed framebuffer
+ */
+struct drm_fb_gem {
+	/**
+	 * @base: Base DRM framebuffer
+	 */
+	struct drm_framebuffer base;
+	/**
+	 * @obj: GEM object array backing the framebuffer. One object per
+	 * plane.
+	 */
+	struct drm_gem_object *obj[4];
+};
+
+static inline struct drm_fb_gem *to_fb_gem(struct drm_framebuffer *fb)
+{
+	return container_of(fb, struct drm_fb_gem, base);
+}
+
+struct drm_gem_object *drm_fb_gem_get_obj(struct drm_framebuffer *fb,
+					  unsigned int plane);
+struct drm_fb_gem *
+drm_fb_gem_alloc(struct drm_device *dev,
+		 const struct drm_mode_fb_cmd2 *mode_cmd,
+		 struct drm_gem_object **obj, unsigned int num_planes,
+		 const struct drm_framebuffer_funcs *funcs);
+void drm_fb_gem_destroy(struct drm_framebuffer *fb);
+int drm_fb_gem_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
+			     unsigned int *handle);
+
+struct drm_framebuffer *
+drm_fb_gem_create_with_funcs(struct drm_device *dev, struct drm_file *file,
+			     const struct drm_mode_fb_cmd2 *mode_cmd,
+			     const struct drm_framebuffer_funcs *funcs);
+struct drm_framebuffer *
+drm_fb_gem_create(struct drm_device *dev, struct drm_file *file,
+		  const struct drm_mode_fb_cmd2 *mode_cmd);
+
+
+int drm_fb_gem_prepare_fb(struct drm_plane *plane,
+			  struct drm_plane_state *state);
+
+
+
+
+#ifdef CONFIG_DEBUG_FS
+struct seq_file;
+
+int drm_fb_gem_debugfs_show(struct seq_file *m, void *arg);
+#endif
+
+#endif
+