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?
--
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
