[RFC][PATCH 00/11] DRM driver for fbdev devices

Wed Mar 27 14:46:08 UTC 2019

Hi

Am 27.03.19 um 10:41 schrieb Daniel Vetter:
>> I use the current approach because it does not require modifications to
>> DRM or fbdev. Not copying the fbdev driver code has the advantage that
>> any fix that goes into fbdev is also used by fbdevdrm.
>>
>> OTOH, that has some problems. At least the event-reporting hooks appear
>> to be fragile. You mentioned that you have patches for replacing it. I'd
>> be happy to use something else. Filtering out generic and DRM-based
>> drivers is also not optimal.
>>
>> Instead of adding the porting helpers to the DRM core, I'd suggest to
>> add them to fbdevdrm. Fbdevdrm would have to duplicate some of the fbdev
>> functionality and provide a replacement for register_framebuffer.
>>
>> Porting would then mean to
>>
>>  1) create a new DRM driver by copying fbdevdrm, and
>>  2) copy the fbdev driver code to the new DRM driver and rename the
>> function calls accordingly. At this point the new driver should already
>> work to some extend.
>>  3) Finally, do the refactoring and get it out of staging.
> 
> Maybe the idea behind helpers isn't fully clear:
> 
> https://blog.ffwll.ch/2016/12/midlayers-once-more-with-feeling.html
> 
> The idea with helpers is essentially to have all the flexibility of
> copypasting (you can refactor as you see fit), without having to
> actually copypaste a lot of the code. That's why I think an fbdevdrm
> helper suits, at least as long as we're actively transitioning. Worked
> fairly well for the atomic transition at least.

That's not really a problem from my side, but maybe we misunderstood
each other.

Modeling fbdevdrm after CMA helpers or simple drm would put it next to
central DRM core and helper modules. That seems like an odd place for
transitional helper functions that are supposed to be replaced by the
actual DRM helpers.

I don't want some kind of abstraction layer. I'm proposing a design like
tinydrm, where a separate module offers helpers for fbdev drivers that
are about to be converted.

The difference might not be that significant, but just a question of
were the code is located.

>>> - Improve the other helpers we have as needed - there's probably room for
>>>   a simple ttm version, inspired by all these simple display chips (e.g.
>>>   ast/cirrus/bochs/mga all solve similar problems like this).
>>
>> Makes sense, but appears to be unrelated.
> 
> It's the main reason to have drivers in upstream - sharing code and
> infrastructure.

I meant that creating 'simple TTM' is unrelated to fbdevdrm. It would
make sense in any case.

Best regards
Thomas

At least for me the main benefit in porting a bunch
> more fbdev drivers over isn't the old hw support (that hw is quietly
> dying anyway, we just have to wait). But improving support for new hw
> that fills similar niches, and making it easier for everyone else. Ofc
> I'm not going to stop anyone who's going to do all the porting work,
> but we can't realistically require it (e.g. legacy kms drivers also
> don't support atomic, since simply not possible without rewriting the
> driver).
> 
> Cheers, Daniel
>>
>> Best regards
>> Thomas
>>
>>> - Treat any such drivers as CONFIG_STAGING until they've become fully
>>>   native, i.e. if no one bothers to convert them, we'll drop them again.
>>>
>>> The above is kinda similar in spirit to the transitional helpers we've
>>> used to upgrade the big drivers from legacy kms to atomic.
>>>
>>> Thoughts?
>>>
>>> Cheers, Daniel
>>>
>>>>
>>>>  drivers/gpu/drm/Kconfig                     |   2 +
>>>>  drivers/gpu/drm/Makefile                    |   1 +
>>>>  drivers/gpu/drm/fbdevdrm/Kconfig            |  13 +
>>>>  drivers/gpu/drm/fbdevdrm/Makefile           |  11 +
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_bo.c      | 276 ++++++++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_bo.h      |  58 ++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_device.c  |  96 +++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_device.h  |  55 ++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_drv.c     | 347 +++++++++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_format.c  | 441 ++++++++++++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_format.h  |  26 +
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c   | 195 +++++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h   |  53 ++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_modeset.c | 746 ++++++++++++++++++++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_modeset.h |  38 +
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_primary.c | 498 +++++++++++++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_primary.h |  27 +
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_ttm.c     | 202 ++++++
>>>>  drivers/gpu/drm/fbdevdrm/fbdevdrm_ttm.h     |  35 +
>>>>  19 files changed, 3120 insertions(+)
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/Kconfig
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/Makefile
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_bo.c
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_bo.h
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_device.c
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_device.h
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_drv.c
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_format.c
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_format.h
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_modeset.c
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_modeset.h
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_primary.c
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_primary.h
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_ttm.c
>>>>  create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_ttm.h
>>>>
>>>> --
>>>> 2.21.0
>>>>
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
>> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
>> HRB 21284 (AG Nürnberg)
>>
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20190327/9ca7dd2f/attachment-0001.sig>