* [PATCH] drm/virtio: fix unblank
@ 2020-08-07 10:54 Gerd Hoffmann
2020-08-07 13:09 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2020-08-07 10:54 UTC (permalink / raw)
To: dri-devel
Cc: Gerd Hoffmann, 1882851, David Airlie, Daniel Vetter, Chia-I Wu,
open list:VIRTIO GPU DRIVER, open list
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.
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 | 1 +
drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 9ff9f4ac0522..7b0c319f23c9 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 need_update;
};
#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 cc7fd957a307..378be5956b30 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -100,6 +100,7 @@ static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
output->enabled = true;
+ output->need_update = true;
}
static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 52d24179bcec..5948031a9ce8 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -163,7 +163,8 @@ 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->need_update) {
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,
@@ -178,6 +179,7 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane,
plane->state->src_h >> 16,
plane->state->src_x >> 16,
plane->state->src_y >> 16);
+ output->need_update = false;
}
virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle,
--
2.18.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/virtio: fix unblank
2020-08-07 10:54 [PATCH] drm/virtio: fix unblank Gerd Hoffmann
@ 2020-08-07 13:09 ` Daniel Vetter
2020-08-17 9:03 ` Gerd Hoffmann
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2020-08-07 13:09 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: dri-devel, 1882851, David Airlie, Daniel Vetter, Chia-I Wu,
open list:VIRTIO GPU DRIVER, open list
On Fri, Aug 07, 2020 at 12:54:29PM +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.
>
> 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 | 1 +
> drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++-
> 3 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 9ff9f4ac0522..7b0c319f23c9 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 need_update;
> };
> #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 cc7fd957a307..378be5956b30 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -100,6 +100,7 @@ static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
> struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
>
> output->enabled = true;
> + output->need_update = true;
> }
>
> static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index 52d24179bcec..5948031a9ce8 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -163,7 +163,8 @@ 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->need_update) {
Uh instead of hand-rolling what's essentially a drm_crtc_needs_modeset
check, why not use that one? atomic helpers try to keep the usual suspects
for state transitions already handy, to avoid every driver rolling their
own. Or do I miss something here?
-Daniel
> 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,
> @@ -178,6 +179,7 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane,
> plane->state->src_h >> 16,
> plane->state->src_x >> 16,
> plane->state->src_y >> 16);
> + output->need_update = false;
> }
>
> virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle,
> --
> 2.18.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/virtio: fix unblank
2020-08-07 13:09 ` Daniel Vetter
@ 2020-08-17 9:03 ` Gerd Hoffmann
2020-08-17 10:19 ` Gerd Hoffmann
0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2020-08-17 9:03 UTC (permalink / raw)
To: dri-devel, 1882851, David Airlie, Chia-I Wu,
open list:VIRTIO GPU DRIVER, open list
Hi,
> > --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> > @@ -100,6 +100,7 @@ static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
> > struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
> >
> > output->enabled = true;
> > + output->need_update = true;
> > --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> > @@ -163,7 +163,8 @@ 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->need_update) {
>
> Uh instead of hand-rolling what's essentially a drm_crtc_needs_modeset
> check, why not use that one? atomic helpers try to keep the usual suspects
> for state transitions already handy, to avoid every driver rolling their
> own. Or do I miss something here?
Well, the virtio-gpu virtual hardware can't do plane updates and crtc
updates independant from each other. So the crtc callbacks handle
disable only (we don't need a fb for that) and leave the enable to the
plane update.
I suspect calling drm_atomic_crtc_needs_modeset() in plane update isn't
going to fly ...
take care,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/virtio: fix unblank
2020-08-17 9:03 ` Gerd Hoffmann
@ 2020-08-17 10:19 ` Gerd Hoffmann
0 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2020-08-17 10:19 UTC (permalink / raw)
To: dri-devel, 1882851, David Airlie, Chia-I Wu,
open list:VIRTIO GPU DRIVER, open list
On Mon, Aug 17, 2020 at 11:03:42AM +0200, Gerd Hoffmann wrote:
> Hi,
>
> > > --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> > > +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> > > @@ -100,6 +100,7 @@ static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
> > > struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
> > >
> > > output->enabled = true;
> > > + output->need_update = true;
>
> > > --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> > > +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> > > @@ -163,7 +163,8 @@ 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->need_update) {
> >
> > Uh instead of hand-rolling what's essentially a drm_crtc_needs_modeset
> > check, why not use that one? atomic helpers try to keep the usual suspects
> > for state transitions already handy, to avoid every driver rolling their
> > own. Or do I miss something here?
>
> Well, the virtio-gpu virtual hardware can't do plane updates and crtc
> updates independant from each other. So the crtc callbacks handle
> disable only (we don't need a fb for that) and leave the enable to the
> plane update.
>
> I suspect calling drm_atomic_crtc_needs_modeset() in plane update isn't
> going to fly ...
Digged a bit more, seems crtc_state->*_changed is cleared after modeset
so the following plane update wouldn't see it. Which I think means
there is no way around tracking that in need_update.
output->enabled is probably not needed though, seems I can replace that
by either output->crtc.state->enable or ->active. Not fully sure which
one, probably ->active.
take care,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drm/virtio: fix unblank
[not found] <20200818072511.6745-1-kraxel@redhat.com>
@ 2020-08-25 21:56 ` Diego Viola
0 siblings, 0 replies; 6+ messages in thread
From: Diego Viola @ 2020-08-25 21:56 UTC (permalink / raw)
To: dri-devel; +Cc: linux-kernel, Gerd Hoffmann, 1882851, Jiri Slaby, Diego Viola
From: Gerd Hoffmann <kraxel@redhat.com>
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>
Tested-by: Diego Viola <diego.viola@gmail.com>
---
drivers/gpu/drm/virtio/virtgpu_display.c | 11 +++++++++++
drivers/gpu/drm/virtio/virtgpu_drv.h | 1 +
drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++-
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index af55b334be2f..35b5c80f5d85 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_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_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.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] drm/virtio: fix unblank
@ 2020-06-12 11:13 Gerd Hoffmann
0 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2020-06-12 11:13 UTC (permalink / raw)
To: dri-devel
Cc: Gerd Hoffmann, 1882851, David Airlie, Daniel Vetter, Chia-I Wu,
open list:VIRTIO GPU DRIVER, open list
When going through a disable/enable cycle without changing the framebuffer
the optimization added by commit 3954ff10e06e causes the screen stay
blank. Add a bool to force an update to fix that.
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 | 1 +
drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 7879ff58236f..6d5410d5dd84 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 need_update;
};
#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 2b7e6ae65546..44e9c7b874f5 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -99,6 +99,7 @@ static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
output->enabled = true;
+ output->need_update = true;
}
static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 52d24179bcec..5948031a9ce8 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -163,7 +163,8 @@ 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->need_update) {
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,
@@ -178,6 +179,7 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane,
plane->state->src_h >> 16,
plane->state->src_x >> 16,
plane->state->src_y >> 16);
+ output->need_update = false;
}
virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle,
--
2.18.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-25 21:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-07 10:54 [PATCH] drm/virtio: fix unblank Gerd Hoffmann
2020-08-07 13:09 ` Daniel Vetter
2020-08-17 9:03 ` Gerd Hoffmann
2020-08-17 10:19 ` Gerd Hoffmann
[not found] <20200818072511.6745-1-kraxel@redhat.com>
2020-08-25 21:56 ` Diego Viola
-- strict thread matches above, loose matches on Subject: below --
2020-06-12 11:13 Gerd Hoffmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).