[PATCH 5/5] drm/udl: Replace fbdev code with generic emulation

[Noralf Trønnes]

Den 25.10.2019 10.00, skrev Daniel Vetter:
> On Fri, Oct 25, 2019 at 09:47:46AM +0200, Daniel Vetter wrote:
>> On Thu, Oct 24, 2019 at 04:42:37PM +0200, Thomas Zimmermann wrote:
>>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>>> ---
>>>  drivers/gpu/drm/udl/udl_drv.c     |   3 +
>>>  drivers/gpu/drm/udl/udl_drv.h     |   4 -
>>>  drivers/gpu/drm/udl/udl_fb.c      | 263 +-----------------------------
>>>  drivers/gpu/drm/udl/udl_main.c    |   2 -
>>>  drivers/gpu/drm/udl/udl_modeset.c |   3 +-
>>>  5 files changed, 8 insertions(+), 267 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
>>> index 15ad7a338f9d..6beaa1109c2c 100644
>>> --- a/drivers/gpu/drm/udl/udl_drv.c
>>> +++ b/drivers/gpu/drm/udl/udl_drv.c
>>> @@ -7,6 +7,7 @@
>>>  
>>>  #include <drm/drm_crtc_helper.h>
>>>  #include <drm/drm_drv.h>
>>> +#include <drm/drm_fb_helper.h>
>>>  #include <drm/drm_file.h>
>>>  #include <drm/drm_ioctl.h>
>>>  #include <drm/drm_probe_helper.h>
>>> @@ -62,6 +63,8 @@ static struct drm_driver driver = {
>>>  	.driver_features = DRIVER_MODESET | DRIVER_GEM,
>>>  	.release = udl_driver_release,
>>>  
>>> +	.lastclose = drm_fb_helper_lastclose,
>>> +
>>>  	/* gem hooks */
>>>  	.gem_free_object_unlocked = udl_gem_free_object,
>>>  	.gem_vm_ops = &udl_gem_vm_ops,
>>> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
>>> index 12a970fd9a87..5f8a7ac084f6 100644
>>> --- a/drivers/gpu/drm/udl/udl_drv.h
>>> +++ b/drivers/gpu/drm/udl/udl_drv.h
>>> @@ -50,8 +50,6 @@ struct urb_list {
>>>  	size_t size;
>>>  };
>>>  
>>> -struct udl_fbdev;
>>> -
>>>  struct udl_device {
>>>  	struct drm_device drm;
>>>  	struct device *dev;
>>> @@ -65,7 +63,6 @@ struct udl_device {
>>>  	struct urb_list urbs;
>>>  	atomic_t lost_pixels; /* 1 = a render op failed. Need screen refresh */
>>>  
>>> -	struct udl_fbdev *fbdev;
>>>  	char mode_buf[1024];
>>>  	uint32_t mode_buf_len;
>>>  	atomic_t bytes_rendered; /* raw pixel-bytes driver asked to render */
>>> @@ -111,7 +108,6 @@ int udl_init(struct udl_device *udl);
>>>  void udl_fini(struct drm_device *dev);
>>>  
>>>  int udl_fbdev_init(struct drm_device *dev);
>>> -void udl_fbdev_cleanup(struct drm_device *dev);
>>>  void udl_fbdev_unplug(struct drm_device *dev);
>>>  struct drm_framebuffer *
>>>  udl_fb_user_fb_create(struct drm_device *dev,
>>> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
>>> index ef3504d06343..43a1da3a56c3 100644
>>> --- a/drivers/gpu/drm/udl/udl_fb.c
>>> +++ b/drivers/gpu/drm/udl/udl_fb.c
>>> @@ -19,19 +19,9 @@
>>>  
>>>  #include "udl_drv.h"
>>>  
>>> -#define DL_DEFIO_WRITE_DELAY    (HZ/20) /* fb_deferred_io.delay in jiffies */
>>> -
>>> -static int fb_defio = 0;  /* Optionally enable experimental fb_defio mmap support */
> 
> Correction on my enthusiasm, this here is a problem:
> 
> The udl defio support as-is is broken, fbdev defio and shmem are fight
> over the page flags. Not a problem with the old code, since disabled by
> default. But will be a problem with the new code. I guess you didn't test
> fbdev mmap? We unfortunately also lack an easy igt testcase for this ...

This is where the shadow buffer comes to the rescue. fbdev gets a
vmalloc buffer and this is blitted on the shmem buffer in the defio
callback before calling the framebuffer .dirty callback. So the defio
internals never sees the shmem buffer. At least this worked when I was
writing the shmem helper.

Noralf.

> 
> The problem is fairly tricky to solve, here's an untested idea that might
> work:
> 
> https://dri.freedesktop.org/docs/drm/gpu/todo.html#generic-fbdev-defio-support
> 
> Cheers, Daniel
>