From 7e348890a09d33dc567b58b914e460e9bab3c2e9 Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Fri, 3 Oct 2025 08:15:54 +0200 Subject: [PATCH] FPGA: Implement high threshold to mark pixels above certain value as overloads --- acquisition_device/FPGAAcquisitionDevice.cpp | 13 +++++--- common/DatasetSettings.cpp | 17 +++++++++- common/DatasetSettings.h | 3 ++ common/DiffractionExperiment.cpp | 9 ++++++ common/DiffractionExperiment.h | 3 ++ fpga/hdl/action_config.v | 33 +++++++++++++++----- fpga/hls/data_collection_fsm.cpp | 9 ++++-- fpga/hls/hls_jfjoch.h | 3 +- fpga/hls/pixel_threshold.cpp | 8 +++-- fpga/hls_simulation/HLSDevice.cpp | 12 ++++--- fpga/hls_simulation/hls_cores.h | 3 +- fpga/pcie_driver/jfjoch_fpga.h | 3 +- fpga/scripts/jfjoch.tcl | 3 +- tests/FPGAHLSModulesTest.cpp | 27 ++++++++++------ tests/FPGAIntegrationTest.cpp | 8 +++-- 15 files changed, 116 insertions(+), 38 deletions(-) diff --git a/acquisition_device/FPGAAcquisitionDevice.cpp b/acquisition_device/FPGAAcquisitionDevice.cpp index 9650d6c2..72f8687a 100644 --- a/acquisition_device/FPGAAcquisitionDevice.cpp +++ b/acquisition_device/FPGAAcquisitionDevice.cpp @@ -256,10 +256,15 @@ void FPGAAcquisitionDevice::FillActionRegister(const DiffractionExperiment& x, D job.sqrtmult = x.GetLossyCompressionPoisson().value(); } - if (x.GetPixelValueLowThreshold()) { - job.mode |= MODE_THRESHOLD; - job.pxlthreshold = x.GetPixelValueLowThreshold().value(); - } + if (x.GetPixelValueLowThreshold()) + job.pxlthreshold_min = x.GetPixelValueLowThreshold().value(); + else + job.pxlthreshold_min = INT32_MIN; + + if (x.GetPixelValueHighThreshold()) + job.pxlthreshold_max = x.GetPixelValueHighThreshold().value(); + else + job.pxlthreshold_max = INT32_MIN; } void FPGAAcquisitionDevice::Start(const DiffractionExperiment &experiment, uint32_t flag) { diff --git a/common/DatasetSettings.cpp b/common/DatasetSettings.cpp index 1421cbe9..df7763ba 100644 --- a/common/DatasetSettings.cpp +++ b/common/DatasetSettings.cpp @@ -324,17 +324,32 @@ DatasetSettings &DatasetSettings::PixelValueLowThreshold(const std::optional &input) { + if (!input || (input == 0)) + pixel_value_high_threshold = {}; + else { + check_min("Pixel value high threshold", input.value(), 1); + check_max("Pixel value high threshold", input.value(), INT32_MAX); + pixel_value_high_threshold = input; + } + return *this; +} + std::optional DatasetSettings::GetPixelValueLowThreshold() const { return pixel_value_low_threshold; } +std::optional DatasetSettings::GetPixelValueHighThreshold() const { + return pixel_value_high_threshold; +} + bool DatasetSettings::IsWriteNXmxHDF5Master() const { return write_nxmx_hdf5_master; } diff --git a/common/DatasetSettings.h b/common/DatasetSettings.h index e0a0a7dd..38795c06 100644 --- a/common/DatasetSettings.h +++ b/common/DatasetSettings.h @@ -50,6 +50,7 @@ class DatasetSettings { std::string group; std::optional compression_poisson_factor; std::optional pixel_value_low_threshold; + std::optional pixel_value_high_threshold; std::optional save_calibration; bool write_nxmx_hdf5_master; @@ -97,6 +98,7 @@ public: DatasetSettings& WriteNXmxHDF5Master(bool input); DatasetSettings& SaveCalibration(std::optional input); DatasetSettings& PixelValueLowThreshold(const std::optional &input); + DatasetSettings& PixelValueHighThreshold(const std::optional &input); DatasetSettings& PolarizationFactor(const std::optional &input); DatasetSettings& PoniRot1_rad(float input); DatasetSettings& PoniRot2_rad(float input); @@ -144,6 +146,7 @@ public: std::optional GetLossyCompressionPoisson() const; std::optional GetPixelValueLowThreshold() const; + std::optional GetPixelValueHighThreshold() const; bool IsWriteNXmxHDF5Master() const; std::optional IsSaveCalibration() const; diff --git a/common/DiffractionExperiment.cpp b/common/DiffractionExperiment.cpp index 620c4297..00af8ab3 100644 --- a/common/DiffractionExperiment.cpp +++ b/common/DiffractionExperiment.cpp @@ -1094,6 +1094,15 @@ DiffractionExperiment &DiffractionExperiment::PixelValueLowThreshold(const std:: return *this; } +std::optional DiffractionExperiment::GetPixelValueHighThreshold() const { + return dataset.GetPixelValueHighThreshold(); +} + +DiffractionExperiment &DiffractionExperiment::PixelValueHighThreshold(const std::optional &input) { + dataset.PixelValueHighThreshold(input); + return *this; +} + DiffractionExperiment &DiffractionExperiment::ImportInstrumentMetadata(const InstrumentMetadata &input) { instrument = input; return *this; diff --git a/common/DiffractionExperiment.h b/common/DiffractionExperiment.h index 24715290..a9407219 100644 --- a/common/DiffractionExperiment.h +++ b/common/DiffractionExperiment.h @@ -334,6 +334,9 @@ public: std::optional GetPixelValueLowThreshold() const; DiffractionExperiment &PixelValueLowThreshold(const std::optional& input); + std::optional GetPixelValueHighThreshold() const; + DiffractionExperiment &PixelValueHighThreshold(const std::optional& input); + DiffractionExperiment& BitDepthImage(const std::optional &input); DiffractionExperiment& PixelSigned(const std::optional &input); diff --git a/fpga/hdl/action_config.v b/fpga/hdl/action_config.v index cd8b9cc1..0f7fb394 100644 --- a/fpga/hdl/action_config.v +++ b/fpga/hdl/action_config.v @@ -1,6 +1,8 @@ // SPDX-FileCopyrightText: 2024 Filip Leonarski, Paul Scherrer Institute // SPDX-License-Identifier: CERN-OHL-S-2.0 +localparam signed [31:0] INT32_MIN = 32'sh80000000; + // parameters imported from source files `define JFJOCH_VARIANT 32'h0 `define JFJOCH_SYNTH_TIME 32'd0 @@ -82,8 +84,10 @@ `define ADDR_NSUMMATION 16'h0220 `define ADDR_SQRTMULT 16'h0224 -`define ADDR_PXLTHRESHOLD 16'h0228 -`define ADDR_DATA_STREAM 16'h022C +`define ADDR_PXLTHRESHOLD_MIN 16'h0228 +`define ADDR_PXLTHRESHOLD_MAX 16'h022C + +`define ADDR_DATA_STREAM 16'h0230 module action_config #(parameter C_S_AXI_ADDR_WIDTH = 16, @@ -123,7 +127,8 @@ module action_config output reg [3:0] nstorage_cells , output reg [7:0] nsummation , output reg [7:0] sqrtmult , - output reg [31:0] pxlthreshold , + output reg [31:0] pxlthreshold_min , + output reg [31:0] pxlthreshold_max , output wire [31:0] hbm_size_bytes , output reg [31:0] data_stream , output reg [31:0] spot_finder_count_threshold, @@ -397,8 +402,11 @@ always @(posedge clk) begin `ADDR_SQRTMULT: begin rdata <= sqrtmult; end - `ADDR_PXLTHRESHOLD: begin - rdata <= pxlthreshold; + `ADDR_PXLTHRESHOLD_MIN: begin + rdata <= pxlthreshold_min; + end + `ADDR_PXLTHRESHOLD_MAX: begin + rdata <= pxlthreshold_max; end `ADDR_DATA_STREAM: begin rdata <= data_stream; @@ -703,10 +711,19 @@ end always @(posedge clk) begin if (!resetn) - pxlthreshold <= 0; + pxlthreshold_min <= INT32_MIN; else if (reg_data_collection_idle) begin - if (w_hs && waddr == `ADDR_PXLTHRESHOLD) - pxlthreshold <= (s_axi_WDATA[31:0] & wmask[31:0]) | (sqrtmult & !wmask[31:0]); + if (w_hs && waddr == `ADDR_PXLTHRESHOLD_MIN) + pxlthreshold_min <= s_axi_WDATA; + end +end + +always @(posedge clk) begin + if (!resetn) + pxlthreshold_max <= INT32_MIN; + else if (reg_data_collection_idle) begin + if (w_hs && waddr == `ADDR_PXLTHRESHOLD_MAX) + pxlthreshold_max <= s_axi_WDATA; end end diff --git a/fpga/hls/data_collection_fsm.cpp b/fpga/hls/data_collection_fsm.cpp index 267fcc10..cb714491 100644 --- a/fpga/hls/data_collection_fsm.cpp +++ b/fpga/hls/data_collection_fsm.cpp @@ -17,7 +17,8 @@ void data_collection_fsm(AXI_STREAM ð_in, ap_uint<4> nstorage_cells, ap_uint<8> nsummation, ap_uint<8> sqrtmult, - ap_uint<32> pxlthreshold, + ap_uint<32> pxlthreshold_min, + ap_uint<32> pxlthreshold_max, volatile rcv_state_t &data_collection_state) { #pragma HLS INTERFACE ap_ctrl_none port=return @@ -36,7 +37,8 @@ void data_collection_fsm(AXI_STREAM ð_in, #pragma HLS INTERFACE ap_none register port=nstorage_cells #pragma HLS INTERFACE ap_none register port=nsummation #pragma HLS INTERFACE ap_none register port=sqrtmult -#pragma HLS INTERFACE ap_none register port=pxlthreshold +#pragma HLS INTERFACE ap_none register port=pxlthreshold_min +#pragma HLS INTERFACE ap_none register port=pxlthreshold_max #pragma HLS INTERFACE ap_none register port=data_collection_state #pragma HLS PIPELINE II=1 style=flp @@ -83,7 +85,8 @@ void data_collection_fsm(AXI_STREAM ð_in, ACT_REG_NSTORAGE_CELLS(packet_out.data) = nstorage_cells + 1; ACT_REG_NSUMMATION(packet_out.data) = nsummation; ACT_REG_SQRTMULT(packet_out.data) = sqrtmult; - ACT_REG_THRESHOLD(packet_out.data) = pxlthreshold; + ACT_REG_THRESHOLD_MIN(packet_out.data) = pxlthreshold_min; + ACT_REG_THRESHOLD_MAX(packet_out.data) = pxlthreshold_max; packet_out.user = 0; packet_out.last = 1; packet_out.dest = 0; diff --git a/fpga/hls/hls_jfjoch.h b/fpga/hls/hls_jfjoch.h index 16520b7a..6d49e154 100644 --- a/fpga/hls/hls_jfjoch.h +++ b/fpga/hls/hls_jfjoch.h @@ -77,7 +77,8 @@ typedef hls::stream STREAM_768; #define ACT_REG_NSTORAGE_CELLS(x) ((x)(148, 144)) // 5 bit #define ACT_REG_NSUMMATION(x) ((x)(167, 160)) // 8 bit (0..255) #define ACT_REG_SQRTMULT(x) ((x)(175, 168)) // 8 bit (0..255) -#define ACT_REG_THRESHOLD(x) ((x)(255,224)) // 32 bit +#define ACT_REG_THRESHOLD_MIN(x) ((x)(255, 224)) // 32 bit +#define ACT_REG_THRESHOLD_MAX(x) ((x)(287, 256)) // 32 bit struct axis_datamover_ctrl { ap_uint<40+64> data; diff --git a/fpga/hls/pixel_threshold.cpp b/fpga/hls/pixel_threshold.cpp index e863eff2..1b196389 100644 --- a/fpga/hls/pixel_threshold.cpp +++ b/fpga/hls/pixel_threshold.cpp @@ -19,8 +19,8 @@ void pixel_threshold(STREAM_768 &data_in, } ap_uint<32> data_collection_mode = ACT_REG_MODE(packet_in.data); - ap_uint<1> apply_threshold = ((data_collection_mode & MODE_THRESHOLD) ? 1 : 0); - ap_int<32> threshold = ACT_REG_THRESHOLD(packet_in.data); + ap_int<32> threshold_min = ACT_REG_THRESHOLD_MIN(packet_in.data); + ap_int<32> threshold_max = ACT_REG_THRESHOLD_MAX(packet_in.data); data_in >> packet_in; @@ -29,8 +29,10 @@ void pixel_threshold(STREAM_768 &data_in, ap_int<24> pixel[32]; unpack32(packet_in.data, pixel); for (int i = 0; i < 32; i++) { - if (apply_threshold && (pixel[i] != INT24_MIN) && (pixel[i] < threshold)) + if ((threshold_min != INT32_MIN) && (pixel[i] != INT24_MIN) && (pixel[i] < threshold_min)) pixel[i] = 0; + else if ((threshold_max != INT32_MIN) && (pixel[i] > threshold_max)) + pixel[i] = INT24_MAX; } packet_in.data = pack32(pixel); diff --git a/fpga/hls_simulation/HLSDevice.cpp b/fpga/hls_simulation/HLSDevice.cpp index 7da00b2c..7752a083 100644 --- a/fpga/hls_simulation/HLSDevice.cpp +++ b/fpga/hls_simulation/HLSDevice.cpp @@ -370,7 +370,8 @@ void HLSDevice::HLSMainThread() { cfg.nstorage_cells, cfg.nsummation , cfg.sqrtmult, - cfg.pxlthreshold, + cfg.pxlthreshold_min, + cfg.pxlthreshold_max, state); impl_->run_data_collection = 1; @@ -392,7 +393,8 @@ void HLSDevice::HLSMainThread() { cfg.nstorage_cells, cfg.nsummation , cfg.sqrtmult, - cfg.pxlthreshold, + cfg.pxlthreshold_min, + cfg.pxlthreshold_max, state); impl_->run_data_collection = 0; @@ -408,7 +410,8 @@ void HLSDevice::HLSMainThread() { cfg.nstorage_cells, cfg.nsummation, cfg.sqrtmult, - cfg.pxlthreshold, + cfg.pxlthreshold_min, + cfg.pxlthreshold_max, state); hls_cores.emplace_back([&] { @@ -469,7 +472,8 @@ void HLSDevice::HLSMainThread() { cfg.nstorage_cells, cfg.nsummation, cfg.sqrtmult, - cfg.pxlthreshold, + cfg.pxlthreshold_min, + cfg.pxlthreshold_max, state); } done = true; diff --git a/fpga/hls_simulation/hls_cores.h b/fpga/hls_simulation/hls_cores.h index 01031330..bab2b3a9 100644 --- a/fpga/hls_simulation/hls_cores.h +++ b/fpga/hls_simulation/hls_cores.h @@ -111,7 +111,8 @@ void data_collection_fsm(AXI_STREAM ð_in, ap_uint<4> nstorage_cells, ap_uint<8> nsummation, ap_uint<8> sqrtmult, - ap_uint<32> pxlthreshold, + ap_uint<32> pxlthreshold_min, + ap_uint<32> pxlthreshold_max, volatile rcv_state_t &data_collection_state); void host_writer(STREAM_512 &data_in, diff --git a/fpga/pcie_driver/jfjoch_fpga.h b/fpga/pcie_driver/jfjoch_fpga.h index 709e26af..df3208e4 100644 --- a/fpga/pcie_driver/jfjoch_fpga.h +++ b/fpga/pcie_driver/jfjoch_fpga.h @@ -108,7 +108,8 @@ struct DataCollectionConfig { uint32_t nstorage_cells; // Number of storage cells minus one (0 = 1SC, 1 = 2SC, ..., 15 = 16SC) uint32_t nsummation; // Summation of frames minus one (0 = no summation, 1 = 2 frames, 2 = 3 frames, ..., 255 = 256 frames) uint32_t sqrtmult; // Multiplication factor (minus one) of pixel value BEFORE taking square root function - should be square of the coeff used in other work - int32_t pxlthreshold; // pixels with values lower than threshold at set to zero + int32_t pxlthreshold_min; // pixels with values lower than threshold are set to zero (after conversion to photons; before summation) + int32_t pxlthreshold_max; // pixels with values higher than threshold are set as overload (after conversion to photons; before summation) int32_t data_stream; // number of the data stream (checked against upper 8-bit of UDP destination port) }; diff --git a/fpga/scripts/jfjoch.tcl b/fpga/scripts/jfjoch.tcl index c17cde11..d70c636b 100644 --- a/fpga/scripts/jfjoch.tcl +++ b/fpga/scripts/jfjoch.tcl @@ -561,7 +561,8 @@ proc create_hier_cell_jungfraujoch { parentCell nameHier } { connect_bd_net -net action_config_0_nmodules [get_bd_pins action_config_0/nmodules] [get_bd_pins data_collection_fsm_0/nmodules] connect_bd_net -net action_config_0_nstorage_cells [get_bd_pins action_config_0/nstorage_cells] [get_bd_pins data_collection_fsm_0/nstorage_cells] connect_bd_net -net action_config_0_nsummation [get_bd_pins action_config_0/nsummation] [get_bd_pins data_collection_fsm_0/nsummation] - connect_bd_net -net action_config_0_pxlthreshold [get_bd_pins action_config_0/pxlthreshold] [get_bd_pins data_collection_fsm_0/pxlthreshold] + connect_bd_net -net action_config_0_pxlthreshold_min [get_bd_pins action_config_0/pxlthreshold_min] [get_bd_pins data_collection_fsm_0/pxlthreshold_min] + connect_bd_net -net action_config_0_pxlthreshold_max [get_bd_pins action_config_0/pxlthreshold_max] [get_bd_pins data_collection_fsm_0/pxlthreshold_max] connect_bd_net -net action_config_0_spot_finder_count_threshold [get_bd_pins action_config_0/spot_finder_count_threshold] [get_bd_pins image_processing/in_count_threshold] connect_bd_net -net action_config_0_spot_finder_d_max [get_bd_pins action_config_0/spot_finder_d_max] [get_bd_pins image_processing/in_max_d_value] connect_bd_net -net action_config_0_spot_finder_d_min [get_bd_pins action_config_0/spot_finder_d_min] [get_bd_pins image_processing/in_min_d_value] diff --git a/tests/FPGAHLSModulesTest.cpp b/tests/FPGAHLSModulesTest.cpp index 4a4605af..c2c50fdb 100644 --- a/tests/FPGAHLSModulesTest.cpp +++ b/tests/FPGAHLSModulesTest.cpp @@ -36,7 +36,8 @@ TEST_CASE("HLS_DataCollectionFSM","[OpenCAPI]") { act_reg.nstorage_cells, act_reg.nsummation, act_reg.sqrtmult, - act_reg.pxlthreshold, + act_reg.pxlthreshold_min, + act_reg.pxlthreshold_max, state); REQUIRE(idle_data_collection == 1); REQUIRE(addr1.empty()); @@ -57,7 +58,8 @@ TEST_CASE("HLS_DataCollectionFSM","[OpenCAPI]") { act_reg.nstorage_cells, act_reg.nsummation, act_reg.sqrtmult, - act_reg.pxlthreshold, + act_reg.pxlthreshold_min, + act_reg.pxlthreshold_max, state); REQUIRE(idle_data_collection == 0); REQUIRE(addr1.empty()); @@ -76,7 +78,8 @@ TEST_CASE("HLS_DataCollectionFSM","[OpenCAPI]") { act_reg.nstorage_cells, act_reg.nsummation, act_reg.sqrtmult, - act_reg.pxlthreshold, + act_reg.pxlthreshold_min, + act_reg.pxlthreshold_max, state); REQUIRE(idle_data_collection == 0); REQUIRE(addr1.empty()); @@ -98,7 +101,8 @@ TEST_CASE("HLS_DataCollectionFSM","[OpenCAPI]") { act_reg.nstorage_cells, act_reg.nsummation, act_reg.sqrtmult, - act_reg.pxlthreshold, + act_reg.pxlthreshold_min, + act_reg.pxlthreshold_max, state); REQUIRE(idle_data_collection == 0); REQUIRE(addr1.empty()); @@ -117,7 +121,8 @@ TEST_CASE("HLS_DataCollectionFSM","[OpenCAPI]") { act_reg.nstorage_cells, act_reg.nsummation, act_reg.sqrtmult, - act_reg.pxlthreshold, + act_reg.pxlthreshold_min, + act_reg.pxlthreshold_max, state); REQUIRE(idle_data_collection == 0); @@ -138,7 +143,8 @@ TEST_CASE("HLS_DataCollectionFSM","[OpenCAPI]") { act_reg.nstorage_cells, act_reg.nsummation, act_reg.sqrtmult, - act_reg.pxlthreshold, + act_reg.pxlthreshold_min, + act_reg.pxlthreshold_max, state); REQUIRE(idle_data_collection == 0); @@ -160,7 +166,8 @@ TEST_CASE("HLS_DataCollectionFSM","[OpenCAPI]") { act_reg.nstorage_cells, act_reg.nsummation, act_reg.sqrtmult, - act_reg.pxlthreshold, + act_reg.pxlthreshold_min, + act_reg.pxlthreshold_max, state); REQUIRE(idle_data_collection == 0); @@ -180,7 +187,8 @@ TEST_CASE("HLS_DataCollectionFSM","[OpenCAPI]") { act_reg.nstorage_cells, act_reg.nsummation, act_reg.sqrtmult, - act_reg.pxlthreshold, + act_reg.pxlthreshold_min, + act_reg.pxlthreshold_max, state); REQUIRE(idle_data_collection == 0); @@ -201,7 +209,8 @@ TEST_CASE("HLS_DataCollectionFSM","[OpenCAPI]") { act_reg.nstorage_cells, act_reg.nsummation, act_reg.sqrtmult, - act_reg.pxlthreshold, + act_reg.pxlthreshold_min, + act_reg.pxlthreshold_max, state); REQUIRE( state == RCV_WAIT_FOR_START); diff --git a/tests/FPGAIntegrationTest.cpp b/tests/FPGAIntegrationTest.cpp index 7efeae28..6ed18119 100644 --- a/tests/FPGAIntegrationTest.cpp +++ b/tests/FPGAIntegrationTest.cpp @@ -859,7 +859,7 @@ TEST_CASE("HLS_C_Simulation_check_threshold_full_range", "[FPGA][Full]") { const uint16_t nmodules = 1; - for (int threshold: {1, 2,4,8}) { + for (int threshold: {1, 2, 4, 8}) { DiffractionExperiment x(DetJF(nmodules)); x.Mode(DetectorMode::Raw).PixelSigned(true).BitDepthImage(16); @@ -2029,7 +2029,9 @@ TEST_CASE("HLS_C_Simulation_internal_packet_generator_pixel_threshold_summation" DiffractionExperiment x(DetJF(nmodules)); x.Mode(DetectorMode::Raw); - x.UseInternalPacketGenerator(true).ImagesPerTrigger(1).PedestalG0Frames(0).Summation(4).PixelValueLowThreshold(4).AutoSummation(true); + x.UseInternalPacketGenerator(true).ImagesPerTrigger(1).PedestalG0Frames(0).Summation(4) + .PixelValueLowThreshold(4).AutoSummation(true).PixelSigned(true) + .PixelValueHighThreshold(500); HLSSimulatedDevice test(0, 64); @@ -2039,6 +2041,7 @@ TEST_CASE("HLS_C_Simulation_internal_packet_generator_pixel_threshold_summation" frame [1024+3] = 4; frame [1024+4] = 5; frame [1024+5] = 100; + frame [1024+6] = 501; for (int m = 0; m < x.GetModulesNum(); m++) test.SetInternalGeneratorFrame(frame.data(), m); @@ -2054,6 +2057,7 @@ TEST_CASE("HLS_C_Simulation_internal_packet_generator_pixel_threshold_summation" REQUIRE(imageBuf[1024+3] == 4*4); REQUIRE(imageBuf[1024+4] == 4*5); REQUIRE(imageBuf[1024+5] == 4*100); + REQUIRE(imageBuf[1024+6] == INT16_MAX); CHECK(test.GetDeviceOutput(0, 0)->module_statistics.pixel_sum == 4 * (4+5+100)); CHECK(test.GetDeviceOutput(0,0)->module_statistics.max_value == 4*100);