Re: [PATCH 4/4] drm/tinydrm: Add RePaper e-ink driver
Den 07.06.2017 14.14, skrev Emil Velikov:
On 7 June 2017 at 12:52, Noralf Trønnes wrote:
Hi Emil,
Den 07.06.2017 11.30, skrev Emil Velikov:
Hi Noralf,
Small drive-by comment, noticed while going through my morning coffee.
By no means a full review.
On 2 June 2017 at 12:49, Noralf Trønnes wrote:
+/* pixels on display are numbered from 1 */
+static void repaper_all_pixels(struct repaper_epd *epd, u8 **pp,
+ const u8 *data, u8 fixed_value, const u8
*mask,
+ enum repaper_stage stage)
+{
+ unsigned int b;
+
+ for (b = epd->width / 8; b > 0; b--) {
+ if (data) {
+ u16 pixels = repaper_interleave_bits(data[b -
1]);
+ u16 pixel_mask = 0x;
+
+ if (mask) {
+ u16 pixel_mask =
repaper_interleave_bits(mask[b - 1]);
+
+ pixel_mask = (pixel_mask ^ pixels) &
0x;
+ pixel_mask |= pixel_mask << 1;
The second u16 pixel_mask seems very suspicious - likely a copy/paste
mistake ?
Actually it's a correct copy. The same pattern is used in three places.
I did look at the datasheet to try and understand what was going on,
but the current and previous frame is applied in normal and inverted
form in several stages, the pixels are converted to black/white/nochange
and then I lost track of it all.
So I decided that the Pervasive people who wrote the driver probably
knew what they where doing. At least it works!
What I meant was:
Within the 'if (mask)' block you have a second, shadowing declaration
of pixel_mask.
Hence all the calculation done within is discarded and not used
further down in the code.
Ah, I see, it's indeed lost and useless.
I'll see what happens if I don't shadow the variable.
Thanks.
Noralf.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/tinydrm: Add RePaper e-ink driver
Den 07.06.2017 14.11, skrev Ilia Mirkin:
On Wed, Jun 7, 2017 at 7:52 AM, Noralf Trønnes wrote:
Hi Emil,
Den 07.06.2017 11.30, skrev Emil Velikov:
Hi Noralf,
Small drive-by comment, noticed while going through my morning coffee.
By no means a full review.
On 2 June 2017 at 12:49, Noralf Trønnes wrote:
+/* pixels on display are numbered from 1 */
+static void repaper_all_pixels(struct repaper_epd *epd, u8 **pp,
+ const u8 *data, u8 fixed_value, const u8
*mask,
+ enum repaper_stage stage)
+{
+ unsigned int b;
+
+ for (b = epd->width / 8; b > 0; b--) {
+ if (data) {
+ u16 pixels = repaper_interleave_bits(data[b -
1]);
+ u16 pixel_mask = 0x;
+
+ if (mask) {
+ u16 pixel_mask =
repaper_interleave_bits(mask[b - 1]);
+
+ pixel_mask = (pixel_mask ^ pixels) &
0x;
+ pixel_mask |= pixel_mask << 1;
The second u16 pixel_mask seems very suspicious - likely a copy/paste
mistake ?
Actually it's a correct copy. The same pattern is used in three places.
I did look at the datasheet to try and understand what was going on,
but the current and previous frame is applied in normal and inverted
form in several stages, the pixels are converted to black/white/nochange
and then I lost track of it all.
So I decided that the Pervasive people who wrote the driver probably
knew what they where doing. At least it works!
It's doubling up the bits... 0x is a 0101010101010101 pattern, so
pixel_mask |= pixel_mask << 1 (after pixel_mask is x & 0x) is just
saying that the same bit should be repeated twice.
As for why you'd want to do that ... no clue. Maybe it's a
2-bit-per-pixel packed mask and you want to set each pixel to 11 or 00
here?
Yes each pixel is represented with 2 bits:
11 - black
10 - white
00 - nothing
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/tinydrm: Add RePaper e-ink driver
On 7 June 2017 at 12:52, Noralf Trønnes wrote:
> Hi Emil,
>
>
> Den 07.06.2017 11.30, skrev Emil Velikov:
>>
>> Hi Noralf,
>>
>> Small drive-by comment, noticed while going through my morning coffee.
>> By no means a full review.
>>
>> On 2 June 2017 at 12:49, Noralf Trønnes wrote:
>>
>>
>>> +/* pixels on display are numbered from 1 */
>>> +static void repaper_all_pixels(struct repaper_epd *epd, u8 **pp,
>>> + const u8 *data, u8 fixed_value, const u8
>>> *mask,
>>> + enum repaper_stage stage)
>>> +{
>>> + unsigned int b;
>>> +
>>> + for (b = epd->width / 8; b > 0; b--) {
>>> + if (data) {
>>> + u16 pixels = repaper_interleave_bits(data[b -
>>> 1]);
>>> + u16 pixel_mask = 0x;
>>> +
>>> + if (mask) {
>>> + u16 pixel_mask =
>>> repaper_interleave_bits(mask[b - 1]);
>>> +
>>> + pixel_mask = (pixel_mask ^ pixels) &
>>> 0x;
>>> + pixel_mask |= pixel_mask << 1;
>>
>> The second u16 pixel_mask seems very suspicious - likely a copy/paste
>> mistake ?
>
>
> Actually it's a correct copy. The same pattern is used in three places.
> I did look at the datasheet to try and understand what was going on,
> but the current and previous frame is applied in normal and inverted
> form in several stages, the pixels are converted to black/white/nochange
> and then I lost track of it all.
> So I decided that the Pervasive people who wrote the driver probably
> knew what they where doing. At least it works!
>
What I meant was:
Within the 'if (mask)' block you have a second, shadowing declaration
of pixel_mask.
Hence all the calculation done within is discarded and not used
further down in the code.
Am I missing something?
-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/tinydrm: Add RePaper e-ink driver
On Wed, Jun 7, 2017 at 7:52 AM, Noralf Trønnes wrote:
> Hi Emil,
>
>
> Den 07.06.2017 11.30, skrev Emil Velikov:
>>
>> Hi Noralf,
>>
>> Small drive-by comment, noticed while going through my morning coffee.
>> By no means a full review.
>>
>> On 2 June 2017 at 12:49, Noralf Trønnes wrote:
>>
>>
>>> +/* pixels on display are numbered from 1 */
>>> +static void repaper_all_pixels(struct repaper_epd *epd, u8 **pp,
>>> + const u8 *data, u8 fixed_value, const u8
>>> *mask,
>>> + enum repaper_stage stage)
>>> +{
>>> + unsigned int b;
>>> +
>>> + for (b = epd->width / 8; b > 0; b--) {
>>> + if (data) {
>>> + u16 pixels = repaper_interleave_bits(data[b -
>>> 1]);
>>> + u16 pixel_mask = 0x;
>>> +
>>> + if (mask) {
>>> + u16 pixel_mask =
>>> repaper_interleave_bits(mask[b - 1]);
>>> +
>>> + pixel_mask = (pixel_mask ^ pixels) &
>>> 0x;
>>> + pixel_mask |= pixel_mask << 1;
>>
>> The second u16 pixel_mask seems very suspicious - likely a copy/paste
>> mistake ?
>
>
> Actually it's a correct copy. The same pattern is used in three places.
> I did look at the datasheet to try and understand what was going on,
> but the current and previous frame is applied in normal and inverted
> form in several stages, the pixels are converted to black/white/nochange
> and then I lost track of it all.
> So I decided that the Pervasive people who wrote the driver probably
> knew what they where doing. At least it works!
It's doubling up the bits... 0x is a 0101010101010101 pattern, so
pixel_mask |= pixel_mask << 1 (after pixel_mask is x & 0x) is just
saying that the same bit should be repeated twice.
As for why you'd want to do that ... no clue. Maybe it's a
2-bit-per-pixel packed mask and you want to set each pixel to 11 or 00
here?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/tinydrm: Add RePaper e-ink driver
Hi Emil,
Den 07.06.2017 11.30, skrev Emil Velikov:
Hi Noralf,
Small drive-by comment, noticed while going through my morning coffee.
By no means a full review.
On 2 June 2017 at 12:49, Noralf Trønnes wrote:
+/* pixels on display are numbered from 1 */
+static void repaper_all_pixels(struct repaper_epd *epd, u8 **pp,
+ const u8 *data, u8 fixed_value, const u8 *mask,
+ enum repaper_stage stage)
+{
+ unsigned int b;
+
+ for (b = epd->width / 8; b > 0; b--) {
+ if (data) {
+ u16 pixels = repaper_interleave_bits(data[b - 1]);
+ u16 pixel_mask = 0x;
+
+ if (mask) {
+ u16 pixel_mask = repaper_interleave_bits(mask[b
- 1]);
+
+ pixel_mask = (pixel_mask ^ pixels) & 0x;
+ pixel_mask |= pixel_mask << 1;
The second u16 pixel_mask seems very suspicious - likely a copy/paste mistake ?
Actually it's a correct copy. The same pattern is used in three places.
I did look at the datasheet to try and understand what was going on,
but the current and previous frame is applied in normal and inverted
form in several stages, the pixels are converted to black/white/nochange
and then I lost track of it all.
So I decided that the Pervasive people who wrote the driver probably
knew what they where doing. At least it works!
Noralf.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/tinydrm: Add RePaper e-ink driver
Hi Noralf,
Small drive-by comment, noticed while going through my morning coffee.
By no means a full review.
On 2 June 2017 at 12:49, Noralf Trønnes wrote:
> +/* pixels on display are numbered from 1 */
> +static void repaper_all_pixels(struct repaper_epd *epd, u8 **pp,
> + const u8 *data, u8 fixed_value, const u8 *mask,
> + enum repaper_stage stage)
> +{
> + unsigned int b;
> +
> + for (b = epd->width / 8; b > 0; b--) {
> + if (data) {
> + u16 pixels = repaper_interleave_bits(data[b - 1]);
> + u16 pixel_mask = 0x;
> +
> + if (mask) {
> + u16 pixel_mask =
> repaper_interleave_bits(mask[b - 1]);
> +
> + pixel_mask = (pixel_mask ^ pixels) & 0x;
> + pixel_mask |= pixel_mask << 1;
The second u16 pixel_mask seems very suspicious - likely a copy/paste mistake ?
-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/tinydrm: Add RePaper e-ink driver
This adds support for the Pervasive Displays RePaper branded displays.
The controller code is taken from the userspace driver available
through repaper.org. Only the V231 film is supported since the others
are EOL.
Signed-off-by: Noralf Trønnes
---
MAINTAINERS |6 +
drivers/gpu/drm/tinydrm/Kconfig | 12 +
drivers/gpu/drm/tinydrm/Makefile |1 +
drivers/gpu/drm/tinydrm/repaper.c | 1095 +
4 files changed, 1114 insertions(+)
create mode 100644 drivers/gpu/drm/tinydrm/repaper.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 757d487..800afa2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4446,6 +4446,12 @@ M: Dave Airlie
S: Odd Fixes
F: drivers/gpu/drm/mgag200/
+DRM DRIVER FOR PERVASIVE DISPLAYS REPAPER PANELS
+M: Noralf Trønnes
+S: Maintained
+F: drivers/gpu/drm/tinydrm/repaper.c
+F: Documentation/devicetree/bindings/display/repaper.txt
+
DRM DRIVER FOR RAGE 128 VIDEO CARDS
S: Orphan / Obsolete
F: drivers/gpu/drm/r128/
diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index 3504c53..9596e44 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -19,3 +19,15 @@ config TINYDRM_MI0283QT
help
DRM driver for the Multi-Inno MI0283QT display panel
If M is selected the module will be called mi0283qt.
+
+config TINYDRM_REPAPER
+ tristate "DRM support for Pervasive Displays RePaper panels (V231)"
+ depends on DRM_TINYDRM && SPI
+ help
+ DRM driver for the following Pervasive Displays panels:
+ 1.44" TFT EPD Panel (E1144CS021)
+ 1.90" TFT EPD Panel (E1190CS021)
+ 2.00" TFT EPD Panel (E2200CS021)
+ 2.71" TFT EPD Panel (E2271CS021)
+
+ If M is selected the module will be called repaper.
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
index 7a3604c..95bb4d4 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI) += mipi-dbi.o
# Displays
obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o
+obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o
diff --git a/drivers/gpu/drm/tinydrm/repaper.c
b/drivers/gpu/drm/tinydrm/repaper.c
new file mode 100644
index 000..d1eaa88
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -0,0 +1,1095 @@
+/*
+ * DRM driver for Pervasive Displays RePaper branded e-ink panels
+ *
+ * Copyright 2013-2017 Pervasive Displays, Inc.
+ * Copyright 2017 Noralf Trønnes
+ *
+ * The driver supports:
+ * Material Film: Aurora Mb (V231)
+ * Driver IC: G2 (eTC)
+ *
+ * The controller code was taken from the userspace driver:
+ * https://github.com/repaper/gratis
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#define REPAPER_RID_G2_COG_ID 0x12
+
+enum repaper_model {
+ E1144CS021 = 1,
+ E1190CS021,
+ E2200CS021,
+ E2271CS021,
+};
+
+enum repaper_stage { /* Image pixel -> Display pixel */
+ REPAPER_COMPENSATE, /* B -> W, W -> B (Current Image) */
+ REPAPER_WHITE, /* B -> N, W -> W (Current Image) */
+ REPAPER_INVERSE, /* B -> N, W -> B (New Image) */
+ REPAPER_NORMAL /* B -> B, W -> W (New Image) */
+};
+
+enum repaper_epd_border_byte {
+ REPAPER_BORDER_BYTE_NONE,
+ REPAPER_BORDER_BYTE_ZERO,
+ REPAPER_BORDER_BYTE_SET,
+};
+
+struct repaper_epd {
+ struct tinydrm_device tinydrm;
+ struct spi_device *spi;
+
+ struct gpio_desc *panel_on;
+ struct gpio_desc *border;
+ struct gpio_desc *discharge;
+ struct gpio_desc *reset;
+ struct gpio_desc *busy;
+
+ struct thermal_zone_device *thermal;
+
+ unsigned int height;
+ unsigned int width;
+ unsigned int bytes_per_scan;
+ const u8 *channel_select;
+ unsigned int stage_time;
+ unsigned int factored_stage_time;
+ bool middle_scan;
+ bool pre_border_byte;
+ enum repaper_epd_border_byte border_byte;
+
+ u8 *line_buffer;
+ void *current_frame;
+
+ bool enabled;
+ bool cleared;
+ bool partial;
+};
+
+static inline struct repaper_epd *
+epd_from_tinydrm(struct tinydrm_device *tdev)
+{
+ return container_of(tdev, struct repaper_epd, tinydrm);
+}
+
+static int repaper_spi_transfer(struct spi_device *spi, u8 header,
+ const void *tx, void *rx, size_t len)
+{
+ void *txbuf = NULL, *rxbuf = NULL;
+ struct spi_transfer tr[2] = {};
+ u8 *headerbuf;
+ int ret;
+
+ headerbuf = kmalloc(1, GF
