| Message ID | 20191127180035.416209-3-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/rockchip: Use drm_gem_fb_create_with_dirty | expand |
Hi Am 27.11.19 um 19:00 schrieb Daniel Vetter: > We're doing a great job for really simple drivers right now, but still > a lot of boilerplate for the bigger ones. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Just a small remark below, otherwise Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > Documentation/gpu/todo.rst | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 3ec509381fc5..2d85f37284a1 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -182,6 +182,32 @@ Contact: Maintainer of the driver you plan to convert > > Level: Intermediate > > +drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup > +----------------------------------------------------------------- > + > +A lot more drivers could be switched over to the drm_gem_framebuffer helpers. > +Various hold-ups: > + > +- Need to switch over to the generic dirty tracking code using > + drm_atomic_helper_dirtyfb first (e.g. qxl). > + > +- Need to switch to drm_fbdev_generic_setup(), otherwise a lot of the custom fb > + setup code can't be deleted. From what I remember, almost all of the obvious, low-hanging fruits have been picked here. The remaining fbdev users either have HW acceleration (nouveau, gma500), or use the cfb drawing functions. The TODO item should probably mention this, with some advise to do some extra testing for compatibility or performance after moving to generic fbdev. Best regards Thomas > + > +- Many drivers wrap drm_gem_fb_create() only to check for valid formats. For > + atomic drivers we could check for valid formats by calling > + drm_plane_check_pixel_format() against all planes, and pass if any plane > + supports the format. For non-atomic that's not possible since like the format > + list for the primary plane is fake and we'd therefor reject valid formats. > + > +- Many drivers subclass drm_framebuffer, we'd need a embedding compatible > + version of the varios drm_gem_fb_create functions. Maybe called > + drm_gem_fb_create/_with_dirty/_with_funcs as needed. > + > +Contact: Daniel Vetter > + > +Level: Intermediate > + > Clean up mmap forwarding > ------------------------ > >
On Fri, Nov 29, 2019 at 10:34:10AM +0100, Thomas Zimmermann wrote: > Hi > > Am 27.11.19 um 19:00 schrieb Daniel Vetter: > > We're doing a great job for really simple drivers right now, but still > > a lot of boilerplate for the bigger ones. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Just a small remark below, otherwise > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > > > --- > > Documentation/gpu/todo.rst | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > index 3ec509381fc5..2d85f37284a1 100644 > > --- a/Documentation/gpu/todo.rst > > +++ b/Documentation/gpu/todo.rst > > @@ -182,6 +182,32 @@ Contact: Maintainer of the driver you plan to convert > > > > Level: Intermediate > > > > +drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup > > +----------------------------------------------------------------- > > + > > +A lot more drivers could be switched over to the drm_gem_framebuffer helpers. > > +Various hold-ups: > > + > > +- Need to switch over to the generic dirty tracking code using > > + drm_atomic_helper_dirtyfb first (e.g. qxl). > > + > > +- Need to switch to drm_fbdev_generic_setup(), otherwise a lot of the custom fb > > + setup code can't be deleted. > > From what I remember, almost all of the obvious, low-hanging fruits have > been picked here. The remaining fbdev users either have HW acceleration > (nouveau, gma500), or use the cfb drawing functions. I think a bunch of these (from you) aren't merged yet. I'll add a note about sys vs cfb. About gma500/nouveau, I'm kinda tempted to just ditch the acceleration ... but maybe someone cares, dunno. > The TODO item should probably mention this, with some advise to do some > extra testing for compatibility or performance after moving to generic > fbdev. Testing (at least on x86) won't catch the cfb/sysfb stuff, since it's exactly the same asm instructions :-/ tbh I still don't know where this actually makes a difference. -Daniel > > Best regards > Thomas > > > + > > +- Many drivers wrap drm_gem_fb_create() only to check for valid formats. For > > + atomic drivers we could check for valid formats by calling > > + drm_plane_check_pixel_format() against all planes, and pass if any plane > > + supports the format. For non-atomic that's not possible since like the format > > + list for the primary plane is fake and we'd therefor reject valid formats. > > + > > +- Many drivers subclass drm_framebuffer, we'd need a embedding compatible > > + version of the varios drm_gem_fb_create functions. Maybe called > > + drm_gem_fb_create/_with_dirty/_with_funcs as needed. > > + > > +Contact: Daniel Vetter > > + > > +Level: Intermediate > > + > > Clean up mmap forwarding > > ------------------------ > > > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer >
On Fri, Nov 29, 2019 at 07:57:39PM +0100, Daniel Vetter wrote: > On Fri, Nov 29, 2019 at 10:34:10AM +0100, Thomas Zimmermann wrote: > > Hi > > > > Am 27.11.19 um 19:00 schrieb Daniel Vetter: > > > We're doing a great job for really simple drivers right now, but still > > > a lot of boilerplate for the bigger ones. > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > > Just a small remark below, otherwise > > > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > > > > > > --- > > > Documentation/gpu/todo.rst | 26 ++++++++++++++++++++++++++ > > > 1 file changed, 26 insertions(+) > > > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > > index 3ec509381fc5..2d85f37284a1 100644 > > > --- a/Documentation/gpu/todo.rst > > > +++ b/Documentation/gpu/todo.rst > > > @@ -182,6 +182,32 @@ Contact: Maintainer of the driver you plan to convert > > > > > > Level: Intermediate > > > > > > +drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup > > > +----------------------------------------------------------------- > > > + > > > +A lot more drivers could be switched over to the drm_gem_framebuffer helpers. > > > +Various hold-ups: > > > + > > > +- Need to switch over to the generic dirty tracking code using > > > + drm_atomic_helper_dirtyfb first (e.g. qxl). > > > + > > > +- Need to switch to drm_fbdev_generic_setup(), otherwise a lot of the custom fb > > > + setup code can't be deleted. > > > > From what I remember, almost all of the obvious, low-hanging fruits have > > been picked here. The remaining fbdev users either have HW acceleration > > (nouveau, gma500), or use the cfb drawing functions. > > I think a bunch of these (from you) aren't merged yet. I'll add a note > about sys vs cfb. About gma500/nouveau, I'm kinda tempted to just ditch > the acceleration ... but maybe someone cares, dunno. Correction, we already have a task for drm_fbdev_generic_setup, and that mentions the sys/cfb issue already. So I'll leave this as-is. -Daniel > > > The TODO item should probably mention this, with some advise to do some > > extra testing for compatibility or performance after moving to generic > > fbdev. > > Testing (at least on x86) won't catch the cfb/sysfb stuff, since it's > exactly the same asm instructions :-/ tbh I still don't know where this > actually makes a difference. > -Daniel > > > > > Best regards > > Thomas > > > > > + > > > +- Many drivers wrap drm_gem_fb_create() only to check for valid formats. For > > > + atomic drivers we could check for valid formats by calling > > > + drm_plane_check_pixel_format() against all planes, and pass if any plane > > > + supports the format. For non-atomic that's not possible since like the format > > > + list for the primary plane is fake and we'd therefor reject valid formats. > > > + > > > +- Many drivers subclass drm_framebuffer, we'd need a embedding compatible > > > + version of the varios drm_gem_fb_create functions. Maybe called > > > + drm_gem_fb_create/_with_dirty/_with_funcs as needed. > > > + > > > +Contact: Daniel Vetter > > > + > > > +Level: Intermediate > > > + > > > Clean up mmap forwarding > > > ------------------------ > > > > > > > > > > -- > > Thomas Zimmermann > > Graphics Driver Developer > > SUSE Software Solutions Germany GmbH > > Maxfeldstr. 5, 90409 Nürnberg, Germany > > (HRB 36809, AG Nürnberg) > > Geschäftsführer: Felix Imendörffer > > > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
(cc: SPARC64 maintainer) (cc: RISC-V maintainers) Hi Daniel Am 29.11.19 um 20:05 schrieb Daniel Vetter: > On Fri, Nov 29, 2019 at 07:57:39PM +0100, Daniel Vetter wrote: >> On Fri, Nov 29, 2019 at 10:34:10AM +0100, Thomas Zimmermann wrote: >>> Hi >>> >>> Am 27.11.19 um 19:00 schrieb Daniel Vetter: >>>> We're doing a great job for really simple drivers right now, but still >>>> a lot of boilerplate for the bigger ones. >>>> >>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >>> >>> Just a small remark below, otherwise >>> >>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> >>> >>> >>>> --- >>>> Documentation/gpu/todo.rst | 26 ++++++++++++++++++++++++++ >>>> 1 file changed, 26 insertions(+) >>>> >>>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst >>>> index 3ec509381fc5..2d85f37284a1 100644 >>>> --- a/Documentation/gpu/todo.rst >>>> +++ b/Documentation/gpu/todo.rst >>>> @@ -182,6 +182,32 @@ Contact: Maintainer of the driver you plan to convert >>>> >>>> Level: Intermediate >>>> >>>> +drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup >>>> +----------------------------------------------------------------- >>>> + >>>> +A lot more drivers could be switched over to the drm_gem_framebuffer helpers. >>>> +Various hold-ups: >>>> + >>>> +- Need to switch over to the generic dirty tracking code using >>>> + drm_atomic_helper_dirtyfb first (e.g. qxl). >>>> + >>>> +- Need to switch to drm_fbdev_generic_setup(), otherwise a lot of the custom fb >>>> + setup code can't be deleted. >>> >>> From what I remember, almost all of the obvious, low-hanging fruits have >>> been picked here. The remaining fbdev users either have HW acceleration >>> (nouveau, gma500), or use the cfb drawing functions. >> >> I think a bunch of these (from you) aren't merged yet. I'll add a note >> about sys vs cfb. About gma500/nouveau, I'm kinda tempted to just ditch >> the acceleration ... but maybe someone cares, dunno. > > Correction, we already have a task for drm_fbdev_generic_setup, and that > mentions the sys/cfb issue already. So I'll leave this as-is. Maybe refer to the related TODO item. > -Daniel > >> >>> The TODO item should probably mention this, with some advise to do some >>> extra testing for compatibility or performance after moving to generic >>> fbdev. >> >> Testing (at least on x86) won't catch the cfb/sysfb stuff, since it's >> exactly the same asm instructions :-/ tbh I still don't know where this >> actually makes a difference. I briefly looked through the code of the CFB helpers. They use the helpers at [1] for accessing the framebuffer. Those are special for several architectures. [2] SPARC64 expands to assembly instructions. [3] The rest appears to have regular memory instructions in their implementations of __raw_readb(). Although not listed here, I found that RISC V has assembly code in __raw_readb(). [4] In the CFB code, there's also bit-shifting code that cares about endianess. [5] In the end it seems to depends on FBINFO_FOREIGN_ENDIAN, but the only user I could find was mb862xxfb. [6] I don't know what the hand-crafted assembly instructions do in detail, but for the moment we seem to be good without CFB code. Best regards Thomas [1] https://elixir.bootlin.com/linux/v5.4/source/include/linux/fb.h#L564 [2] https://elixir.bootlin.com/linux/v5.4/source/include/linux/fb.h#L527 [3] https://elixir.bootlin.com/linux/v5.4/source/arch/sparc/include/asm/io_64.h#L20 [4] https://elixir.bootlin.com/linux/v5.4/source/arch/riscv/include/asm/io.h#L59 [5] https://elixir.bootlin.com/linux/v5.4/source/include/linux/fb.h#L578 [6] https://elixir.bootlin.com/linux/v5.4/source/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c#L501 >> -Daniel >> >>> >>> Best regards >>> Thomas >>> >>>> + >>>> +- Many drivers wrap drm_gem_fb_create() only to check for valid formats. For >>>> + atomic drivers we could check for valid formats by calling >>>> + drm_plane_check_pixel_format() against all planes, and pass if any plane >>>> + supports the format. For non-atomic that's not possible since like the format >>>> + list for the primary plane is fake and we'd therefor reject valid formats. >>>> + >>>> +- Many drivers subclass drm_framebuffer, we'd need a embedding compatible >>>> + version of the varios drm_gem_fb_create functions. Maybe called >>>> + drm_gem_fb_create/_with_dirty/_with_funcs as needed. >>>> + >>>> +Contact: Daniel Vetter >>>> + >>>> +Level: Intermediate >>>> + >>>> Clean up mmap forwarding >>>> ------------------------ >>>> >>>> >>> >>> -- >>> Thomas Zimmermann >>> Graphics Driver Developer >>> SUSE Software Solutions Germany GmbH >>> Maxfeldstr. 5, 90409 Nürnberg, Germany >>> (HRB 36809, AG Nürnberg) >>> Geschäftsführer: Felix Imendörffer >>> >> >> >> >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> http://blog.ffwll.ch >
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 3ec509381fc5..2d85f37284a1 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -182,6 +182,32 @@ Contact: Maintainer of the driver you plan to convert Level: Intermediate +drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup +----------------------------------------------------------------- + +A lot more drivers could be switched over to the drm_gem_framebuffer helpers. +Various hold-ups: + +- Need to switch over to the generic dirty tracking code using + drm_atomic_helper_dirtyfb first (e.g. qxl). + +- Need to switch to drm_fbdev_generic_setup(), otherwise a lot of the custom fb + setup code can't be deleted. + +- Many drivers wrap drm_gem_fb_create() only to check for valid formats. For + atomic drivers we could check for valid formats by calling + drm_plane_check_pixel_format() against all planes, and pass if any plane + supports the format. For non-atomic that's not possible since like the format + list for the primary plane is fake and we'd therefor reject valid formats. + +- Many drivers subclass drm_framebuffer, we'd need a embedding compatible + version of the varios drm_gem_fb_create functions. Maybe called + drm_gem_fb_create/_with_dirty/_with_funcs as needed. + +Contact: Daniel Vetter + +Level: Intermediate + Clean up mmap forwarding ------------------------
We're doing a great job for really simple drivers right now, but still a lot of boilerplate for the bigger ones. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- Documentation/gpu/todo.rst | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)