Hi Noralf, On Wednesday, 13 September 2017 18:19:22 EEST Noralf Trønnes wrote: > Den 13.09.2017 07.09, skrev Laurent Pinchart: > > On Monday, 11 September 2017 17:31:54 EEST Noralf Trønnes wrote: > >> Hi, > >> > >> I want to start out by saying that this patchset is low priority for me > >> and if no one has interest or time to review this, that is just fine. I > >> was in the flow and just typed it out. > >> > >> This patchset adds a way for fbdev emulation code to create a > >> framebuffer that is backed by a dumb buffer. drm_fb_helper gets a > >> drm_file to hang the objects on, drm_framebuffer_create_dumb() creates > >> the framebuffer and drm_fb_helper_fini() destroys it. > >> I have verified that all cma drivers supports dumb buffers, so > >> converting the library should be fine for all. > > > > Stupid question, what does this give us ? The series makes the call stack > > more complex (up to a point where I'm getting trouble just following it), > > what's the advantage that offsets that ? > > The short answer is that it avoids the need for special fbdev > _with_funcs() functions in libraries like cma for framebuffers with the > dirty callback set. > > I'm suprised that you think this more complex, I find it more coherent > when fbdev userspace is doing things more like DRM userspace, like > hanging the objects on a drm_file and cleaning up the objects when done. Maybe moving to a new code base gave me the wrong feeling that the result is more complex. The current implementation is certainly not simple. > The longer and more diffuse answer is that it's annoying me that many > drivers carry the burden of fbdev emulation just to get a console! I totally agree with that. Ideally I'd like to remove 100% of fbdev-related code from drivers. This includes - initialization and cleanup of fbdev helpers - fbdev restore in last_close() - forwarding of hotplug events to fbdev compatibility layer In practice we'll likely need to keep one initialization function (or a drm_driver flag) to let drivers opt-in to fbdev compatibility, but the rest should go away. Or maybe we could even enable fbdev compatibility in all drivers unconditionally. > And an in-kernel drm console, as David Herrmann described it a year ago, > would allocate a framebuffer using something like what I have done here. > But maybe it's better to do this the other way around: first get the > drm console and then let fbdev use the same mechanisms. Or maybe at that > point, just forget about fbdev and not spend any more time on it :-) When I mention I'd like fbdev to go away completely people complain that I like breaking things :-) > Sidenote: > Do you if any DRM drivers is capable of showing kernel oops'es on the > console? The last time my kernel oopses the i915 driver certainly didn't show me the oops. > > With the exception of vmwgfx that does weird things I won't even try to > > understand, all drivers seem to use the drm_file object passed to the > > .dumb_create() operation just to register the GEM object handle. I wonder > > whether a better solution to use .dumb_create() for framebuffer emulation > > wouldn't be to move the GEM object handle registration from the > > .dumb_create() implementation to its caller in the core. > > Can you elaborate what you mean by this? Again vmwgfx aside (and that's not a detail, we'd still have to handle that driver), the reason why the .dumb_create() operation needs a drm_file pointer is to register the GEM object handle for the dumb buffer. We could perform that registration in the caller of .dumb_create(), in which case we wouldn't need to pass the drm_file to the operation. > >> A patch by David Herrmann from a year ago made this easy. It was the > >> last piece in his work to make it possible to create a drm_file for > >> in-kernel use, but it never got merged. > >> > >> I've cc'ed intel-gfx since that will give CI runs of the core patches if > >> I understood Daniel right. > >> > >> Noralf. > >> > >> David Herrmann (1): > >> drm: provide management functions for drm_file > >> > >> Noralf Trønnes (7): > >> drm/framebuffer: Add drm_framebuffer_create_dumb() > >> drm/auth: Export drm_dropmaster_ioctl() > >> drm/fb-helper: Allocate a drm_file > >> drm/fb-cma-helper: Use drm_framebuffer_create_dumb() > >> drm/fb-cma-helper: Drop unnecessary fbdev buffer offset > >> drm/tinydrm: Use drm_fbdev_cma_init() > >> drm/fb-cma-helper: Remove drm_fbdev_cma_init_with_funcs() > >> > >> drivers/gpu/drm/drm_auth.c | 1 + > >> drivers/gpu/drm/drm_fb_cma_helper.c | 111 ++-------- > >> drivers/gpu/drm/drm_fb_helper.c | 22 +- > >> drivers/gpu/drm/drm_file.c | 323 ++++++++++++-------- > >> drivers/gpu/drm/drm_framebuffer.c | 61 ++++++ > >> drivers/gpu/drm/drm_internal.h | 2 - > >> drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 5 +- > >> include/drm/drm_auth.h | 2 + > >> include/drm/drm_fb_helper.h | 9 + > >> include/drm/drm_file.h | 2 + > >> include/drm/drm_framebuffer.h | 4 + > >> 11 files changed, 305 insertions(+), 237 deletions(-) -- Regards, Laurent Pinchart _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
