| Message ID | 20250718082402.1207460-1-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 07. 18. 10:23 keltezéssel, Naushir Patuck írta: > There is a possible static initialisation issue with accessing the > HardwareConfigMap static object through Controller::getHardwareConfig(). > Fix this by providing a static function &hardwareConfigMap() to access > the object. > > Though not proven, this is possibly the cause of a very infrequent > lockup in https://github.com/raspberrypi/rpicam-apps/issues/799. I really believe non-`constinit` (C++20) globals should removed altogether... A simple array with a for loop would be sufficient in many (most?) cases. And what cannot be made `constinit` should be moved into a getter function like this. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/rpi/controller/controller.cpp | 119 ++++++++++++++------------ > 1 file changed, 64 insertions(+), 55 deletions(-) > > diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp > index 651fff632400..72c0592c9585 100644 > --- a/src/ipa/rpi/controller/controller.cpp > +++ b/src/ipa/rpi/controller/controller.cpp > @@ -21,61 +21,70 @@ using namespace std::literals::chrono_literals; > > LOG_DEFINE_CATEGORY(RPiController) > > -static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap = { > - { > - "bcm2835", > +namespace { > + > +const std::map<std::string, Controller::HardwareConfig> &hardwareConfigMap() { const std::map<std::string, Controller::HardwareConfig> &hardwareConfigMap() { Regards, Barnabás Pőcze > + > + static const std::map<std::string, Controller::HardwareConfig> map = { > { > - /* > - * There are only ever 15 AGC regions computed by the firmware > - * due to zoning, but the HW defines AGC_REGIONS == 16! > - */ > - .agcRegions = { 15 , 1 }, > - .agcZoneWeights = { 15 , 1 }, > - .awbRegions = { 16, 12 }, > - .cacRegions = { 0, 0 }, > - .focusRegions = { 4, 3 }, > - .numHistogramBins = 128, > - .numGammaPoints = 33, > - .pipelineWidth = 13, > - .statsInline = false, > - .minPixelProcessingTime = 0s, > - .dataBufferStrided = true, > - } > - }, > - { > - "pisp", > + "bcm2835", > + { > + /* > + * There are only ever 15 AGC regions computed by the firmware > + * due to zoning, but the HW defines AGC_REGIONS == 16! > + */ > + .agcRegions = { 15 , 1 }, > + .agcZoneWeights = { 15 , 1 }, > + .awbRegions = { 16, 12 }, > + .cacRegions = { 0, 0 }, > + .focusRegions = { 4, 3 }, > + .numHistogramBins = 128, > + .numGammaPoints = 33, > + .pipelineWidth = 13, > + .statsInline = false, > + .minPixelProcessingTime = 0s, > + .dataBufferStrided = true, > + } > + }, > { > - .agcRegions = { 0, 0 }, > - .agcZoneWeights = { 15, 15 }, > - .awbRegions = { 32, 32 }, > - .cacRegions = { 8, 8 }, > - .focusRegions = { 8, 8 }, > - .numHistogramBins = 1024, > - .numGammaPoints = 64, > - .pipelineWidth = 16, > - .statsInline = true, > - > - /* > - * The constraint below is on the rate of pixels going > - * from CSI2 peripheral to ISP-FE (400Mpix/s, plus tiny > - * overheads per scanline, for which 380Mpix/s is a > - * conservative bound). > - * > - * There is a 64kbit data FIFO before the bottleneck, > - * which means that in all reasonable cases the > - * constraint applies at a timescale >= 1 scanline, so > - * adding horizontal blanking can prevent loss. > - * > - * If the backlog were to grow beyond 64kbit during a > - * single scanline, there could still be loss. This > - * could happen using 4 lanes at 1.5Gbps at 10bpp with > - * frames wider than ~16,000 pixels. > - */ > - .minPixelProcessingTime = 1.0us / 380, > - .dataBufferStrided = false, > - } > - }, > -}; > + "pisp", > + { > + .agcRegions = { 0, 0 }, > + .agcZoneWeights = { 15, 15 }, > + .awbRegions = { 32, 32 }, > + .cacRegions = { 8, 8 }, > + .focusRegions = { 8, 8 }, > + .numHistogramBins = 1024, > + .numGammaPoints = 64, > + .pipelineWidth = 16, > + .statsInline = true, > + > + /* > + * The constraint below is on the rate of pixels going > + * from CSI2 peripheral to ISP-FE (400Mpix/s, plus tiny > + * overheads per scanline, for which 380Mpix/s is a > + * conservative bound). > + * > + * There is a 64kbit data FIFO before the bottleneck, > + * which means that in all reasonable cases the > + * constraint applies at a timescale >= 1 scanline, so > + * adding horizontal blanking can prevent loss. > + * > + * If the backlog were to grow beyond 64kbit during a > + * single scanline, there could still be loss. This > + * could happen using 4 lanes at 1.5Gbps at 10bpp with > + * frames wider than ~16,000 pixels. > + */ > + .minPixelProcessingTime = 1.0us / 380, > + .dataBufferStrided = false, > + } > + }, > + }; > + > + return map; > +} > + > +} /* namespace */ > > Controller::Controller() > : switchModeCalled_(false) > @@ -211,12 +220,12 @@ const std::string &Controller::getTarget() const > > const Controller::HardwareConfig &Controller::getHardwareConfig() const > { > - auto cfg = HardwareConfigMap.find(getTarget()); > + auto cfg = hardwareConfigMap().find(getTarget()); > > /* > * This really should not happen, the IPA ought to validate the target > * on initialisation. > */ > - ASSERT(cfg != HardwareConfigMap.end()); > + ASSERT(cfg != hardwareConfigMap().end()); > return cfg->second; > }
diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp index 651fff632400..72c0592c9585 100644 --- a/src/ipa/rpi/controller/controller.cpp +++ b/src/ipa/rpi/controller/controller.cpp @@ -21,61 +21,70 @@ using namespace std::literals::chrono_literals; LOG_DEFINE_CATEGORY(RPiController) -static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap = { - { - "bcm2835", +namespace { + +const std::map<std::string, Controller::HardwareConfig> &hardwareConfigMap() { + + static const std::map<std::string, Controller::HardwareConfig> map = { { - /* - * There are only ever 15 AGC regions computed by the firmware - * due to zoning, but the HW defines AGC_REGIONS == 16! - */ - .agcRegions = { 15 , 1 }, - .agcZoneWeights = { 15 , 1 }, - .awbRegions = { 16, 12 }, - .cacRegions = { 0, 0 }, - .focusRegions = { 4, 3 }, - .numHistogramBins = 128, - .numGammaPoints = 33, - .pipelineWidth = 13, - .statsInline = false, - .minPixelProcessingTime = 0s, - .dataBufferStrided = true, - } - }, - { - "pisp", + "bcm2835", + { + /* + * There are only ever 15 AGC regions computed by the firmware + * due to zoning, but the HW defines AGC_REGIONS == 16! + */ + .agcRegions = { 15 , 1 }, + .agcZoneWeights = { 15 , 1 }, + .awbRegions = { 16, 12 }, + .cacRegions = { 0, 0 }, + .focusRegions = { 4, 3 }, + .numHistogramBins = 128, + .numGammaPoints = 33, + .pipelineWidth = 13, + .statsInline = false, + .minPixelProcessingTime = 0s, + .dataBufferStrided = true, + } + }, { - .agcRegions = { 0, 0 }, - .agcZoneWeights = { 15, 15 }, - .awbRegions = { 32, 32 }, - .cacRegions = { 8, 8 }, - .focusRegions = { 8, 8 }, - .numHistogramBins = 1024, - .numGammaPoints = 64, - .pipelineWidth = 16, - .statsInline = true, - - /* - * The constraint below is on the rate of pixels going - * from CSI2 peripheral to ISP-FE (400Mpix/s, plus tiny - * overheads per scanline, for which 380Mpix/s is a - * conservative bound). - * - * There is a 64kbit data FIFO before the bottleneck, - * which means that in all reasonable cases the - * constraint applies at a timescale >= 1 scanline, so - * adding horizontal blanking can prevent loss. - * - * If the backlog were to grow beyond 64kbit during a - * single scanline, there could still be loss. This - * could happen using 4 lanes at 1.5Gbps at 10bpp with - * frames wider than ~16,000 pixels. - */ - .minPixelProcessingTime = 1.0us / 380, - .dataBufferStrided = false, - } - }, -}; + "pisp", + { + .agcRegions = { 0, 0 }, + .agcZoneWeights = { 15, 15 }, + .awbRegions = { 32, 32 }, + .cacRegions = { 8, 8 }, + .focusRegions = { 8, 8 }, + .numHistogramBins = 1024, + .numGammaPoints = 64, + .pipelineWidth = 16, + .statsInline = true, + + /* + * The constraint below is on the rate of pixels going + * from CSI2 peripheral to ISP-FE (400Mpix/s, plus tiny + * overheads per scanline, for which 380Mpix/s is a + * conservative bound). + * + * There is a 64kbit data FIFO before the bottleneck, + * which means that in all reasonable cases the + * constraint applies at a timescale >= 1 scanline, so + * adding horizontal blanking can prevent loss. + * + * If the backlog were to grow beyond 64kbit during a + * single scanline, there could still be loss. This + * could happen using 4 lanes at 1.5Gbps at 10bpp with + * frames wider than ~16,000 pixels. + */ + .minPixelProcessingTime = 1.0us / 380, + .dataBufferStrided = false, + } + }, + }; + + return map; +} + +} /* namespace */ Controller::Controller() : switchModeCalled_(false) @@ -211,12 +220,12 @@ const std::string &Controller::getTarget() const const Controller::HardwareConfig &Controller::getHardwareConfig() const { - auto cfg = HardwareConfigMap.find(getTarget()); + auto cfg = hardwareConfigMap().find(getTarget()); /* * This really should not happen, the IPA ought to validate the target * on initialisation. */ - ASSERT(cfg != HardwareConfigMap.end()); + ASSERT(cfg != hardwareConfigMap().end()); return cfg->second; }
There is a possible static initialisation issue with accessing the HardwareConfigMap static object through Controller::getHardwareConfig(). Fix this by providing a static function &hardwareConfigMap() to access the object. Though not proven, this is possibly the cause of a very infrequent lockup in https://github.com/raspberrypi/rpicam-apps/issues/799. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/ipa/rpi/controller/controller.cpp | 119 ++++++++++++++------------ 1 file changed, 64 insertions(+), 55 deletions(-)