Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1383754

Re: [PATCH 8/8] drm/udl: Use drm_fb_helper deferred_io support

From Daniel Vetter <daniel@ffwll.ch>
Newsgroups linux.kernel
Subject Re: [PATCH 8/8] drm/udl: Use drm_fb_helper deferred_io support
Date 2016-04-20 23:30 +0200
Message-ID <rq7R9-6eA-11@gated-at.bofh.it> (permalink)
References <rq2op-1Bz-3@gated-at.bofh.it> <rq2oq-1Bz-9@gated-at.bofh.it> <rq4zU-3eo-15@gated-at.bofh.it> <rq5YZ-4rG-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wed, Apr 20, 2016 at 9:20 PM, Noralf Trønnes <noralf@tronnes.org> wrote:
>>> @@ -330,20 +203,20 @@ static int udl_fb_open(struct fb_info *info, int
>>> user)
>>>         ufbdev->fb_count++;
>>>   -     if (fb_defio && (info->fbdefio == NULL)) {
>>> -               /* enable defio at last moment if not disabled by client
>>> */
>>> +       if (!info->fbdefio) {
>>> +               /* enable defio at last moment */
>>>                 struct fb_deferred_io *fbdefio;
>>>                 fbdefio = kmalloc(sizeof(struct fb_deferred_io),
>>> GFP_KERNEL);
>>> -
>>>                 if (fbdefio) {
>>>                         fbdefio->delay = DL_DEFIO_WRITE_DELAY;
>>> -                       fbdefio->deferred_io = udlfb_dpy_deferred_io;
>>> +                       fbdefio->deferred_io = drm_fb_helper_deferred_io;
>>
>> Why all these changes here? I figured just exchanging the deferred_io
>> pointer should be all that's really needed in the setup code?
>
>
> Because we always need to initialize deferred_io since we use it's worker
> to handle fb_{fillrect,copyarea,imageblit} damage in drm_fb_helper.
>
> The previous code didn't use deferred_io to handle these, it just handled
> the damage directly unless it was running in atomic context, in which case
> it just recorded the damage and returned, leaving it to the next call to
> push the changes.

That kind of explanation needs to be added to the commit message. I
completely missed that udl doesn't have an async work item for defio
from atomic.

> And in the following code I fixed a null pointer problem as well, maybe I
> shouldn't have packed it in here. If fbdefio allocation fails == NULL,
> fb_deferred_io_init() will trigger a BUG().

Yeah, better to split that out into a separate bugfix I think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH 8/8] drm/udl: Use drm_fb_helper deferred_io support Noralf Trønnes <noralf@tronnes.org> - 2016-04-20 17:40 +0200
  Re: [PATCH 8/8] drm/udl: Use drm_fb_helper deferred_io support Daniel Vetter <daniel@ffwll.ch> - 2016-04-20 20:00 +0200
    Re: [PATCH 8/8] drm/udl: Use drm_fb_helper deferred_io support Noralf Trønnes <noralf@tronnes.org> - 2016-04-20 21:30 +0200
      Re: [PATCH 8/8] drm/udl: Use drm_fb_helper deferred_io support Daniel Vetter <daniel@ffwll.ch> - 2016-04-20 23:30 +0200

csiph-web