Hi Am 13.07.20 um 18:21 schrieb Daniel Vetter: > On Fri, Jul 10, 2020 at 08:28:16AM +0200, Thomas Zimmermann wrote: >> Hi >> >> Am 09.07.20 um 21:30 schrieb Sam Ravnborg: >>> Mark reported that sparc64 would panic while booting using qemu. >>> Mark bisected this to a patch that introduced generic fbdev emulation to >>> the bochs DRM driver. >>> Mark pointed out that a similar bug was fixed before where >>> the sys helpers was replaced by cfb helpers. >>> >>> The culprint here is that the framebuffer reside in IO memory which >>> requires SPARC ASI_PHYS (physical) loads and stores. >>> >>> The current bohcs DRM driver uses a shadow buffer. >>> So all copying to the framebuffer happens in >>> drm_fb_helper_dirty_blit_real(). >>> >>> The fix is to replace the memcpy with memcpy_toio() from io.h. >>> >>> memcpy_toio() uses writeb() where the original fbdev code >>> used sbus_memcpy_toio(). The latter uses sbus_writeb(). >>> >>> The difference between writeb() and sbus_memcpy_toio() is >>> that writeb() writes bytes in little-endian, where sbus_writeb() writes >>> bytes in big-endian. As endian does not matter for byte writes they are >>> the same. So we can safely use memcpy_toio() here. >>> >>> For many architectures memcpy_toio() is a simple memcpy(). >>> One sideeffect that is unknow is if this has any impact on other >>> architectures. >>> So far the analysis tells that this change is OK for other arch's. >>> but testing would be good. >>> >>> Signed-off-by: Sam Ravnborg <[email protected]> >>> Reported-by: Mark Cave-Ayland <[email protected]> >>> Tested-by: Mark Cave-Ayland <[email protected]> >>> Cc: Mark Cave-Ayland <[email protected]> >>> Cc: Thomas Zimmermann <[email protected]> >>> Cc: Gerd Hoffmann <[email protected]> >>> Cc: "David S. Miller" <[email protected]> >>> Cc: [email protected] >> >> So this actually is a problem in practice. Do you know how userspace >> handles this? >> >> For this patch >> >> Acked-by: Thomas Zimmermann <[email protected]> >> >> but I'd like to have someone with more architecture expertise ack this >> as well. >> >> Best regards >> Thomas >> >>> --- >>> drivers/gpu/drm/drm_fb_helper.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> 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); > > I don't think we can do this unconditionally, there's fbdev-helper drivers > using shmem helpers, and for shmem memcpy_toio is wrong. We need a switch > to fix this properly I think.
I once has a patch set for this problem, but it didn't make it. [1]
Buffers can move between I/O and system memory, so a simple flag would
not work. I'd propose this
bool drm_gem_is_iomem(struct drm_gem_object *obj)
{
if (obj->funcs && obj->funcs->is_iomem)
return obj->funcs->is_iomem(obj);
return false;
}
Most GEM implmentations wouldn't bother, but VRAM helpers could set the
is_iomem function and return the current state. Fbdev helpers can then
pick the correct memcpy_*() function.
Best regards
Thomas
[1]
https://lore.kernel.org/dri-devel/[email protected]/
>
> What Dave Airlie mentioned is just about memcpy_toio vs the sparc bus
> version, for which we don't have any drivers really. But I do think we
> need to differentiate between memcpy and memcpy_tio. That's what this
> entire annoying _cfb_ vs _sys_ business is all about, and also what gem
> vram helpers have to deal with.
> -Daniel
>
>>> src += fb->pitches[0];
>>> dst += fb->pitches[0];
>>> }
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
>
>
>
>
>> _______________________________________________
>> dri-devel mailing list
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
