-
Notifications
You must be signed in to change notification settings - Fork 11
Part1 of : Feat double buffer pushtype #534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
e478626
f6ccad9
b654575
f9b47fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| // SPDX-FileCopyrightText: Deutsches Elektronen-Synchrotron DESY, MSK, ChimeraTK Project <[email protected]> | ||
| // SPDX-License-Identifier: LGPL-3.0-or-later | ||
| #pragma once | ||
|
|
||
| #include <nlohmann/json.hpp> | ||
|
|
||
| #include <optional> | ||
|
|
||
| namespace nlohmann { | ||
|
|
||
| template<typename T> | ||
| struct adl_serializer<std::optional<T>> { | ||
| static void to_json(json& j, const std::optional<T>& opt) { | ||
| if(opt.has_value()) { | ||
| j = *opt; | ||
| } | ||
| else { | ||
| j = nullptr; | ||
| } | ||
| } | ||
|
|
||
| static void from_json(const json& j, std::optional<T>& opt) { | ||
| if(j.is_null()) { | ||
| opt = std::nullopt; | ||
| } | ||
| else { | ||
| opt = j.get<T>(); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| } // namespace nlohmann |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| // SPDX-FileCopyrightText: Deutsches Elektronen-Synchrotron DESY, MSK, ChimeraTK Project <[email protected]> | ||
| // SPDX-License-Identifier: LGPL-3.0-or-later | ||
| #pragma once | ||
|
|
||
| #include "NDRegisterAccessor.h" | ||
| #include "NDRegisterAccessorDecorator.h" | ||
| #include "NumericAddressedBackend.h" | ||
| #include "RegisterInfo.h" | ||
| #include "TransferElement.h" | ||
|
|
||
| #include <string> | ||
|
|
||
| namespace ChimeraTK { | ||
|
|
||
| template<typename UserType> | ||
| class NumericDoubleBufferAccessorDecorator : public NDRegisterAccessorDecorator<UserType> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to name this "Numeric"? It is foreseen to also be used by the LMAP backend, eventually. |
||
| public: | ||
| using ChimeraTK::NDRegisterAccessorDecorator<UserType>::buffer_2D; | ||
| using ChimeraTK::NDRegisterAccessorDecorator<UserType>::_target; | ||
| NumericDoubleBufferAccessorDecorator(const boost::shared_ptr<ChimeraTK::NDRegisterAccessor<UserType>>& target, | ||
| std::optional<NumericAddressedRegisterInfo::DoubleBufferInfo> doubleBufferConfig, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am wondering what the plugin does if there is nothing in the optional. This should just be the DoubleBufferInfo |
||
| const boost::shared_ptr<DeviceBackend>& backend, | ||
| std::shared_ptr<NumericAddressedBackend::DoubleBufferControlState> controlState); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would move the DoubleBufferControlState into the DoubleBufferAccessorDecorator file. Like that it can also be used by the lmap backend, which should not depend on the NumericAddressedBackend. |
||
|
|
||
| void doPreRead(TransferType type) override; | ||
|
|
||
| void doReadTransferSynchronously() override; | ||
|
|
||
| void doPostRead(TransferType type, bool hasNewData) override; | ||
|
|
||
| [[nodiscard]] bool isWriteable() const override { return false; } | ||
|
|
||
| void doPreWrite(TransferType, VersionNumber) override { | ||
| throw ChimeraTK::logic_error("NumericAddressBackend DoubleBufferPlugin: Writing is not allowed atm."); | ||
| } | ||
|
|
||
| void doPostWrite(TransferType, VersionNumber) override { | ||
| // do not throw here again | ||
| } | ||
| // below functions are needed for TransferGroup to work | ||
| std::vector<boost::shared_ptr<TransferElement>> getHardwareAccessingElements() override; | ||
|
|
||
| std::list<boost::shared_ptr<TransferElement>> getInternalElements() override { return {}; } | ||
|
|
||
| void replaceTransferElement(boost::shared_ptr<ChimeraTK::TransferElement> /* newElement */) override { | ||
| // do nothing, we do not support merging of DoubleBufferAccessorDecorators | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will have to support merging eventually because we don't want to do individual transfers of neighbouring registers. Once we can describe individual channels of multiplexed registers, we have to support it. We cannot transfer the whole DAQ area 16 times and only extract one channel. But to be honest, I have no idea yet how to combine that with double buffering. Elements are merged when you put them into the transfer group, but we cannot put the registers of the promary and the secondary accessor into the same transfer group. So we probably need two "under the hood" transfer groups for each DoubleBufferControlState. Now that I think about it, I don't even know if it works cleanly in the current implementation. |
||
| } | ||
| [[nodiscard]] bool mayReplaceOther(const boost::shared_ptr<TransferElement const>& other) const override; | ||
|
|
||
| private: | ||
| // we know that plugin exists at least as long as any register (of the catalogue) refers to it, | ||
| // so no shared_ptr required here | ||
|
Comment on lines
+51
to
+52
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now invalid comment, copied from the LNM backend |
||
| std::optional<NumericAddressedRegisterInfo::DoubleBufferInfo> _doubleBufferInfo; | ||
| boost::shared_ptr<DeviceBackend> _backend; | ||
| std::shared_ptr<NumericAddressedBackend::DoubleBufferControlState> _controlState; | ||
| boost::shared_ptr<ChimeraTK::NDRegisterAccessor<UserType>> _secondBufferReg; | ||
| boost::shared_ptr<ChimeraTK::NDRegisterAccessor<uint32_t>> _enableDoubleBufferReg; | ||
| boost::shared_ptr<ChimeraTK::NDRegisterAccessor<uint32_t>> _currentBufferNumberReg; | ||
| uint32_t _currentBuffer{0}; | ||
| }; | ||
| } // namespace ChimeraTK | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,21 +50,29 @@ namespace ChimeraTK { | |
| [[nodiscard]] DataType getRawType() const; /**< Return raw type matching the given width */ | ||
| }; | ||
|
|
||
| struct DoubleBufferInfo { | ||
| uint64_t offset; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| RegisterPath enableRegisterPath; | ||
| RegisterPath inactiveBufferRegisterPath; | ||
| uint32_t index; | ||
| }; | ||
|
|
||
| /** | ||
| * Constructor to set all data members for scalar/1D registers. They all have default values, so this also acts as | ||
| * default constructor. | ||
| */ | ||
| explicit NumericAddressedRegisterInfo(RegisterPath const& pathName_ = {}, uint32_t nElements_ = 0, | ||
| uint64_t address_ = 0, uint32_t nBytes_ = 0, uint64_t bar_ = 0, uint32_t width_ = 32, | ||
| int32_t nFractionalBits_ = 0, bool signedFlag_ = true, Access dataAccess_ = Access::READ_WRITE, | ||
| Type dataType_ = Type::FIXED_POINT, std::vector<size_t> interruptId_ = {}); | ||
| Type dataType_ = Type::FIXED_POINT, std::vector<size_t> interruptId_ = {}, | ||
| std::optional<DoubleBufferInfo> doubleBuffer_ = std::nullopt); | ||
|
|
||
| /** | ||
| * Constructor to set all data members for 2D registers. | ||
| */ | ||
| NumericAddressedRegisterInfo(RegisterPath const& pathName_, uint64_t bar_, uint64_t address_, uint32_t nElements_, | ||
| uint32_t elementPitchBits_, std::vector<ChannelInfo> channelInfo_, Access dataAccess_, | ||
| std::vector<size_t> interruptId_); | ||
| std::vector<size_t> interruptId_, std::optional<DoubleBufferInfo> doubleBuffer_ = std::nullopt); | ||
|
|
||
| NumericAddressedRegisterInfo(const NumericAddressedRegisterInfo&) = default; | ||
|
|
||
|
|
@@ -115,7 +123,7 @@ namespace ChimeraTK { | |
|
|
||
| Access registerAccess; /**< Data access direction: Read, write, read and write or interrupt */ | ||
| std::vector<size_t> interruptId; | ||
|
|
||
| std::optional<DoubleBufferInfo> doubleBuffer; | ||
| /** Define per-channel information (bit interpretation etc.), 1D/scalars have exactly one entry. */ | ||
| std::vector<ChannelInfo> channels; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,8 @@ | |
|
|
||
| #include "JsonMapFileParser.h" | ||
|
|
||
| #include "JsonExtensions.h" | ||
|
|
||
| #include <nlohmann/json.hpp> | ||
|
|
||
| #include <boost/algorithm/string.hpp> | ||
|
|
@@ -113,6 +115,31 @@ namespace ChimeraTK::detail { | |
| size_t numberOfElements{1}; | ||
| size_t bytesPerElement{0}; | ||
|
|
||
| struct DoubleBufferingInfo { | ||
| struct Address { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a struct Address below which is different. Something is not right here.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, this is in the DoubleBufferingInfo, the other is outside. Are they the same? Why not put the Address from below to the top and use it here. If it's different, then SecondaryAddress is a better name here. |
||
| AddressType type{AddressType::DMA}; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is DMA the default? It can be anything. |
||
| size_t channel{0}; | ||
| HexValue offset{0}; | ||
|
|
||
| NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(Address, type, channel, offset) | ||
| }; | ||
|
|
||
| Address secondaryBufferAddress; | ||
| std::string enableRegister; | ||
| std::string readBufferRegister; | ||
| size_t index{0}; | ||
|
|
||
| void fill(NumericAddressedRegisterInfo& info) const { | ||
| info.doubleBuffer->offset = secondaryBufferAddress.offset.v; | ||
| info.doubleBuffer->enableRegisterPath = enableRegister; | ||
| info.doubleBuffer->inactiveBufferRegisterPath = readBufferRegister; | ||
| } | ||
|
|
||
| NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT( | ||
| DoubleBufferingInfo, secondaryBufferAddress, enableRegister, readBufferRegister, index) | ||
| }; | ||
| std::optional<DoubleBufferingInfo> doubleBuffering; | ||
|
|
||
| struct Address { | ||
| AddressType type{AddressType::IO}; | ||
| size_t channel{0}; | ||
|
|
@@ -220,6 +247,15 @@ namespace ChimeraTK::detail { | |
| info.interruptId = triggeredByInterrupt; | ||
| info.channels.emplace_back(0, NumericAddressedRegisterInfo::Type::VOID, 0, 0, false); | ||
| } | ||
| if(!info.doubleBuffer) { | ||
| info.doubleBuffer = NumericAddressedRegisterInfo::DoubleBufferInfo{}; | ||
| } | ||
| if(doubleBuffering.has_value()) { | ||
| doubleBuffering->fill(info); | ||
| } | ||
| else { | ||
| info.doubleBuffer.reset(); | ||
| } | ||
| } | ||
|
|
||
| std::vector<JsonAddressSpaceEntry> children; | ||
|
|
@@ -235,14 +271,31 @@ namespace ChimeraTK::detail { | |
| fill(my, parentName); | ||
| my.computeDataDescriptor(); | ||
| catalogue.addRegister(my); | ||
| if(doubleBuffering.has_value()) { | ||
| // Create the .buf0 register as a copy of the main one | ||
| NumericAddressedRegisterInfo buf0Register = my; | ||
| buf0Register.pathName = my.pathName + ".BUF0"; | ||
| buf0Register.doubleBuffer.reset(); // it's a simple view of the buffer | ||
| buf0Register.computeDataDescriptor(); | ||
| catalogue.addRegister(buf0Register); | ||
| NumericAddressedRegisterInfo buf1Register = my; | ||
| buf1Register.pathName = my.pathName + ".BUF1"; | ||
| buf1Register.doubleBuffer.reset(); // it's a simple view of the buffer | ||
| buf1Register.address = doubleBuffering->secondaryBufferAddress.offset.v; | ||
| // buf1Register.bar = doubleBuffering->secondaryBufferAddress.channel + | ||
| // (doubleBuffering->secondaryBufferAddress.type == AddressType::DMA ? 13 : 0); | ||
| buf1Register.computeDataDescriptor(); | ||
| catalogue.addRegister(buf1Register); | ||
| } | ||
| } | ||
| for(const auto& child : children) { | ||
| child.addInfos(catalogue, parentName / name); | ||
| } | ||
| } | ||
|
|
||
| NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(JsonAddressSpaceEntry, name, engineeringUnit, description, access, | ||
| triggeredByInterrupt, numberOfElements, bytesPerElement, address, representation, children, channelTabs) | ||
| triggeredByInterrupt, numberOfElements, bytesPerElement, address, representation, children, channelTabs, | ||
| doubleBuffering) | ||
| }; | ||
|
|
||
| /********************************************************************************************************************/ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name should not be ND, that means n-dimensional (in contrast to oneD and twoD and scalar)