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

[Bruno PrÃmont]

On Fri, 30 April 2010 Florian Tobias Schandinat <FlorianSchandinat@xxxxxx> wrote:
> 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

Ok, but then it would help to have the comment explain that ioremap
is allowed to fail but will disable accel or what other impact it has
(probably also put similar comment at declaration of vdev->engine_mmio)

Thanks,
Bruno


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