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

[Thomas Zimmermann]

Hi Noralf

Am 25.10.19 um 13:22 schrieb 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.

There's already a patchset on the ML, cleaning up the docs around fb
defio. Please take a look.

Best regards
Thomas

> 
> 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
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20191025/26b2dc0f/attachment.sig>