Re: [PATCH 2/3] Define fb_open_adj_file()

[Max Staudt]

Hi Daniel,

Thanks for the feedback! Comments below:


On 05/23/2016 03:44 PM, Daniel Vetter wrote:
> Do we _really_ care about fbdev mmap support so much that we want to add
> more hacks all over the place (in each driver) to make it work? Given that
> fbdev is officially in the "no more drivers" territory, I'm not sure we
> want this either.
> 
> The trouble is that fbdev mmap and kms dumb buffer mmap have different
> semantics and so every driver needs to implement both if they're special
> in any kind of way.

DRM drivers already have fbdev hacks in place in order to implement both semantics.

With this change, only bochsdrm(fb) will need to be touched, and if no further DRM driver implements fbdev support the way bochsdrmfb does, then no further driver will need this code.


This patch set is really just about setting file->f_mapping in bochsdrmfb's fb_open(), so we finally get rid of the WARNING.

We could also pass file as a third paramenter in fb_open(), changing the function definition in all fbdev drivers. I'm happy to change the patch in that way if this is preferred.


> If we can't say no to fbdev mmap then I think we should implement
> something that works for all drivers. I'm thinking of allocating just a
> pile of pages in the fbdev emulation, and then copying them over using the
> defio stuff we just merged for 4.7 into a dumb bo, plus calling dirtyfb.
> Or at least something along those lines. Of couse just opt-in, in case the
> driver can do a more traditional mmio fbdev mmapping.

Well, the mmap code is standard fbdev and it's already there, so why not keep it?

All this patch series does is allowing bochsdrmfb to work properly until fbdev support is finally removed from the kernel.



Max