| Message ID | 20200818072511.6745-2-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/virtio: fix unblank | expand |
On 18. 08. 20, 9:25, Gerd Hoffmann wrote: > When going through a disable/enable cycle without changing the > framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio: > skip set_scanout if framebuffer didn't change") causes the screen stay > blank. Add a bool to force an update to fix that. > > v2: use drm_atomic_crtc_needs_modeset() (Daniel). > > Cc: 1882851@bugs.launchpad.net > Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Tested-by: Jiri Slaby <jirislaby@kernel.org> > --- > drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + > drivers/gpu/drm/virtio/virtgpu_display.c | 11 +++++++++++ > drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++- > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > index 9ff9f4ac0522..4ab1b0ba2925 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > @@ -138,6 +138,7 @@ struct virtio_gpu_output { > int cur_x; > int cur_y; > bool enabled; > + bool needs_modeset; > }; > #define drm_crtc_to_virtio_gpu_output(x) \ > container_of(x, struct virtio_gpu_output, crtc) > diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c > index 2c2742b8d657..6c26b41f4e0d 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_display.c > +++ b/drivers/gpu/drm/virtio/virtgpu_display.c > @@ -123,6 +123,17 @@ static int virtio_gpu_crtc_atomic_check(struct drm_crtc *crtc, > static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc, > struct drm_crtc_state *old_state) > { > + struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); > + > + /* > + * virtio-gpu can't do modeset and plane update operations > + * independant from each other. So the actual modeset happens > + * in the plane update callback, and here we just check > + * whenever we must force the modeset. > + */ > + if (drm_atomic_crtc_needs_modeset(crtc->state)) { > + output->needs_modeset = true; > + } > } > > static const struct drm_crtc_helper_funcs virtio_gpu_crtc_helper_funcs = { > diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c > index 52d24179bcec..65757409d9ed 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_plane.c > +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c > @@ -163,7 +163,9 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, > plane->state->src_w != old_state->src_w || > plane->state->src_h != old_state->src_h || > plane->state->src_x != old_state->src_x || > - plane->state->src_y != old_state->src_y) { > + plane->state->src_y != old_state->src_y || > + output->needs_modeset) { > + output->needs_modeset = false; > DRM_DEBUG("handle 0x%x, crtc %dx%d+%d+%d, src %dx%d+%d+%d\n", > bo->hw_res_handle, > plane->state->crtc_w, plane->state->crtc_h, >
On Mon, Aug 24, 2020 at 09:24:40AM +0200, Jiri Slaby wrote: > On 18. 08. 20, 9:25, Gerd Hoffmann wrote: > > When going through a disable/enable cycle without changing the > > framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio: > > skip set_scanout if framebuffer didn't change") causes the screen stay > > blank. Add a bool to force an update to fix that. > > > > v2: use drm_atomic_crtc_needs_modeset() (Daniel). > > > > Cc: 1882851@bugs.launchpad.net > > Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > Tested-by: Jiri Slaby <jirislaby@kernel.org> Ping. Need an ack or an review to merge this. thanks, Gerd
On Fri, Aug 28, 2020 at 01:27:59PM +0200, Gerd Hoffmann wrote: > On Mon, Aug 24, 2020 at 09:24:40AM +0200, Jiri Slaby wrote: > > On 18. 08. 20, 9:25, Gerd Hoffmann wrote: > > > When going through a disable/enable cycle without changing the > > > framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio: > > > skip set_scanout if framebuffer didn't change") causes the screen stay > > > blank. Add a bool to force an update to fix that. > > > > > > v2: use drm_atomic_crtc_needs_modeset() (Daniel). > > > > > > Cc: 1882851@bugs.launchpad.net > > > Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > > > Tested-by: Jiri Slaby <jirislaby@kernel.org> > > Ping. Need an ack or an review to merge this. In case you still need that, on both patches: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > thanks, > Gerd > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Aug 18, 2020 at 09:25:10AM +0200, Gerd Hoffmann wrote: > When going through a disable/enable cycle without changing the > framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio: > skip set_scanout if framebuffer didn't change") causes the screen stay > blank. Add a bool to force an update to fix that. > > v2: use drm_atomic_crtc_needs_modeset() (Daniel). > > Cc: 1882851@bugs.launchpad.net > Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + > drivers/gpu/drm/virtio/virtgpu_display.c | 11 +++++++++++ > drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++- > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > index 9ff9f4ac0522..4ab1b0ba2925 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > @@ -138,6 +138,7 @@ struct virtio_gpu_output { > int cur_x; > int cur_y; > bool enabled; > + bool needs_modeset; Maybe for a follow-up in -next: The clean atomic way of doing this is to put this into a virtio_crtc_state, compute it in atomic_check, and then fish it out (through old_state->state lookup, somewhat contrived I know) in the commit side. Putting random atomic commit state tracking stuff into non-state structures without appropriate amounts of locks is kinda iffy and means more work for reviewers pondering whether it all works correctly. Cheers, Daniel > }; > #define drm_crtc_to_virtio_gpu_output(x) \ > container_of(x, struct virtio_gpu_output, crtc) > diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c > index 2c2742b8d657..6c26b41f4e0d 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_display.c > +++ b/drivers/gpu/drm/virtio/virtgpu_display.c > @@ -123,6 +123,17 @@ static int virtio_gpu_crtc_atomic_check(struct drm_crtc *crtc, > static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc, > struct drm_crtc_state *old_state) > { > + struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); > + > + /* > + * virtio-gpu can't do modeset and plane update operations > + * independant from each other. So the actual modeset happens > + * in the plane update callback, and here we just check > + * whenever we must force the modeset. > + */ > + if (drm_atomic_crtc_needs_modeset(crtc->state)) { > + output->needs_modeset = true; > + } > } > > static const struct drm_crtc_helper_funcs virtio_gpu_crtc_helper_funcs = { > diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c > index 52d24179bcec..65757409d9ed 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_plane.c > +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c > @@ -163,7 +163,9 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, > plane->state->src_w != old_state->src_w || > plane->state->src_h != old_state->src_h || > plane->state->src_x != old_state->src_x || > - plane->state->src_y != old_state->src_y) { > + plane->state->src_y != old_state->src_y || > + output->needs_modeset) { > + output->needs_modeset = false; > DRM_DEBUG("handle 0x%x, crtc %dx%d+%d+%d, src %dx%d+%d+%d\n", > bo->hw_res_handle, > plane->state->crtc_w, plane->state->crtc_h, > -- > 2.18.4 >
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 9ff9f4ac0522..4ab1b0ba2925 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -138,6 +138,7 @@ struct virtio_gpu_output { int cur_x; int cur_y; bool enabled; + bool needs_modeset; }; #define drm_crtc_to_virtio_gpu_output(x) \ container_of(x, struct virtio_gpu_output, crtc) diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c index 2c2742b8d657..6c26b41f4e0d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_display.c +++ b/drivers/gpu/drm/virtio/virtgpu_display.c @@ -123,6 +123,17 @@ static int virtio_gpu_crtc_atomic_check(struct drm_crtc *crtc, static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { + struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); + + /* + * virtio-gpu can't do modeset and plane update operations + * independant from each other. So the actual modeset happens + * in the plane update callback, and here we just check + * whenever we must force the modeset. + */ + if (drm_atomic_crtc_needs_modeset(crtc->state)) { + output->needs_modeset = true; + } } static const struct drm_crtc_helper_funcs virtio_gpu_crtc_helper_funcs = { diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index 52d24179bcec..65757409d9ed 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -163,7 +163,9 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, plane->state->src_w != old_state->src_w || plane->state->src_h != old_state->src_h || plane->state->src_x != old_state->src_x || - plane->state->src_y != old_state->src_y) { + plane->state->src_y != old_state->src_y || + output->needs_modeset) { + output->needs_modeset = false; DRM_DEBUG("handle 0x%x, crtc %dx%d+%d+%d, src %dx%d+%d+%d\n", bo->hw_res_handle, plane->state->crtc_w, plane->state->crtc_h,
When going through a disable/enable cycle without changing the framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") causes the screen stay blank. Add a bool to force an update to fix that. v2: use drm_atomic_crtc_needs_modeset() (Daniel). Cc: 1882851@bugs.launchpad.net Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + drivers/gpu/drm/virtio/virtgpu_display.c | 11 +++++++++++ drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++- 3 files changed, 15 insertions(+), 1 deletion(-)