Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751153AbcD1HSF (ORCPT ); Thu, 28 Apr 2016 03:18:05 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:34141 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750810AbcD1HSD (ORCPT ); Thu, 28 Apr 2016 03:18:03 -0400 Date: Thu, 28 Apr 2016 09:17:53 +0200 From: Daniel Vetter To: Noralf =?iso-8859-1?Q?Tr=F8nnes?= Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org, laurent.pinchart@ideasonboard.com, tomi.valkeinen@ti.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 3/7] drm/fb-helper: Add fb_deferred_io support Message-ID: <20160428071753.GJ2558@phenom.ffwll.local> Mail-Followup-To: Noralf =?iso-8859-1?Q?Tr=F8nnes?= , dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org, laurent.pinchart@ideasonboard.com, tomi.valkeinen@ti.com, linux-kernel@vger.kernel.org References: <1461780996-8612-1-git-send-email-noralf@tronnes.org> <1461780996-8612-4-git-send-email-noralf@tronnes.org> <20160427192052.GG2558@phenom.ffwll.local> <57211204.7080703@tronnes.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <57211204.7080703@tronnes.org> X-Operating-System: Linux phenom 4.6.0-rc5+ User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2630 Lines: 65 On Wed, Apr 27, 2016 at 09:24:52PM +0200, Noralf Tr?nnes wrote: > > Den 27.04.2016 21:20, skrev Daniel Vetter: > >On Wed, Apr 27, 2016 at 08:16:32PM +0200, Noralf Tr?nnes wrote: > >>This adds deferred io support to drm_fb_helper. > >>The fbdev framebuffer changes are flushed using the callback > >>(struct drm_framebuffer *)->funcs->dirty() by a dedicated worker > >>ensuring that it always runs in process context. > >> > >>Signed-off-by: Noralf Tr?nnes > >>--- > >> > >>Changes since v2: > >>- FB_DEFERRED_IO is now always selected by DRM_KMS_FB_HELPER, ifdef removed > >>- The drm_clip_rect utility functions are dropped, so open code it > >>- docs: use & to denote structs > >> > >>Changes since v1: > >>- Use a dedicated worker to run the framebuffer flushing like qxl does > >>- Add parameter descriptions to drm_fb_helper_deferred_io > >> > >> drivers/gpu/drm/Kconfig | 1 + > >> drivers/gpu/drm/drm_fb_helper.c | 109 +++++++++++++++++++++++++++++++++++++++- > >> include/drm/drm_fb_helper.h | 15 ++++++ > >> 3 files changed, 124 insertions(+), 1 deletion(-) > >> > >>diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > >>index 9e4f2f1..8e6f34b 100644 > >>--- a/drivers/gpu/drm/Kconfig > >>+++ b/drivers/gpu/drm/Kconfig > >>@@ -52,6 +52,7 @@ config DRM_KMS_FB_HELPER > >> select FB_CFB_FILLRECT > >> select FB_CFB_COPYAREA > >> select FB_CFB_IMAGEBLIT > >>+ select FB_DEFERRED_IO > >> help > >> FBDEV helpers for KMS drivers. > >> > >>diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > >>index 855108e..5112b5d 100644 > >>--- a/drivers/gpu/drm/drm_fb_helper.c > >>+++ b/drivers/gpu/drm/drm_fb_helper.c > >>@@ -48,6 +48,8 @@ MODULE_PARM_DESC(fbdev_emulation, > >> > >> static LIST_HEAD(kernel_fb_helper_list); > >> > >>+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper); > >Instead of this forward decl just inline the few setup commands into > >drm_fb_helper_prepare. > > That would mean that I have to forward declare drm_fb_helper_dirty_work() > instead. Is that better? > Or should I relocate the functions? Yeah, just move them all I think. This means that usually the setup function for a component is at the very bottom, and also that you have the inverted reading order with first reading leaf/helper functions, then the bigger ones that assemble things together. Personally I find that backwards, but otoh consistency is more important. And avoid forward decls for static functions is standard style in the kernel. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch