Re: [PATCH 4/4] drm/tinydrm: Add RePaper e-ink driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Den 07.06.2017 14.11, skrev Ilia Mirkin:
On Wed, Jun 7, 2017 at 7:52 AM, Noralf Trønnes <noralf@xxxxxxxxxxx> 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 <noralf@xxxxxxxxxxx> 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 = 0xffff;
+
+                       if (mask) {
+                               u16 pixel_mask =
repaper_interleave_bits(mask[b - 1]);
+
+                               pixel_mask = (pixel_mask ^ pixels) &
0x5555;
+                               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... 0x5555 is a 0101010101010101 pattern, so
pixel_mask |= pixel_mask << 1 (after pixel_mask is x & 0x5555) 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux