diff --git a/libraries/dsp/st/doc/st_histogram_status.txt b/libraries/dsp/st/doc/st_histogram_status.txt index f657cbdac864fb1e3fb1bbdac710093afe9ed959..0a33378b78c5c86bdb2ce6816537ed25db08dfa7 100644 --- a/libraries/dsp/st/doc/st_histogram_status.txt +++ b/libraries/dsp/st/doc/st_histogram_status.txt @@ -3,15 +3,19 @@ Status of st_histogram on june 2021 after merge to master in GitLab. EK = Eric Kooistra DS = Daniel van der Schuur -In summary the things to do still for st_histogram are (this can be done in new Jira story): +In summary the things to do still for st_histogram are (this can be done in +new Jira story): - add top level functional description (see 2b, 3) - explain why common_paged_ram_r_w was not used (see 2a) - remove control via MM (see 3, 4, 5a) - verify using DC signal (see 5b) -- verify that sum(bins) has expected constant value every sync interval (see 5c) +- verify that sum(bins) has expected constant value every sync interval + (see 5c) -Below, for reference, EK copied the left over comments and answers from the GitLab merge ( https://git.astron.nl/desp/hdl/-/merge_requests/101 ) of the st_histogram VHDL in L2SDP-151 into L2SDP-143 and then into the master. +Below, for reference, EK copied the left over comments and answers from the +GitLab merge ( https://git.astron.nl/desp/hdl/-/merge_requests/101 ) of the +st_histogram VHDL in L2SDP-151 into L2SDP-143 and then into the master. 1) mms_st_histogram.vhd - no comments @@ -22,20 +26,31 @@ a) EK comment Why common_ram_r_w was selected instead of common_ram_cr_cw ? -Waarom gebruik je niet common_paged_ram_r_w.vhd voor de RAM ? Kun je dat in comment aangeven, of vermelden dat common_paged_ram_r_w ook gebruikt had kunnen worden. +Waarom gebruik je niet common_paged_ram_r_w.vhd voor de RAM ? Kun je dat in +comment aangeven, of vermelden dat common_paged_ram_r_w ook gebruikt had +kunnen worden. b) EK comment -Waarom kan read en write tegelijk voorkomen? --> Wat beslist de bus arbiter dan? --> Wat is het gevolg dan voor de bin waardes ? +Waarom kan read en write tegelijk voorkomen? --> Wat beslist de bus arbiter +dan? --> Wat is het gevolg dan voor de bin waardes ? -Hoe ziet de st_histogram werking er in de tijd van een sync interval uit? Tellen de bins echt alle samples, of gaan er steed bijv K samples per sync interval verloren, dat is acceptabel mits K constant is en K << N = nof samples per sync. +Hoe ziet de st_histogram werking er in de tijd van een sync interval uit? +Tellen de bins echt alle samples, of gaan er steeds bijvoorbeeld K samples +per sync interval verloren, dat is acceptabel mits K constant is en mits +K << N = nof samples per sync. -Eigenlijk ontbreekt nog de beschrijving op functioneel niveau, los van de implementatie details, bijvoorbeeld zaken als: +Eigenlijk ontbreekt nog de beschrijving op functioneel niveau, los van de +implementatie details, bijvoorbeeld zaken als: -* The number of bins in the histogram g_nof_bins is a power of 2 and g_nof_bins_w <= g_data_w, so each bin counts the occurance of a subrange with 2**(g_data_w - g_nof_bins_w) samples values. -* The sample values in range g_data_w get mapped to g_nof_bins by counting how often the sample value(s) for that bin occur during a sync interval. -* The sum of the histogram bin values in each sync interval is : sum(bins) = N - K (klopt dat)? +* The number of bins in the histogram g_nof_bins is a power of 2 and + g_nof_bins_w <= g_data_w, so each bin counts the occurance of a subrange + with 2**(g_data_w - g_nof_bins_w) samples values. +* The sample values in range g_data_w get mapped to g_nof_bins by counting + how often the sample value(s) for that bin occur during a sync interval. +* The sum of the histogram bin values in each sync interval is : sum(bins) + = N - K (klopt dat)? @@ -43,60 +58,90 @@ Eigenlijk ontbreekt nog de beschrijving op functioneel niveau, los van de implem ==> EK comment -Ik snap niet waarom ram_clear en ram_fill nodig zijn op het MM interface. Voor ADUH_MON mean, power en voor SST is dat ook niet nodig. +Ik snap niet waarom ram_clear en ram_fill nodig zijn op het MM interface. +Voor ADUH_MON mean, power en voor SST is dat ook niet nodig. -Eigenlijk moet het zo zijn dat de user op elk moment st_histogram RAM uit mag lezen, zonder extra polling of controlling. Als de user toevallig leest terwijl de pages swap, dan zal een deel van de bins bij het ene sync interval horen en de rest van de bins uit het volgende sync interval komen, maar dat is niet erg, want waarschijnlijk veranderd de histogram toch niet veel van sync interval op sync interval. Daarbij komt dat de kans dat de user precies tijdens een sync het histogram leest klein is, want lezen duurt veel korter dan 1 sync interval. +Eigenlijk moet het zo zijn dat de user op elk moment st_histogram RAM uit mag +lezen, zonder extra polling of controlling. Als de user toevallig leest +terwijl de pages swap, dan zal een deel van de bins bij het ene sync interval +horen en de rest van de bins uit het volgende sync interval komen, maar dat +is niet erg, want waarschijnlijk verandert de histogram toch niet veel van +sync interval op sync interval. Daarbij komt dat de kans dat de user precies +tijdens een sync het histogram leest klein is, want lezen duurt veel korter +dan 1 sync interval. -Kunnen we st_histogram_reg.vhd verwijderen van het MM interface en vervangen door een intern VHDL process dat op fixed momenten de write velden bedient ? +Kunnen we st_histogram_reg.vhd verwijderen van het MM interface en vervangen +door een intern VHDL process dat op fixed momenten de write velden bedient ? -In een los Python scriptje is st_histogram_reg nog wel te bedienen, maar in een compleet M&C systeem (driver) is het denk ik te ingewikkeld en te belastend. +In een los Python scriptje is st_histogram_reg nog wel te bedienen, maar in +een compleet M&C systeem (driver) is het denk ik te ingewikkeld en te +belastend. ==> DS antwoord: -De ram_clear control is per abuis achtergebleven; deze was al vervangen voor een automatisch VHDL proces. Ik heb dit gefixt in de code. +De ram_clear control is per abuis achtergebleven; deze was al vervangen voor +een automatisch VHDL proces. Ik heb dit gefixt in de code. -De ram_fill control bedient in essentie een g_nof_instances (12 voor LOFAR) naar 1 DP->MM selector. Er is dus maar 1 MM RAM en de user kiest middels de ram_fill control van welke van de g_nof_instances (12 voor lofar) de inhoud in de MM RAM moet staan. Dit bespaart g_nof_instances-1 RAM blocks. +De ram_fill control bedient in essentie een g_nof_instances (12 voor LOFAR) +naar 1 DP->MM selector. Er is dus maar 1 MM RAM en de user kiest middels de +ram_fill control van welke van de g_nof_instances (12 voor lofar) de inhoud +in de MM RAM moet staan. Dit bespaart g_nof_instances-1 RAM blocks. -Goed punt over de M&C belasting hierdoor. We zouden hier (automated VHDL proces ipv MM control) een JIRA ticket van kunnen maken. +Goed punt over de M&C belasting hierdoor. We zouden hier (automated VHDL +proces ipv MM control) een JIRA ticket van kunnen maken. 4) tb_mms_st_histogram.vhd ==> EK comment: -Only needs to verify that the MM interface is connected properly, so should be a minimal tb. The details of the st_histogram inner workings have already been verified by tb_st_histogram. +Only needs to verify that the MM interface is connected properly, so should +be a minimal tb. The details of the st_histogram inner workings have already +been verified by tb_st_histogram. ==> DS antwoord: -tb_st_histogram tests only a single instance. tb_mms_st_histogram tests mms_st_histogram which has multi-instance support and the ram_fill control discussed above. Depending on what we do with ram_fill topic, we might no longer need the tb_mms. Possible JIRA ticket +tb_st_histogram tests only a single instance. tb_mms_st_histogram tests +mms_st_histogram which has multi-instance support and the ram_fill control +discussed above. Depending on what we do with ram_fill topic, we might no +longer need the tb_mms. Possible JIRA ticket 5) tb_st_histogram.vhd a) EK comment: -The tb does not use reg_mosi, is that not needed ? But why is reg_mosi port then on the st_histogram ? +The tb does not use reg_mosi, is that not needed ? But why is reg_mosi port +then on the st_histogram ? ==> DS antwoord: -st_histogram.vhd does not have a reg_mosi port. tb_st_histogram tests only a single instance. tb_mms_st_histogram tests mms_st_histogram which has multi-instance support and the ram_fill control discussed above. Depending on what we do with ram_fill topic, we might no longer need the tb_mms. Possible JIRA ticket +st_histogram.vhd does not have a reg_mosi port. tb_st_histogram tests only a +single instance. tb_mms_st_histogram tests mms_st_histogram which has +multi-instance support and the ram_fill control discussed above. Depending +on what we do with ram_fill topic, we might no longer need the tb_mms. +Possible JIRA ticket b) EK comment: What kind of data is used, does proc_dp_gen_block_data increment it? -Have you tested using DC data ? I think having multiple sample values in a row that are important to try too. +Have you tested using DC data ? I think having multiple sample values in a row +that are important to try too. ==> DS antwoord: -Good point, we're only using counter data at this point. st_histogram should work with DC data but this really needs to be added to the TB. Possible JIRA ticket +Good point, we're only using counter data at this point. st_histogram should +work with DC data but this really needs to be added to the TB. Possible JIRA +ticket. c) EK comment: -Can you also check in a process that sum(bins) has always the expected constant value ? +Can you also check in a process that sum(bins) has always the expected +constant value ? ==> DS antwoord: Collapse replies -This would be a good addition to the TB. Possible (par of) JIRE ticket +This would be a good addition to the TB. Possible (par of) JIRE ticket.