[PATCH] drm/bochs: switch fb_ops over to use drm_fb_helper_cfb helpers

[PATCH] drm/bochs: switch fb_ops over to use drm_fb_helper_cfb helpers

[Mark Cave-Ayland]

The current drm_fb_helper_sys helpers referenced in fb_ops assume that the
video memory is in system RAM. This is not the case for sparc which uses direct
physical memory accesses for IO memory and causes the bochs_drm module to panic
immediately upon startup as it tries to initialise the framebuffer.

Switching fb_ops over to use the drm_fb_helper_cfb helpers ensures that the
correct accesses are used on sparc, fixing the panic and allowing the
bochs_drm module to function under qemu-system-sparc64.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 drivers/gpu/drm/bochs/bochs_fbdev.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c b/drivers/gpu/drm/bochs/bochs_fbdev.c
index 932a769..d3a75c2 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -23,9 +23,9 @@ static int bochsfb_mmap(struct fb_info *info,
 static struct fb_ops bochsfb_ops = {
 	.owner = THIS_MODULE,
 	DRM_FB_HELPER_DEFAULT_OPS,
-	.fb_fillrect = drm_fb_helper_sys_fillrect,
-	.fb_copyarea = drm_fb_helper_sys_copyarea,
-	.fb_imageblit = drm_fb_helper_sys_imageblit,
+	.fb_fillrect = drm_fb_helper_cfb_fillrect,
+	.fb_copyarea = drm_fb_helper_cfb_copyarea,
+	.fb_imageblit = drm_fb_helper_cfb_imageblit,
 	.fb_mmap = bochsfb_mmap,
 };
 
-- 
1.7.10.4

Re: [PATCH] drm/bochs: switch fb_ops over to use drm_fb_helper_cfb helpers

[Daniel Vetter]

On Sun, Jul 02, 2017 at 10:52:43PM +0100, Mark Cave-Ayland wrote:
> The current drm_fb_helper_sys helpers referenced in fb_ops assume that the
> video memory is in system RAM. This is not the case for sparc which uses direct
> physical memory accesses for IO memory and causes the bochs_drm module to panic
> immediately upon startup as it tries to initialise the framebuffer.
> 
> Switching fb_ops over to use the drm_fb_helper_cfb helpers ensures that the
> correct accesses are used on sparc, fixing the panic and allowing the
> bochs_drm module to function under qemu-system-sparc64.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

So is this generally true for bochs, or is this now going to broken
platforms where the bochs framebuffer is in system memory?

I'm not entirely clear on that from your commit message ...

Thanks, Daniel

> ---
>  drivers/gpu/drm/bochs/bochs_fbdev.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c b/drivers/gpu/drm/bochs/bochs_fbdev.c
> index 932a769..d3a75c2 100644
> --- a/drivers/gpu/drm/bochs/bochs_fbdev.c
> +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
> @@ -23,9 +23,9 @@ static int bochsfb_mmap(struct fb_info *info,
>  static struct fb_ops bochsfb_ops = {
>  	.owner = THIS_MODULE,
>  	DRM_FB_HELPER_DEFAULT_OPS,
> -	.fb_fillrect = drm_fb_helper_sys_fillrect,
> -	.fb_copyarea = drm_fb_helper_sys_copyarea,
> -	.fb_imageblit = drm_fb_helper_sys_imageblit,
> +	.fb_fillrect = drm_fb_helper_cfb_fillrect,
> +	.fb_copyarea = drm_fb_helper_cfb_copyarea,
> +	.fb_imageblit = drm_fb_helper_cfb_imageblit,
>  	.fb_mmap = bochsfb_mmap,
>  };
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/bochs: switch fb_ops over to use drm_fb_helper_cfb helpers

[Mark Cave-Ayland]

On 03/07/17 09:11, Daniel Vetter wrote:

> On Sun, Jul 02, 2017 at 10:52:43PM +0100, Mark Cave-Ayland wrote:
>> The current drm_fb_helper_sys helpers referenced in fb_ops assume that the
>> video memory is in system RAM. This is not the case for sparc which uses direct
>> physical memory accesses for IO memory and causes the bochs_drm module to panic
>> immediately upon startup as it tries to initialise the framebuffer.
>>
>> Switching fb_ops over to use the drm_fb_helper_cfb helpers ensures that the
>> correct accesses are used on sparc, fixing the panic and allowing the
>> bochs_drm module to function under qemu-system-sparc64.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> So is this generally true for bochs, or is this now going to broken
> platforms where the bochs framebuffer is in system memory?
> 
> I'm not entirely clear on that from your commit message ...

Here's my original analysis on the problem:
http://lkml.iu.edu/hypermail/linux/kernel/1706.3/04745.html where you
can see the issue is that on sparc you can't just assume that address
returned from ioremap() is a virtual address.

Most of the context for this change comes from commit 68648ed which has
this initial paragraph:

<quote>
fbdev: add drawing functions for framebuffers in system RAM

The generic drawing functions (cfbimgblt, cfbcopyarea, cfbfillrect)
assume that the framebuffer is in IO memory.  However, we have 3 drivers
(hecubafb, arcfb, and vfb) where the framebuffer is allocated from
system RAM (via vmalloc). Using _raw_read/write and family for these
drivers (as used in the cfb* functions) is illegal, especially in other
platforms.
</quote>

Given that bochs_drm is a PCI device which supports both memory and IO
accesses then I would think that the above comment about using
_raw_read/write isn't relevant here and this is the correct fix (for
reference I do see that nouveau/radeon/i915 which are also PCI-based are
already using the cfb helper functions).

Hopefully Gerd has experience using bochs_drm with other non-x86 systems
and can comment further.


ATB,

Mark.

Re: [PATCH] drm/bochs: switch fb_ops over to use drm_fb_helper_cfb helpers

[Gerd Hoffmann]

  Hi,

> Hopefully Gerd has experience using bochs_drm with other non-x86
> systems
> and can comment further.

Just pushed to drm-misc-next, fix is correct.  bochs video memory is a
pci bar, and the framebuffer is stored there.

Using sys instead of cfb is probably a leftover from cirrus, where the
framebuffer actually in a shadow copy in system ram, due to the low
amout of (pci) video memory.

cheers,
  Gerd

Re: [PATCH] drm/bochs: switch fb_ops over to use drm_fb_helper_cfb helpers

[Mark Cave-Ayland]

On 03/07/17 09:55, Gerd Hoffmann wrote:

>   Hi,
> 
>> Hopefully Gerd has experience using bochs_drm with other non-x86
>> systems
>> and can comment further.
> 
> Just pushed to drm-misc-next, fix is correct.  bochs video memory is a
> pci bar, and the framebuffer is stored there.
> 
> Using sys instead of cfb is probably a leftover from cirrus, where the
> framebuffer actually in a shadow copy in system ram, due to the low
> amout of (pci) video memory.

Great, thanks a lot! Whilst investigating this I noticed that there are
quite a few reports of having to blacklist bochs_drm (particularly on
Xen HVM) so it would be interesting to know if this fix improves things
there too.


ATB,

Mark.