From patchwork Tue Feb 20 16:43:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Scally X-Patchwork-Id: 19516 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 49F5CC3261 for ; Tue, 20 Feb 2024 16:43:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id F130B62816; Tue, 20 Feb 2024 17:43:37 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="LcByrR4I"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 153C062805 for ; Tue, 20 Feb 2024 17:43:25 +0100 (CET) Received: from mail.ideasonboard.com (cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5E19714B0; Tue, 20 Feb 2024 17:43:18 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1708447398; bh=SusT3ITljRDCsmlVRh/ICXW4OxPR3B3+GORNLGYAD4w=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LcByrR4ItRnyy8mugEt/t/GncQogvZku/K2GrlrSqOVHhpMd/0RaGdvKOF2ZSmMsz nOGTVuxPVj814ldhbN4Ka0xZPwua/8uQRAyWBZAV21vrPG9Y/DwjB6YR/v+o8GrRHN w6yTDIkJl3fTIgHkAVOfYch423I6bx8Hi7VweJ8g= From: Daniel Scally To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 5/7] libcamera: rkisp1: Remove RkISP1FrameInfo Date: Tue, 20 Feb 2024 16:43:15 +0000 Message-Id: <20240220164317.998477-6-dan.scally@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240220164317.998477-1-dan.scally@ideasonboard.com> References: <20240220164317.998477-1-dan.scally@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The RkISP1FrameInfo class existed to hold varying bits of info to help guarantee that a Request is not completed until all of the image buffers are returned to userspace and the metadata has been filled by the IPA module. Now that we're no longer using it for that function it can be removed. Remove the class and refactor its code out to the rest of the Pipeline Handler. Signed-off-by: Daniel Scally --- Changes in v2: - None src/libcamera/pipeline/rkisp1/rkisp1.cpp | 287 +++++------------------ 1 file changed, 64 insertions(+), 223 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 43e9d98b..ec4665a9 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -54,45 +54,13 @@ using InternalStream = Request::Private::InternalStream; class PipelineHandlerRkISP1; class RkISP1CameraData; -struct RkISP1FrameInfo { - unsigned int frame; - Request *request; - - FrameBuffer *paramBuffer; - FrameBuffer *statBuffer; - FrameBuffer *mainPathBuffer; - FrameBuffer *selfPathBuffer; - - bool paramDequeued; - bool metadataProcessed; -}; - -class RkISP1Frames -{ -public: - RkISP1Frames(PipelineHandler *pipe); - - RkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request, - bool isRaw); - int destroy(unsigned int frame); - void clear(); - - RkISP1FrameInfo *find(unsigned int frame); - RkISP1FrameInfo *find(FrameBuffer *buffer); - RkISP1FrameInfo *find(Request *request); - -private: - PipelineHandlerRkISP1 *pipe_; - std::map frameInfo_; -}; - class RkISP1CameraData : public Camera::Private { public: RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath, RkISP1SelfPath *selfPath) - : Camera::Private(pipe), frameInfo_(pipe), - mainPath_(mainPath), selfPath_(selfPath) + : Camera::Private(pipe), mainPath_(mainPath), + selfPath_(selfPath) { } @@ -104,7 +72,6 @@ public: std::unique_ptr sensor_; std::unique_ptr delayedCtrls_; std::vector ipaBuffers_; - RkISP1Frames frameInfo_; RkISP1MainPath *mainPath_; RkISP1SelfPath *selfPath_; @@ -172,7 +139,6 @@ private: } friend RkISP1CameraData; - friend RkISP1Frames; int initLinks(Camera *camera, const CameraSensor *sensor, const RkISP1CameraConfiguration &config); @@ -208,131 +174,6 @@ private: const MediaPad *ispSink_; }; -RkISP1Frames::RkISP1Frames(PipelineHandler *pipe) - : pipe_(static_cast(pipe)) -{ -} - -RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request, - bool isRaw) -{ - unsigned int frame = request->sequence(); - - FrameBuffer *paramBuffer = nullptr; - FrameBuffer *statBuffer = nullptr; - - if (!isRaw) { - if (pipe_->availableParamBuffers_.empty()) { - LOG(RkISP1, Error) << "Parameters buffer underrun"; - return nullptr; - } - - if (pipe_->availableStatBuffers_.empty()) { - LOG(RkISP1, Error) << "Statistic buffer underrun"; - return nullptr; - } - - paramBuffer = pipe_->availableParamBuffers_.front(); - pipe_->availableParamBuffers_.pop(); - paramBuffer->_d()->setRequest(request); - - statBuffer = pipe_->availableStatBuffers_.front(); - pipe_->availableStatBuffers_.pop(); - statBuffer->_d()->setRequest(request); - } - - FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_); - FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_); - - RkISP1FrameInfo *info = new RkISP1FrameInfo; - - info->frame = frame; - info->request = request; - info->paramBuffer = paramBuffer; - info->mainPathBuffer = mainPathBuffer; - info->selfPathBuffer = selfPathBuffer; - info->statBuffer = statBuffer; - info->paramDequeued = false; - info->metadataProcessed = false; - - frameInfo_[frame] = info; - - return info; -} - -int RkISP1Frames::destroy(unsigned int frame) -{ - RkISP1FrameInfo *info = find(frame); - if (!info) - return -ENOENT; - - pipe_->availableParamBuffers_.push(info->paramBuffer); - pipe_->availableStatBuffers_.push(info->statBuffer); - - frameInfo_.erase(info->frame); - - delete info; - - return 0; -} - -void RkISP1Frames::clear() -{ - for (const auto &entry : frameInfo_) { - RkISP1FrameInfo *info = entry.second; - - pipe_->availableParamBuffers_.push(info->paramBuffer); - pipe_->availableStatBuffers_.push(info->statBuffer); - - delete info; - } - - frameInfo_.clear(); -} - -RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame) -{ - auto itInfo = frameInfo_.find(frame); - - if (itInfo != frameInfo_.end()) - return itInfo->second; - - LOG(RkISP1, Fatal) << "Can't locate info from frame"; - - return nullptr; -} - -RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer) -{ - for (auto &itInfo : frameInfo_) { - RkISP1FrameInfo *info = itInfo.second; - - if (info->paramBuffer == buffer || - info->statBuffer == buffer || - info->mainPathBuffer == buffer || - info->selfPathBuffer == buffer) - return info; - } - - LOG(RkISP1, Fatal) << "Can't locate info from buffer"; - - return nullptr; -} - -RkISP1FrameInfo *RkISP1Frames::find(Request *request) -{ - for (auto &itInfo : frameInfo_) { - RkISP1FrameInfo *info = itInfo.second; - - if (info->request == request) - return info; - } - - LOG(RkISP1, Fatal) << "Can't locate info from request"; - - return nullptr; -} - PipelineHandlerRkISP1 *RkISP1CameraData::pipe() { return static_cast(Camera::Private::pipe()); @@ -386,20 +227,37 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision) void RkISP1CameraData::paramFilled(unsigned int frame) { PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe(); - RkISP1FrameInfo *info = frameInfo_.find(frame); - if (!info) + + Request *request = queuedRequests_[frame]; + ASSERT(request); + + FrameBuffer *paramBuffer = request->_d()->findInternalBuffer(InternalStream::Parameters); + ASSERT(paramBuffer); + + FrameBuffer *mainPathBuffer = request->findBuffer(&mainPathStream_); + FrameBuffer *selfPathBuffer = request->findBuffer(&selfPathStream_); + + if (pipe->availableStatBuffers_.empty()) { + LOG(RkISP1, Fatal) << "Statistics buffer underrun"; return; + } + + FrameBuffer *statBuffer = pipe->availableStatBuffers_.front(); + pipe->availableStatBuffers_.pop(); + request->_d()->addInternalBuffer(InternalStream::Statistics, statBuffer); - info->paramBuffer->_d()->metadata().planes()[0].bytesused = + paramBuffer->_d()->metadata().planes()[0].bytesused = sizeof(struct rkisp1_params_cfg); - pipe->param_->queueBuffer(info->paramBuffer); - pipe->stat_->queueBuffer(info->statBuffer); + pipe->param_->queueBuffer(paramBuffer); + pipe->stat_->queueBuffer(statBuffer); + + if (mainPathBuffer) + mainPath_->queueBuffer(mainPathBuffer); + + if (selfPath_ && selfPathBuffer) + selfPath_->queueBuffer(selfPathBuffer); - if (info->mainPathBuffer) - mainPath_->queueBuffer(info->mainPathBuffer); - if (selfPath_ && info->selfPathBuffer) - selfPath_->queueBuffer(info->selfPathBuffer); } void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame, @@ -410,15 +268,16 @@ void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame, void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata) { - RkISP1FrameInfo *info = frameInfo_.find(frame); - if (!info) - return; + Request *request = queuedRequests_[frame]; + ASSERT(request); - info->request->metadata().merge(metadata); - info->metadataProcessed = true; + FrameBuffer *statBuffer = request->_d()->findInternalBuffer(InternalStream::Statistics); + ASSERT(statBuffer); - pipe()->completeBuffer(info->request, info->statBuffer); - pipe()->tryCompleteRequest(info->request); + request->metadata().merge(metadata); + pipe()->availableStatBuffers_.push(statBuffer); + pipe()->completeBuffer(request, statBuffer); + pipe()->tryCompleteRequest(request); } /* ----------------------------------------------------------------------------- @@ -1016,7 +875,6 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) } ASSERT(data->queuedRequests_.empty()); - data->frameInfo_.clear(); freeBuffers(camera); @@ -1026,24 +884,34 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request) { RkISP1CameraData *data = cameraData(camera); + FrameBuffer *paramBuffer = nullptr; - RkISP1FrameInfo *info = data->frameInfo_.create(data, request, isRaw_); - if (!info) - return -ENOENT; + if (!isRaw_) { + if (availableParamBuffers_.empty()) { + LOG(RkISP1, Error) << "Parameters buffer underrun"; + return -ENOENT; + } + + paramBuffer = availableParamBuffers_.front(); + availableParamBuffers_.pop(); + paramBuffer->_d()->setRequest(request); + } - request->_d()->addInternalBuffer(InternalStream::Statistics, info->statBuffer); - request->_d()->addInternalBuffer(InternalStream::Parameters, info->paramBuffer); + request->_d()->addInternalBuffer(InternalStream::Parameters, paramBuffer); data->ipa_->queueRequest(request->sequence(), request->controls()); if (isRaw_) { - if (info->mainPathBuffer) - data->mainPath_->queueBuffer(info->mainPathBuffer); + FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_); + FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_); + + if (mainPathBuffer) + data->mainPath_->queueBuffer(mainPathBuffer); - if (data->selfPath_ && info->selfPathBuffer) - data->selfPath_->queueBuffer(info->selfPathBuffer); + if (data->selfPath_ && selfPathBuffer) + data->selfPath_->queueBuffer(selfPathBuffer); } else { data->ipa_->fillParamsBuffer(request->sequence(), - info->paramBuffer->cookie()); + paramBuffer->cookie()); } return 0; @@ -1238,20 +1106,9 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) void PipelineHandlerRkISP1::tryCompleteRequest(Request *request) { - RkISP1CameraData *data = cameraData(activeCamera_); - - RkISP1FrameInfo *info = data->frameInfo_.find(request); - if (!info) - return; - if (request->hasPendingBuffers()) return; - if (!isRaw_ && !info->paramDequeued) - return; - - data->frameInfo_.destroy(info->frame); - completeRequest(request); } @@ -1259,11 +1116,6 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); - - RkISP1FrameInfo *info = data->frameInfo_.find(buffer); - if (!info) - return; - const FrameMetadata &metadata = buffer->metadata(); Request *request = buffer->request(); @@ -1283,9 +1135,6 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer) data->ipa_->processStatsBuffer(request->sequence(), 0, ctrls); } - } else { - if (isRaw_) - info->metadataProcessed = true; } completeBuffer(request, buffer); @@ -1295,15 +1144,11 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer) void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer) { ASSERT(activeCamera_); - RkISP1CameraData *data = cameraData(activeCamera_); - - RkISP1FrameInfo *info = data->frameInfo_.find(buffer); - if (!info) - return; + Request *request = buffer->request(); - info->paramDequeued = true; - completeBuffer(info->request, buffer); - tryCompleteRequest(info->request); + completeBuffer(buffer->request(), buffer); + tryCompleteRequest(request); + availableParamBuffers_.push(buffer); } void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer) @@ -1312,18 +1157,14 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer) RkISP1CameraData *data = cameraData(activeCamera_); Request *request = buffer->request(); - RkISP1FrameInfo *info = data->frameInfo_.find(buffer); - if (!info) - return; - if (buffer->metadata().status == FrameMetadata::FrameCancelled) { - info->metadataProcessed = true; - completeBuffer(info->request, buffer); - tryCompleteRequest(info->request); + completeBuffer(request, buffer); + tryCompleteRequest(request); + availableStatBuffers_.push(buffer); return; } - data->ipa_->processStatsBuffer(request->sequence(), info->statBuffer->cookie(), + data->ipa_->processStatsBuffer(request->sequence(), buffer->cookie(), data->delayedCtrls_->get(buffer->metadata().sequence)); }