Add support for CH1115 Controller
Subsystems (3)
v4 cover Add support for CH1115 Controller
This series add support for the CH1115 Controller [1]. It is a monochrome dot matrix OLED controller that supports several interfaces such as SPI and I2C.
This driver only supports the I2C interface, but all common parts could easily be put into a common file to be used with other interfaces. I only have I2C to test with.
This is the first driver I write. The work is based on the driver located at:
- drivers/gpu/drm/sitronix/st7571-i2c.c
- drivers/gpu/drm/sitronix/st7571.c
Tested with Raspberry Pi 3b+.
Changelog:
v4:
General:
- Patch series based in kernel v7.3-rc1
Driver:
- Fixed nits found by Christophe JAILLET.
- Fixed max_width and max_height in modeset to avoid having bigger framebuffers than resolution.
v3: [5]
- General:
- Patch series based in kernel v7.2
- In dt-bindings:
- Removed "chipwealth,contrast" property (added backlight_device support in driver's code)
- Renamed "chipwealth,invert" -> "chipwealth,inverted"
- Added default value to "chipwealth,offset"
- Removed 'status = "okay"' from example
- Indented example with 4 spaces as stated in the documentation [4]
- In driver code:
- Added support for backlight_device
- Replaced drm_dev_unregister with drm_dev_unplug to cover the dt overlay unload case
- Executing drm shutdown operations before turning off the display
- Checking that the width (panel-timing.hactive) and height (panel-timing.vactive) parameters are greater than 0
v2: [3]
- General:
- Patch series based in kernel v7.2
- In dt-bindings:
- Fixed linter problems
- Replaced first_page property with offset
- In driver code:
- Fixed dst_pitch calculation
- Renamed file from ch1115.c to ch1115-i2c.c
- Support flip-vertical and flip-horizontal dt properties
- Fixed several things found by sashiko-ai
v1: [2]
[1] https://datasheet4u.com/download/1576606/CH1115.html
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/
[4] https://docs.kernel.org/devicetree/bindings/writing-schema.html#annotated-example-schema
[5] https://lore.kernel.org/all/[email protected]/
Nicolás Antinori (3):
dt-bindings: display: add Chipwealth CH1115 OLED Controller drm/ch1115: add support for Chipwealth CH1115 OLED controller
MAINTAINERS: add entry for Chipwealth CH1115 OLED Controller
.../bindings/display/chipwealth,ch1115.yaml | 81 ++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + MAINTAINERS | 6 + drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/chipwealth/Kconfig | 16 + drivers/gpu/drm/chipwealth/Makefile | 3 + drivers/gpu/drm/chipwealth/ch1115-i2c.c | 885 ++++++++++++++++++ 8 files changed, 995 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml create mode 100644 drivers/gpu/drm/chipwealth/Kconfig create mode 100644 drivers/gpu/drm/chipwealth/Makefile create mode 100644 drivers/gpu/drm/chipwealth/ch1115-i2c.c
--
2.47.3
PATCH v4 1/3 dt-bindings: display: add Chipwealth CH1115 OLED Controller
Chipwealth CH1115 is a controller for monochrome dot matrix OLED panels.
---
Notes:
- Patch series based on 7.3-rc1
- Carried "Reviewed-by: Krzysztof Kozlowski <...>" from v3 since this patch did not change.
.../bindings/display/chipwealth,ch1115.yaml | 81 +++++++++++++++++++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + 2 files changed, 83 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +81 -0
@@ -0,0 +1,81 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/chipwealth,ch1115.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Chipwealth CH1115 Display Controller + +maintainers: + - Nicolás Antinori <[email protected]> + +description: + Chipwealth CH1115 is a controller for monochrome dot matrix OLED + panels. + + https://datasheet4u.com/download/1576606/CH1115.html + +allOf: + - $ref: panel/panel-common.yaml# + +properties: + compatible: + const: chipwealth,ch1115 + + reg: + maxItems: 1 + + chipwealth,inverted: + type: boolean + description: + Display pixels are inverted, i.e. 0 is white and 1 is black. + + chipwealth,offset: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 63 + default: 0 + description: + Display start line offset. Specifies the vertical alignment and row shift + along the Y-coordinate. + + width-mm: true + height-mm: true + flip-horizontal: true + flip-vertical: true + panel-timing: true + +required: + - compatible + - reg + - width-mm + - height-mm + - panel-timing + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + display@3c { + compatible = "chipwealth,ch1115"; + reg = <0x3c>; + width-mm = <12>; + height-mm = <8>; + chipwealth,offset = <8>; + panel-timing { + clock-frequency = <0>; + hactive = <88>; + vactive = <48>; + hfront-porch = <0>; + hback-porch = <0>; + hsync-len = <0>; + vsync-len = <0>; + vfront-porch = <0>; + vback-porch = <0>; + }; + }; + };
Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0
@@ -331,6 +331,8 @@ patternProperties: description: ChipOne "^chipspark,.*": description: ChipSPARK + "^chipwealth,.*": + description: Chip Wealth Technology, Ltd. "^chongzhou,.*": description: Shenzhen Chongzhou Electronic Technology Co., Ltd "^chrontel,.*":
--
2.47.3
PATCH v4 2/3 drm/ch1115: add support for Chipwealth CH1115 OLED controller
In reply to Nicolás AntinoriChipwealth CH1115 is a controller for monochrome dot matrix OLED panels.
Add DRM support for this chip using i2c bus.
---
Notes:
- Patch series based on 7.3-rc1
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/chipwealth/Kconfig | 16 + drivers/gpu/drm/chipwealth/Makefile | 3 + drivers/gpu/drm/chipwealth/ch1115-i2c.c | 885 ++++++++++++++++++++++++ 5 files changed, 906 insertions(+) create mode 100644 drivers/gpu/drm/chipwealth/Kconfig create mode 100644 drivers/gpu/drm/chipwealth/Makefile create mode 100644 drivers/gpu/drm/chipwealth/ch1115-i2c.c
drivers/gpu/drm/Kconfig +1 -0
@@ -307,6 +307,7 @@ source "drivers/gpu/drm/aspeed/Kconfig" source "drivers/gpu/drm/ast/Kconfig" source "drivers/gpu/drm/atmel-hlcdc/Kconfig" source "drivers/gpu/drm/bridge/Kconfig" +source "drivers/gpu/drm/chipwealth/Kconfig" source "drivers/gpu/drm/etnaviv/Kconfig" source "drivers/gpu/drm/exynos/Kconfig" source "drivers/gpu/drm/fsl-dcu/Kconfig"
drivers/gpu/drm/Makefile +1 -0
@@ -238,6 +238,7 @@ obj-$(CONFIG_DRM_SPRD) += sprd/ obj-$(CONFIG_DRM_LOONGSON) += loongson/ obj-$(CONFIG_DRM_POWERVR) += imagination/ obj-$(CONFIG_DRM_VERISILICON_DC) += verisilicon/ +obj-y += chipwealth/ # Ensure drm headers are self-contained and pass kernel-doc hdrtest-files := \
drivers/gpu/drm/chipwealth/Kconfig new file +16 -0
@@ -0,0 +1,16 @@ +# SPDX-License-Identifier: GPL-2.0-only +config DRM_CH1115_I2C + tristate "DRM support for Ch1115 OLED panels (I2C BUS)" + depends on DRM && I2C + select DRM_CLIENT_SELECTION + select DRM_GEM_SHMEM_HELPER + select DRM_KMS_HELPER + select REGMAP_I2C + select VIDEOMODE_HELPERS + help + Chipwealth CH1115 is a controller for monochrome dot matrix OLED + panels. + + DRM Driver for Ch1115 OLED panels connected via I2C. + + If M is selected the module will be called ch1115-i2c.
drivers/gpu/drm/chipwealth/Makefile new file +3 -0
@@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0-only + +obj-$(CONFIG_DRM_CH1115_I2C) += ch1115-i2c.o
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +885 -0
885 lines not on this page · raw message
--
2.47.3
[email protected]
AI reviewRe: [PATCH v4 2/3] drm/ch1115: add support for Chipwealth CH1115 OLED controller
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] Heap buffer overflow due to undersized `hwbuf` allocation. - [High] Calling sleeping functions in…
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Heap buffer overflow due to undersized hwbuf allocation.
- [High] Calling sleeping functions in atomic context.
- [High] Missing required Kconfig dependencies leading to build failures or NULL pointer dereferences.
- [High] Visual corruption due to horizontally unaligned damage rectangles.
--
commit fe052a7a7d4de23b8d160888c9f118bbd31d156e
Author: Nicolás Antinori <[email protected]>
drm/ch1115: add support for Chipwealth CH1115 OLED controller
Chipwealth CH1115 is a controller for monochrome dot matrix OLED panels.
Add DRM support for this chip using i2c bus.
drivers/gpu/drm/chipwealth/Kconfig line 4
Quoted 10 lines
drivers/gpu/drm/chipwealth/Kconfig new file +4
-0@@ -0,0 +1,16 @@ +# SPDX-License-Identifier: GPL-2.0-only +config DRM_CH1115_I2C + tristate "DRM support for Ch1115 OLED panels (I2C BUS)" + depends on DRM && I2C
[Severity: High]
Is there a regression here due to missing Kconfig dependencies?
The driver code calls of_get_display_timing(), which requires CONFIG_OF_VIDEOMODE. If not enabled, it returns -ENOSYS and probe fails unconditionally.
Additionally, if CONFIG_BACKLIGHT_CLASS_DEVICE is compiled as a module (=m) and this driver is built-in (=y), linking will fail due to unresolved symbols.
Should this include "depends on OF" (or OF_VIDEOMODE) and "select BACKLIGHT_CLASS_DEVICE"?
Quoted 5 lines
drivers/gpu/drm/chipwealth/Kconfig new file +5
-0+ select DRM_CLIENT_SELECTION + select DRM_GEM_SHMEM_HELPER + select DRM_KMS_HELPER + select REGMAP_I2C + select VIDEOMODE_HELPERS
[ ... ]
Quoted 5 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +0
-0
[ ... ]
Quoted 5 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +5
-0+static void ch1115_prepare_buffer (struct ch1115_device *ch1115, +const struct iosys_map *vmap, +struct drm_framebuffer *fb, +struct drm_rect *rect, +struct drm_format_conv_state *fmtcnv_state)
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 246
Quoted 10 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +10
-0+{ +unsigned int dst_pitch; +struct iosys_map dst; + + dst_pitch =DIV_ROUND_UP (ch1115->width,8 ); + rect->y1 =round_down (rect->y1,CH1115_PAGE_HEIGHT ); + rect->y2 = min_t(unsigned int, round_up(rect->y2, CH1115_PAGE_HEIGHT), ch1115->height); + +iosys_map_set_vaddr (&dst, ch1115->hwbuf + rect->y1 * dst_pitch + rect->x1 /8 ); +drm_fb_xrgb8888_to_mono (&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
[Severity: High]
Does this code introduce a regression causing visual corruption by passing horizontally unaligned rectangles to the conversion helper?
This rounds the damage rectangle vertically, but neglects to round it horizontally (rect->x1, rect->x2). If rect->x1 is not a multiple of 8, drm_fb_xrgb8888_to_mono() starts reading from the unaligned rect->x1 and packs those pixels starting at the 0th bit of the given destination address.
This will overwrite unrelated pixels and scramble the image in that byte. When ch1115_transform_xy() later reads these bytes assuming correct alignment, the display will output corrupted artifacts. Should rect->x1 and rect->x2 be rounded to multiples of 8?
Additionally, drm_fb_xrgb8888_to_mono() writes full rows based on the rounded up pitch, which can lead to an out-of-bounds write if the buffer is undersized.
Quoted 6 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +6
-0+} + +static inline u8 ch1115_transform_xy (const char *p,int x,int y,u8 bytes_per_row) +{ +int xrest = x %8 ; +u8 result =0 ;
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 268
Quoted 6 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +6
-0+ x = x /8 ; + y = (y /8 ) *8 ; + +for (int i =0 ; i <8 ; i++) { +int row_idx = y + i; +u8 byte = p[row_idx * bytes_per_row + x];
[Severity: High]
Can this loop introduce a regression by performing out-of-bounds reads when iterating through the vertical page blocks?
If width or height are not multiples of 8, the allocated hwbuf size may be too small because it stores data with a pitch of DIV_ROUND_UP(width, 8) and up to round_up(height, 8) rows. This could cause this loop to read past the end of the buffer.
Quoted 7 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +7
-0+u8 bit = (byte >> xrest) &1 ; + + result |= (bit << i); + } + +return result; +}
[ ... ]
Quoted 3 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +3
-0+static void ch1115_primary_plane_helper_atomic_update (struct drm_plane *plane, +struct drm_atomic_commit *state) +{
[ ... ]
Quoted 7 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +7
-0+if (!fb) +return ; + +if (drm_gem_fb_begin_cpu_access (fb,DMA_FROM_DEVICE )) +return ; + +if (!drm_dev_enter (dev, &idx))
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 412
Quoted 10 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +10
-0+ goto out_drm_gem_fb_end_cpu_access; + +drm_atomic_helper_damage_iter_init (&iter, old_plane_state, plane_state); +drm_atomic_for_each_plane_damage (&iter, &damage) { +ch1115_prepare_buffer (ch1115, + &shadow_plane_state->data[0 ], + fb, &damage, + &shadow_plane_state->fmtcnv_state); + +ch1115_fb_update_rect (fb, &damage);
[Severity: High]
Does this code introduce a regression by calling sleeping functions in an atomic context?
Plane atomic update callbacks execute in atomic context during non-blocking commits and must not sleep.
Here, drm_gem_fb_begin_cpu_access() is called, which can sleep. Additionally, ch1115_fb_update_rect() triggers I2C transfers via regmap, which is also a sleeping function.
Executing I2C transfers, mutex locks, and CPU syncs in atomic context can cause scheduling while atomic panics and potential deadlocks.
Quoted 7 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +7
-0+ } + +drm_dev_exit (idx); + +out_drm_gem_fb_end_cpu_access: +drm_gem_fb_end_cpu_access (fb,DMA_FROM_DEVICE ); +}
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 529
Quoted 13 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +13
-0+static void ch1115_encoder_atomic_enable (struct drm_encoder *encoder, +struct drm_atomic_commit *state) +{ +struct drm_device *drm = encoder->dev; +struct ch1115_device *ch1115 =drm_to_ch1115 (drm); + +ch1115_oled_init (ch1115); + +u8 command =CH1115_CMD_ON ; + +ch1115_send_command_list (ch1115, &command,1 ); + +backlight_enable (ch1115->bl_dev);
[Severity: High]
Does this cause a regression by calling a sleeping backlight API inside an atomic encoder callback?
Encoder atomic enable and disable callbacks run in atomic context during non-blocking commits.
backlight_enable() acquires a mutex internally, and both this and the I2C transfers above will sleep, which violates the atomic context requirements.
Quoted 1 line
+}
[ ... ]
Quoted 2 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +2
-0+static int ch1115_probe (struct i2c_client *client) +{
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 780
Quoted 9 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +9
-0+ ch1115->pages =DIV_ROUND_UP (ch1115->height,CH1115_PAGE_HEIGHT ); + ch1115->bytes_per_row =DIV_ROUND_UP (ch1115->width,8 ); + ch1115->x_start_offset = ch1115->flip_horizontal + ?CH1115_MAX_WIDTH - ch1115->width + :0 ; + ch1115->mode =ch1115_mode (ch1115); + + ch1115->hwbuf =devm_kzalloc (dev, (ch1115->width * ch1115->height) /8 , +GFP_KERNEL );
[Severity: High]
Does this buffer allocation introduce a regression by failing to account for row pitch padding and page alignment?
The hwbuf is allocated as (width * height) / 8 bytes. If width or height are not multiples of 8, this size is too small.
The buffer stores data with a pitch of DIV_ROUND_UP(width, 8) and up to round_up(height, 8) rows. For example, if width=12 and height=12, the allocated size is 18 bytes.
Later, drm_fb_xrgb8888_to_mono() writes 12 rows with a pitch of 2 (24 bytes), causing an out-of-bounds write of 6 bytes. This memory corruption can lead to system instability.
Quoted 5 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +5
-0+if (!ch1115->hwbuf) +return -ENOMEM ; + + ch1115->row =devm_kzalloc (dev, ch1115->width,GFP_KERNEL ); +if (!ch1115->row)
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 795
Quoted 10 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +10
-0+return -ENOMEM ; + +// Backlight + bl =devm_backlight_device_register (dev,dev_name (dev), dev, ch1115, + &ch1115_bl_ops,NULL ); +if (IS_ERR (bl)) +return dev_err_probe (dev,PTR_ERR (bl), +"Unable to register backlight device\n" ); + + bl->props.brightness = ch1115->contrast;
[Severity: High]
Can this introduce a regression where bl is NULL, causing a NULL pointer dereference?
If CONFIG_BACKLIGHT_CLASS_DEVICE is disabled, the fallback stub for devm_backlight_device_register() returns NULL.
Since IS_ERR(bl) does not check for NULL, the next line will dereference a NULL pointer.
Quoted 2 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +2
-0+ bl->props.max_brightness =CH1115_MAX_CONTRAST ; + ch1115->bl_dev = bl;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
PATCH v4 3/3 MAINTAINERS: add entry for Chipwealth CH1115 OLED Controller
In reply to Nicolás AntinoriAdd MAINTAINERS entry for the Chipwealth CH1115 dot matrix OLED controller.
--- MAINTAINERS | 6 ++++++ 1 file changed, 6 insertions(+)
MAINTAINERS +6 -0
@@ -8665,6 +8665,12 @@ T: git https://gitlab.freedesktop.org/drm/misc/kernel.git F: Documentation/devicetree/bindings/display/panel/samsung,lms380kf01.yaml F: drivers/gpu/drm/panel/panel-widechips-ws2401.c +DRM DRIVER FOR CHIPWEALTH CH1115 PANELS +M: Nicolás Antinori <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml +F: drivers/gpu/drm/chipwealth/ch1115-i2c.c + DRM DRIVERS M: David Airlie <[email protected]> M: Simona Vetter <[email protected]>
--
2.47.3