PYNQ icon indicating copy to clipboard operation
PYNQ copied to clipboard

Register called "width" causes RecursionError

Open MarioMihaly opened this issue 7 months ago • 1 comments

  • PYNQ version (e.g. v2.6): v2.7
  • Board name (e.g. Pynq-Z2): ADM-XRC-9Z1 (custom US+ MPSoC)
  • Pull request type (bug fix / new feature): bug fix
  • Pull request number: -
  • Description: Register access using register_map breaks down for Vitis HLS IP with width argument

I created the following BGR2RGB HLS IP using the Vitis Vision Library:

[!NOTE] config.h contains constants and data types, utils.h contains xfMat2axis to produce a stream where last is only asserted for the last pixel, compatible with standard DMA engines. These files are omitted for brevity, can be provided on request.

// bgr2rgb.h
#include "config.h"

// Accelerated function prototype
extern "C"
{
    void bgr2rgb_accel( stream_t& stream_in, stream_t& stream_out, unsigned int width, unsigned int height );
}
// bgr2rgb.cpp
#include "bgr2rgb.h"
#include "utils.h"
#include <common/xf_common.hpp>
#include <imgproc/xf_cvt_color.hpp>

extern "C"
{
    void bgr2rgb_accel( stream_t& stream_in, stream_t& stream_out, unsigned int width, unsigned int height )
    {
#pragma HLS INTERFACE axis register both port = stream_in
#pragma HLS INTERFACE axis register both port = stream_out
#pragma HLS INTERFACE s_axilite          port = width
#pragma HLS INTERFACE s_axilite          port = height
#pragma HLS INTERFACE s_axilite          port = return

        xf::cv::Mat<TYPE, HEIGHT, WIDTH, NPC_T> img_in( width, height );
        xf::cv::Mat<TYPE, HEIGHT, WIDTH, NPC_T> img_out( width, height );

#pragma HLS DATAFLOW

        // Convert stream in to xf::cv::Mat
        axis2xfMat<DATA_WIDTH, TYPE, HEIGHT, WIDTH, NPC_T>( stream_in, img_in );

        // Run xfOpenCV kernel:
        xf::cv::bgr2rgb<TYPE, TYPE, HEIGHT, WIDTH, NPC_T>( img_in, img_out );

        // Convert xf::cv::Mat to stream
        xfMat2axis<DATA_WIDTH, TYPE, HEIGHT, WIDTH, NPC_T>( img_out, stream_out );
        return;
    }
}

Using the following custom driver, when the config, start or stop methods are called or any direct calls to self.register_map causes RecursionError:

from pynq import DefaultIP

class BGR2RGB(DefaultIP):
    """Colour channel conversion IP handling BGR to RGB conversion."""

    bindto = ["xilinx.com:hls:bgr2rgb_accel:1.0"]

    def __init__(self, description):
        super().__init__(description)

    def config(self, height: int, width: int) -> None:
        """
        Configure the IP with the image resolution.

        Args:
            height (int): image height in pixels.
            width (int): image width in pixels.
        """

        self.register_map.height = height
        self.register_map.width = width

    def start(self) -> None:
        """Start the IP in auto restart mode."""
        self.register_map.CTRL = 0x81

    def stop(self) -> None:
        """Stop the IP."""
        self.register_map.CTRL = 0x00

Example stack trace:

[!NOTE] Stack trace is from larger design that works if I use (rows, cols) instead of (height, width).

---------------------------------------------------------------------------
RecursionError                            Traceback (most recent call last)
<ipython-input-5-88146cb72c9d> in <module>
      3 std_devs = (255.0, 255.0, 255.0)
      4 
----> 5 ol.config(['bgr2rgb', 'letterbox', 'preprocess'], in_h, in_w, out_h, out_w, std_devs=std_devs)

/usr/local/share/pynq-venv/lib/python3.8/site-packages/pynq_mlvap/mlvap.py in config(self, steps, in_h, in_w, out_h, out_w, means, std_devs, fix_point)
    223         for step in steps:
    224             if step == "bgr2rgb":
--> 225                 self._bgr2rgb.config(next_h, next_w)
    226                 self._steps.append(self._bgr2rgb)
    227             elif step == "resize":

/usr/local/share/pynq-venv/lib/python3.8/site-packages/pynq_mlvap/libs.py in config(self, height, width)
     78         """
     79 
---> 80         self.register_map.height = height
     81         self.register_map.width = width
     82 

/usr/local/share/pynq-venv/lib/python3.8/site-packages/pynq/overlay.py in register_map(self)
    737         if not hasattr(self, '_register_map'):
    738             if self._registers:
--> 739                 self._register_map = RegisterMap.create_subclass(
    740                     self._register_name,
    741                     self._registers)(self.mmio.array)

/usr/local/share/pynq-venv/lib/python3.8/site-packages/pynq/registers.py in __init__(self, buffer)
    402                 continue
    403 
--> 404             self._instances[k] = v[0](
    405                 address=v[1], width=align_width, buffer=array, access=v[5])
    406 

/usr/local/share/pynq-venv/lib/python3.8/site-packages/pynq/registers.py in __init__(self, address, width, debug, buffer, access)
    141 
    142         self.address = address
--> 143         self.width = width
    144         self.debug = debug
    145         self.access = access

/usr/local/share/pynq-venv/lib/python3.8/site-packages/pynq/registers.py in _reordered_setitem(self, value, index)
    234 
    235         """
--> 236         return self.__setitem__(index, value)
    237 
    238     def __str__(self):

/usr/local/share/pynq-venv/lib/python3.8/site-packages/pynq/registers.py in __setitem__(self, index, value)
    203 
    204         """
--> 205         lower, upper, reverse = _calc_index(index, self.width)
    206         if upper == lower:
    207             if value != 0 and value != 1:

/usr/local/share/pynq-venv/lib/python3.8/site-packages/pynq/registers.py in __getitem__(self, index)
    173 
    174         """
--> 175         lower, upper, reverse = _calc_index(index, self.width)
    176         curr_val = int(self._buffer[0])
    177         if self.access == 'write-only':

... last 1 frames repeated, from the frame below ...

/usr/local/share/pynq-venv/lib/python3.8/site-packages/pynq/registers.py in __getitem__(self, index)
    173 
    174         """
--> 175         lower, upper, reverse = _calc_index(index, self.width)
    176         curr_val = int(self._buffer[0])
    177         if self.access == 'write-only':

RecursionError: maximum recursion depth exceeded while calling a Python object

MarioMihaly avatar Jul 07 '25 09:07 MarioMihaly

Hi @MarioMihaly

Thanks for submitting this. I'm surprised this hasn't come up before.

I guess there should be a reserved name check to avoid conflicts like this. This would at least give a useful error message for the user.

From a quick glance this reserved list would contain: address, width, debug, access and buffer.

Will look into fixing this soon.

jogomojo avatar Jul 07 '25 13:07 jogomojo