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 <sam@xxxxxxxxxxxx>
>>> Reported-by: Mark Cave-Ayland <mark.cave-ayland@xxxxxxxxxxxx>
>>> Tested-by: Mark Cave-Ayland <mark.cave-ayland@xxxxxxxxxxxx>
>>> Cc: Mark Cave-Ayland <mark.cave-ayland@xxxxxxxxxxxx>
>>> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
>>> Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
>>> Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
>>> Cc: sparclinux@xxxxxxxxxxxxxxx
>>
>> So this actually is a problem in practice. Do you know how userspace
>> handles this?
>>
>> For this patch
>>
>> Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>>
>> 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/20191106093121.21762-1-tzimmermann@xxxxxxx/
>
> 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
>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
