| Message ID | 1447678222-6858-2-git-send-email-daniels@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2015-11-16 at 12:50 +0000, Daniel Stone wrote: > Rockchip previously treated a pageflip to the same framebuffer as a > no-op, discarding the event if one was requested. This breaks Weston, > which, when idle, sends a no-op vblank event to discover vblank > timings if the vblank query interface is not usable. > > Silently dropping events is also quite a hostile thing to do to > userspace in general. Tested on a Radxa Rock 2 square board, the combination of this patch and the previous in this series makes weston (drm compositor, pixman renderer) work nicely on this hardware while before it would get stuck right away for the reasons pointed out by daniels Tested-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > Signed-off-by: Daniel Stone <daniels@collabora.com> > Cc: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > Cc: Heiko Stuebner <heiko@sntech.de> > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 30 ++++++++++++++----- > ---------- > 1 file changed, 14 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index ddf6dc2..dad607e 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -916,25 +916,23 @@ static int vop_update_plane_event(struct > drm_plane *plane, > * unreference any previous framebuffers. > */ > mutex_lock(&vop->vsync_mutex); > - if (fb != vop_win_last_pending_fb(vop_win)) { > - ret = drm_vblank_get(plane->dev, vop->pipe); > - if (ret) { > - DRM_ERROR("failed to get vblank, %d\n", > ret); > - mutex_unlock(&vop->vsync_mutex); > - return ret; > - } > + ret = drm_vblank_get(plane->dev, vop->pipe); > + if (ret) { > + DRM_ERROR("failed to get vblank, %d\n", ret); > + mutex_unlock(&vop->vsync_mutex); > + return ret; > + } > > - drm_framebuffer_reference(fb); > + drm_framebuffer_reference(fb); > > - ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, > event); > - if (ret) { > - drm_vblank_put(plane->dev, vop->pipe); > - mutex_unlock(&vop->vsync_mutex); > - return ret; > - } > - > - vop->vsync_work_pending = true; > + ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, event); > + if (ret) { > + drm_vblank_put(plane->dev, vop->pipe); > + mutex_unlock(&vop->vsync_mutex); > + return ret; > } > + > + vop->vsync_work_pending = true; > mutex_unlock(&vop->vsync_mutex); > > spin_lock(&vop->reg_lock);
Am Montag, 16. November 2015, 12:50:22 schrieb Daniel Stone: > Rockchip previously treated a pageflip to the same framebuffer as a > no-op, discarding the event if one was requested. This breaks Weston, > which, when idle, sends a no-op vblank event to discover vblank > timings if the vblank query interface is not usable. > > Silently dropping events is also quite a hostile thing to do to > userspace in general. > > Signed-off-by: Daniel Stone <daniels@collabora.com> > Cc: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > Cc: Heiko Stuebner <heiko@sntech.de> on a rk3288_veyron_jerry Tested-by: Heiko Stuebner <heiko@sntech.de> Everything up to GLES2 using the binary userspace driver still works
Hi, ? 2015?11?16? 20:50, Daniel Stone ??: > Rockchip previously treated a pageflip to the same framebuffer as a > no-op, discarding the event if one was requested. This breaks Weston, > which, when idle, sends a no-op vblank event to discover vblank > timings if the vblank query interface is not usable. > > Silently dropping events is also quite a hostile thing to do to > userspace in general. > > Signed-off-by: Daniel Stone <daniels@collabora.com> > Cc: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > Cc: Heiko Stuebner <heiko@sntech.de> > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 30 ++++++++++++++--------------- > 1 file changed, 14 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index ddf6dc2..dad607e 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -916,25 +916,23 @@ static int vop_update_plane_event(struct drm_plane *plane, > * unreference any previous framebuffers. > */ > mutex_lock(&vop->vsync_mutex); > - if (fb != vop_win_last_pending_fb(vop_win)) { There is a warning for building. CC drivers/gpu/drm/rockchip/rockchip_drm_vop.o drivers/gpu/drm/rockchip/rockchip_drm_vop.c:753:32: warning: 'vop_win_last_pending_fb' defined but not used [-Wunused-function] Maybe, we can also remove the vop_win_last_pending_fb() function. > - ret = drm_vblank_get(plane->dev, vop->pipe); > - if (ret) { > - DRM_ERROR("failed to get vblank, %d\n", ret); > - mutex_unlock(&vop->vsync_mutex); > - return ret; > - } > + ret = drm_vblank_get(plane->dev, vop->pipe); > + if (ret) { > + DRM_ERROR("failed to get vblank, %d\n", ret); > + mutex_unlock(&vop->vsync_mutex); > + return ret; > + } > > - drm_framebuffer_reference(fb); > + drm_framebuffer_reference(fb); > > - ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, event); > - if (ret) { > - drm_vblank_put(plane->dev, vop->pipe); > - mutex_unlock(&vop->vsync_mutex); > - return ret; > - } > - > - vop->vsync_work_pending = true; > + ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, event); > + if (ret) { > + drm_vblank_put(plane->dev, vop->pipe); > + mutex_unlock(&vop->vsync_mutex); > + return ret; > } > + > + vop->vsync_work_pending = true; > mutex_unlock(&vop->vsync_mutex); > > spin_lock(&vop->reg_lock);
On 2015?11?23? 15:46, Caesar Wang wrote: > Hi, > > ? 2015?11?16? 20:50, Daniel Stone ??: >> Rockchip previously treated a pageflip to the same framebuffer as a >> no-op, discarding the event if one was requested. This breaks Weston, >> which, when idle, sends a no-op vblank event to discover vblank >> timings if the vblank query interface is not usable. >> >> Silently dropping events is also quite a hostile thing to do to >> userspace in general. >> >> Signed-off-by: Daniel Stone <daniels@collabora.com> >> Cc: Sjoerd Simons <sjoerd.simons@collabora.co.uk> >> Cc: Heiko Stuebner <heiko@sntech.de> >> --- >> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 30 >> ++++++++++++++--------------- >> 1 file changed, 14 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> index ddf6dc2..dad607e 100644 >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> @@ -916,25 +916,23 @@ static int vop_update_plane_event(struct >> drm_plane *plane, >> * unreference any previous framebuffers. >> */ >> mutex_lock(&vop->vsync_mutex); >> - if (fb != vop_win_last_pending_fb(vop_win)) { > > There is a warning for building. > > CC drivers/gpu/drm/rockchip/rockchip_drm_vop.o > drivers/gpu/drm/rockchip/rockchip_drm_vop.c:753:32: warning: > 'vop_win_last_pending_fb' defined but not used [-Wunused-function] > > Maybe, we can also remove the vop_win_last_pending_fb() function. > Can you test this problem with my new atomic patches? I think atomic patch fix the problem. Thanks. >> - ret = drm_vblank_get(plane->dev, vop->pipe); >> - if (ret) { >> - DRM_ERROR("failed to get vblank, %d\n", ret); >> - mutex_unlock(&vop->vsync_mutex); >> - return ret; >> - } >> + ret = drm_vblank_get(plane->dev, vop->pipe); >> + if (ret) { >> + DRM_ERROR("failed to get vblank, %d\n", ret); >> + mutex_unlock(&vop->vsync_mutex); >> + return ret; >> + } >> - drm_framebuffer_reference(fb); >> + drm_framebuffer_reference(fb); >> - ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, event); >> - if (ret) { >> - drm_vblank_put(plane->dev, vop->pipe); >> - mutex_unlock(&vop->vsync_mutex); >> - return ret; >> - } >> - >> - vop->vsync_work_pending = true; >> + ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, event); >> + if (ret) { >> + drm_vblank_put(plane->dev, vop->pipe); >> + mutex_unlock(&vop->vsync_mutex); >> + return ret; >> } >> + >> + vop->vsync_work_pending = true; >> mutex_unlock(&vop->vsync_mutex); >> spin_lock(&vop->reg_lock); > > >
Hi Mark, Caesar, On 2 December 2015 at 01:01, Mark yao <mark.yao@rock-chips.com> wrote: > On 2015?11?23? 15:46, Caesar Wang wrote: >> There is a warning for building. >> >> CC drivers/gpu/drm/rockchip/rockchip_drm_vop.o >> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:753:32: warning: >> 'vop_win_last_pending_fb' defined but not used [-Wunused-function] >> >> Maybe, we can also remove the vop_win_last_pending_fb() function. You're absolutely right, and I have removed this in the next patchset, which I will find the time to post shortly. > Can you test this problem with my new atomic patches? I think atomic patch > fix the problem. I need some more time to do a full review of the atomic work. But either way, atomic is not appropriate for a -fixes pull - I would like to see this in 4.4 if possible, rather than 4.6. Cheers, Daniel
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index ddf6dc2..dad607e 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -916,25 +916,23 @@ static int vop_update_plane_event(struct drm_plane *plane, * unreference any previous framebuffers. */ mutex_lock(&vop->vsync_mutex); - if (fb != vop_win_last_pending_fb(vop_win)) { - ret = drm_vblank_get(plane->dev, vop->pipe); - if (ret) { - DRM_ERROR("failed to get vblank, %d\n", ret); - mutex_unlock(&vop->vsync_mutex); - return ret; - } + ret = drm_vblank_get(plane->dev, vop->pipe); + if (ret) { + DRM_ERROR("failed to get vblank, %d\n", ret); + mutex_unlock(&vop->vsync_mutex); + return ret; + } - drm_framebuffer_reference(fb); + drm_framebuffer_reference(fb); - ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, event); - if (ret) { - drm_vblank_put(plane->dev, vop->pipe); - mutex_unlock(&vop->vsync_mutex); - return ret; - } - - vop->vsync_work_pending = true; + ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, event); + if (ret) { + drm_vblank_put(plane->dev, vop->pipe); + mutex_unlock(&vop->vsync_mutex); + return ret; } + + vop->vsync_work_pending = true; mutex_unlock(&vop->vsync_mutex); spin_lock(&vop->reg_lock);
Rockchip previously treated a pageflip to the same framebuffer as a no-op, discarding the event if one was requested. This breaks Weston, which, when idle, sends a no-op vblank event to discover vblank timings if the vblank query interface is not usable. Silently dropping events is also quite a hostile thing to do to userspace in general. Signed-off-by: Daniel Stone <daniels@collabora.com> Cc: Sjoerd Simons <sjoerd.simons@collabora.co.uk> Cc: Heiko Stuebner <heiko@sntech.de> --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 30 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 16 deletions(-)