From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lars-Peter Clausen Subject: Re: [PATCH v2] DRM: Add DRM kms/fb cma helper Date: Thu, 31 May 2012 11:34:37 +0200 Message-ID: <4FC73B2D.3020406@metafoo.de> References: <1338391815-12050-1-git-send-email-lars@metafoo.de> <20120531081355.GP30400@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mailhost.informatik.uni-hamburg.de (mailhost.informatik.uni-hamburg.de [134.100.9.70]) by gabe.freedesktop.org (Postfix) with ESMTP id E47A89E7CE for ; Thu, 31 May 2012 02:39:14 -0700 (PDT) In-Reply-To: <20120531081355.GP30400@pengutronix.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: Sascha Hauer Cc: kernel@pengutronix.de, Laurent Pinchart , dri-devel@lists.freedesktop.org, Rob Clark List-Id: dri-devel@lists.freedesktop.org On 05/31/2012 10:13 AM, Sascha Hauer wrote: > On Wed, May 30, 2012 at 05:30:15PM +0200, Lars-Peter Clausen wrote: >> This patch 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 >> >> --- >> Changes since v1: >> * Some spelling fixes >> * Add missing kfree in drm_fb_cma_alloc error path >> * Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second >> parameter to specify the plane. >> --- >> drivers/gpu/drm/Kconfig | 11 + >> drivers/gpu/drm/Makefile | 1 + >> drivers/gpu/drm/drm_fb_cma_helper.c | 384 +++++++++++++++++++++++++++++++++++ >> include/drm/drm_fb_cma_helper.h | 27 +++ >> 4 files changed, 423 insertions(+) >> create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c >> create mode 100644 include/drm/drm_fb_cma_helper.h >> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> index a78dfbe..e7c0a3d 100644 >> --- a/drivers/gpu/drm/Kconfig >> +++ b/drivers/gpu/drm/Kconfig >> @@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER >> help >> Choose this if you need the GEM cma helper functions >> >> +config DRM_KMS_CMA_HELPER >> + tristate >> + select DRM_GEM_CMA_HELPER >> + select DRM_KMS_HELPER >> + select FB_SYS_FILLRECT >> + select FB_SYS_COPYAREA >> + select FB_SYS_IMAGEBLIT >> + help >> + Choose this if you need the KMS cma helper functions >> + >> + >> config DRM_TDFX >> tristate "3dfx Banshee/Voodoo3+" >> depends on DRM && PCI >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile >> index 6e9e948..5dcb1a5 100644 >> --- a/drivers/gpu/drm/Makefile >> +++ b/drivers/gpu/drm/Makefile >> @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o >> drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o >> >> drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o >> +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o >> >> obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o >> >> 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..8821a98 >> --- /dev/null >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -0,0 +1,384 @@ >> +/* >> + * drm kms/fb cma (contiguous memory allocator) helper functions >> + * >> + * Copyright (C) 2012 Analog Device Inc. >> + * Author: Lars-Peter Clausen >> + * >> + * Based on udl_fbdev.c >> + * Copyright (C) 2012 Red Hat >> + * >> + * 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. >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +struct drm_fb_cma { >> + struct drm_framebuffer fb; >> + struct drm_gem_cma_obj *obj[4]; > > Can we have a define for this magic '4'? It is used several times in > this patch. Yes, had the same though. The magic 4 is actually used all through all of DRM. Something like '#define DRM_MAX_PLANES 4' probably makes sense. > >> +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, >> + struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj, >> + unsigned int num_planes) >> +{ >> + struct drm_fb_cma *fb_cma; >> + int ret; >> + int i; >> + >> + fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL); >> + if (!fb_cma) >> + return ERR_PTR(-ENOMEM); >> + >> + ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs); >> + if (ret) { >> + dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret); >> + kfree(fb_cma); >> + return ERR_PTR(ret); >> + } >> + >> + drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd); >> + >> + for (i = 0; i < num_planes; i++) >> + fb_cma->obj[i] = obj[i]; > > Check for valid num_planes before this loop? > Hm, I think the callers already take care of this. drm_format_num_planes will always return a valid number and the other caller passes 1 unconditionally. > > Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object > in the patch I just sent. > Do you think it makes sense for you to carry this patch as part of your iMX DRM series? Thanks, - Lars