Re: [PATCH] drm/virtio: fix unblank
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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] drm/virtio: fix unblank
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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] drm/virtio: fix unblank
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: 1882...@bugs.launchpad.net
> Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't
> change")
> Signed-off-by: Gerd Hoffmann
> ---
> 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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] drm/virtio: fix unblank
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: 1882...@bugs.launchpad.net
Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't
change")
Signed-off-by: Gerd Hoffmann
---
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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] drm/virtio: fix unblank
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: 1882...@bugs.launchpad.net
Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't
change")
Signed-off-by: Gerd Hoffmann
---
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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
