| Message ID | 20191205160142.3588-5-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Rate-limit shadow-FB-to-console-update to screen refresh | expand |
Hi Thomas, On Thu, 5 Dec 2019 at 16:01, Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Before updating the display from the console's shadow buffer, the dirty > worker now waits for a vblank. This allows several screen updates to pile > up and acts as a rate limiter. If a DRM master is present, it could > interfere with the vblank. Don't wait in this case. > > v3: > * add back helper->lock > * acquire DRM master status while waiting for vblank > v2: > * don't hold helper->lock while waiting for vblank > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/drm_fb_helper.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index fb9bff0f4581..ba20ad92fb90 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -404,8 +404,29 @@ static void drm_fb_helper_dirty_work(struct work_struct *work) > dirty_work); > struct drm_clip_rect *clip = &helper->dirty_clip; > struct drm_clip_rect clip_copy; > + struct drm_crtc *crtc; > unsigned long flags; > void *vaddr; > + int ret; > + > + /* > + * Rate-limit update frequency to vblank. If there's a DRM master > + * present, it could interfere while we're waiting for the vblank > + * event. Don't wait in this case. > + */ > + mutex_lock(&helper->lock); > + if (!drm_master_internal_acquire(helper->dev)) { > + goto unlock; > + } > + crtc = helper->client.modesets[0].crtc; > + ret = drm_crtc_vblank_get(crtc); > + if (!ret) { > + drm_crtc_wait_one_vblank(crtc); > + drm_crtc_vblank_put(crtc); > + } > + drm_master_internal_release(helper->dev); > +unlock: > + mutex_unlock(&helper->lock); > This hunk seems surprisingly similar to the FBIO_WAITFORVSYNC code in drm_fb_helper_ioctl(). Modulo the really neat comment, which answers the "why do we use modeset[0]" instead of any other (or all). Can I suggest fleshing that function out (alongside all the locking) into a helper and reusing it here. Pretty sure we can live with the (very unlikely) EBUSY -> ENOTTY change, in drm_fb_helper_ioctl() HTH Emil
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index fb9bff0f4581..ba20ad92fb90 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -404,8 +404,29 @@ static void drm_fb_helper_dirty_work(struct work_struct *work) dirty_work); struct drm_clip_rect *clip = &helper->dirty_clip; struct drm_clip_rect clip_copy; + struct drm_crtc *crtc; unsigned long flags; void *vaddr; + int ret; + + /* + * Rate-limit update frequency to vblank. If there's a DRM master + * present, it could interfere while we're waiting for the vblank + * event. Don't wait in this case. + */ + mutex_lock(&helper->lock); + if (!drm_master_internal_acquire(helper->dev)) { + goto unlock; + } + crtc = helper->client.modesets[0].crtc; + ret = drm_crtc_vblank_get(crtc); + if (!ret) { + drm_crtc_wait_one_vblank(crtc); + drm_crtc_vblank_put(crtc); + } + drm_master_internal_release(helper->dev); +unlock: + mutex_unlock(&helper->lock); spin_lock_irqsave(&helper->dirty_lock, flags); clip_copy = *clip;
Before updating the display from the console's shadow buffer, the dirty worker now waits for a vblank. This allows several screen updates to pile up and acts as a rate limiter. If a DRM master is present, it could interfere with the vblank. Don't wait in this case. v3: * add back helper->lock * acquire DRM master status while waiting for vblank v2: * don't hold helper->lock while waiting for vblank Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_fb_helper.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)