Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751806AbcDTVXB (ORCPT ); Wed, 20 Apr 2016 17:23:01 -0400 Received: from mail-oi0-f43.google.com ([209.85.218.43]:35289 "EHLO mail-oi0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751092AbcDTVW6 convert rfc822-to-8bit (ORCPT ); Wed, 20 Apr 2016 17:22:58 -0400 MIME-Version: 1.0 X-Originating-IP: [2a02:168:56b5:0:ac27:b86c:7764:9429] In-Reply-To: <5717D679.5070809@tronnes.org> References: <1461165929-11344-1-git-send-email-noralf@tronnes.org> <1461165929-11344-9-git-send-email-noralf@tronnes.org> <20160420175903.GS2510@phenom.ffwll.local> <5717D679.5070809@tronnes.org> Date: Wed, 20 Apr 2016 23:22:52 +0200 X-Google-Sender-Auth: Kr0kkCf_0VCnivZb4Hee4QjcDrw Message-ID: Subject: Re: [PATCH 8/8] drm/udl: Use drm_fb_helper deferred_io support From: Daniel Vetter To: =?UTF-8?Q?Noralf_Tr=C3=B8nnes?= Cc: dri-devel , Linux Fbdev development list , Laurent Pinchart , Tomi Valkeinen , Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1878 Lines: 44 On Wed, Apr 20, 2016 at 9:20 PM, Noralf Trønnes wrote: >>> @@ -330,20 +203,20 @@ static int udl_fb_open(struct fb_info *info, int >>> user) >>> ufbdev->fb_count++; >>> - if (fb_defio && (info->fbdefio == NULL)) { >>> - /* enable defio at last moment if not disabled by client >>> */ >>> + if (!info->fbdefio) { >>> + /* enable defio at last moment */ >>> struct fb_deferred_io *fbdefio; >>> fbdefio = kmalloc(sizeof(struct fb_deferred_io), >>> GFP_KERNEL); >>> - >>> if (fbdefio) { >>> fbdefio->delay = DL_DEFIO_WRITE_DELAY; >>> - fbdefio->deferred_io = udlfb_dpy_deferred_io; >>> + fbdefio->deferred_io = drm_fb_helper_deferred_io; >> >> Why all these changes here? I figured just exchanging the deferred_io >> pointer should be all that's really needed in the setup code? > > > Because we always need to initialize deferred_io since we use it's worker > to handle fb_{fillrect,copyarea,imageblit} damage in drm_fb_helper. > > The previous code didn't use deferred_io to handle these, it just handled > the damage directly unless it was running in atomic context, in which case > it just recorded the damage and returned, leaving it to the next call to > push the changes. That kind of explanation needs to be added to the commit message. I completely missed that udl doesn't have an async work item for defio from atomic. > And in the following code I fixed a null pointer problem as well, maybe I > shouldn't have packed it in here. If fbdefio allocation fails == NULL, > fb_deferred_io_init() will trigger a BUG(). Yeah, better to split that out into a separate bugfix I think. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch