Fix occasional screen corruption by changing GLCD locking

[jburgess777]

This moves mutex protection in GLCD up a layer which fixes the occasional
screen corruption seen in some of the apps.

The locking helps avoid the situation where multiple tasks could call
GotoXY() to update the internal position without taking a lock. This
position determines what data is accessed by ReadData() and WriteData().
An invalid position could cause memory access beyond the framebuffer.

Moving the locking up to the higher level drawing routines causes
the mutex to be held for longer but also means the display update
is more likely to be triggered after a complete drawing operation
instead of in the middle.

[marksteward]

Thanks for this - unfortunately, it conflicts with changes I've been making today that I'm just about to push. Could you pop onto IRC to discuss?

[drrk]

This also is not FreeRTOS safe, as the release function does not check if the scheduler is running.
Although unlikely, it is possible for the scheduler to have been stopped after the mutex is created, and calling the Give function will crash if the scheduler is not running.

[marksteward]

The scheduler can't be stopped on this arch, and there's various other bits of code that assume it never stops. Happy to take this and leave scheduler stopping for the future, but won't be possible this evening.

[jburgess777]

I'll join IRC if you tell me what server & channel. I was going to ask where to discuss making these kinds of changes.

[jburgess777]

Another solution would be to remove GotoXY() and change ReadData() and WriteData() to take the X/Y positions instead.

I don't have any previous experience with FreeRTOS so it isn't clear to me what the rules are for cases like these. If the scheduler was stopped then how would the task release the mutex, could it forget about it or would it need to reset it another way?

I copied the "if (mutex == 0) { initialize }" code which was there before but that doesn't look safe to me either. I don't what safe model to follow.

[marksteward]

We're on #tilda on Freenode. 9a9a143 is the commit which I think corrects all the locking apart from gText. Not too worried about the level locking is at, as we're not talking about a very large screen, but I do find having three different files involved confusing, so need to understand it better. There are a few functions which don't seem to be at the right abstraction level.

[marekventur]

Has this been resolved?

[drrk]

Well it can't be merged against master now, so it needs to be looked at, but as discussed the entire graphics stack is going to be replaced longterm, so its depend how much work @jburgess777 and @marksteward want to do on code with a lifespan.