Den 07.06.2017 14.14, skrev Emil Velikov:
On 7 June 2017 at 12:52, 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!
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
