[PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>