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

LCD_CAM example doesn't work in release #1532

Open
JurajSadel opened this issue May 2, 2024 · 7 comments
Open

LCD_CAM example doesn't work in release #1532

JurajSadel opened this issue May 2, 2024 · 7 comments
Labels
bug Something isn't working peripheral:lcd_cam LCD_CAM peripheral
Milestone

Comments

@JurajSadel
Copy link
Contributor

While the lcd_i8080 example works as expected in debug (display blinking in red and blue), there is some issue in release profile - display blinking in black and white. Needs further investigation.

@JurajSadel JurajSadel added bug Something isn't working help wanted Extra attention is needed labels May 2, 2024
@jessebraham jessebraham added peripheral:lcd_cam LCD_CAM peripheral and removed help wanted Extra attention is needed labels May 3, 2024
@Dominaezzz Dominaezzz mentioned this issue Jun 4, 2024
5 tasks
@Dominaezzz
Copy link
Contributor

See #1651 (comment)

In short, the buffers being used end up in non-DMA capable memory, but only in release mode.

@bjoernQ
Copy link
Contributor

bjoernQ commented Jun 5, 2024

That it's only happening in release mode doesn't make much sense to me but from a brief look something like this

        i8080.send(CMD_CSCON, 0, &[0xC3]).unwrap(); // Enable extension command 2 part I

Will locate the array in DROM. Probably making it &mut [0xC3] would help. Unfortunately, it won't help with an empty array

@MabezDev
Copy link
Member

MabezDev commented Jun 5, 2024

The FlashSafeDma wrapper would help here (but is probably missing some methods/traits for LCD_CAM), but maybe if every driver that uses DMA might need this we should build it into the DMA driver itself. I guess the hard part is sizing the temporary buffer, it might be doable once we have some configuration system.

@bjoernQ
Copy link
Contributor

bjoernQ commented Jun 5, 2024

I thought about adding the "valid_ram_address" check in prepare_transfer_without_start but then I was wondering why using DROM doesn't trigger a DMA error and found this

image

Seems there are a lot of constraints when accessing external memory via DMA - not sure if we just shouldn't support it

@Dominaezzz
Copy link
Contributor

Dominaezzz commented Jun 5, 2024

My thoughts on this are similar to my proposal for SPI. The hal should have special buffer structs like DmaBuf, DmaExtRamBuf, etc. that validate the necessary hardware constraints in the constructor, then the user can then reuse these without paying for additional checks during each transfer. I feel like these better exposes the hardware capabilities (and sacrifices some ease of use).

On top of this can be a layer that does memcpy like FlashSafeDma does, and gives users a "it just works" experience (even if it's sorta slow).

but maybe if every driver that uses DMA might need this we should build it into the DMA driver itself.

I though the same thing but you then have the complication that every driver will have it's own way of how to correctly split up one big transfer into multiple chunks. i.e. You don't really want to restart the command phase of the transfer.

@Dominaezzz
Copy link
Contributor

Short term though, should the example be patched to ensure the buffers are in DRAM? Or should it wait for the driver to be changed?

@bjoernQ
Copy link
Contributor

bjoernQ commented Jun 5, 2024

Short term though, should the example be patched to ensure the buffers are in DRAM? Or should it wait for the driver to be changed?

I don't have a strong opinion here - leaving it as is doesn't make anything worse but on the other hand (and since the xtask always runs examples in release mode) it would probably be nice to have this working "out-of-the-box"

@jessebraham jessebraham added this to the 0.19.0 milestone Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working peripheral:lcd_cam LCD_CAM peripheral
Projects
Status: Todo
Development

No branches or pull requests

5 participants