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

["Russell King (Oracle)" <linux@armlinux.org.uk>]

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!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel