Hi Daniel On Mon, Jul 27, 2020 at 11:20:13AM +0200, Daniel Vetter wrote: > On Sat, Jul 25, 2020 at 9:10 PM Sam Ravnborg <sam@xxxxxxxxxxxx> wrote: > > > > From 1323a7433691aee112a9b2df8041b5024895a77e Mon Sep 17 00:00:00 2001 > > From: Sam Ravnborg <sam@xxxxxxxxxxxx> > > Date: Thu, 9 Jul 2020 21:30:16 +0200 > > Subject: [PATCH v2 1/1] drm/drm_fb_helper: fix fbdev with sparc64 > > > > Recent kernels have been reported to panic using the bochs_drm framebuffer under > > qemu-system-sparc64 which was bisected to commit 7a0483ac4ffc "drm/bochs: switch to > > generic drm fbdev emulation". The backtrace indicates that the shadow framebuffer > > copy in drm_fb_helper_dirty_blit_real() is trying to access the real framebuffer > > using a virtual address rather than use an IO access typically implemented using a > > physical (ASI_PHYS) access on SPARC. > > > > 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 side-effect that is unknown 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. > > The rules are that officially we have to use the io functions for > __mmio pointers. We just drop these sparse annotations on the floor. > I'd replace this with something like: > > "Note that this only fixes bochs, in general fbdev helpers still have > issues with mixing up system memory and mmio space. Fixing that will > require a lot more work." OK, done. > > > v2: > > - Added missing __iomem cast (kernel test robot) > > - Made changelog readable and fix typos (Mark) > > - Add flag to select iomem - and set it in the bochs driver > > > > Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx> > > Reported-by: Mark Cave-Ayland <mark.cave-ayland@xxxxxxxxxxxx> > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > > 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 > > Link: https://patchwork.freedesktop.org/patch/msgid/20200709193016.291267-1-sam@xxxxxxxxxxxx > > --- > > > > This fix introducing a flag in mode_config is at best a band-aid > > solution until we have a proper fix. > > We need to propagate the info about iomem so it is not a driver flag > > thing. > > > > There is also the issue with sys* versus cfb* functions, where cfb* > > functions are used for iomem. > > I did not manage to make the bochs driver work with the cfb* functions, > > for some unknown reason booting would be stuck waiting for the console > > mutex when usign the cfb* functions. > > > > I consider this fix OK to get the kernel working for sparc64 with the > > bochs driver for now. And with the fbdev_uses_iomem flag no other > > drivers will see any changes. > > > > Sam > > > > drivers/gpu/drm/bochs/bochs_kms.c | 1 + > > drivers/gpu/drm/drm_fb_helper.c | 6 +++++- > > include/drm/drm_mode_config.h | 9 +++++++++ > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c > > index 05d8373888e8..079f46f5cdb6 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.fbdev_use_iomem = 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..89cfd68ef400 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -399,7 +399,11 @@ 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); > > + if (!fb_helper->dev->mode_config.fbdev_use_iomem) > > + memcpy(dst, src, len); > > + else > > + memcpy_toio((void __iomem *)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..c24c066bdd9c 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; > > > > + /** > > + * @fbdev_use_iomem: > > + * > > + * Set to true if framebuffer reside in iomem. > > + * When set to true memcpy_toio() is used when copying the framebuffer in > > + * drm_fb_helper.drm_fb_helper_dirty_blit_real() > > I'd add a "FIXME: This should be replaced with a per-mapping is_iomem > flag (like ttm does), and then used everywhere in fbdev code." > OK. > With those two nits: Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> Thanks. Applied to drm-misc-fixes and will push out when build testing finish. Sam > > > + */ > > + bool fbdev_use_iomem; > > + > > /** > > * @quirk_addfb_prefer_xbgr_30bpp: > > * > > -- > > 2.25.1 > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
