From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v3] DRM: Add DRM kms/fb cma helper Date: Thu, 19 Jul 2012 15:33:16 +0200 Message-ID: <1951188.4zC90J4lPi@avalon> References: <1341239867-10384-1-git-send-email-lars@metafoo.de> <3176651.XRgMMYj3qf@avalon> <50080CE2.9070803@metafoo.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [95.142.166.194]) by gabe.freedesktop.org (Postfix) with ESMTP id 07C649E9F3 for ; Thu, 19 Jul 2012 06:33:10 -0700 (PDT) In-Reply-To: <50080CE2.9070803@metafoo.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Lars-Peter Clausen Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org 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 > >> > >> --- > >> 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