12 Nov 2024 |
| @flovanmt:matrix.org left the room. | 05:29:18 |
almindor | I was able to further simplify the implementation using a specialization hack. At least now it doesn't need Model changes in the driver. https://github.com/almindor/mipidsi/pull/146 | 17:58:43 |
almindor | GrantM11235: rfuest What are your guys' thoughts on how to handle buffers? Is there some abstraction already for this? I think best we can do is something like a Buffers type that can then be used for various parts of the stack e.g. DI would use it to obtain a buffer for spi cases, mipidsi/other drivers could use it to obtain a buffer for their buffering needs and ultimately the user would construct this so that the actual buffer could point to say DMA friendly memory. | 21:34:55 |
GrantM11235 | IMO, only the interface impl should have a buffer and deal with endianness | 21:44:54 |
GrantM11235 | In reply to @grantm11235:matrix.org
Something like this:
pub trait Interface<P: Copy> {
type Error: core::fmt::Debug;
fn send_command(&mut self, command: u8, args: &[u8]) -> Result<(), Self::Error>;
fn send_pixels(&mut self, pixels: impl Iterator<Item = P>) -> Result<(), Self::Error>;
fn send_repeated_pixel(&mut self, pixel: P, count: u32) -> Result<(), Self::Error> {
self.send_pixels((0..count).map(|_| pixel))
}
fn flush(&mut self) -> Result<(), Self::Error>;
}
Are you interested in a PR like this to replace d-i with a custom trait? I had a 90% working version a while ago but I ran into a snag somewhere. | 21:48:09 |
almindor | We can try it, my main concern is that I need a buffer inside mipidsi too | 21:50:26 |
almindor | so it'll also need to be accessible from the "DI" | 21:50:34 |
almindor | I'd also like to avoid polluting with even more generic params but I'm not sure if that's possible | 21:51:01 |
almindor | IMO the buffer should just be a &mut [u8] direct, I don't see a reason for anything different. If someone needs say u16 stepping they can still use it without performance impact | 21:51:36 |
GrantM11235 | In reply to @almindor:matrix.org We can try it, my main concern is that I need a buffer inside mipidsi too For what? (I haven't looked inside mipidsi in a while) | 21:58:24 |
almindor | In reply to @grantm11235:matrix.org For what? (I haven't looked inside mipidsi in a while) Well in this case for example for the fill_solid optimization (see above PR) | 22:35:34 |
almindor | doing a buffer instead of iterator is > 2x faster | 22:36:07 |
GrantM11235 | Yeah, but that would be replaced with a repeat pixel method in the new interface trait. Do you need a buffer for anything else? | 22:37:13 |
almindor | there's batching, although that one is currently using a specific buffer (e.g. rows/columns kind of thing instead of u8s) | 22:38:01 |
almindor | it's used for sort of contiguous batches that are more complex than a rect | 22:38:34 |
almindor | it'll "rect them up" as much as possible and batch send those | 22:38:43 |
almindor | this is generic, outside of e-g stuff | 22:38:58 |
almindor | https://github.com/almindor/mipidsi/blob/master/mipidsi/src/batch.rs this stuff | 22:41:37 |
almindor | not sure if it's worth forcing into a general buffer tho | 22:42:00 |
almindor | the problem with the DI approach (in general) is that it's a information lossy abstraction | 22:44:49 |
almindor | we lose "metainformation" that can be quite valuable, such as in this case, you cannot tell DI "oh and batch these up according to pixel continuity" you have to do it driver side | 22:45:21 |
GrantM11235 | I think it is possible to batch contiguous-ish draw_iter pixels without a buffer | 22:48:00 |
almindor | this comes into play with things like Circle for example. Rects will use solid or contiguous, but non rect shapes will just send a pixel iterator | 22:48:12 |
almindor | I remember it has major impact on performance for those, like visible difference between non-batched and batched, coz the other alternative is "pixel by pixel, write each" essentially.
The DI is not away of what a pixel is | 22:49:19 |
almindor | we could possibly introduce somehing like a closure argument of "is-contiguous -> bool" kind of thing for auto batching I guess | 22:49:46 |
GrantM11235 | Basically, you start with the first pixel and guess what the bounding box should be. Then for each pixel, if it is the next one in the box, send it, otherwise guess again what the bounding box should be | 22:50:00 |
almindor | how do you guess though? | 22:50:20 |
almindor | you only see a pixel, e.g. point + color | 22:50:29 |
GrantM11235 | For the first pixel, guess that it starts at that pixel and goes to the bottom right of the screen | 22:51:16 |
almindor | the best I can think of is keeping the previous pixel cached in the driver, and providing the DI with a "is-contiguous" callback to see if it can keep batching | 22:51:29 |