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

Groups > linux.kernel > #1577886

Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC

Show all headers | View raw

On Thu, Feb 09, 2017 at 05:38:26PM +0000, Daniel Stone wrote:
> Hi,
> 
> On 9 February 2017 at 17:01, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Feb 02, 2017 at 11:31:57AM +0100, Maxime Ripard wrote:
> >> +int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, unsigned long arg)
> >> +{
> >> +     struct drm_fb_helper *fb_helper = info->par;
> >> +     struct drm_device *dev = fb_helper->dev;
> >> +     unsigned int i;
> >> +     int ret = 0;
> >> +
> >> +     drm_modeset_lock_all(dev);
> >> +     if (!drm_fb_helper_is_bound(fb_helper)) {
> >> +             drm_modeset_unlock_all(dev);
> >> +             return -EBUSY;
> >> +     }
> >> +
> >> +     switch (cmd) {
> >> +     case FBIO_WAITFORVSYNC:
> >> +             for (i = 0; i < fb_helper->crtc_count; i++) {
> >> +                     struct drm_mode_set *mode_set;
> >> +                     struct drm_crtc *crtc;
> >> +
> >> +                     mode_set = &fb_helper->crtc_info[i].mode_set;
> >> +                     crtc = mode_set->crtc;
> >> +
> >> +                     /*
> >> +                      * Only call drm_crtc_wait_one_vblank for crtcs that
> >> +                      * are currently enabled.
> >> +                      */
> >> +                     if (!crtc->enabled)
> >> +                             continue;
> >
> > This needs locking
> 
> More locking than the drm_modeset_lock_all() above the switch? Ouch. :)

Oh, I didn't spot that one. Well, that one should be removed imo, we dont
want any vblank wait path to hold these locks, it looks bad. Otoh we still
need to grab the dev->mode_config.mutex, because that's also the fbdev
lock. There's a plan somewhere to give the fbdev helpers their own lock,
so switching to just mode_config.mutex + the changes I suggestd would be
good.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

Thread

[PATCH v2 0/2] drm: Support framebuffer panning Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-02-02 11:40 +0100
  [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-02-02 11:40 +0100
    Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC Daniel Vetter <daniel@ffwll.ch> - 2017-02-09 18:10 +0100
      Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC Daniel Vetter <daniel@ffwll.ch> - 2017-02-09 20:10 +0100
      Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC Daniel Stone <daniel@fooishbar.org> - 2017-02-10 00:00 +0100
    Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC Ville Syrjälä <ville.syrjala@linux.intel.com> - 2017-02-10 15:30 +0100
      Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-02-13 11:40 +0100
        Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC Ville Syrjälä <ville.syrjala@linux.intel.com> - 2017-02-13 15:50 +0100

csiph-web