Re: [PATCH 13/30] viafb: Separate global and fb-specific data

From: Florian Tobias Schandinat
Date: Fri Apr 30 2010 - 14:44:26 EST


Jonathan Corbet schrieb:
On Fri, 30 Apr 2010 20:07:02 +0200
Florian Tobias Schandinat <FlorianSchandinat@xxxxxx> wrote:

Please don't do this. The reason for not failing in viafb is that ioremap failure of the engine is not critical it just disables the hardware acceleration but otherwiese it works quite nice. The problem is that ioremap failures are quite common as we really try to remap huge amount (sometimes above 128 or even 256 MB).

But...this region is small should never be hard to remap. Are you sure
you've seen trouble with the engine region?

Well I agree that after the reordering you did it should more likely work. But it remains valid that viafb can work without it and so it should (IMHO).
Yes there were actually problems, especially since the really old code did not even catch the error:
https://bugzilla.kernel.org/show_bug.cgi?id=13976

In fact the bug (or missing feature) is that the first ioremap is fatal and that needs to be fixed on the long run. Sorry but I don't want to make viafb unusable for many people if they don't add the "vmalloc=" kernel option so failing here would be a step in the wrong direction.

Is the real problem, perhaps, that we're remapping the entire
framebuffer? One wonders if we could get away without doing that.
Some quick greppery suggests that the cursor code goes straight to a

small region of framebuffer memory. With working acceleration, we
shouldn't need cfb_*(). But, probably, I'm missing something?

That mostly works I just tried it once for fun. But that's probably nothing we want to have in mainline as the hardware acceleration functions do at least have the possibility to fail. Possible solutions would include reducing the remapped memory on error (example: dividing it each time by 2 and failing if not at least 4 MB could be reamapped) or remapping it on demand: framebuffer only as big as needed (often between 4 and 8MB) with the possibility to "grow" and the cursor. The last would be my favourite as it does not needlessly eats memory space but is hardest to implement. Probably it will be fixed the next time I'll work around the memory management (which will be also tricky due to the framebuffer interface)

Meanwhile, I can let initialization continue without the engine space.
It kills interrupts/dma/camera, of course, but so be it.

Yeah at least for viafb that would be nice as it does not use any of these until now.


Thanks,

Florian Tobias Schandinat
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/