From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E6DA4C47094 for ; Mon, 7 Jun 2021 12:39:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C9B8960FEE for ; Mon, 7 Jun 2021 12:39:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230217AbhFGMlj (ORCPT ); Mon, 7 Jun 2021 08:41:39 -0400 Received: from perceval.ideasonboard.com ([213.167.242.64]:57286 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230198AbhFGMli (ORCPT ); Mon, 7 Jun 2021 08:41:38 -0400 Received: from [192.168.1.111] (91-157-208-71.elisa-laajakaista.fi [91.157.208.71]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 858AB8DB; Mon, 7 Jun 2021 14:39:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1623069586; bh=W9n/33z2bW0ox0ItPnZjbzarLY7HCIwV9z97A+0qRw0=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=n8EN6u+Uh0jXPS0rCXR7EwoYDa7QPNsMiC1I2NO777iKVgkG2cTPdjQCEEPhAvKXP TkA79uHOqveDci2xNDao6aqLxDEE1wlo55u7/FoJRwoHBRC/lDxZbmJ0L4ayCcpzo9 9tG4ArW0cWGNFvk4onJ2ID50oWOrANXjhE6jfkN4= Subject: Re: [PATCH v3 32/38] media: ti-vpe: cal: use CSI-2 frame number To: Laurent Pinchart Cc: Pratyush Yadav , Lokesh Vutla , linux-media@vger.kernel.org References: <20210524110909.672432-1-tomi.valkeinen@ideasonboard.com> <20210524110909.672432-33-tomi.valkeinen@ideasonboard.com> From: Tomi Valkeinen Message-ID: Date: Mon, 7 Jun 2021 15:39:45 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On 04/06/2021 17:04, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Mon, May 24, 2021 at 02:09:03PM +0300, Tomi Valkeinen wrote: >> The driver fills buf->vb.sequence with an increasing number which is >> incremented by the driver. This feels a bit pointless, as the userspace >> could as well track that kind of number itself. Instead, lets use the > > s/lets/let's/ > >> frame number provided in the CSI-2 data from the sensor. >> >> Signed-off-by: Tomi Valkeinen >> --- >> drivers/media/platform/ti-vpe/cal.c | 7 +++++-- >> drivers/media/platform/ti-vpe/cal.h | 1 - >> 2 files changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c >> index 888706187fd1..62c45add4efe 100644 >> --- a/drivers/media/platform/ti-vpe/cal.c >> +++ b/drivers/media/platform/ti-vpe/cal.c >> @@ -493,7 +493,6 @@ void cal_ctx_unprepare(struct cal_ctx *ctx) >> >> void cal_ctx_start(struct cal_ctx *ctx) >> { >> - ctx->sequence = 0; >> ctx->dma.state = CAL_DMA_RUNNING; >> >> /* Configure the CSI-2, pixel processing and write DMA contexts. */ >> @@ -586,6 +585,10 @@ static inline void cal_irq_wdma_start(struct cal_ctx *ctx) >> static inline void cal_irq_wdma_end(struct cal_ctx *ctx) >> { >> struct cal_buffer *buf = NULL; >> + u32 frame_num; >> + >> + frame_num = cal_read(ctx->cal, CAL_CSI2_STATUS(ctx->phy->instance, >> + ctx->csi2_ctx)) & 0xffff; >> >> spin_lock(&ctx->dma.lock); >> >> @@ -607,7 +610,7 @@ static inline void cal_irq_wdma_end(struct cal_ctx *ctx) >> if (buf) { >> buf->vb.vb2_buf.timestamp = ktime_get_ns(); >> buf->vb.field = ctx->v_fmt.fmt.pix.field; >> - buf->vb.sequence = ctx->sequence++; >> + buf->vb.sequence = frame_num; > > We'll need something a bit more complicated. The CSI-2 frame number is > not mandatory, and when used, it is a 16-bit number starting at 1 and > counting to an unspecified value larger than one, resetting to 1 at the > end of the cycle. The V4L2 sequence number, on the other hand, is a > monotonic counter starting at 0 and wrapping only at 2^32-1. We should > thus keep a software sequence counter and > > - increase it by 1 if the frame number is zero > - increase it by frame_num - last_frame_num (with wrap-around of > frame_num handled) otherwise Ok... I wonder if we need a new field for this, though. The problem I was solving when I changed this to use the CSI-2 frame-number was how to associate a pixel frame and a metadata frame. Their CSI-2 frame-numbers match (as they are from the same original CSI-2 frame), so the userspace can use that to figure the matching frames. While the above method you suggest should give us identical sequence numbers for pixel and metadata, I think it's going a bit away from my intended purpose, and possibly risks ending up with different sequences for pixel and metadata. So do we need the sequence number, as it is currently, and something new for this buffer matching purpose? Tomi