Hi
Am 24.07.20 um 06:53 schrieb Dave Airlie:
On Tue, 14 Jul 2020 at 18:56, Thomas Zimmermann tzimmermann@suse.de wrote:
Hi
Am 14.07.20 um 10:41 schrieb Daniel Vetter:
On Tue, Jul 14, 2020 at 08:41:58AM +0200, Thomas Zimmermann wrote:
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@ravnborg.org > Reported-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk > Tested-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk > Cc: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk > Cc: Thomas Zimmermann tzimmermann@suse.de > Cc: Gerd Hoffmann kraxel@redhat.com > Cc: "David S. Miller" davem@davemloft.net > Cc: sparclinux@vger.kernel.org
So this actually is a problem in practice. Do you know how userspace handles this?
For this patch
Acked-by: Thomas Zimmermann tzimmermann@suse.de
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.
Hm wasn't the (long term at least) idea to add the is_iomem flag to the vmap functions? is_iomem is kinda only well-defined if there's a vmap of the buffer around (which also pins it), or in general when the buffer is pinned. Outside of that an ->is_iomem function doesn't make much sense.
Oh. From how I understood the original discussion, you shoot down the idea because sparse would not support it well?
The other idea was to add an additional vmap_iomem() helper that returns an__iomem pointer. Can we try that?
Did we get anywhere with this yet?
Not yet. But I intend to work on it ASAP.
Best regards Thomas
Dave. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel