Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected result in SCANVIDEO if sync pins below colour pins #36

Open
Memotech-Bill opened this issue May 24, 2022 · 6 comments
Open

Comments

@Memotech-Bill
Copy link

I am trying to build a VGA interface using only 8 GPIO pins. Two pins for sync and two pins each for red, green and blue.

This is not currently working for my board.

If I have the following definitions in my board header file:

#define PICO_SCANVIDEO_SYNC_PIN_BASE 6
#define PICO_SCANVIDEO_HSYNC_PIN 6
#define PICO_SCANVIDEO_VSYNC_PIN 7
#define PICO_SCANVIDEO_ALPHA_PIN -1
#define PICO_SCANVIDEO_COLOR_PIN_BASE 0
#define PICO_SCANVIDEO_COLOR_PIN_COUNT 6
#define PICO_SCANVIDEO_PIXEL_RSHIFT 0
#define PICO_SCANVIDEO_PIXEL_GSHIFT 2
#define PICO_SCANVIDEO_PIXEL_BSHIFT 4
#define PICO_SCANVIDEO_PIXEL_RCOUNT 2
#define PICO_SCANVIDEO_PIXEL_GCOUNT 2
#define PICO_SCANVIDEO_PIXEL_BCOUNT 2

Then, loading this onto the Pico I measure the expected 31.25kHz on GPIO 6 and 60Hz on GPIO 7.

However, my board has the pins in a different order. If I compile with the following board definition:

#define PICO_SCANVIDEO_SYNC_PIN_BASE 0
#define PICO_SCANVIDEO_HSYNC_PIN 0
#define PICO_SCANVIDEO_VSYNC_PIN 1
#define PICO_SCANVIDEO_ALPHA_PIN -1
#define PICO_SCANVIDEO_COLOR_PIN_BASE 2
#define PICO_SCANVIDEO_COLOR_PIN_COUNT 6
#define PICO_SCANVIDEO_PIXEL_RSHIFT 0
#define PICO_SCANVIDEO_PIXEL_GSHIFT 2
#define PICO_SCANVIDEO_PIXEL_BSHIFT 4
#define PICO_SCANVIDEO_PIXEL_RCOUNT 2
#define PICO_SCANVIDEO_PIXEL_GCOUNT 2
#define PICO_SCANVIDEO_PIXEL_BCOUNT 2

and load the resulting executable onto the Pico, I get the expected 60Hz on GPIO 1, but GPIO 0 measures 25MHz!

So it appears that, for some reason, if the sync pins are below the colour pins, what should be HSYNC is actually the pixel clock.

Any obvious reason for this?

@Memotech-Bill
Copy link
Author

As a further experiment, I tried the following board definition:

#define PICO_SCANVIDEO_SYNC_PIN_BASE 2
#define PICO_SCANVIDEO_HSYNC_PIN 2
#define PICO_SCANVIDEO_VSYNC_PIN 3
#define PICO_SCANVIDEO_ALPHA_PIN -1
#define PICO_SCANVIDEO_COLOR_PIN_BASE 4
#define PICO_SCANVIDEO_COLOR_PIN_COUNT 6
#define PICO_SCANVIDEO_PIXEL_RSHIFT 0
#define PICO_SCANVIDEO_PIXEL_GSHIFT 2
#define PICO_SCANVIDEO_PIXEL_BSHIFT 4
#define PICO_SCANVIDEO_PIXEL_RCOUNT 2
#define PICO_SCANVIDEO_PIXEL_GCOUNT 2
#define PICO_SCANVIDEO_PIXEL_BCOUNT 2

This gives the expected 31.25kHz on GPIO 2. So it looks as the problem is only if the HSYNC is on GPIO 0.

@kilograham
Copy link
Contributor

The setup code in scanvideo,c is definitely not correct for certain pin combinations - there are a few PRs from people to fix very scenarios, but you may want to look at the code.

@Memotech-Bill
Copy link
Author

Adding some diagnostics and building with my board configuration with PICO_SCANVIDEO_SYNC_PIN_BASE = 0 and PICO_SCANVIDEO_COLOR_PIN_BASE = 2 gives:

State machine 0: execctrl = 0x0002D580, shiftctrl = 0x400E0000, pinctrl = 0x00600002
pinctrl: in: base = 0, out: base = 2, count = 6, set: base = 0, count = 0, sideset: base = 0, count = 0
State machine 1: execctrl = 0x0001F000, shiftctrl = 0x000C0000, pinctrl = 0x14000000
pinctrl: in: base = 0, out: base = 0, count = 0, set: base = 0, count = 5, sideset: base = 0, count = 0
State machine 2: execctrl = 0x0001F000, shiftctrl = 0x000C0000, pinctrl = 0x14000000
pinctrl: in: base = 0, out: base = 0, count = 0, set: base = 0, count = 5, sideset: base = 0, count = 0
State machine 3: execctrl = 0x0001FD80, shiftctrl = 0x000E0000, pinctrl = 0x20200000
pinctrl: in: base = 0, out: base = 0, count = 2, set: base = 0, count = 0, sideset: base = 0, count = 1

So the timing SM (3) has a sideset count of 1, even though PICO_SCANVIDEO_ENABLE_CLOCK_PIN = 0. Looking at "timing.pio.h" reveals:

static inline pio_sm_config video_htiming_program_get_default_config(uint offset) {
    pio_sm_config c = pio_get_default_sm_config();
    sm_config_set_wrap(&c, offset + video_htiming_wrap_target, offset + video_htiming_wrap);
    sm_config_set_sideset(&c, 1, false, false);
    return c;
}

So the PIO compiler is generating a default configuration which unconditionally sets a sideset count of 1. However, since PICO_SCANVIDEO_ENABLE_CLOCK_PIN = 0, the sideset pin base is being left undefined (and is defaulting to zero).

It is suggested that routine setup_sm in pico-extras/src/rp2_common/pico_scanvideo_dpi/scanvideo.c should be revised as follows

void setup_sm(int sm, uint offset) {
#ifndef NDEBUG
    printf("Setting up SM %d\n", sm);
#endif
 
    pio_sm_config config = is_scanline_sm(sm) ? video_mode.pio_program->configure_pio(video_pio, sm, offset) :
                           video_htiming_program_get_default_config(offset);
 
#if PICO_SCANVIDEO_ENABLE_VIDEO_CLOCK_DOWN
    sm_config_set_clkdiv_int_frac(&config, video_clock_down_times_2 / 2, (video_clock_down_times_2 & 1u) << 7u);
#endif
 
    if (!is_scanline_sm(sm)) {
        // enable auto-pull
        sm_config_set_out_shift(&config, true, true, 32);
        const uint BASE = PICO_SCANVIDEO_SYNC_PIN_BASE; // hsync and vsync are +0 and +1, clock is +2
        uint pin_count;
#if PICO_SCANVIDEO_ENABLE_DEN_PIN
        pin_count = 3;
        // 3 OUT pins and maybe 1 sideset pin following them
#else
        // 2 OUT pins and 1 sideset pin following them
        pin_count = 2;
#endif
        sm_config_set_out_pins(&config, BASE, pin_count);
#if PICO_SCANVIDEO_ENABLE_CLOCK_PIN
        // side set pin as well
        sm_config_set_sideset_pins(&config, BASE + pin_count);
        pin_count++;
#else
        sm_config_set_sideset(&config, 0, false, false);
#endif
        pio_sm_set_consecutive_pindirs(video_pio, sm, BASE, pin_count, true);
    }
 
    pio_sm_init(video_pio, sm, offset, &config); // now paused
}

@lurch
Copy link
Contributor

lurch commented May 25, 2022

It is suggested that...

If you've tested that as working, you should probably submit a PR ?

@Memotech-Bill
Copy link
Author

Memotech-Bill commented May 25, 2022

I have now tested it, and it does not work :(

Changing the sideset count changes the sidesest data in the PIO program into delay data, wrecking the timing.

Three options:

  • Have a separate PIO program without sideset instructions for the case without a clock output.
  • Assign the sideset base to a pin which is not configured for PIO control.
  • Dynamically edit the PIO program to remove the sideset data

Edit:
Since the SCANVIDEO code already dynamically edits the "timing.pio" program for the polarity of the clock signal, making the same code dynamically remove the sideset if there is no clock is probably the way to go.

I have partially tested this, and the sync frequencies are now correct. I have yet to check that I now have a working display signal.

@Memotech-Bill
Copy link
Author

I have now tested my revisions to dynamically edit the timing PIO program to remove the pixel clock side set if PICO_SCANVIDEO_ENABLE_CLOCK_PIN is zero (disabled) and confirmed that I now get a working VGA signal.

I have created a pull request with my changes: https://github.com/raspberrypi/pico-extras/pull/37/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants