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@xxxxxxxxxxx>
> > ---
>
> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
