On Wed, Sep 12, 2018 at 01:06:07PM +0200, Noralf Trønnes wrote: > > Den 12.09.2018 12.57, skrev Noralf Trønnes: > > (Cc: Daniel Vetter) > > > > Somehow that CC was dropped somewhere after leaving email client. > Trying once more. Yeah I just made myself unpopular. If you want SMEM_START, then you need to carry a local patch now. virt_to_pfn on the vmap should work. It's about 2 lines, including the change to drop HIDE_SMEM_START. And if consensus is that hiding SMEM_START is not a nice idea, then I'm sure we can reintroduce it through some slightly unpretty backdoor, even with Noralf's generic code. So not really a good reason to reject the cleanup I think. -Daniel > > Den 12.09.2018 11.56, skrev Maxime Ripard: > > > On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote: > > > > Hi Noralf, > > > > > > > > On 08/09/2018 15:46, Noralf Trønnes wrote: > > > > > The CMA helper is already using the > > > > > drm_fb_helper_generic_probe part of > > > > > the generic fbdev emulation. This patch makes full use of the generic > > > > > fbdev emulation by using its drm_client callbacks. This means that > > > > > drm_mode_config_funcs->output_poll_changed and > > > > > drm_driver->lastclose are > > > > > now handled by the emulation code. Additionally fbdev > > > > > unregister happens > > > > > automatically on drm_dev_unregister(). > > > > > > > > > > The drm_fbdev_generic_setup() call is put after > > > > > drm_dev_register() in the > > > > > driver. This is done to highlight the fact that fbdev emulation is an > > > > > internal client that makes use of the driver, it is not part of the > > > > > driver as such. If fbdev setup fails, an error is printed, > > > > > but the driver > > > > > succeeds probing. > > > > I can't really ack/nack this move, on one side it's great to have a > > > > generic fbdev emulation instead of the old fbdev code, but on the > > > > other side the Amlogic platform (like allwinner, samsung, rockchip, > > > > ...) still relies on an Fbdev variant of the libMali that uses > > > > smem_start... > > > > > > > > I know it's dirty and fbdev based code should be legacy now, but I > > > > consider it like an user-space breakage... > > > > > > > > I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that > > > > could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM > > > > driver, it won't be the case and it will never be the case until the > > > > Lima projects finalizes. > > > > > > > > So for me it's a no-go until Lima lands upstream in Linux *and* Mesa. > > > My feelings exactly. If the choice is between reducing the DRM driver > > > by a couple of dozens of lines or keeping the mali working, I'll pick > > > the latter, every single day. > > > > I don't know the reasoning for blocking smem_start other than what Daniel > > wrote in these commit messages: > > > > da6c7707caf3736c1cf968606bd97c07e79625d4 > > fbdev: Add FBINFO_HIDE_SMEM_START flag > > > > DRM drivers really, really, really don't want random userspace to > > share buffer behind it's back, bypassing the dma-buf buffer sharing > > machanism. For that reason we've ruthlessly rejected any IOCTL > > exposing the physical address of any graphics buffer. > > > > Unfortunately fbdev comes with that built-in. We could just set > > smem_start to 0, but that means we'd have to hand-roll our own fb_mmap > > implementation. For good reasons many drivers do that, but > > smem_start/length is still super convenient. > > > > Hence instead just stop the leak in the ioctl, to keep fb mmap working > > as-is. A second patch will set this flag for all drm drivers. > > > > > > 6be8f3bd2c78915a9f3a058a346ae93068d35c01 > > drm/fb: Stop leaking physical address > > > > For buffer sharing, use dma-buf instead. We can't set smem_start to 0 > > unconditionally since that's used by the fbdev mmap default > > implementation. And we have plenty of userspace which would like to > > keep that working. > > > > This might break legit userspace - if it does we need to look at a > > case-by-cases basis how to handle that. Worst case I expect overrides > > for only specific drivers, since anything remotely modern should be > > using dma-buf/prime now (which is about 7 years old now for DRM > > drivers). > > > > This issue was uncovered because Noralf's rework to implement a > > generic fb_probe also implements it's own fb_mmap callback. Which > > means smem_start didn't have to be set anymore, which blew up some > > blob in userspace rather badly. > > > > > > Noralf. > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > -- 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
