AM335x garbled LCD and SGX messages

Tomi Valkeinen said:

I don't have a clue about ARM instruction set, but it's a 32 bit CPU.  Are you saying it has a store instruction for 64 bit data, which it manages to write atomically? Isn't usually the rule of thumb that on 32 bit cpu, only 32 bit values are handled atomically, not 64 bit.

ARM has 64-bit load/store. I don't think they're architectually guaranteed to be atomic unless you use LDREX / STREX. Aligned 64-bit load/store is definitely atomic on the cortex-a8 though. All memory paths internally are 64-bit or 128-bit wide, as is the external bus interface, so there's simply no opportunity whatsoever to break it into words.

On a cortex-A8 you can actually make much bigger transfers that are still atomic from the cpu's point of view: an LDM / STM instruction cannot be not interrupted. An STM however completes once the data is in the write buffer*, and I don't exclude the possiblity you can get other transfers inserted afterward, though aligned 64-bit transfers will stay atomic all the way to the L3. It's also quite possible bursts stay atomic in the A8. Once a burst goes out the bus interface, I'm reasonably sure it remains atomic while travelling on the L3 interconnect.

(* except stores to strongly-ordered memory)

Jonathan Fisher90 said:

the issue has not yet gone away

It may be interesting to note there are more LCDC problems, with a possibility they may be linked:

1. When using the framebuffer console, the screen content may jump 32 pixels to the right (with wrap to next line) and jump back again.  Both jumps seem to require some sort of trigger (seems fairly random, like typing a command or triggering a vbell) but appears stable when fully idle. Robert Nelson reports he's seen this one all the way back to the 3.8 kernels.

2. Someone with severe garbling during some kinds of intensive memory access from the cpu. This was apparently traced to fifo underruns occurring on lcdc (see also this). Interestingly, 32-pixel shift was also seen, often as a prelude to full-blown garbling.

3. I have acquired a truly bizarre one: on a kernel with no framebuffer console (CONFIG_VT=n) and a 1024x768@60 lcd panel attached (not via HDMI) the last pixel (1023) of each line is corrupted, specifically it displays a copy of pixel 991 = 1023 - 32 of that line. The phenomenon was first noticed with a Qt5 application (QPA=linuxfb) but it also appears if I manually write to /dev/fb0. The content of /dev/fb0 still reads back correctly. Since this variant of the problem is completely reproducible and stable it should be easier to debug, although I'm still not sure where to begin. Unfortunately the flimsy prototype setup broke and the problem didn't manifest itself on a small low-res HDMI screen I attached. I haven't had time yet to dig further into this since other stuff has priority for me now, but I'll certainly need to face it again once the new prototype shows up on my desk.

All these have in common that they're highly mysterious, there's a "32-pixel" thing that shows up, although in different ways, and no obvious fault in software or kernel driver. The more blatent garbling in problem 2 also seems to link it to your problem.

Another thing they have in common is that all of them could be caused by problems with the async fifo between LCDC's dma controller and the raster controller...

Manuel Odendahl32 said:

we realized that the libEGL was using tilcdc_crtc_mode_set_base to induce a page flip, not tilcdc_crtc_page_flip.

Hmm that's interesting. What How did libEGL do the page flip? What API functions it calls? I was under the impression the mode_set_base will be called only for full  mode-sets, which is a "heavy" operation (i.e. no guarantee that you can get full fps with it).

In any case, just for your information, I think all this has changed quite a bit with the later releases, which have the updated SGX driver. And tilcdc is about to be converted to support the so called atomic modesetting, which also changes how the driver works (for better, I think). We need to verify that all the code paths in the new code handle this problem correctly.

Manuel Odendahl32 said:

We also added a 64 bit bus write for the FB addresses, we didn't go entirely crazy on the atomicity but this also seems to help (mostly it removes the unnecessary memory barrier between the 2 register writes). 

So you actually saw a diff in your testing with and without 64 bit write? It sounds very unlikely that the HW would read the registers exactly between the two 32 bit writes, but if you actually saw it affect, I guess it's safer to add that to the mainline driver too.

I'm not sure what libEGL methods are called, we use Qt and basically defer all the low-level things to it. Sadly libEGL itself is a binary blob.

As for 64 bit, I am fairly sure we saw a significant difference between 32 bit and 64 bit writes. (the 32 bit write also has memory barriers and a function call iirc). I don't exactly remember the duration of our tests, but it looks from our database records that we had around 70 individual snowcrashes with normal kernel vs 3 with 64 bit writes, in approximately the same period of time. I really don't remember what this means exactly however.

We will stick to our current setup for a bit now I think, but good to hear that the new system will improve things further.

Cheers, Manuel

Manuel Odendahl said:

I'm not sure what libEGL methods are called, we use Qt and basically defer all the low-level things to it. Sadly libEGL itself is a binary blob.

Ok, no prob. I can dig it up from somewhere.

Manuel Odendahl said:

As for 64 bit, I am fairly sure we saw a significant difference between 32 bit and 64 bit writes. (the 32 bit write also has memory barriers and a function call iirc). I don't exactly remember the duration of our tests, but it looks from our database records that we had around 70 individual snowcrashes with normal kernel vs 3 with 64 bit writes, in approximately the same period of time. I really don't remember what this means exactly however.

Ok. Were those tests made without the mode_set_base improvement? In other words, did you need both fixes (64bit write and the mode_set_base) to get a reliable system, or would only mode_set_base be enough?

I think both improvements make sense, though, I'm just curious.

Tomi Valkeinen said:

I was under the impression the mode_set_base will be called only for full mode-sets, which is a "heavy" operation (i.e. no guarantee that you can get full fps with it).

I'd presume full mode-sets call mode_set rather than mode_set_base?

My current impression is that (atomic) modesetting basically replaces a whole pile of older calls, including page-flip, which is now essentially a special case of drm_mode_atomic but just to update the FB_ID property of one given CRTC.

Matthijs van Duin said:

page-flip, which is now essentially a special case of drm_mode_atomic but just to update the FB_ID property of one given CRTC.

Just to add an example of when the basic page-flip call just doesn't suffice: say you have a windowed UI that uses double-buffering, and you're using an overlay for video playback inside a window. Now when that window is being moved, the page flip of the primary layer (containing the window decoration) needs to coincide with an update of the overlay coordinates to avoid undesirable artifacts. With the atomic modesetting API, that's just two extra properties to include in the call.

Obviously not applicable with tilcdc which has no overlays, but it was just an example. (It does apply to omapdrm of course, which fortunately seems to handle it well: I recently made a little test that animated the position and size of an overlay using atomic modesetting, and it ran flawlessly on my omap5 uevm at 60 fps with no significant CPU load).

Update: actually, a far more serious problem is that page_flip only works for the primary layer. Comments in drm_atomic.c and drm_atomic_helper.c actually refer to it as "legacy page_flip", so my guess would be that eventually the call will be removed from drivers and will just be emulated using modesetting by the DRM core for userspace compat.

Matthijs van Duin said:

I'd presume full mode-sets call mode_set rather than mode_set_base?

Well, with "full mode-set" I was referring to modesetting in general, versus page-flipping. mode_set_base is part of that.

Matthijs van Duin said:

My current impression is that (atomic) modesetting basically replaces a whole pile of older calls, including page-flip, which is now essentially a special case of drm_mode_atomic but just to update the FB_ID property of one given CRTC.

Atomic modesetting does replace the driver's configuration machinery almost totally. The legacy API will still remain, of course.

Matthijs van Duin said:

actually, a far more serious problem is that page_flip only works for the primary layer. Comments in drm_atomic.c and drm_atomic_helper.c actually refer to it as "legacy page_flip", so my guess would be that eventually the call will be removed from drivers and will just be emulated using modesetting by the DRM core for userspace compat.

Yes, driver's page_flip goes away when the driver is converted to atomic modesetting. See mainline omap_crtc.c for example. Same will happen with tilcdc when it's converted to atomic modesetting.

But this is going quite out of topic =).