drivers: display: ssd1322 driver #75550
Conversation
zephyrbot
added
area: Devicetree Binding
|
|
||
| row-offset: | ||
| type: int | ||
| default: 0 |
There was a problem hiding this comment.
There was a problem hiding this comment.
Generally looks ok, (although the default values do need justification).
@hlukasz would you prefer to merge your PR first once the merge window opens, then have this PR contain only the enhancements? Or would it be better for you to merge both commits within this PR? By default I'd bias towards merging #68608 first, but it would be more efficient to batch the commits into one PR
| }; | ||
|
|
||
| static inline int ssd1322_write_command(const struct device *dev, uint8_t cmd, | ||
| const uint8_t *buf, size_t len) | ||
| static inline int ssd1322_write_command(const struct device *dev, uint8_t cmd, const uint8_t *buf, |
There was a problem hiding this comment.
Please do not make formatting changes like this in the same commit as functional additions. If you would like to format a file before editing, IE with clang-format, then please do so and keep the formatting changes in one commit. This makes the review process much easier
|
I don't have strong opinion on that. If we can merge both patches as one MR in next merging window, let's go that way. |
Ok, we can merge both in one PR then. @topisani when you get a chance- it probably makes the most sense to squash the driver itself into one commit, since the driver introduction and extensions are happening in one PR. Please make sure that @hlukasz is a co-author on the commit, and that copyrights are retained- I want to be sure credit is present where it is due :) This also allows us to avoid the clang-format changes, since the driver changes will all be in one commit |
Initial support for SSD1322 OLED display driver. Only 1 bit color mode is supported. Most options map directly to values documented in the datasheet, except segments-per-pixel, which I had to add to support the Newhaven NHD-2.7-12864WDW3. This is a slightly odd feature, but in practice it is a lot nicer to support it in the driver, and since we're currently remapping pixels anyway, it makes sense to implement here. This driver also has a configurable buffer size for the pixel conversion. By using a larger buffer, we can potentially use DMA for the SPI transfer. The default is set to 8, which is the smallest value that supports segments-per-pixel = 2 Initial driver implementation by Lukasz Hawrylko <[email protected]>. Additional options and configurability by Tobias Pisani <[email protected]> Signed-off-by: Lukasz Hawrylko <[email protected]> Signed-off-by: Tobias Pisani <[email protected]> Co-authored-by: Lukasz Hawrylko <[email protected]> Co-authored-by: Tobias Pisani <[email protected]>
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
@jfischer-no would you be willing to review this PR? I know you had some comments on #68608, and I'm wondering if we could them resolved here to get this driver moving forwards |
This is based on #68608, but adds more configuration options
Most options map directly to values documented in the datasheet, except segments-per-pixel, which I had to add to support the Newhaven NHD-2.7-12864WDW3. This is a slightly odd feature, but in practice it is a lot nicer to support it in the driver, and since we're currently remapping pixels anyway, it makes sense to implement here in my opinion.
This commit also adds a configurable buffer size for the pixel conversion. By using a larger buffer, we can potentially use DMA for the SPI transfer. The default is set to 8, which is the smallest value that supports
segments-per-pixel = 2