| Message ID | 20231115102954.7102-1-tzimmermann@suse.de |
|---|---|
| Headers | show |
| Series | fbdev: Modularize helpers for struct fb_ops | expand |
Thomas Zimmermann <tzimmermann@suse.de> writes: > Fix build by using the correct name for the initializer macro > for struct fb_ops. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Fixes: 9037afde8b9d ("fbdev/acornfb: Use fbdev I/O helpers") > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Helge Deller <deller@gmx.de> > Cc: Javier Martinez Canillas <javierm@redhat.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: <stable@vger.kernel.org> # v6.6+ > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > Only initialize mmap and draw helpers with macros; leave read/write > callbacks to driver implementations. Fixes the following warnings: > > CC [M] drivers/video/fbdev/sm712fb.o > sm712fb.c:1355:25: warning: initialized field overwritten [-Woverride-init] > 1355 | .fb_fillrect = cfb_fillrect, > | ^~~~~~~~~~~~ > sm712fb.c:1355:25: note: (near initialization for 'smtcfb_ops.fb_fillrect') > sm712fb.c:1356:25: warning: initialized field overwritten [-Woverride-init] > 1356 | .fb_imageblit = cfb_imageblit, > | ^~~~~~~~~~~~~ > sm712fb.c:1356:25: note: (near initialization for 'smtcfb_ops.fb_imageblit') > sm712fb.c:1357:25: warning: initialized field overwritten [-Woverride-init] > 1357 | .fb_copyarea = cfb_copyarea, > | ^~~~~~~~~~~~ > sm712fb.c:1357:25: note: (near initialization for 'smtcfb_ops.fb_copyarea') > sm712fb.c:1358:25: warning: initialized field overwritten [-Woverride-init] > 1358 | .fb_read = smtcfb_read, > | ^~~~~~~~~~~ > sm712fb.c:1358:25: note: (near initialization for 'smtcfb_ops.fb_read') > sm712fb.c:1359:25: warning: initialized field overwritten [-Woverride-init] > 1359 | .fb_write = smtcfb_write, > | ^~~~~~~~~~~~ > sm712fb.c:1359:25: note: (near initialization for 'smtcfb_ops.fb_write') > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Fixes: 586132cf1d38 ("fbdev/sm712fb: Initialize fb_ops to fbdev I/O-memory helpers") > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Javier Martinez Canillas <javierm@redhat.com> > Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com> > Cc: Teddy Wang <teddy.wang@siliconmotion.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Helge Deller <deller@gmx.de> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: linux-fbdev@vger.kernel.org > --- > drivers/video/fbdev/sm712fb.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > The cfag12864bfb driver operates on system memory. Mark the framebuffer > accordingly. Helpers operating on the framebuffer memory will test for > the presence of this flag. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Miguel Ojeda <ojeda@kernel.org> > --- > drivers/auxdisplay/cfag12864bfb.c | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > Initialize the instance of struct fb_ops with fbdev initializer > macros for framebuffers in virtual address space. Set the read/write, > draw and mmap callbacks to the correct implementation and avoid > implicit defaults. Also select the necessary helpers in Kconfig. > > Fbdev drivers sometimes rely on the callbacks being NULL for a > default I/O-memory-based implementation to be invoked; hence > requiring the I/O helpers to be built in any case. Setting all > callbacks in all drivers explicitly will allow to make the I/O > helpers optional. This benefits systems that do not use these > functions. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Miguel Ojeda <ojeda@kernel.org> > Cc: Robin van der Gracht <robin@protonic.nl> > --- > drivers/auxdisplay/Kconfig | 5 +---- > drivers/auxdisplay/ht16k33.c | 7 ++----- > 2 files changed, 3 insertions(+), 9 deletions(-) > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > The picolcd_fb driver operates on system memory. Mark the framebuffer > accordingly. Helpers operating on the framebuffer memory will test > for the presence of this flag. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Cc: "Bruno Prémont" <bonbons@linux-vserver.org> > Cc: Jiri Kosina <jikos@kernel.org> > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com> > Cc: linux-input@vger.kernel.org > --- > drivers/hid/hid-picolcd_fb.c | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > The sh_mobile_lcdcfb driver operates on DMA-able system memory. Mark > the framebuffer accordingly. Helpers operating on the framebuffer memory > will test for the presence of this flag. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > The driver uses deferred I/O. Select the correct helpers via > FB_SYSMEM_HELPERS_DEFERRED in the Kconfig file. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > The driver uses deferred I/O. Select the correct helpers via > FB_SYSMEM_HELPERS_DEFERRED in the Kconfig file. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > The au1200fb driver operates on DMA-able system memory. Mark the > framebuffer accordingly. Helpers operating on the framebuffer memory > will test for the presence of this flag. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > Initialize the instance of struct fb_ops with fbdev initializer > macros for framebuffers in DMA-able virtual address space. Set the > read/write, draw and mmap callbacks to the correct implementation > and avoid implicit defaults. Also select the necessary helpers in > Kconfig. > > Fbdev drivers sometimes rely on the callbacks being NULL for a > default I/O-memory-based implementation to be invoked; hence > requiring the I/O helpers to be built in any case. Setting all > callbacks in all drivers explicitly will allow to make the I/O > helpers optional. This benefits systems that do not use these > functions. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > Initialize the instance of struct fb_ops with fbdev initializer > macros for framebuffers in DMA-able virtual address space. Set the > read/write, draw and mmap callbacks to the correct implementation > and avoid implicit defaults. Also select the necessary helpers in > Kconfig. > > Fbdev drivers sometimes rely on the callbacks being NULL for a > default I/O-memory-based implementation to be invoked; hence > requiring the I/O helpers to be built in any case. Setting all > callbacks in all drivers explicitly will allow to make the I/O > helpers optional. This benefits systems that do not use these > functions. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > Initialize the instance of struct fb_ops with fbdev initializer > macros for framebuffers in DMA-able address space. This explictily > sets the read/write, draw and mmap callbacks to the correct default > implementation. Also select the necessary helpers in Kconfig. > > Fbdev drivers sometimes rely on the callbacks being NULL for a > default implementation to be invoked; hence requireing the I/O > helpers to be built in any case. Setting all callbacks in all > drivers explicitly will allow to make the I/O helpers optional. > This benefits systems that do not use these functions. > > Set the callbacks via macros. No functional changes. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > Initialize all instances of struct fb_ops with fbdev initializer > macros for framebuffers in I/O address space. Set the read/write, > draw and mmap callbacks to the correct implementation and avoid > implicit defaults. Also select the necessary helpers in Kconfig. > > Fbdev drivers sometimes rely on the callbacks being NULL for a > default I/O-memory-based implementation to be invoked; hence > requiring the I/O helpers to be built in any case. Setting all > callbacks in all drivers explicitly will allow to make the I/O > helpers optional. This benefits systems that do not use these > functions. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com> > Cc: Teddy Wang <teddy.wang@siliconmotion.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: linux-staging@lists.linux.dev > --- > drivers/staging/sm750fb/sm750.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > Rename the token to harmonize naming among various helpers. For > example, I/O-memory helpers use FB_IOMEM_FOPS. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- I wonder if the object names should also be changed to fb_iomem_fops.o and fb_sysmem_fops.o for consistency with the Kconfig symbols names. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > If a driver sets struct fb_ops.fb_mmap, the fbdev core automatically > calls pgprot_decrypted(). But the default fb_mmap code doesn't handle > pgprot_decrypted(). > > Move the call to pgprot_decrypted() into each drivers' fb_mmap function. > This only concerns fb_mmap functions for system and DMA memory. For > I/O memory, which is the default case, nothing changes. The fb_mmap > for I/O-memory can later be moved into a helper as well. > > DRM's fbdev emulation handles pgprot_decrypted() internally via the > Prime helpers. Fbdev doesn't have to do anything in this case. In > cases where DRM uses deferred I/O, this patch updates fb_mmap correctly. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann <tzimmermann@suse.de> writes: > Drop the default implementations for file read, write and mmap > operations. Each fbdev driver must now provide an implementation > and select any necessary helpers. If no implementation has been > set, fbdev returns an errno code to user space. The code is the > same as if the operation had not been set in the file_operations > struct. > > This change makes the fbdev helpers for I/O memory optional. Most > systems only use system-memory framebuffers via DRM's fbdev emulation. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- [...] > @@ -34,13 +34,13 @@ static ssize_t fb_read(struct file *file, char __user *buf, size_t count, loff_t > if (!info) > return -ENODEV; > > + if (!info->fbops->fb_read) > + return -EINVAL; > + Can we also add a warn here? In case that it was missed to set a driver callback. Probably can be figured out from the -EINVAL but better to be explicit about the issue to make finding that easier. And same for the other fops. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
On Thu, 16 Nov 2023 11:27:55 +0100 Javier Martinez Canillas wrote: > Thomas Zimmermann <tzimmermann@suse.de> writes: > > > The picolcd_fb driver operates on system memory. Mark the framebuffer > > accordingly. Helpers operating on the framebuffer memory will test > > for the presence of this flag. > > > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > Cc: "Bruno Prémont" <bonbons@linux-vserver.org> > > Cc: Jiri Kosina <jikos@kernel.org> > > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > Cc: linux-input@vger.kernel.org > > --- > > drivers/hid/hid-picolcd_fb.c | 1 + > > 1 file changed, 1 insertion(+) > > > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Acked-by: Bruno Prémont <bonbons@linux-vserver.org>