Re: [Intel-gfx] [PATCH v2 02/12] drm/armada: Use regular fbdev I/O helpers

Russell King (Oracle)
2023-05-15T18:13:38Z

On Mon, May 15, 2023 at 07:55:44PM +0200, Sam Ravnborg wrote:
> Hi Thomas,
>
> On Mon, May 15, 2023 at 11:40:23AM +0200, Thomas Zimmermann wrote:
> > Use the regular fbdev helpers for framebuffer I/O instead of DRM's
> > helpers. Armada does not use damage handling, so DRM's fbdev helpers
> > are mere wrappers around the fbdev code.
> >
> > By using fbdev helpers directly within each DRM fbdev emulation,
> > we can eventually remove DRM's wrapper functions entirely.
> >
> > v2:
> > * use FB_IO_HELPERS option
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Russell King <linux@armlinux.org.uk>
> > ---
> > drivers/gpu/drm/armada/Kconfig | 1 +
> > drivers/gpu/drm/armada/armada_fbdev.c | 9 ++++-----
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/armada/Kconfig b/drivers/gpu/drm/armada/Kconfig
> > index f5c66d89ba99..5afade25e217 100644
> > --- a/drivers/gpu/drm/armada/Kconfig
> > +++ b/drivers/gpu/drm/armada/Kconfig
> > @@ -3,6 +3,7 @@ config DRM_ARMADA
> > tristate "DRM support for Marvell Armada SoCs"
> > depends on DRM && HAVE_CLK && ARM && MMU
> > select DRM_KMS_HELPER
> > + select FB_IO_HELPERS if DRM_FBDEV_EMULATION
> > help
> > Support the "LCD" controllers found on the Marvell Armada 510
> > devices. There are two controllers on the device, each controller
> > diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c
> > index 0a5fd1aa86eb..6c3bbaf53569 100644
> > --- a/drivers/gpu/drm/armada/armada_fbdev.c
> > +++ b/drivers/gpu/drm/armada/armada_fbdev.c
> > @@ -5,6 +5,7 @@
> > */
> >
> > #include <linux/errno.h>
> > +#include <linux/fb.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> >
> > @@ -34,11 +35,9 @@ static void armada_fbdev_fb_destroy(struct fb_info *info)
> > static const struct fb_ops armada_fb_ops = {
> > .owner = THIS_MODULE,
> > DRM_FB_HELPER_DEFAULT_OPS,
> > - .fb_read = drm_fb_helper_cfb_read,
> > - .fb_write = drm_fb_helper_cfb_write,
> I had expected to see
> .fb_read = fb_io_read,
>
> But maybe this only used when using damage handling?
>
> Likewise for drm_fb_helper_cfb_write.
>
> ??
>
> > - .fb_fillrect = drm_fb_helper_cfb_fillrect,
> > - .fb_copyarea = drm_fb_helper_cfb_copyarea,
> > - .fb_imageblit = drm_fb_helper_cfb_imageblit,
> > + .fb_fillrect = cfb_fillrect,
> > + .fb_copyarea = cfb_copyarea,
> > + .fb_imageblit = cfb_imageblit,
>
> This part is as expected.

Well, to me it looks like this has gone through an entire circular set
of revisions:

commit e8b70e4dd7b5dad7c2379de6e0851587bf86bfd6
Author: Archit Taneja <architt@codeaurora.org>
Date: Wed Jul 22 14:58:04 2015 +0530

drm/armada: Use new drm_fb_helper functions

- .fb_fillrect = cfb_fillrect,
- .fb_copyarea = cfb_copyarea,
- .fb_imageblit = cfb_imageblit,
+ .fb_fillrect = drm_fb_helper_cfb_fillrect,
+ .fb_copyarea = drm_fb_helper_cfb_copyarea,
+ .fb_imageblit = drm_fb_helper_cfb_imageblit,

commit 983780918c759fdbbf0bf033e701bbff75d2af23
Author: Thomas Zimmermann <tzimmermann@suse.de>
Date: Thu Nov 3 16:14:40 2022 +0100

drm/fb-helper: Perform all fbdev I/O with the same implementation

+ .fb_read = drm_fb_helper_cfb_read,
+ .fb_write = drm_fb_helper_cfb_write,

and now effectively those two changes are being reverted, so we'd
now be back to the pre-July 2015 state of affairs. As I believe
the fbdev layer has been stable, this change merely reverts the
driver back to what it once was.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!