[PATCH 0/8] drm/fb-helper: Use drm_file to get a dumb framebuffer

[Noralf Trønnes]

Den 13.09.2017 07.09, skrev Laurent Pinchart:
> Hi Noralf,
>
> Thank you for the patches.
>
> 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.

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!
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 :-)

Sidenote:
Do you if any DRM drivers is capable of showing kernel oops'es on the
console?

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

Noralf.

>> 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(-)