Hi
Am 15.05.23 um 20:04 schrieb Russell King (Oracle):
> 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 at suse.de>
>>> Cc: Russell King <linux at 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 at 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 at 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.
Not quite. One long-standing problem has been that fbdev does not
protect its public interfaces with CONFIG_FB. If fbdev had been
disabled, DRM drivers could no longer be linked/loaded. DRM wrappers
solved this. The issue has recently been fixed for all of DRM. DRM does
not build it's fbdev emulation if CONFIG_FB has been disabled.
Another thing was that the original DRM wrappers might have been
different from fbdev's I/O helpers in subtle ways. But now they are
simple wrappers around their fbdev counterparts; plus the option of
additional damage handling. But such damage handling is better
implemented by the driver itself. The two cases that require it, i915
and fbdev-generic, are different enough that each should probably have
it's own code.
Best regards
Thomas
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20230516/0f0578b8/attachment.sig>