Panic booting qemu-system-sparc64 with bochs_drm

Panic booting qemu-system-sparc64 with bochs_drm

[Mark Cave-Ayland]

Hi all,

I've been receiving reports that newer sparc64 kernels have started to panic on boot
under qemu-system-sparc64 with bochs_drm enabled which I was able to confirm locally
building git master:


[    9.007161] [drm] Found bochs VGA, ID 0xb0c5.
[    9.007840] [drm] Framebuffer size 16384 kB @ 0x1ff22000000, mmio @ 0x1ff23000000.
[    9.012567] [TTM] Zone  kernel: Available graphics memory: 51496 KiB
[    9.013551] [TTM] Initializing pool allocator
[    9.032757] [drm] Found EDID data blob.
[    9.061904] [drm] Initialized bochs-drm 1.0.0 20130925 for 0000:01:02.0 on minor 0
[    9.336819] Unable to handle kernel paging request at virtual address 000001ff221d0000
[    9.337177] tsk->{mm,active_mm}->context = 0000000000000000
[    9.337283] tsk->{mm,active_mm}->pgd = fffff80000402000
[    9.337372]               \|/ ____ \|/
[    9.337372]               "@'/ .. \`@"
[    9.337372]               /_| \__/ |_\
[    9.337372]                  \__U_/
[    9.337468] kworker/0:0(5): Oops [#1]
[    9.339359] CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 5.8.0-rc3+ #55
[    9.341360] Workqueue: events drm_fb_helper_dirty_work
[    9.341775] TSTATE: 0000000080001605 TPC: 000000000077441c TNPC: 0000000000774420
Y: 00000000    Not tainted
[    9.341894] TPC: <memcpy+0x121c/0x13c0>
[    9.342015] g0: 0000000000000000 g1: 0000000000000000 g2: 0000000000000000 g3:
fffff800043d2c00
[    9.342094] g4: fffff8000410eac0 g5: fffff800064cc000 g6: fffff80004124000 g7:
0000000000000010
[    9.342173] o0: 000001ff221d0000 o1: 0000000100220000 o2: 0000000000000000 o3:
000001fe21fb0000
[    9.342254] o4: 000001ff221d0000 o5: 0000000000000000 sp: fffff800041273d1 ret_pc:
0000000000805b18
[    9.342325] RPC: <drm_fb_helper_dirty_work+0xf8/0x180>
[    9.342591] l0: fffff80007819cc0 l1: fffff800043df8cc l2: 0000000001356200 l3:
fffff800064cc000
[    9.342670] l4: fffff80004004200 l5: 0000000000000000 l6: 0000000000000025 l7:
fffff80004002500
[    9.342750] i0: fffff800043df8d0 i1: fffff800040106b0 i2: 0000000000000020 i3:
fffff800043e5500
[    9.342829] i4: 00000000000001d1 i5: 0000000100220000 i6: fffff80004127491 i7:
0000000000481fec
[    9.342960] I7: <process_one_work+0x18c/0x540>
[    9.343308] Call Trace:
[    9.344077] [<0000000000481fec>] process_one_work+0x18c/0x540
[    9.344267] [<00000000004824c4>] worker_thread+0x124/0x580
[    9.344310] [<0000000000489758>] kthread+0xf8/0x120
[    9.344357] [<00000000004060a4>] ret_from_fork+0x1c/0x2c
[    9.344714] [<0000000000000000>] 0x0


The error "Unable to handle kernel paging request at virtual address
000001ff221d0000" is caused by trying to access the framebuffer using a virtual
address, rather than using IO accessors which access the framebuffer correctly using
SPARC ASI_PHYS (physical) loads and stores. In some ways this is similar to the bug I
reported a couple of years back at
https://lists.freedesktop.org/archives/dri-devel/2017-June/145793.html which was
fixed with https://lists.freedesktop.org/archives/dri-devel/2017-July/145935.html.

According to git bisect the regression is introduced by the following commit:

$ git bisect bad
7a0483ac4ffca4998945c159b28afdde8353cc84 is the first bad commit
commit 7a0483ac4ffca4998945c159b28afdde8353cc84
Author: Gerd Hoffmann <kraxel@redhat.com>
Date:   Fri Jan 11 06:37:50 2019 +0100

    drm/bochs: switch to generic drm fbdev emulation

    Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
    Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Link:
http://patchwork.freedesktop.org/patch/msgid/20190111053752.4004-15-kraxel@redhat.com

:040000 040000 1917943277034f620af03ac1a2fa5db48b7b224c
6d7a3c316a68efbffd398d6c2b7eebefb47bc92d M      drivers


The commit following this one at
https://patchwork.freedesktop.org/patch/276488/?series=54269&rev=4 removes
bochsfb_ops and the cfb helpers which was the original fix introduced by my second
patch above, so I'm unsure how to approach fixing this with the switch to
drm_fbdev_generic_setup().

Can anyone point me in the right direction?


Many thanks,

Mark.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Mark Cave-Ayland]

On 03/07/2020 22:57, Mark Cave-Ayland wrote:

> Hi all,
> 
> I've been receiving reports that newer sparc64 kernels have started to panic on boot
> under qemu-system-sparc64 with bochs_drm enabled which I was able to confirm locally
> building git master:
> 
> 
> [    9.007161] [drm] Found bochs VGA, ID 0xb0c5.
> [    9.007840] [drm] Framebuffer size 16384 kB @ 0x1ff22000000, mmio @ 0x1ff23000000.
> [    9.012567] [TTM] Zone  kernel: Available graphics memory: 51496 KiB
> [    9.013551] [TTM] Initializing pool allocator
> [    9.032757] [drm] Found EDID data blob.
> [    9.061904] [drm] Initialized bochs-drm 1.0.0 20130925 for 0000:01:02.0 on minor 0
> [    9.336819] Unable to handle kernel paging request at virtual address 000001ff221d0000
> [    9.337177] tsk->{mm,active_mm}->context = 0000000000000000
> [    9.337283] tsk->{mm,active_mm}->pgd = fffff80000402000
> [    9.337372]               \|/ ____ \|/
> [    9.337372]               "@'/ .. \`@"
> [    9.337372]               /_| \__/ |_\
> [    9.337372]                  \__U_/
> [    9.337468] kworker/0:0(5): Oops [#1]
> [    9.339359] CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 5.8.0-rc3+ #55
> [    9.341360] Workqueue: events drm_fb_helper_dirty_work
> [    9.341775] TSTATE: 0000000080001605 TPC: 000000000077441c TNPC: 0000000000774420
> Y: 00000000    Not tainted
> [    9.341894] TPC: <memcpy+0x121c/0x13c0>
> [    9.342015] g0: 0000000000000000 g1: 0000000000000000 g2: 0000000000000000 g3:
> fffff800043d2c00
> [    9.342094] g4: fffff8000410eac0 g5: fffff800064cc000 g6: fffff80004124000 g7:
> 0000000000000010
> [    9.342173] o0: 000001ff221d0000 o1: 0000000100220000 o2: 0000000000000000 o3:
> 000001fe21fb0000
> [    9.342254] o4: 000001ff221d0000 o5: 0000000000000000 sp: fffff800041273d1 ret_pc:
> 0000000000805b18
> [    9.342325] RPC: <drm_fb_helper_dirty_work+0xf8/0x180>
> [    9.342591] l0: fffff80007819cc0 l1: fffff800043df8cc l2: 0000000001356200 l3:
> fffff800064cc000
> [    9.342670] l4: fffff80004004200 l5: 0000000000000000 l6: 0000000000000025 l7:
> fffff80004002500
> [    9.342750] i0: fffff800043df8d0 i1: fffff800040106b0 i2: 0000000000000020 i3:
> fffff800043e5500
> [    9.342829] i4: 00000000000001d1 i5: 0000000100220000 i6: fffff80004127491 i7:
> 0000000000481fec
> [    9.342960] I7: <process_one_work+0x18c/0x540>
> [    9.343308] Call Trace:
> [    9.344077] [<0000000000481fec>] process_one_work+0x18c/0x540
> [    9.344267] [<00000000004824c4>] worker_thread+0x124/0x580
> [    9.344310] [<0000000000489758>] kthread+0xf8/0x120
> [    9.344357] [<00000000004060a4>] ret_from_fork+0x1c/0x2c
> [    9.344714] [<0000000000000000>] 0x0
> 
> 
> The error "Unable to handle kernel paging request at virtual address
> 000001ff221d0000" is caused by trying to access the framebuffer using a virtual
> address, rather than using IO accessors which access the framebuffer correctly using
> SPARC ASI_PHYS (physical) loads and stores. In some ways this is similar to the bug I
> reported a couple of years back at
> https://lists.freedesktop.org/archives/dri-devel/2017-June/145793.html which was
> fixed with https://lists.freedesktop.org/archives/dri-devel/2017-July/145935.html.
> 
> According to git bisect the regression is introduced by the following commit:
> 
> $ git bisect bad
> 7a0483ac4ffca4998945c159b28afdde8353cc84 is the first bad commit
> commit 7a0483ac4ffca4998945c159b28afdde8353cc84
> Author: Gerd Hoffmann <kraxel@redhat.com>
> Date:   Fri Jan 11 06:37:50 2019 +0100
> 
>     drm/bochs: switch to generic drm fbdev emulation
> 
>     Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Link:
> http://patchwork.freedesktop.org/patch/msgid/20190111053752.4004-15-kraxel@redhat.com
> 
> :040000 040000 1917943277034f620af03ac1a2fa5db48b7b224c
> 6d7a3c316a68efbffd398d6c2b7eebefb47bc92d M      drivers
> 
> 
> The commit following this one at
> https://patchwork.freedesktop.org/patch/276488/?series=54269&rev=4 removes
> bochsfb_ops and the cfb helpers which was the original fix introduced by my second
> patch above, so I'm unsure how to approach fixing this with the switch to
> drm_fbdev_generic_setup().
> 
> Can anyone point me in the right direction?

Just following up from the original thread on debian-sparc, Sam asked about providing
some instructions to allow others to reproduce the error which are included below:


1) Building QEMU

I'm currently using QEMU git master configured just to build qemu-system-sparc64 as
follows:

./configure --target-list=sparc64-softmmu
make && make install

(Note: the latest release QEMU 5.0 has a regression in OpenBIOS which prevents
-kernel from working correctly. If you install QEMU 5.0 from a package then you can
grab the updated openbios-sparc64 directly from git at
https://git.qemu.org/?p=qemu.git;a=tree;f=pc-bios;h=a835f94751ef7d2e2648ce7c79eac1d6fea9b83c;hb=5f42c3375d45108cf14f50ac8ba57c2865e75e9c
to replace the installed one)


2) Build the kernel

This was done using Debian Buster on amd64 and its pre-packaged sparc64
cross-compilers. With those installed via "aptitude install gcc-sparc64-linux-gnu" I
did the following on a clone of Linux git master:

make ARCH=sparc64 CROSS_COMPILE=sparc64-linux-gnu- O=../rel-sparc64/ sparc64_defconfig
make ARCH=sparc64 CROSS_COMPILE=sparc64-linux-gnu- O=../rel-sparc64/ menuconfig

(Here go to Device Drivers -> Graphics support and enable both "Direct Rendering
Manager" and "DRM Support for bochs dispi vga interface (qemu stdvga)")

Then build the kernel itself:

make ARCH=sparc64 CROSS_COMPILE=sparc64-linux-gnu- O=../rel-sparc64/


3) Boot the kernel in qemu-system-sparc64

This can be done with the following command line:

qemu-system-sparc64 -kernel /path/to/rel-sparc64/vmlinuz

The problem is visible as the screen going black after the bootconsole has finished.
If you want to see the actual panic from my original email then add -nographic onto
the command line above which redirects the console onto a serial port on stdio.


ATB,

Mark.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Sam Ravnborg]

Hi Mark.

Thanks for the report and the informative pointers.

On Fri, Jul 03, 2020 at 10:57:46PM +0100, Mark Cave-Ayland wrote:
> Hi all,
> 
> I've been receiving reports that newer sparc64 kernels have started to panic on boot
> under qemu-system-sparc64 with bochs_drm enabled which I was able to confirm locally
> building git master:
> 
> 
> [    9.007161] [drm] Found bochs VGA, ID 0xb0c5.
> [    9.007840] [drm] Framebuffer size 16384 kB @ 0x1ff22000000, mmio @ 0x1ff23000000.
> [    9.012567] [TTM] Zone  kernel: Available graphics memory: 51496 KiB
> [    9.013551] [TTM] Initializing pool allocator
> [    9.032757] [drm] Found EDID data blob.
> [    9.061904] [drm] Initialized bochs-drm 1.0.0 20130925 for 0000:01:02.0 on minor 0
> [    9.336819] Unable to handle kernel paging request at virtual address 000001ff221d0000
> [    9.337177] tsk->{mm,active_mm}->context = 0000000000000000
> [    9.337283] tsk->{mm,active_mm}->pgd = fffff80000402000
> [    9.337372]               \|/ ____ \|/
> [    9.337372]               "@'/ .. \`@"
> [    9.337372]               /_| \__/ |_\
> [    9.337372]                  \__U_/
> [    9.337468] kworker/0:0(5): Oops [#1]
> [    9.339359] CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 5.8.0-rc3+ #55
> [    9.341360] Workqueue: events drm_fb_helper_dirty_work
> [    9.341775] TSTATE: 0000000080001605 TPC: 000000000077441c TNPC: 0000000000774420
> Y: 00000000    Not tainted
> [    9.341894] TPC: <memcpy+0x121c/0x13c0>
> [    9.342015] g0: 0000000000000000 g1: 0000000000000000 g2: 0000000000000000 g3:
> fffff800043d2c00
> [    9.342094] g4: fffff8000410eac0 g5: fffff800064cc000 g6: fffff80004124000 g7:
> 0000000000000010
> [    9.342173] o0: 000001ff221d0000 o1: 0000000100220000 o2: 0000000000000000 o3:
> 000001fe21fb0000
> [    9.342254] o4: 000001ff221d0000 o5: 0000000000000000 sp: fffff800041273d1 ret_pc:
> 0000000000805b18
> [    9.342325] RPC: <drm_fb_helper_dirty_work+0xf8/0x180>
> [    9.342591] l0: fffff80007819cc0 l1: fffff800043df8cc l2: 0000000001356200 l3:
> fffff800064cc000
> [    9.342670] l4: fffff80004004200 l5: 0000000000000000 l6: 0000000000000025 l7:
> fffff80004002500
> [    9.342750] i0: fffff800043df8d0 i1: fffff800040106b0 i2: 0000000000000020 i3:
> fffff800043e5500
> [    9.342829] i4: 00000000000001d1 i5: 0000000100220000 i6: fffff80004127491 i7:
> 0000000000481fec
> [    9.342960] I7: <process_one_work+0x18c/0x540>
> [    9.343308] Call Trace:
> [    9.344077] [<0000000000481fec>] process_one_work+0x18c/0x540
> [    9.344267] [<00000000004824c4>] worker_thread+0x124/0x580
> [    9.344310] [<0000000000489758>] kthread+0xf8/0x120
> [    9.344357] [<00000000004060a4>] ret_from_fork+0x1c/0x2c
> [    9.344714] [<0000000000000000>] 0x0
> 
> 
> The error "Unable to handle kernel paging request at virtual address
> 000001ff221d0000" is caused by trying to access the framebuffer using a virtual
> address, rather than using IO accessors which access the framebuffer correctly using
> SPARC ASI_PHYS (physical) loads and stores. In some ways this is similar to the bug I
> reported a couple of years back at
> https://lists.freedesktop.org/archives/dri-devel/2017-June/145793.html which was
> fixed with https://lists.freedesktop.org/archives/dri-devel/2017-July/145935.html.
> 
> According to git bisect the regression is introduced by the following commit:
> 
> $ git bisect bad
> 7a0483ac4ffca4998945c159b28afdde8353cc84 is the first bad commit
> commit 7a0483ac4ffca4998945c159b28afdde8353cc84
> Author: Gerd Hoffmann <kraxel@redhat.com>
> Date:   Fri Jan 11 06:37:50 2019 +0100
> 
>     drm/bochs: switch to generic drm fbdev emulation
> 
>     Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Link:
> http://patchwork.freedesktop.org/patch/msgid/20190111053752.4004-15-kraxel@redhat.com
> 
> :040000 040000 1917943277034f620af03ac1a2fa5db48b7b224c
> 6d7a3c316a68efbffd398d6c2b7eebefb47bc92d M      drivers
> 
> 
> The commit following this one at
> https://patchwork.freedesktop.org/patch/276488/?series=54269&rev=4 removes
> bochsfb_ops and the cfb helpers which was the original fix introduced by my second
> patch above, so I'm unsure how to approach fixing this with the switch to
> drm_fbdev_generic_setup().
> 
> Can anyone point me in the right direction?

I tried to take a look at this - came up with the following untested
hack.
The idea is that we in mode_config can specify if we need the cfb
variants. (I do not know what cfb is acronym for?)
Then when we setup the framebuffer we select the relevant fbops.

The oops refers to drm_fb_helper_dirty_work, so I think it is the memcpy
in drm_fb_helper_dirty_blit_real() that hits us.

For now I used fb_memcpy_tofb() - but that is a macro that is
expanded depending on the architecture. I think we can do btter if this
works.

	Sam


diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 853081d186d5..1609ac6efbcb 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -151,6 +151,7 @@ int bochs_kms_init(struct bochs_device *bochs)
 	bochs->dev->mode_config.preferred_depth = 24;
 	bochs->dev->mode_config.prefer_shadow = 0;
 	bochs->dev->mode_config.prefer_shadow_fbdev = 1;
+	bochs->dev->mode_config.use_cfb_for_fbdev = true;
 	bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
 
 	bochs->dev->mode_config.funcs = &bochs_mode_funcs;
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 170aa7689110..44e833b2f015 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -382,8 +382,13 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
 	size_t len = (clip->x2 - clip->x1) * cpp;
 	unsigned int y;
 
+	// TODO
 	for (y = clip->y1; y < clip->y2; y++) {
-		memcpy(dst, src, len);
+		if (fb_helper->dev->mode_config.use_cfb_for_fbdev)
+			fb_memcpy_tofb(dst, src, len);
+		else
+			memcpy(dst, src, len);
+
 		src += fb->pitches[0];
 		dst += fb->pitches[0];
 	}
@@ -2017,6 +2022,21 @@ static const struct fb_ops drm_fbdev_fb_ops = {
 	.fb_imageblit	= drm_fb_helper_sys_imageblit,
 };
 
+static const struct fb_ops drm_fbdev_cfb_fb_ops = {
+	.owner		= THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
+	.fb_open	= drm_fbdev_fb_open,
+	.fb_release	= drm_fbdev_fb_release,
+	.fb_destroy	= drm_fbdev_fb_destroy,
+	.fb_mmap	= drm_fbdev_fb_mmap,
+	.fb_read	= drm_fb_helper_sys_read,
+	.fb_write	= drm_fb_helper_sys_write,
+	.fb_fillrect	= drm_fb_helper_cfb_fillrect,
+	.fb_copyarea	= drm_fb_helper_cfb_copyarea,
+	.fb_imageblit	= drm_fb_helper_cfb_imageblit,
+};
+
+
 static struct fb_deferred_io drm_fbdev_defio = {
 	.delay		= HZ / 20,
 	.deferred_io	= drm_fb_helper_deferred_io,
@@ -2057,7 +2077,11 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
 	if (IS_ERR(fbi))
 		return PTR_ERR(fbi);
 
-	fbi->fbops = &drm_fbdev_fb_ops;
+	if (fb_helper->dev->mode_config.use_cfb_for_fbdev)
+		fbi->fbops = &drm_fbdev_cfb_fb_ops;
+	else
+		fbi->fbops = &drm_fbdev_fb_ops;
+
 	fbi->screen_size = fb->height * fb->pitches[0];
 	fbi->fix.smem_len = fbi->screen_size;
 
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 6c3ef49b46b3..dce9adf7d189 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -865,6 +865,15 @@ struct drm_mode_config {
 	 */
 	bool prefer_shadow_fbdev;
 
+	/**
+	 * @use_cfb_for_fbdev:
+	 *
+	 * Use cfb variants of drm_fb_helper_cfb_{fillrect,copyarea,imageblit}
+	 * The cfb variants are required when the CPU do not allow direct
+	 * access to the framebuffer (for example sparc64)
+	 */
+	bool use_cfb_for_fbdev;
+
 	/**
 	 * @quirk_addfb_prefer_xbgr_30bpp:
 	 *

> 
> 
> Many thanks,
> 
> Mark.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Mark Cave-Ayland]

On 04/07/2020 08:23, Sam Ravnborg wrote:

> I tried to take a look at this - came up with the following untested
> hack.
> The idea is that we in mode_config can specify if we need the cfb
> variants. (I do not know what cfb is acronym for?)
> Then when we setup the framebuffer we select the relevant fbops.
> 
> The oops refers to drm_fb_helper_dirty_work, so I think it is the memcpy
> in drm_fb_helper_dirty_blit_real() that hits us.
> 
> For now I used fb_memcpy_tofb() - but that is a macro that is
> expanded depending on the architecture. I think we can do btter if this
> works.
> 
> 	Sam
> 
> 
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
> index 853081d186d5..1609ac6efbcb 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -151,6 +151,7 @@ int bochs_kms_init(struct bochs_device *bochs)
>  	bochs->dev->mode_config.preferred_depth = 24;
>  	bochs->dev->mode_config.prefer_shadow = 0;
>  	bochs->dev->mode_config.prefer_shadow_fbdev = 1;
> +	bochs->dev->mode_config.use_cfb_for_fbdev = true;
>  	bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
>  
>  	bochs->dev->mode_config.funcs = &bochs_mode_funcs;
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 170aa7689110..44e833b2f015 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -382,8 +382,13 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
>  	size_t len = (clip->x2 - clip->x1) * cpp;
>  	unsigned int y;
>  
> +	// TODO
>  	for (y = clip->y1; y < clip->y2; y++) {
> -		memcpy(dst, src, len);
> +		if (fb_helper->dev->mode_config.use_cfb_for_fbdev)
> +			fb_memcpy_tofb(dst, src, len);
> +		else
> +			memcpy(dst, src, len);
> +
>  		src += fb->pitches[0];
>  		dst += fb->pitches[0];
>  	}
> @@ -2017,6 +2022,21 @@ static const struct fb_ops drm_fbdev_fb_ops = {
>  	.fb_imageblit	= drm_fb_helper_sys_imageblit,
>  };
>  
> +static const struct fb_ops drm_fbdev_cfb_fb_ops = {
> +	.owner		= THIS_MODULE,
> +	DRM_FB_HELPER_DEFAULT_OPS,
> +	.fb_open	= drm_fbdev_fb_open,
> +	.fb_release	= drm_fbdev_fb_release,
> +	.fb_destroy	= drm_fbdev_fb_destroy,
> +	.fb_mmap	= drm_fbdev_fb_mmap,
> +	.fb_read	= drm_fb_helper_sys_read,
> +	.fb_write	= drm_fb_helper_sys_write,
> +	.fb_fillrect	= drm_fb_helper_cfb_fillrect,
> +	.fb_copyarea	= drm_fb_helper_cfb_copyarea,
> +	.fb_imageblit	= drm_fb_helper_cfb_imageblit,
> +};
> +
> +
>  static struct fb_deferred_io drm_fbdev_defio = {
>  	.delay		= HZ / 20,
>  	.deferred_io	= drm_fb_helper_deferred_io,
> @@ -2057,7 +2077,11 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
>  	if (IS_ERR(fbi))
>  		return PTR_ERR(fbi);
>  
> -	fbi->fbops = &drm_fbdev_fb_ops;
> +	if (fb_helper->dev->mode_config.use_cfb_for_fbdev)
> +		fbi->fbops = &drm_fbdev_cfb_fb_ops;
> +	else
> +		fbi->fbops = &drm_fbdev_fb_ops;
> +
>  	fbi->screen_size = fb->height * fb->pitches[0];
>  	fbi->fix.smem_len = fbi->screen_size;
>  
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 6c3ef49b46b3..dce9adf7d189 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -865,6 +865,15 @@ struct drm_mode_config {
>  	 */
>  	bool prefer_shadow_fbdev;
>  
> +	/**
> +	 * @use_cfb_for_fbdev:
> +	 *
> +	 * Use cfb variants of drm_fb_helper_cfb_{fillrect,copyarea,imageblit}
> +	 * The cfb variants are required when the CPU do not allow direct
> +	 * access to the framebuffer (for example sparc64)
> +	 */
> +	bool use_cfb_for_fbdev;
> +
>  	/**
>  	 * @quirk_addfb_prefer_xbgr_30bpp:
>  	 *

Thanks for the quick response, Sam! I tried the above diff and it stills seems to
provoke a memory access error (although now the kernel hangs rather than panicking
and continuing to boot).

Digging a bit more I can see the fault occurring in cfb_imageblit():


IN: cfb_imageblit
0x00000000007ad8c0:  ldsb  [ %i5 ], %g1
0x00000000007ad8c4:  add  %g3, 4, %i3
0x00000000007ad8c8:  sra  %g1, %g2, %g1
0x00000000007ad8cc:  and  %g1, %o5, %g1
0x00000000007ad8d0:  srl  %g1, 0, %g1
0x00000000007ad8d4:  sllx  %g1, 2, %g1
0x00000000007ad8d8:  ld  [ %o4 + %g1 ], %g1
0x00000000007ad8dc:  and  %i2, %g1, %g1
0x00000000007ad8e0:  xor  %g1, %i1, %g1
0x00000000007ad8e4:  sta  %g1, [ %g3 ] #ASI_PHYS_BYPASS_EC_WITH_EBIT
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0x00000000007ad8e8:  cmp  %g2, 0
0x00000000007ad8ec:  bne  %icc, 0x7ad8b4
0x00000000007ad8f0:  mov  %i3, %g3


According to gdbstub the destination address in $g3 looks like this:

Breakpoint 1, 0x00000000007ad8e4 in cfb_imageblit ()
(gdb) i r $g3
g3             0x100220000      4297195520


The 0x100220000 address still isn't right. On sun4u the PCI address space is mapped
at physical address 0x1fe00000000 and adding these two together gives 0x1ff00220000
which seems closer, but still not the correct framebuffer address 0x1ff22000000 which
is reported at boot:

[    9.007161] [drm] Found bochs VGA, ID 0xb0c5.
[    9.007840] [drm] Framebuffer size 16384 kB @ 0x1ff22000000, mmio @ 0x1ff23000000.



ATB,

Mark.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Mark Cave-Ayland]

On 04/07/2020 12:11, Mark Cave-Ayland wrote:

> According to gdbstub the destination address in $g3 looks like this:
> 
> Breakpoint 1, 0x00000000007ad8e4 in cfb_imageblit ()
> (gdb) i r $g3
> g3             0x100220000      4297195520
> 
> 
> The 0x100220000 address still isn't right. On sun4u the PCI address space is mapped
> at physical address 0x1fe00000000 and adding these two together gives 0x1ff00220000
> which seems closer, but still not the correct framebuffer address 0x1ff22000000 which
> is reported at boot:
> 
> [    9.007161] [drm] Found bochs VGA, ID 0xb0c5.
> [    9.007840] [drm] Framebuffer size 16384 kB @ 0x1ff22000000, mmio @ 0x1ff23000000.

As a comparison, I took the last known good commit 7a0483ac4ffc~1: "drm/bochs: add
basic prime support" and added some debug in cfb_imageblit() to allow me to pick out
p->screen_base:

(gdb) i r $o1
o1             0x1ff22000000    2195298713600

When running git master with your patch in the same way I get a completely different
value:

(gdb) i r $o1
o1             0x100050000      4295294976

Does p->screen_base need to be initialised differently when using the cfb helpers?


ATB,

Mark.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Sam Ravnborg]

Hi Mark.

On Sat, Jul 04, 2020 at 02:09:38PM +0100, Mark Cave-Ayland wrote:
> On 04/07/2020 12:11, Mark Cave-Ayland wrote:
> 
> > According to gdbstub the destination address in $g3 looks like this:
> > 
> > Breakpoint 1, 0x00000000007ad8e4 in cfb_imageblit ()
> > (gdb) i r $g3
> > g3             0x100220000      4297195520
> > 
> > 
> > The 0x100220000 address still isn't right. On sun4u the PCI address space is mapped
> > at physical address 0x1fe00000000 and adding these two together gives 0x1ff00220000
> > which seems closer, but still not the correct framebuffer address 0x1ff22000000 which
> > is reported at boot:
> > 
> > [    9.007161] [drm] Found bochs VGA, ID 0xb0c5.
> > [    9.007840] [drm] Framebuffer size 16384 kB @ 0x1ff22000000, mmio @ 0x1ff23000000.
> 
> As a comparison, I took the last known good commit 7a0483ac4ffc~1: "drm/bochs: add
> basic prime support" and added some debug in cfb_imageblit() to allow me to pick out
> p->screen_base:
> 
> (gdb) i r $o1
> o1             0x1ff22000000    2195298713600
> 
> When running git master with your patch in the same way I get a completely different
> value:
> 
> (gdb) i r $o1
> o1             0x100050000      4295294976
> 
> Does p->screen_base need to be initialised differently when using the cfb helpers?

I think what is happening is that the bochs driver request a shadow copy
for the frambuffer. And with the change to fbops we now use the cfb_
functions to write to the shadow framebuffer - which is not in any IO
space. So this does not work.

So maybe all we need is the fix in drm_fb_helper_dirty_blit_real().
If you try to undo the change so fbops is set to &drm_fbdev_fb_ops,
but keep the fix in drm_fb_helper_dirty_blit_real() how does it then
behave?

I did not find time to follow your instructions to test this myself with
qemu - sorry.

	Sam


> 
> 
> ATB,
> 
> Mark.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Mark Cave-Ayland]

On 04/07/2020 14:41, Sam Ravnborg wrote:

> I think what is happening is that the bochs driver request a shadow copy
> for the frambuffer. And with the change to fbops we now use the cfb_
> functions to write to the shadow framebuffer - which is not in any IO
> space. So this does not work.
> 
> So maybe all we need is the fix in drm_fb_helper_dirty_blit_real().
> If you try to undo the change so fbops is set to &drm_fbdev_fb_ops,
> but keep the fix in drm_fb_helper_dirty_blit_real() how does it then
> behave?

Bingo! I just tried that and the framebuffer is now working under qemu-system-sparc64
again - thank you so much for the help! From what you said I guess
drm_fb_helper_dirty_blit_real() is responsible syncing the shadow copy?

Below is the current working diff based upon your previous one: it certainly feels
like the difference in memcpy() behaviour should be hidden away in fb_memcpy_tofb()
or similar.


diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 05d8373888e8..8dcc09f1ba1d 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -146,6 +146,7 @@ int bochs_kms_init(struct bochs_device *bochs)
        bochs->dev->mode_config.preferred_depth = 24;
        bochs->dev->mode_config.prefer_shadow = 0;
        bochs->dev->mode_config.prefer_shadow_fbdev = 1;
+       bochs->dev->mode_config.use_cfb_for_fbdev = true;
        bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;

        bochs->dev->mode_config.funcs = &bochs_mode_funcs;
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 5609e164805f..18d89388b125 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -398,8 +398,13 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper
*fb_helper,
        size_t len = (clip->x2 - clip->x1) * cpp;
        unsigned int y;

+       // TODO
        for (y = clip->y1; y < clip->y2; y++) {
-               memcpy(dst, src, len);
+               if (fb_helper->dev->mode_config.use_cfb_for_fbdev)
+                       fb_memcpy_tofb(dst, src, len);
+               else
+                       memcpy(dst, src, len);
+
                src += fb->pitches[0];
                dst += fb->pitches[0];
        }
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 6c3ef49b46b3..dce9adf7d189 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -865,6 +865,15 @@ struct drm_mode_config {
         */
        bool prefer_shadow_fbdev;

+       /**
+        * @use_cfb_for_fbdev:
+        *
+        * Use cfb variants of drm_fb_helper_cfb_{fillrect,copyarea,imageblit}
+        * The cfb variants are required when the CPU do not allow direct
+        * access to the framebuffer (for example sparc64)
+        */
+       bool use_cfb_for_fbdev;
+
        /**
         * @quirk_addfb_prefer_xbgr_30bpp:
         *

> I did not find time to follow your instructions to test this myself with
> qemu - sorry.

No worries, I do appreciate that as a maintainer it can be hard to fit these things
around life, family, job etc.


ATB,

Mark.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Sam Ravnborg]

Hi Mark.

On Sat, Jul 04, 2020 at 03:16:47PM +0100, Mark Cave-Ayland wrote:
> On 04/07/2020 14:41, Sam Ravnborg wrote:
> 
> > I think what is happening is that the bochs driver request a shadow copy
> > for the frambuffer. And with the change to fbops we now use the cfb_
> > functions to write to the shadow framebuffer - which is not in any IO
> > space. So this does not work.
> > 
> > So maybe all we need is the fix in drm_fb_helper_dirty_blit_real().
> > If you try to undo the change so fbops is set to &drm_fbdev_fb_ops,
> > but keep the fix in drm_fb_helper_dirty_blit_real() how does it then
> > behave?
> 
> Bingo! I just tried that and the framebuffer is now working under qemu-system-sparc64
> again - thank you so much for the help! From what you said I guess
> drm_fb_helper_dirty_blit_real() is responsible syncing the shadow copy?
> 
> Below is the current working diff based upon your previous one: it certainly feels
> like the difference in memcpy() behaviour should be hidden away in fb_memcpy_tofb()
> or similar.

From your feedback so far I thnk the minimal fix would be like this:

> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> .. static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
>         size_t len = (clip->x2 - clip->x1) * cpp;
>         unsigned int y;
> 
>         for (y = clip->y1; y < clip->y2; y++) {
> -               memcpy(dst, src, len);
> +               fb_memcpy_tofb(dst, src, len);
>                 src += fb->pitches[0];
>                 dst += fb->pitches[0];
>         }

(Hand edited, patch s not a valid syntax)

But I need feedback from someone that know all this a bit better
to judge if this is an OK change.
For once - this will only work with shadow buffers.

	Sam



> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 6c3ef49b46b3..dce9adf7d189 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -865,6 +865,15 @@ struct drm_mode_config {
>          */
>         bool prefer_shadow_fbdev;
> 
> +       /**
> +        * @use_cfb_for_fbdev:
> +        *
> +        * Use cfb variants of drm_fb_helper_cfb_{fillrect,copyarea,imageblit}
> +        * The cfb variants are required when the CPU do not allow direct
> +        * access to the framebuffer (for example sparc64)
> +        */
> +       bool use_cfb_for_fbdev;
> +
>         /**
>          * @quirk_addfb_prefer_xbgr_30bpp:
>          *
> 
> > I did not find time to follow your instructions to test this myself with
> > qemu - sorry.
> 
> No worries, I do appreciate that as a maintainer it can be hard to fit these things
> around life, family, job etc.
> 
> 
> ATB,
> 
> Mark.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Mark Cave-Ayland]

On 04/07/2020 15:52, Sam Ravnborg wrote:

> Hi Mark.
> 
> On Sat, Jul 04, 2020 at 03:16:47PM +0100, Mark Cave-Ayland wrote:
>> On 04/07/2020 14:41, Sam Ravnborg wrote:
>>
>>> I think what is happening is that the bochs driver request a shadow copy
>>> for the frambuffer. And with the change to fbops we now use the cfb_
>>> functions to write to the shadow framebuffer - which is not in any IO
>>> space. So this does not work.
>>>
>>> So maybe all we need is the fix in drm_fb_helper_dirty_blit_real().
>>> If you try to undo the change so fbops is set to &drm_fbdev_fb_ops,
>>> but keep the fix in drm_fb_helper_dirty_blit_real() how does it then
>>> behave?
>>
>> Bingo! I just tried that and the framebuffer is now working under qemu-system-sparc64
>> again - thank you so much for the help! From what you said I guess
>> drm_fb_helper_dirty_blit_real() is responsible syncing the shadow copy?
>>
>> Below is the current working diff based upon your previous one: it certainly feels
>> like the difference in memcpy() behaviour should be hidden away in fb_memcpy_tofb()
>> or similar.
> 
>>From your feedback so far I thnk the minimal fix would be like this:
> 
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> .. static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
>>         size_t len = (clip->x2 - clip->x1) * cpp;
>>         unsigned int y;
>>
>>         for (y = clip->y1; y < clip->y2; y++) {
>> -               memcpy(dst, src, len);
>> +               fb_memcpy_tofb(dst, src, len);
>>                 src += fb->pitches[0];
>>                 dst += fb->pitches[0];
>>         }
> 
> (Hand edited, patch s not a valid syntax)
> 
> But I need feedback from someone that know all this a bit better
> to judge if this is an OK change.
> For once - this will only work with shadow buffers.

Hi Sam,

Yes, that's correct - I can confirm that the simplified diff below works:

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 5609e164805f..83af05fac604 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -399,7 +399,7 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper
*fb_helper,
        unsigned int y;

        for (y = clip->y1; y < clip->y2; y++) {
-               memcpy(dst, src, len);
+               fb_memcpy_tofb(dst, src, len);
                src += fb->pitches[0];
                dst += fb->pitches[0];
        }

I guess that the next step is to wait for advice from Gerd as to whether this is
sufficient, or if other changes are required to allow non-shadow buffers to work on
architectures similar to SPARC64 too.


ATB,

Mark.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Gerd Hoffmann]

> Yes, that's correct - I can confirm that the simplified diff below works:
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 5609e164805f..83af05fac604 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -399,7 +399,7 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper
> *fb_helper,
>         unsigned int y;
> 
>         for (y = clip->y1; y < clip->y2; y++) {
> -               memcpy(dst, src, len);
> +               fb_memcpy_tofb(dst, src, len);

fb_memcpy_tofb is #defined to sbus_memcpy_toio @ sparc which looks
wrong to me given that this is a pci not a sbus device.  sparc also has
memcpy_toio which looks better to me.

There are blit helpers in drm_format_helper.c which already use
memcpy_toio(), I guess we should do the same here.  Not fully sure we
can use memcpy_toio() unconditionally here.  Given that a shadow
framebuffer makes sense only in case the real framebuffer is not in
normal ram we probably can.

take care,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Mark Cave-Ayland]

On 07/07/2020 08:03, Gerd Hoffmann wrote:

>> Yes, that's correct - I can confirm that the simplified diff below works:
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 5609e164805f..83af05fac604 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -399,7 +399,7 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper
>> *fb_helper,
>>         unsigned int y;
>>
>>         for (y = clip->y1; y < clip->y2; y++) {
>> -               memcpy(dst, src, len);
>> +               fb_memcpy_tofb(dst, src, len);
> 
> fb_memcpy_tofb is #defined to sbus_memcpy_toio @ sparc which looks
> wrong to me given that this is a pci not a sbus device.  sparc also has
> memcpy_toio which looks better to me.
> 
> There are blit helpers in drm_format_helper.c which already use
> memcpy_toio(), I guess we should do the same here.  Not fully sure we
> can use memcpy_toio() unconditionally here.  Given that a shadow
> framebuffer makes sense only in case the real framebuffer is not in
> normal ram we probably can.

Thanks Gerd - I've just tested the diff below with memcpy_toio() and that works too:

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 5609e164805f..4d05b0ab1592 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -399,7 +399,7 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper
*fb_helper,
        unsigned int y;

        for (y = clip->y1; y < clip->y2; y++) {
-               memcpy(dst, src, len);
+               memcpy_toio(dst, src, len);
                src += fb->pitches[0];
                dst += fb->pitches[0];
        }

Presumably there is some existing mechanism that ensures SPARC will always choose a
shadow framebuffer?


ATB,

Mark.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Gerd Hoffmann]

> Thanks Gerd - I've just tested the diff below with memcpy_toio() and that works too:
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 5609e164805f..4d05b0ab1592 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -399,7 +399,7 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper
> *fb_helper,
>         unsigned int y;
> 
>         for (y = clip->y1; y < clip->y2; y++) {
> -               memcpy(dst, src, len);
> +               memcpy_toio(dst, src, len);
>                 src += fb->pitches[0];
>                 dst += fb->pitches[0];
>         }
> 
> Presumably there is some existing mechanism that ensures SPARC will always choose a
> shadow framebuffer?

bochs-drm always runs with a shadow framebuffer (that allows to swap
the real framebuffer into and out of vram as needed).  With other
drivers this is in the hands of the driver.  It might not be needed,
virtio-gpu for example uses normal ram as framebuffer storage.

take care,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Sam Ravnborg]

Hi Gerd.

On Tue, Jul 07, 2020 at 09:03:41AM +0200, Gerd Hoffmann wrote:
> > Yes, that's correct - I can confirm that the simplified diff below works:
> > 
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index 5609e164805f..83af05fac604 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -399,7 +399,7 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper
> > *fb_helper,
> >         unsigned int y;
> > 
> >         for (y = clip->y1; y < clip->y2; y++) {
> > -               memcpy(dst, src, len);
> > +               fb_memcpy_tofb(dst, src, len);
> 
> fb_memcpy_tofb is #defined to sbus_memcpy_toio @ sparc which looks
> wrong to me given that this is a pci not a sbus device.  sparc also has
> memcpy_toio which looks better to me.
Looked at sbus_memcpy_toio and memcpy_toio for sparc64.
They are essential the same. Only read bytes in little-endian format,
the other read bytes in big-endian format. So thats the same.

I will prepare a proper patch with focus on fixin sparc64 only.

> 
> There are blit helpers in drm_format_helper.c which already use
> memcpy_toio(), I guess we should do the same here.  Not fully sure we
> can use memcpy_toio() unconditionally here.  Given that a shadow
> framebuffer makes sense only in case the real framebuffer is not in
> normal ram we probably can.
Not so sure about this part.
We unconditionally enable the use of a shadow framebuffer.
But on some archs the framebuffer is not in io space - but then
on these architectures memcpy_toio boils down to a simple memcpy.
So maybe in the end everything is fine.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Panic booting qemu-system-sparc64 with bochs_drm

[Mark Cave-Ayland]

On 07/07/2020 20:52, Sam Ravnborg wrote:

> Hi Gerd.
> 
> On Tue, Jul 07, 2020 at 09:03:41AM +0200, Gerd Hoffmann wrote:
>>> Yes, that's correct - I can confirm that the simplified diff below works:
>>>
>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>>> index 5609e164805f..83af05fac604 100644
>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>> @@ -399,7 +399,7 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper
>>> *fb_helper,
>>>         unsigned int y;
>>>
>>>         for (y = clip->y1; y < clip->y2; y++) {
>>> -               memcpy(dst, src, len);
>>> +               fb_memcpy_tofb(dst, src, len);
>>
>> fb_memcpy_tofb is #defined to sbus_memcpy_toio @ sparc which looks
>> wrong to me given that this is a pci not a sbus device.  sparc also has
>> memcpy_toio which looks better to me.
> Looked at sbus_memcpy_toio and memcpy_toio for sparc64.
> They are essential the same. Only read bytes in little-endian format,
> the other read bytes in big-endian format. So thats the same.
> 
> I will prepare a proper patch with focus on fixin sparc64 only.

Thanks Sam! If you want to add me as a CC then I'm happy to test if needed.

>> There are blit helpers in drm_format_helper.c which already use
>> memcpy_toio(), I guess we should do the same here.  Not fully sure we
>> can use memcpy_toio() unconditionally here.  Given that a shadow
>> framebuffer makes sense only in case the real framebuffer is not in
>> normal ram we probably can.
> Not so sure about this part.
> We unconditionally enable the use of a shadow framebuffer.
> But on some archs the framebuffer is not in io space - but then
> on these architectures memcpy_toio boils down to a simple memcpy.
> So maybe in the end everything is fine.

I'm afraid this part is beyond my current knowledge of the various framebuffer
implementations within the kernel :/


ATB,

Mark.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel