-
Notifications
You must be signed in to change notification settings - Fork 8.2k
drivers: ssd16xx: Imlement better partial refresh #48163
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
drivers: ssd16xx: Imlement better partial refresh #48163
Conversation
|
@galak Could you review the DT changes please? In particular, is using hard-coded node names the best way to specify different display profiles? |
35eccd1 to
46875f5
Compare
galak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
am I missing 'partial' profile described in the dts bindings? I see 'full' mentioned.
@mbolivar-nordic I don't think we have a way to specify a node name as part of a binding, seems like that might be useful. Wonder if we have some other cases of specific node names being utilized.
Will take a look, off the top of my head that seems ok. Will see if I think of any better way to handle that. |
46875f5 to
6795b7d
Compare
Fixed.
I was hoping to be able to do that, but I couldn't find that ability when checking the schema specification. I think that might be useful for debugging. Another option would be to have a |
d847398 to
131e81b
Compare
You're right, we don't have that.
Child nodes of the |
131e81b to
d4f9cd1
Compare
d4f9cd1 to
144ca4d
Compare
|
@jfischer-no I have rebased this change onto the latest main after the opt overrides PR was merged. This is now ready for review. |
|
@jfischer-no @galak ping |
jfischer-no
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested, looks great, thank you
galak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some updates to the binding to add some compatibles for the child node (and look to utilize those rather than names).
| - 'full' - Normal / full refresh. | ||
| - 'partial' - Partial refresh. | ||
|
|
||
| properties: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a zephyr specific config, so I'd ask to add a compatible to the child node. Something like zephyr,ssd16xx-display-profile and maybe look to utilize an additional compatible like zephyr,-ssd16xx-display-profile-full and zephyr,-ssd16xx-display-profile-partial (we probably don't have the ability to describe that properly in the binding beyond in the description field at this time).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a compatible really necessary? There are a few cases where Linux uses device tree objects without a compatible. For example, display timings (https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/display/panel/display-timings.yaml) nodes don't use a compatible. I realise that Zephyr uses device trees in a subtly different way though, so I don't know what's works best with Zephyr's design philosophy.
I think I would like to avoid mode-specific compatibles since that won't scale. I can see reasons for adding support for more refresh profiles in the future. One possibility that I explored was to have a device-specific API to select a refresh profile. When trying that, I briefly considered (ab)using a reg property and providing well-known names for standard profiles such as full and partial. This seems a bit smelly though.
Another, potentially cleaner approach, would be to generate structs for all of the provided profiles and have a macro to refer to those instances using their node label. In this case, we could have a properties that point to the relevant default profiles (full/partial).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linux devicetree's have a regularity problem in that the same issue is solved different ways. Since you are giving meaning to the profile, I think it makes sense to describe that with a compatible rather than saying the node name should be X or Y. I don't know enough about the domain area of display choices to really say much more about how many different profiles you might have, and how you might choose between them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think part of the problem here is that we potentially need more profiles than the current two. I can see the logic in having a single "zephyr,ssd16xx-refresh-profile" compatible if that makes works better for the tooling though. It's also not necessarily a Zephyr-specific config. The refresh profile describes a property of the hardware that the driver needs to know to configure the display correctly.
Would it be better to have named properties in the main device node that points to specific well-known profiles (full/partial)? We could use a generic compatible (e.g., "solomon,ssd16xx-refresh-profile") for the profiles in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's worth noting that #48383 has now been merged. If we end up changing the way profiles are handled here, we'll need to update that driver as well for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really care about the approach we use here whether it is a child node naming convention, a property that points to a profile, or something else. However, I do feel strongly about type-specific compatibles. I don't believe that will work since I can see a path towards multiple profiles that can be selected at runtime (fast, fast differential, full, black, red, etc.). I don't have any patches planned to implement this in the near term though.
One option I was considering briefly was to add a 'reg' property to the sub node and provide well-known full/partial IDs in a header file. This would neatly scale to an arbitrary number of user-defined profiles.
144ca4d to
481b2bf
Compare
481b2bf to
15d14d0
Compare
|
@galak Could you revisit this? |
|
@andysan It would be nice if you picked it up again and rebase, thanks. |
|
@andysan please rebase |
The SSD16xx driver currently provides basic support for most chips in the Solomon Systech SSD16xx range of e-paper drivers. We currently use the SSD1608, SSD1673, SSD1675A, and SSD1681 in various boards supported by Zephyr. The main user-facing difference between the various SSD16xx chips is the resolution they support (sources & gates), but there are other differences as well. For example: * 8 or 16 bits used to represent x coordinates * 8 or 16 bits used to represent y coordinates * Differences in refresh configuration (SSD16XX_CMD_UPDATE_CTRL2) * Differences in LUT sizes The driver currently assumes that the user specifies the number of bits used to describe coordinates. However, as we add support for more chips, more of the differences will become apparent and need workaround. Comparing data sheets from different chips in the SSD16xx range suggests that there are (at least) two different generations present. These differ in the size of the LUTs they expect and the way they handle partial refresh. This impacts register layout where SSD16XX_CMD_UPDATE_CTRL2 uses bit 3 selects "mode 2" whereas older devices uses this for a mode referred to as "initial". In order to add support for partial refresh in newer devices, we need to be able to distinguish between the different generations of the chip. It might be possible to add a DT property to indicate the revision, but that seems like a bit of an anti-pattern and it would be hard for users to specify the correct chip generation. This change introduces chip-specific compatible strings instead of the generic SSD16xx. There is unfortunately clear pattern that can be used to distinguish different generations, so the full chip name must be specified. A benefit of this is that we don't need to specify the width of the fields describing coordinates in device trees. Signed-off-by: Andreas Sandberg <[email protected]>
Update the device tree bindings for the SSD16xx driver to make it possible to specify multiple refresh profiles. The only profile currently supported is the 'full' profile. Signed-off-by: Andreas Sandberg <[email protected]>
Add support for partial refresh profiles. This makes it possible to
use partial refresh on generation 2 devices which are able to store
partial refresh LUTs in OTP.
Partial refresh is only enabled if a partial profile has been
provided. The display will use the full refresh profile if in this
case.
Devices that need custom LUTs and voltages can specify them separately
for the full and partial profiles. The controller will be reset when
changing profiles which means that profiles always override the
default reset values. This means that it is, for example, possible to
use default values and LUTs from OTP for a full refresh and a custom
profile for partial refreshes.
For example, to use a GoodDisplay GDEY027T91 with partial refresh
simply use the following device tree fragment:
display: ssd1680@0 {
compatible = "solomon,ssd1680";
spi-max-frequency = <4000000>;
duplex = <SPI_HALF_DUPLEX>;
reg = <0>;
dc-gpios = <&arduino_header 15 GPIO_ACTIVE_LOW>;
reset-gpios = <&arduino_header 14 GPIO_ACTIVE_LOW>;
busy-gpios = <&arduino_header 13 GPIO_ACTIVE_HIGH>;
/* Enable the built-in temperature sensor */
tssv = <0x80>;
width = <264>;
height = <176>;
/* Enable partial refresh using built-in LUT */
partial {
};
};
Signed-off-by: Andreas Sandberg <[email protected]>
Remove the optional call to ssd16xx_update_display() in ssd16xx_clear_cntlr_mem(). This doesn't really belong in that function and just adds a non-obvious boolean argument to the function. Signed-off-by: Andreas Sandberg <[email protected]>
Add support for the SSD1680 EPD driver chip with support for up to 296x176 pixel displays. Signed-off-by: Andreas Sandberg <[email protected]>
The SSD16xx driver used to use the SCREEN_INFO_DOUBLE_BUFFER flag to indicate to the LVGL integration that it needs writes to be performed twice. This was required because partial writes require both the old and new buffer to be written. This behavior is really an implementation detail and only applies to partial refresh. Do this buffer maintenance in the driver instead. Signed-off-by: Andreas Sandberg <[email protected]>
15d14d0 to
3eefe98
Compare
Overview
There is currently no way to specify different configuration overrides for different refresh types (full/partial). This pull request addresses that by introducing different refresh profiles in the device tree. There is also no way to use partial refresh on SSD168x devices that are able to select partial refresh LUTs from OTP. This pull request addresses these deficiencies.
It consists of three major changes:
Device-specific compatibles
The SSD16xx driver currently provides basic support for most chips in the Solomon Systech SSD16xx range of e-paper drivers. We currently use the SSD1608, SSD1673, SSD1675A, and SSD1681 in various boards
supported by Zephyr.
The main user-facing difference between the various SSD16xx chips is the resolution they support (sources & gates), but there are other differences as well. For example:
SSD16XX_CMD_UPDATE_CTRL2)The driver currently assumes that the user specifies the number of bits used to describe coordinates. However, as we add support for more chips, more of the differences will become apparent and need workaround.
Comparing data sheets from different chips in the SSD16xx range suggests that there are (at least) two different generations present. These differ in the size of the LUTs they expect and the way they handle partial refresh. This impacts register layout where
SSD16XX_CMD_UPDATE_CTRL2uses bit 3 selects "mode 2" whereas olderdevices uses this for a mode referred to as "initial".
In order to add support for partial refresh in newer devices, we need to be able to distinguish between the different generations of the chip. It might be possible to add a DT property to indicate the revision, but that seems like a bit of an anti-pattern and it would be hard for users to specify the correct chip generation.
This change introduces chip-specific compatible strings instead of the generic SSD16xx. There is unfortunately clear pattern that can be used to distinguish different generations, so the full chip name must be specified. A benefit of this is that we don't need to specify the width of the fields describing coordinates in device trees.
Add support for separate refresh profiles
Add support for separate partial and full refresh profiles. This makes it possible to use partial refresh on generation 2 devices which are able to store partial refresh LUTs in OTP.
Partial refresh is only enabled if a partial profile has been provided. The display will use the full refresh profile if in this case.
Devices that need custom LUTs and voltages can specify them separately for the full and partial profiles. The controller will be reset when changing profiles which means that profiles always override the default reset values. This means that it is, for example, possible to use default values and LUTs from OTP for a full refresh and a custom profile for partial refreshes.
For example, to use a GoodDisplay GDEY027T91 with partial refresh simply use the following device tree fragment:
Updated device profiles
Since this change introduces different partial and full profiles, it also updates multiple in-tree boards. These boards have been updated with the configuration from the match GoodDisplay example code.
Testing
This has so far only been tested on a GDEY027T91. I don't own any boards using the older controllers, so I would really appreciate it if someone with access to boards using SSD1608/SSD1673/SSD1675A could test this change on their devices.
NB: This change depends on #47712. The commits that are a part of that PR should be reviewed there. I'm posting this PR before the other one has been merged primarily to get early feedback on the DT changes. I will rebase it once that change has been merged.