On Thursday 19 July 2012 14:46:38 Laurent Pinchart wrote:
On Monday 02 July 2012 16:37:47 Lars-Peter Clausen wrote:
This patchset introduces a set of helper function for implementing the KMS framebuffer layer for drivers which use the drm gem CMA helper function.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Note: This patch depends on Sascha's "DRM: add drm gem CMA helper" patch
Changes since v2:
- Adapt to changes in the GEM CMA helper
- Add basic buffer size checking in drm_fb_cma_create
Changes since v1:
- Some spelling fixes
- Add missing kfree in drm_fb_cma_alloc error path
- Add multi-plane support
drivers/gpu/drm/Kconfig | 10 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_fb_cma_helper.c | 393 ++++++++++++++++++++++++++++++++ include/drm/drm_fb_cma_helper.h | 27 +++ 4 files changed, 431 insertions(+) create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c create mode 100644 include/drm/drm_fb_cma_helper.h
[snip]
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c new file mode 100644 index 0000000..9042233 --- /dev/null +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
[snip]
+/**
- drm_fb_cma_create() - (struct drm_mode_config_funcs *)->fb_create
callback function
- If your hardware has special alignment or pitch requirements these
should be
- checked before calling this function.
- */
+struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
- struct drm_file *file_priv, struct drm_mode_fb_cmd2 *mode_cmd)
+{
- struct drm_fb_cma *fb_cma;
- struct drm_gem_cma_object *objs[4];
- struct drm_gem_object *obj;
- int ret;
- int i;
- for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
obj = drm_gem_object_lookup(dev, file_priv, mode_cmd->handles[i]);if (!obj) {dev_err(dev->dev, "Failed to lookup GEM object\n");ret = -ENXIO;goto err_gem_object_unreference;}if (obj->size < mode_cmd->height * mode_cmd->pitches[i]) {Shouldn't this be
if (obj->size < mode_cmd->height * mode_cmd->pitches[i] + mode_cmd->offsets[i])
?
And, for multiplanar formats, we need to divide height by the plane vertical subsampling factor. What about something like this ?
struct drm_fb_cma *fb_cma; struct drm_gem_cma_object *objs[4]; struct drm_gem_object *obj; int ret; int i;
for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) { unsigned int min_size;
obj = drm_gem_object_lookup(dev, file_priv, mode_cmd->handles[i]); if (!obj) { dev_err(dev->dev, "Failed to lookup GEM object\n"); ret = -ENXIO; goto err_gem_object_unreference; }
min_size = mode_cmd->pitches[i] * mode_cmd->height; if (i) min_size /= drm_format_vert_chroma_subsampling(mode_cmd->pixel_format); min_size += mode_cmd->offsets[i];
if (obj->size < min_size) { drm_gem_object_unreference_unlocked(obj); ret = -EINVAL; goto err_gem_object_unreference; } objs[i] = to_drm_gem_cma_obj(obj); }
On a totally unrelated note, I'm wondering whether we should replace the various DRM format information calls (drm_format_num_planes, drm_fb_get_bpp_depth, ...) by a single drm_get_format_info() function that would return a const pointer to format information. We could keep the existing API as static inline functions. This would save CPU time when we need to repeatedly access various information about a single format.
drm_gem_object_unreference_unlocked(obj);ret = -EINVAL;goto err_gem_object_unreference;}objs[i] = to_drm_gem_cma_obj(obj);- }
- fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i);
- if (IS_ERR(fb_cma)) {
ret = PTR_ERR(fb_cma);goto err_gem_object_unreference;- }
- return &fb_cma->fb;
+err_gem_object_unreference:
- for (i--; i >= 0; i--)
drm_gem_object_unreference_unlocked(&objs[i]->base);- return ERR_PTR(ret);
+} +EXPORT_SYMBOL_GPL(drm_fb_cma_create);