| Message ID | 20170616073915.5027-6-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1dLlrC-0005nx-Eo; Fri, 16 Jun 2017 07:39:50 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.89/mailfrontend-8) with esmtp id 1dLlrA-0004vQ-km; Fri, 16 Jun 2017 09:39:50 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752771AbdFPHjn (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Fri, 16 Jun 2017 03:39:43 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:34102 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752729AbdFPHjm (ORCPT <rfc822;linux-media@vger.kernel.org>); Fri, 16 Jun 2017 03:39:42 -0400 Received: by mail-pf0-f196.google.com with SMTP id d5so4719604pfe.1 for <linux-media@vger.kernel.org>; Fri, 16 Jun 2017 00:39:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=gU4wdEtxQBVsvKw+UEW+7uHdoN0bLDTeQIaGN2P1nGw=; b=g2CJVN3D+ciNJRSFeEHHOUS3t36+SrqOI78PHWgHW8bHEd9TxXX5Cy5rv1pKY+u3u/ D+i/Sm0h3QMdpVNQ9ISRAhmzw0DIXkH/QCWijVAUBN/Bwl8optePSflBm97gAjc6BrrG MDbjk5EiTfXEOUrrldTH9+wy3SnduLWTultzfDpetf6geo9XXpPrENf5Rg+KO+oyovWe z2wvbMHHO8cSFQVE4p5VoNO+AApqPbSskSC8F/Jrm+XktRNstn0iUXbGAu98qxYemp6T DaPu1XT7a7JYkUwMoRj8TiVuqRO/+x5EDiGdkz6ySKpLqxaAL8oKezXP+lSVazmDjT3u he7w== X-Gm-Message-State: AKS2vOxQBBc5XIhrCr1wZyGeLCwYOB1vwjyBsxGy+zLUE7M1qOka2EtP a7XPNlG2Mu9IwGMuJ40= X-Received: by 10.99.171.71 with SMTP id k7mr9619713pgp.244.1497598776371; Fri, 16 Jun 2017 00:39:36 -0700 (PDT) Received: from jade.nodan1.kt.home.ne.jp (202-72-64-244.koalanet.ne.jp. [202.72.64.244]) by smtp.gmail.com with ESMTPSA id a2sm2760568pfj.8.2017.06.16.00.39.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 16 Jun 2017 00:39:35 -0700 (PDT) From: Gustavo Padovan <gustavo@padovan.org> To: linux-media@vger.kernel.org Cc: Hans Verkuil <hverkuil@xs4all.nl>, Javier Martinez Canillas <javier@osg.samsung.com>, Mauro Carvalho Chehab <mchehab@osg.samsung.com>, Shuah Khan <shuahkh@osg.samsung.com>, Gustavo Padovan <gustavo.padovan@collabora.com> Subject: [PATCH 05/12] [media] vivid: assign the specific device to the vb2_queue->dev Date: Fri, 16 Jun 2017 16:39:08 +0900 Message-Id: <20170616073915.5027-6-gustavo@padovan.org> X-Mailer: git-send-email 2.9.4 In-Reply-To: <20170616073915.5027-1-gustavo@padovan.org> References: <20170616073915.5027-1-gustavo@padovan.org> Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2017.6.16.73017 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_2000_2999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, IN_REP_TO 0, LEGITIMATE_SIGNS 0, MSG_THREAD 0, MULTIPLE_REAL_RCPTS 0, NO_URI_HTTPS 0, REFERENCES 0, __ANY_URI 0, __CC_NAME 0, __CC_NAME_DIFF_FROM_ACC 0, __CC_REAL_NAMES 0, __HAS_CC_HDR 0, __HAS_FROM 0, __HAS_LIST_ID 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MIME_TEXT_P 0, __MIME_TEXT_P1 0, __MULTIPLE_RCPTS_CC_X2 0, __NO_HTML_TAG_RAW 0, __REFERENCES 0, __SANE_MSGID 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS , __YOUTUBE_RCVD 0' |
Commit Message
Gustavo F. Padovan
June 16, 2017, 7:39 a.m. UTC
From: Gustavo Padovan <gustavo.padovan@collabora.com> Instead of assigning the global v4l2 device, assign the specific device. This was causing trouble when using using V4L2 events with vivid devices. The device's queue should be the same we opened in userspace. Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> --- drivers/media/platform/vivid/vivid-core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Comments
On 06/16/17 09:39, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > Instead of assigning the global v4l2 device, assign the specific device. > This was causing trouble when using using V4L2 events with vivid > devices. The device's queue should be the same we opened in userspace. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> Can you add a line to the commit log that says that this is needed for the upcoming V4L2_EVENT_BUF_QUEUED support? This log message suggests that the current vivid code is wrong, which it isn't. It just needs to be changed so V4L2_EVENT_BUF_QUEUED can be supported. After making that change: Acked-by: Hans Verkuil <hans.verkuil@cisco.com> Regards, Hans > --- > drivers/media/platform/vivid/vivid-core.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c > index ef344b9..8843170 100644 > --- a/drivers/media/platform/vivid/vivid-core.c > +++ b/drivers/media/platform/vivid/vivid-core.c > @@ -1070,7 +1070,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > q->min_buffers_needed = 2; > q->lock = &dev->mutex; > - q->dev = dev->v4l2_dev.dev; > + q->dev = &dev->vid_cap_dev.dev; > > ret = vb2_queue_init(q); > if (ret) > @@ -1090,7 +1090,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > q->min_buffers_needed = 2; > q->lock = &dev->mutex; > - q->dev = dev->v4l2_dev.dev; > + q->dev = &dev->vid_out_dev.dev; > > ret = vb2_queue_init(q); > if (ret) > @@ -1110,7 +1110,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > q->min_buffers_needed = 2; > q->lock = &dev->mutex; > - q->dev = dev->v4l2_dev.dev; > + q->dev = &dev->vbi_cap_dev.dev; > > ret = vb2_queue_init(q); > if (ret) > @@ -1130,7 +1130,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > q->min_buffers_needed = 2; > q->lock = &dev->mutex; > - q->dev = dev->v4l2_dev.dev; > + q->dev = &dev->vbi_out_dev.dev; > > ret = vb2_queue_init(q); > if (ret) > @@ -1149,7 +1149,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > q->min_buffers_needed = 8; > q->lock = &dev->mutex; > - q->dev = dev->v4l2_dev.dev; > + q->dev = &dev->sdr_cap_dev.dev; > > ret = vb2_queue_init(q); > if (ret) >
On 06/16/2017 01:39 AM, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > Instead of assigning the global v4l2 device, assign the specific device. > This was causing trouble when using using V4L2 events with vivid > devices. The device's queue should be the same we opened in userspace. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> > --- > drivers/media/platform/vivid/vivid-core.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c > index ef344b9..8843170 100644 > --- a/drivers/media/platform/vivid/vivid-core.c > +++ b/drivers/media/platform/vivid/vivid-core.c > @@ -1070,7 +1070,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > q->min_buffers_needed = 2; > q->lock = &dev->mutex; > - q->dev = dev->v4l2_dev.dev; > + q->dev = &dev->vid_cap_dev.dev; Does this work in all cases? My concern is that in some code paths q->dev might be used to initiate release perhaps. Fore example v4l2_dev.release is vivid_dev_release() dev->v4l2_dev.release = vivid_dev_release; vid_cap_dev release is video_device_release_empty This is one difference, but there might be others and the code paths that might depend on q->dev being the v4l2_dev.dev which is the global dev. > > ret = vb2_queue_init(q); > if (ret) > @@ -1090,7 +1090,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > q->min_buffers_needed = 2; > q->lock = &dev->mutex; > - q->dev = dev->v4l2_dev.dev; > + q->dev = &dev->vid_out_dev.dev; > > ret = vb2_queue_init(q); > if (ret) > @@ -1110,7 +1110,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > q->min_buffers_needed = 2; > q->lock = &dev->mutex; > - q->dev = dev->v4l2_dev.dev; > + q->dev = &dev->vbi_cap_dev.dev; > > ret = vb2_queue_init(q); > if (ret) > @@ -1130,7 +1130,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > q->min_buffers_needed = 2; > q->lock = &dev->mutex; > - q->dev = dev->v4l2_dev.dev; > + q->dev = &dev->vbi_out_dev.dev; > > ret = vb2_queue_init(q); > if (ret) > @@ -1149,7 +1149,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > q->min_buffers_needed = 8; > q->lock = &dev->mutex; > - q->dev = dev->v4l2_dev.dev; > + q->dev = &dev->sdr_cap_dev.dev; > > ret = vb2_queue_init(q); > if (ret) > thanks, -- Shuah
2017-07-07 Shuah Khan <shuahkh@osg.samsung.com>: > On 06/16/2017 01:39 AM, Gustavo Padovan wrote: > > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > > > Instead of assigning the global v4l2 device, assign the specific device. > > This was causing trouble when using using V4L2 events with vivid > > devices. The device's queue should be the same we opened in userspace. > > > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> > > --- > > drivers/media/platform/vivid/vivid-core.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c > > index ef344b9..8843170 100644 > > --- a/drivers/media/platform/vivid/vivid-core.c > > +++ b/drivers/media/platform/vivid/vivid-core.c > > @@ -1070,7 +1070,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > > q->min_buffers_needed = 2; > > q->lock = &dev->mutex; > > - q->dev = dev->v4l2_dev.dev; > > + q->dev = &dev->vid_cap_dev.dev; > > Does this work in all cases? My concern is that in some code paths > q->dev might be used to initiate release perhaps. > > Fore example v4l2_dev.release is vivid_dev_release() > dev->v4l2_dev.release = vivid_dev_release; > > vid_cap_dev release is video_device_release_empty > > This is one difference, but there might be others and the code paths > that might depend on q->dev being the v4l2_dev.dev which is the global > dev. Sure, I'll check this again. Gustavo
2017-07-07 Shuah Khan <shuahkh@osg.samsung.com>: > On 06/16/2017 01:39 AM, Gustavo Padovan wrote: > > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > > > Instead of assigning the global v4l2 device, assign the specific device. > > This was causing trouble when using using V4L2 events with vivid > > devices. The device's queue should be the same we opened in userspace. > > > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> > > --- > > drivers/media/platform/vivid/vivid-core.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c > > index ef344b9..8843170 100644 > > --- a/drivers/media/platform/vivid/vivid-core.c > > +++ b/drivers/media/platform/vivid/vivid-core.c > > @@ -1070,7 +1070,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) > > q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > > q->min_buffers_needed = 2; > > q->lock = &dev->mutex; > > - q->dev = dev->v4l2_dev.dev; > > + q->dev = &dev->vid_cap_dev.dev; > > Does this work in all cases? My concern is that in some code paths > q->dev might be used to initiate release perhaps. > > Fore example v4l2_dev.release is vivid_dev_release() > dev->v4l2_dev.release = vivid_dev_release; > > vid_cap_dev release is video_device_release_empty > > This is one difference, but there might be others and the code paths > that might depend on q->dev being the v4l2_dev.dev which is the global > dev. The release call comes from the v4l2-core as we pass the v4l2 device on v4l2_register_device(). q->dev is in just one ocasion and setting it to a different device doesn't change the behavior. That code just check if it is queued or not. Gustavo
diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c index ef344b9..8843170 100644 --- a/drivers/media/platform/vivid/vivid-core.c +++ b/drivers/media/platform/vivid/vivid-core.c @@ -1070,7 +1070,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; q->min_buffers_needed = 2; q->lock = &dev->mutex; - q->dev = dev->v4l2_dev.dev; + q->dev = &dev->vid_cap_dev.dev; ret = vb2_queue_init(q); if (ret) @@ -1090,7 +1090,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; q->min_buffers_needed = 2; q->lock = &dev->mutex; - q->dev = dev->v4l2_dev.dev; + q->dev = &dev->vid_out_dev.dev; ret = vb2_queue_init(q); if (ret) @@ -1110,7 +1110,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; q->min_buffers_needed = 2; q->lock = &dev->mutex; - q->dev = dev->v4l2_dev.dev; + q->dev = &dev->vbi_cap_dev.dev; ret = vb2_queue_init(q); if (ret) @@ -1130,7 +1130,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; q->min_buffers_needed = 2; q->lock = &dev->mutex; - q->dev = dev->v4l2_dev.dev; + q->dev = &dev->vbi_out_dev.dev; ret = vb2_queue_init(q); if (ret) @@ -1149,7 +1149,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; q->min_buffers_needed = 8; q->lock = &dev->mutex; - q->dev = dev->v4l2_dev.dev; + q->dev = &dev->sdr_cap_dev.dev; ret = vb2_queue_init(q); if (ret)