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.
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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
