From 43b70caf03aae086a96fda2f8465ec478ac77fcd Mon Sep 17 00:00:00 2001 From: Rick Altherr Date: Thu, 1 Feb 2018 12:11:25 -0800 Subject: [PATCH] xc7series: Dynamically allocate config packets when writing bitstreams ConfigurationPacket assumes that the payload data is owned by someone else. For frame data, that is generally true. For initialization and finalization sequences, those payloads need to be created and managed. Instead, dynamically allocate packets which allows for using subclasses of ConfigurationPacket that store the payload with the packet. Signed-off-by: Rick Altherr --- .../xilinx/xc7series/bitstream_writer.h | 2 +- lib/xilinx/xc7series/bitstream_writer.cc | 6 +-- lib/xilinx/xc7series/bitstream_writer_test.cc | 38 +++++++++++-------- tools/xc7patch.cc | 8 ++-- 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/lib/include/prjxray/xilinx/xc7series/bitstream_writer.h b/lib/include/prjxray/xilinx/xc7series/bitstream_writer.h index ae0634d7..806cc9a2 100644 --- a/lib/include/prjxray/xilinx/xc7series/bitstream_writer.h +++ b/lib/include/prjxray/xilinx/xc7series/bitstream_writer.h @@ -23,7 +23,7 @@ namespace xc7series { class BitstreamWriter { public: typedef std::array header_t; - typedef std::vector packets_t; + typedef std::vector> packets_t; // Only defined if a packet exists typedef absl::optional> op_data_t; typedef absl::Span::iterator data_iterator_t; diff --git a/lib/xilinx/xc7series/bitstream_writer.cc b/lib/xilinx/xc7series/bitstream_writer.cc index 3a7da30d..eb388ca5 100644 --- a/lib/xilinx/xc7series/bitstream_writer.cc +++ b/lib/xilinx/xc7series/bitstream_writer.cc @@ -28,7 +28,7 @@ BitstreamWriter::BitstreamWriter(const packets_t& packets) BitstreamWriter::packet_iterator BitstreamWriter::iterator::packet_begin() { // itr_packets = packets.begin(); - const ConfigurationPacket& packet = *itr_packets_; + const ConfigurationPacket& packet = **itr_packets_; return BitstreamWriter::packet_iterator( &packet, BitstreamWriter::packet_iterator::STATE_HEADER, @@ -36,7 +36,7 @@ BitstreamWriter::packet_iterator BitstreamWriter::iterator::packet_begin() { } BitstreamWriter::packet_iterator BitstreamWriter::iterator::packet_end() { - const ConfigurationPacket& packet = *itr_packets_; + const ConfigurationPacket& packet = **itr_packets_; return BitstreamWriter::packet_iterator( &packet, BitstreamWriter::packet_iterator::STATE_END, @@ -138,7 +138,7 @@ BitstreamWriter::iterator BitstreamWriter::begin() { if (itr_packets != packets_.end()) { // op_packet_itr = packet_begin(); // FIXME: de-duplicate this - const ConfigurationPacket& packet = *itr_packets; + const ConfigurationPacket& packet = **itr_packets; packet_iterator packet_itr = packet_iterator(&packet, packet_iterator::STATE_HEADER, packet.data().begin()); diff --git a/lib/xilinx/xc7series/bitstream_writer_test.cc b/lib/xilinx/xc7series/bitstream_writer_test.cc index 752fa4c9..be2d41ec 100644 --- a/lib/xilinx/xc7series/bitstream_writer_test.cc +++ b/lib/xilinx/xc7series/bitstream_writer_test.cc @@ -50,7 +50,8 @@ void dump_packets(xc7series::BitstreamWriter writer, bool nl = true) { } // Special all 0's -void AddType0(std::vector& packets) { +void AddType0( + std::vector>& packets) { // InitWithWords doesn't like type 0 /* static std::vector words{0x00000000}; @@ -61,27 +62,32 @@ void AddType0(std::vector& packets) { static std::vector words{}; absl::Span word_span(words); // CRC is config value 0 - packets.push_back(xc7series::ConfigurationPacket( + packets.emplace_back(new xc7series::ConfigurationPacket( 0, xc7series::ConfigurationPacket::NOP, xc7series::ConfigurationRegister::CRC, word_span)); } -void AddType1(std::vector& packets) { +void AddType1( + std::vector>& packets) { static std::vector words{MakeType1(0x2, 0x3, 2), 0xAA, 0xBB}; absl::Span word_span(words); auto packet = xc7series::ConfigurationPacket::InitWithWords(word_span); - packets.push_back(*(packet.second)); + packets.emplace_back( + new xc7series::ConfigurationPacket(*(packet.second))); } // Empty -void AddType1E(std::vector& packets) { +void AddType1E( + std::vector>& packets) { static std::vector words{MakeType1(0x2, 0x3, 0)}; absl::Span word_span(words); auto packet = xc7series::ConfigurationPacket::InitWithWords(word_span); - packets.push_back(*(packet.second)); + packets.emplace_back( + new xc7series::ConfigurationPacket(*(packet.second))); } -void AddType2(std::vector& packets) { +void AddType2( + std::vector>& packets) { // Type 1 packet with address // Without this InitWithWords will fail on type 2 xc7series::ConfigurationPacket* packet1; @@ -90,8 +96,9 @@ void AddType2(std::vector& packets) { absl::Span word_span(words); auto packet1_pair = xc7series::ConfigurationPacket::InitWithWords(word_span); - packets.push_back(*(packet1_pair.second)); - packet1 = &packets[0]; + packets.emplace_back( + new xc7series::ConfigurationPacket(*(packet1_pair.second))); + packet1 = packets[0].get(); } // Type 2 packet with data { @@ -100,13 +107,14 @@ void AddType2(std::vector& packets) { absl::Span word_span(words); auto packet = xc7series::ConfigurationPacket::InitWithWords( word_span, packet1); - packets.push_back(*(packet.second)); + packets.emplace_back( + new xc7series::ConfigurationPacket(*(packet.second))); } } // Empty packets should produce just the header TEST(BitstreamWriterTest, WriteHeader) { - std::vector packets; + std::vector> packets; xc7series::BitstreamWriter writer(packets); std::vector words(writer.begin(), writer.end()); @@ -120,7 +128,7 @@ TEST(BitstreamWriterTest, WriteHeader) { } TEST(BitstreamWriterTest, WriteType0) { - std::vector packets; + std::vector> packets; AddType0(packets); xc7series::BitstreamWriter writer(packets); // dump_packets(writer, false); @@ -133,7 +141,7 @@ TEST(BitstreamWriterTest, WriteType0) { EXPECT_EQ(words, ref); } TEST(BitstreamWriterTest, WriteType1) { - std::vector packets; + std::vector> packets; AddType1(packets); xc7series::BitstreamWriter writer(packets); // dump_packets(writer, false); @@ -147,7 +155,7 @@ TEST(BitstreamWriterTest, WriteType1) { } TEST(BitstreamWriterTest, WriteType2) { - std::vector packets; + std::vector> packets; AddType2(packets); xc7series::BitstreamWriter writer(packets); // dump_packets(writer, false); @@ -164,7 +172,7 @@ TEST(BitstreamWriterTest, WriteType2) { } TEST(BitstreamWriterTest, WriteMulti) { - std::vector packets; + std::vector> packets; AddType1(packets); AddType1E(packets); AddType2(packets); diff --git a/tools/xc7patch.cc b/tools/xc7patch.cc index fc4bc6ad..b3ebf6ff 100644 --- a/tools/xc7patch.cc +++ b/tools/xc7patch.cc @@ -120,7 +120,8 @@ int main(int argc, char* argv[]) { frame_data[0x32] |= (ecc & 0x1FFF); } - std::vector out_packets; + std::vector> + out_packets; // Generate a single type 2 packet that writes everything at once. std::vector packet_data; @@ -139,10 +140,11 @@ int main(int argc, char* argv[]) { } packet_data.insert(packet_data.end(), 202, 0); - out_packets.push_back(xc7series::ConfigurationPacket( + // Frame data write + out_packets.emplace_back(new xc7series::ConfigurationPacket( 1, xc7series::ConfigurationPacket::Opcode::Write, xc7series::ConfigurationRegister::FDRI, {})); - out_packets.push_back(xc7series::ConfigurationPacket( + out_packets.emplace_back(new xc7series::ConfigurationPacket( 2, xc7series::ConfigurationPacket::Opcode::Write, xc7series::ConfigurationRegister::FDRI, packet_data));