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 at 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])
> >
> > ?
>
> 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
