Opened 8 years ago

Last modified 6 years ago

#1017 new defect

swizzle and other DCON-related display problems

Reported by: AlbertCahalan Owned by: wad
Priority: normal Milestone: Gen2
Component: hardware Version:
Keywords: Cc:
Blocked By: Blocking:
Deployments affected: Action Needed:
Verified: no

Description

On a test pattern, I'm seeing aliasing errors which indicate that the RGB weight in the swizzle is not correct and which suggest that the swizzle is not operating on linear data.

If the the swizzle convolution is loadable and can do 5x5, and there are gamma tables both before and after the swizzle, then this can be fixed now. Otherwise I guess it will need a new ASIC. (well, maybe it isn't too late for an ASIC fix...?)

First, the sRGB values need to become linear via a lookup table. If YCrCr were supported in the framebuffer (nice idea, just set the high bit) then that would get converted too.

Second, make the result a properly weighted sum of the RGB components. In mono mode, this is how you convert from color to greyscale. In color mode, this is how you change from sRGB primaries and whitepoint to whatever the LCD is natively.

Third (color mode), perform the anti-aliasing convolution. It needs to be such that 1/3 of the weight comes from each type of pixel. A loadable 5x5 matrix or bigger would be great. It could have enforced symmetry along the diagonals to reduce cost; 5x5 only requires 9 distinct values and 7x7 only requires 16 distinct values. Another nice-to-have would be separate R/G/B matrixes, allowing step 2 and 3 to be combined if each output color can be a linear function of the 5x5x3 (x,y,channel) input.

Fourth, go through a second gamma table. This one accounts for any non-linearity in the LCD. It may need to be adjusted according to light level; otherwise it can be fixed.

Change History (11)

comment:1 in reply to: ↑ description ; follow-up: Changed 8 years ago by vmb

Replying to AlbertCahalan:

On a test pattern, I'm seeing aliasing errors which indicate that the RGB weight in the swizzle is not correct and which suggest that the swizzle is not operating on linear data.

If the the swizzle convolution is loadable and can do 5x5, and there are gamma tables both before and after the swizzle, then this can be fixed now. Otherwise I guess it will need a new ASIC. (well, maybe it isn't too late for an ASIC fix...?)

First, the sRGB values need to become linear via a lookup table. If YCrCr were supported in the framebuffer (nice idea, just set the high bit) then that would get converted too.

Second, make the result a properly weighted sum of the RGB components. In mono mode, this is how you convert from color to greyscale. In color mode, this is how you change from sRGB primaries and whitepoint to whatever the LCD is natively.

Third (color mode), perform the anti-aliasing convolution. It needs to be such that 1/3 of the weight comes from each type of pixel. A loadable 5x5 matrix or bigger would be great. It could have enforced symmetry along the diagonals to reduce cost; 5x5 only requires 9 distinct values and 7x7 only requires 16 distinct values. Another nice-to-have would be separate R/G/B matrixes, allowing step 2 and 3 to be combined if each output color can be a linear function of the 5x5x3 (x,y,channel) input.

Fourth, go through a second gamma table. This one accounts for any non-linearity in the LCD. It may need to be adjusted according to light level; otherwise it can be fixed.

As the person who originally designed the hardware filter kernel (which is 3x3, and in order to avoid having full multipliers in the ASIC needs to have all weights be fractional powers of 2), I'd be interested in seeing some of the images that give you aliasing problems.

comment:2 in reply to: ↑ 1 Changed 8 years ago by AlbertCahalan

Replying to vmb:

Replying to AlbertCahalan:

On a test pattern, I'm seeing aliasing errors which indicate that the RGB weight in the swizzle is not correct and which suggest that the swizzle is not operating on linear data.

Here you go:

http://wiki.laptop.org/go/Image:Zone_plate_boys.png

Regarding the filter itself: The colored circles in the zone plate area are obviously bad. The bi-level noise patch shouldn't be showing any color. The bottom row of bi-level pattern patches should not be showing any color. The 3x2 group of patterned yellow and green color patches shouldn't be showing odd colors.

Regarding the filter operating on wrong-gamma data: There are numerous extra aliasing circles in the zone plate area. The top row of bi-level pattern patches does not appear equally bright as the grey square to the left of them.

First, the sRGB values need to become linear via a lookup table. If YCrCr were supported in the framebuffer (nice idea, just set the high bit) then that would get converted too.

I forgot: this would be a good place to add temporal anti-aliasing against the quantization to 6:6:6, especially when the frame buffer is set to 8:8:8. Each table lookup could provide a mask for a pseudo-random number generator, allowing gamma to be properly respected. (actually a pair of masks, equivalent to multiplication by a floating-point value with 1 fraction bit)

As the person who originally designed the hardware filter kernel (which is 3x3, and in order to avoid having full multipliers in the ASIC needs to have all weights be fractional powers of 2), I'd be interested in seeing some of the images that give you aliasing problems.

Multiplication is just adding weights that are fractional powers of 2. For a 3x3 filter labeled "a" through "i" in normal English reading order, this is better:

(a + 2*b + 2*d + 2*e + 4*e + 2*f + 2*h + i)/16

As you can see, the "e" term appears twice. The degree to which you can do this will greatly affect quality. I stuck with 3x3 and only increased the divider to 16, but higher is much better.

I just tested the above in a screen simulator, with both broken and fixed gamma. It's better no matter what the gamma.

On a real B2 OLPC XO, the test pattern looks far better if I tell my image viewer that the screen gamma is 1.0 rather than the standard sRGB gamma. I'm fairly sure this indicates that the filter is operating on raw pixel data, which would normally be in the sRGB color space. This is not a legitimate operation; the data must be linear. If the LCD itself is not linear, then you need a separate gamma lookup table for that.

Got a diagram of the pipeline anywhere? It could go on the wiki. ASIC code would be useful too.

Note: image link is above

comment:3 Changed 8 years ago by jg

  • Component changed from distro to hardware
  • Milestone changed from Untriaged to BTest-3
  • Resolution set to duplicate
  • Status changed from new to closed

The LUT can't effectively be used for gamma correction and is causing other trouble due to a wiring error in the hardware. That will be fixed in B3.

See #336.

comment:4 Changed 8 years ago by AlbertCahalan

  • Resolution duplicate deleted
  • Status changed from closed to reopened

The gamma issue may be nearly a dupe of #336, but the other problem remains. The filter is far from being balanced among red/green/blue, which leads to severe color shifts that vary as images are moved around the screen. Yellow can look orange or green.

comment:5 follow-up: Changed 8 years ago by vmb

Yes, I know that filtering and luminance-extraction oughtn't be done on nonlinear data (I've worked on the design of chips in commercial products that do this thing the right way) but because of limited hardware resources Mark had available in the DCON we had to do a simple hack -- Albert's non-rotationally-symmetric filter probably would have been a better idea. In a later generation of the machine where we get a decent display driver circuit integrated with the graphics subsystem we can do the totally correct thing; if we ever re-do the DCON before then we can try to fit in a better filter even if we can't put it between gamma tables.

So everyone knows what the DCON does:

The filter kernel is (left-to-right, top-to-bottom) (0 1 0, 1 4 1, 0 1 0)/8. The Y channel is computed as
(R >> 2) + (R >> 4) + (G >> 1) + (G >> 4) + (B >> 3)

Note also that the current DCON has only 16-bit color line buffers (cost and space, again), so there isn't going to be a lot of visual finesse available in any event.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 8 years ago by AlbertCahalan

Replying to vmb:

(R >> 2) + (R >> 4) + (G >> 1) + (G >> 4) + (B >> 3)

This notation looks like C, but the DCON document seems to indicate that operations like (R >> 4) do not discard the rightmost bits. I'm pretty sure you keep the bits that overlap between (R >> 2) and (R >> 4), but it isn't clear what happens with the rightmost 2 bits of (R >> 4). They could be discarded, (R >> 2) could be zero-extended on the right by 2 bits, or (R >> 2) could be extended on the right with a copy of the high two bits. I'm guessing the second method, though I think the third method may be a bit better.

Note also that the current DCON has only 16-bit color line buffers (cost and space, again), so there isn't going to be a lot of visual finesse available in any event.

Page 5 of the DCON spec claims that 19-bit 6:7:6 color is stored in the line buffer. Is this incorrect?

comment:7 in reply to: ↑ 6 Changed 8 years ago by AlbertCahalan

Replying to AlbertCahalan:

Page 5 of the DCON spec claims that 19-bit 6:7:6 color is stored in the line buffer. Is this incorrect?

BTW, about the extra three bits... zero, ones, or copies of the high bit?

comment:8 Changed 8 years ago by vmb

Whoops, sorry, for "16-bit" please read "19-bit". It started as 16 bits and then Mark added three more because some of us complained.

As for the calculation, it's not what C would do (unless you started by casting the RGB values to shorts and shifting them left by four before shifting right again, I guess) -- I believe that the adder is wide enough that there is no truncation of the less-significant bits till after the sum is taken.

comment:9 Changed 7 years ago by jg

  • Resolution set to wontfix
  • Status changed from reopened to closed
  • Verified unset

Gamma correction is fixed in B3.

Won't fix the DCON; it is as good as it will get.

comment:10 Changed 7 years ago by AlbertCahalan

  • Milestone changed from BTest-4 to Gen2
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Bug #1671 is the gen2 DCON wishlist. (new since this bug was closed) It now has a reference to this bug.

comment:11 Changed 6 years ago by jg

  • Owner changed from blizzard to wad
  • Status changed from reopened to new
Note: See TracTickets for help on using tickets.