Xilinx / Xilinx/PYNQ

Register called "width" causes RecursionError

Open
#1,480 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Jupyter Notebook
Stars
2.4k
Forks
866
Avg merge
11h 58m
Merged PRs (30d)
32

Description

  • 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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in pynq/overlay.py at the register_map property and pynq/registers.py around RegisterMap initialization and the Register width access shown in the stack trace. Reproduce the failure with the BGR2RGB driver and a register named width, then verify that register_map access and assignment complete without RecursionError while the existing register behavior remains intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
embedded-iot
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.