Hi Lars, On Thursday 19 July 2012 15:34:26 Lars-Peter Clausen wrote: > On 07/19/2012 02:46 PM, 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@xxxxxxxxxx> > >> > >> --- > >> 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]) > > > > ? > > That's actually a good question. I'd expect the offset to be included in the > pitch. > > If you access pixels like mem[offset + x * bpp + y * pitch] then pitch has > to be greater equal to offset + max_x * bpp, otherwise you'd have > overlapping lines. My understanding is that the offset is a linear offset from the start of the buffer to allow X/Y panning. In that case the pitch is a frame buffer property that is not be influenced by the offset at all. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel