MiSTer-devel / GBA_MiSTer

GBA for MiSTer
GNU General Public License v2.0
147 stars 44 forks source link

Update Sys, Organize OSD options, Release 20220417 #116

Closed birdybro closed 2 years ago

birdybro commented 2 years ago
birdybro commented 2 years ago

Here is the timing closure recommendations summary from quartus:

+------------------------------------------------------------------------------------------------------------+
; Setup Summary                                                                                              ;
+-----------------------------------------------------------------------------------+--------+---------------+
; Clock                                                                             ; Slack  ; End Point TNS ;
+-----------------------------------------------------------------------------------+--------+---------------+
; emu|pll|pll_inst|altera_pll_i|general[0].gpll~PLL_OUTPUT_COUNTER|divclk           ; 0.244  ; 0.000         ;
; pll_hdmi|pll_hdmi_inst|altera_pll_i|cyclonev_pll|counter[0].output_counter|divclk ; 0.526  ; 0.000         ;
; emu|pll|pll_inst|altera_pll_i|general[1].gpll~PLL_OUTPUT_COUNTER|divclk           ; 1.757  ; 0.000         ;
; sysmem|fpga_interfaces|clocks_resets|h2f_user0_clk                                ; 3.992  ; 0.000         ;
; spi_sck                                                                           ; 4.784  ; 0.000         ;
; FPGA_CLK1_50                                                                      ; 5.660  ; 0.000         ;
; FPGA_CLK2_50                                                                      ; 11.823 ; 0.000         ;
; pll_audio|pll_audio_inst|altera_pll_i|general[0].gpll~PLL_OUTPUT_COUNTER|divclk   ; 13.712 ; 0.000         ;
+-----------------------------------------------------------------------------------+--------+---------------+

+-----------------------------------------------------------------------------------------------------------+
; Hold Summary                                                                                              ;
+-----------------------------------------------------------------------------------+-------+---------------+
; Clock                                                                             ; Slack ; End Point TNS ;
+-----------------------------------------------------------------------------------+-------+---------------+
; pll_hdmi|pll_hdmi_inst|altera_pll_i|cyclonev_pll|counter[0].output_counter|divclk ; 0.205 ; 0.000         ;
; emu|pll|pll_inst|altera_pll_i|general[0].gpll~PLL_OUTPUT_COUNTER|divclk           ; 0.249 ; 0.000         ;
; emu|pll|pll_inst|altera_pll_i|general[1].gpll~PLL_OUTPUT_COUNTER|divclk           ; 0.254 ; 0.000         ;
; sysmem|fpga_interfaces|clocks_resets|h2f_user0_clk                                ; 0.346 ; 0.000         ;
; FPGA_CLK2_50                                                                      ; 0.383 ; 0.000         ;
; pll_audio|pll_audio_inst|altera_pll_i|general[0].gpll~PLL_OUTPUT_COUNTER|divclk   ; 0.383 ; 0.000         ;
; FPGA_CLK1_50                                                                      ; 0.415 ; 0.000         ;
; spi_sck                                                                           ; 0.425 ; 0.000         ;
+-----------------------------------------------------------------------------------+-------+---------------+

+------------------------------------------------------------------------------------------------------------+
; Recovery Summary                                                                                           ;
+-----------------------------------------------------------------------------------+--------+---------------+
; Clock                                                                             ; Slack  ; End Point TNS ;
+-----------------------------------------------------------------------------------+--------+---------------+
; pll_hdmi|pll_hdmi_inst|altera_pll_i|cyclonev_pll|counter[0].output_counter|divclk ; 3.850  ; 0.000         ;
; sysmem|fpga_interfaces|clocks_resets|h2f_user0_clk                                ; 6.649  ; 0.000         ;
; FPGA_CLK1_50                                                                      ; 12.771 ; 0.000         ;
; emu|pll|pll_inst|altera_pll_i|general[1].gpll~PLL_OUTPUT_COUNTER|divclk           ; 15.931 ; 0.000         ;
+-----------------------------------------------------------------------------------+--------+---------------+

+-----------------------------------------------------------------------------------------------------------+
; Removal Summary                                                                                           ;
+-----------------------------------------------------------------------------------+-------+---------------+
; Clock                                                                             ; Slack ; End Point TNS ;
+-----------------------------------------------------------------------------------+-------+---------------+
; pll_hdmi|pll_hdmi_inst|altera_pll_i|cyclonev_pll|counter[0].output_counter|divclk ; 1.075 ; 0.000         ;
; FPGA_CLK1_50                                                                      ; 1.088 ; 0.000         ;
; sysmem|fpga_interfaces|clocks_resets|h2f_user0_clk                                ; 1.513 ; 0.000         ;
; emu|pll|pll_inst|altera_pll_i|general[1].gpll~PLL_OUTPUT_COUNTER|divclk           ; 2.509 ; 0.000         ;
+-----------------------------------------------------------------------------------+-------+---------------+
sorgelig commented 2 years ago

There is no point to enforce synth/fitter without a real reason. Recent ascal update relaxes the timings. Higher effort may give timing closure according to existing rules but may miss timings not described in rules due to possible "overclock" and other tricks in code. Also don't force new releases in your PRs!

RobertPeip commented 2 years ago

GBA often has timing issues taking me several tries to get a map with timing closure. If it works better that way, I'm fine with it.

I agree that releases should not be forced. In this case i talked with birdy before about it, so it was intended.

birdybro commented 2 years ago

Sorry @sorgelig, I should have mentioned I already discussed this with Robert beforehand.

Robert also helped explain to me the concerns you brought up here and I will greatly expand my testing of cores before I release as a result in the future. I hadn't considered the possibility of things like an increased chance of marginal timings due to out of spec and unconstrained intentional tricks.

sorgelig commented 2 years ago

i see. ok.