From patchwork Fri Apr 18 13:49:34 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Devin Heitmueller X-Patchwork-Id: 23717 X-Patchwork-Delegate: hverkuil@xs4all.nl Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from ) id 1Wb9Ai-0001SC-T9; Fri, 18 Apr 2014 15:49:40 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.72/mailfrontend-7) with esmtp id 1Wb9Ag-0008Ey-2I; Fri, 18 Apr 2014 15:49:40 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751314AbaDRNtg (ORCPT + 1 other); Fri, 18 Apr 2014 09:49:36 -0400 Received: from mail-qa0-f44.google.com ([209.85.216.44]:62968 "EHLO mail-qa0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751185AbaDRNtf (ORCPT ); Fri, 18 Apr 2014 09:49:35 -0400 Received: by mail-qa0-f44.google.com with SMTP id hw13so1588793qab.31 for ; Fri, 18 Apr 2014 06:49:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=XbxK66pCt9OBWivrVt8Oy4z8+Kd50GV1xT26fprtf/s=; b=HkNJ5crkCIgOKOWF1t70KBVj+u0o9cDpCe/KrF82Xa8S26sLYSUltUJ8+FSViwoqgW N4fiG386YUCjfvRqtynwnFJ9Qc9of0nzJ4AQmrAxmMv3UlAfV7q8QMJaLQQ2wOor5RJO AJtK4576twry7LSnab9JAPXNPUuQnj1O5IpHwrUT6ATW3mQ5UrEZzp7LhX2ETltAsrjz IB34tCbo2DFaO3nPVmiVxMAmZNCxkIot53SbRxUbRQ0ZIx6bd0oheMOp+mL6dsQoQuI8 Jxq6624EaTHACECfhxvRASh0KrQgOe5aDv/+RlFfqaQ2Wb/DhUk6dH2P45wRIgWnrcqG xAjQ== X-Gm-Message-State: ALoCoQm7YfosGyYCWjXgjbO6949B21SAAWN7uWWPosCk4XNNsQuDVzO8ew9ruUMSzaAA0eXF0PDO MIME-Version: 1.0 X-Received: by 10.224.115.68 with SMTP id h4mr20726615qaq.35.1397828974432; Fri, 18 Apr 2014 06:49:34 -0700 (PDT) Received: by 10.140.49.198 with HTTP; Fri, 18 Apr 2014 06:49:34 -0700 (PDT) In-Reply-To: <5351106E.4080700@xs4all.nl> References: <1394454049-12879-1-git-send-email-hverkuil@xs4all.nl> <1394454049-12879-4-git-send-email-hverkuil@xs4all.nl> <20140416192343.30a5a8fc@samsung.com> <534F0553.2000808@xs4all.nl> <20140416231730.6252aae7@samsung.com> <534FA3BF.2010308@xs4all.nl> <20140417101310.0111d236@samsung.com> <5351106E.4080700@xs4all.nl> Date: Fri, 18 Apr 2014 09:49:34 -0400 Message-ID: Subject: Re: [REVIEW PATCH 3/3] saa7134: convert to vb2 From: Devin Heitmueller To: Hans Verkuil Cc: Mauro Carvalho Chehab , Linux Media Mailing List , Hans Verkuil Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2014.4.18.133918 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, MSGID_ADDED_BY_MTA 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_2000_2999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, DATE_TZ_NA 0, URI_ENDS_IN_HTML 0, WEBMAIL_SOURCE 0, __ANY_URI 0, __BOUNCE_CHALLENGE_SUBJ 0, __BOUNCE_NDR_SUBJ_EXEMPT 0, __CP_MEDIA_BODY 0, __CP_URI_IN_BODY 0, __CT 0, __CT_TEXT_PLAIN 0, __FORWARDED_MSG 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __PHISH_SPEAR_HTTP_RECEIVED 0, __PHISH_SPEAR_STRUCTURE_1 0, __PHISH_SPEAR_STRUCTURE_2 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __SUBJ_ALPHA_NEGATE 0, __TO_MALFORMED_2 0, __URI_NS , __YOUTUBE_RCVD 0' > This was a bit confusing following the previous paragraph. I meant to say that the > *saa7134* userptr implementation is not USERPTR at all but PAGE_ALIGNED_USERPTR. > > A proper USERPTR implementation (like in bttv) can use any malloc()ed pointer as > it should, but saa7134 can't as it requires the application to align the pointer > to a page boundary, which is non-standard. It's probably worth mentioning at this point that there's a bug in videobuf2_vmalloc() where it forces the buffer provided by userptr mode to be page aligned. This causes issues if you hand it a buffer that isn't actually page aligned, as it writes to an arbitrary offset into the buffer instead of the start of the buffer you provided. I've had the following patch in my private tree for months, but had been hesitant to submit it since I didn't know if it would effect other implementations. I wasn't sure if USERPTR mode required the buffers to be page aligned and I was violating the spec. Devin From: Devin Heitmueller Date: Fri, 17 May 2013 19:53:02 +0000 (-0400) Subject: Fix alignment bug when using VB2 with userptr mode Fix alignment bug when using VB2 with userptr mode If we pass in a USERPTR buffer that isn't page aligned, the driver will align it (and not tell userland). The result is that the driver thinks the video starts in one place while userland thinks it starts in another. This manifests itself as the video being horizontally shifted and there being garbage from the start of the field up to that point. This problem was seen with both the em28xx and uvc drivers when testing on the Davinci platform (where the buffers allocated are not necessarily page aligned). diff --git a/linux/drivers/media/v4l2-core/videobuf2-vmalloc.c b/linux/drivers/media/v4l2-core/videobuf2-vmalloc.c index 94efa04..ad7ce7c 100644 --- a/linux/drivers/media/v4l2-core/videobuf2-vmalloc.c +++ b/linux/drivers/media/v4l2-core/videobuf2-vmalloc.c @@ -82,7 +82,7 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx, unsigned long vaddr, return NULL; buf->write = write; - offset = vaddr & ~PAGE_MASK; + offset = 0; buf->size = size;