Hi,
On 01-10-18 18:52, Daniel Vetter wrote:
On Mon, Oct 01, 2018 at 11:37:29AM +0200, Hans de Goede wrote:
Hi,
On 01-10-18 09:53, Daniel Vetter wrote:
On Wed, Sep 26, 2018 at 09:42:02PM +0200, Hans de Goede wrote:
Atomic modesetting does not use the traditional dpms call backs, instead
we should check crtc_state->active.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Are you sure this does what you want it to do? Atomic helpers fully shut
down the screen when you do a dpms off, "just blanked" kinda doesn't exist
as a state by default.
Yes I'm sure I tested "xset dpms force off" and before this would
result in in the virtual monitors (just windows on the host) to resize to
640x480 and in that 640x480 still show part of the old contents.
Hm, this sounds a bit like a bug in your code somewhere, or at least not
100% converted over to atomic. From a driver pov it should be 100%
equivalent code between dpms off and the xrandr --off. If dpms shows some
garbage without this, then something is wrong.
I believe the 2 paths are different, xrandr --off actually does a modeset
disabling the crtc, where as xset dpms force off just changes the DPMS
property on the connector using the non atomic property ioctls leading to
the following call graph:
drm_mode_obj_set_property_ioctl()
set_property_atomic()
drm_atomic_connector_commit_dpms()
With the last one iterating over all connectors of the crtc to which
the connector in question is connected and then setting
crtc_state->active = false if all connectors have their dpms value
set to a value != DRM_MODE_DPMS_ON.
Followed by a drm_atomic_commit() so all that changes in this code
path is the active property of the crtc_state. Where as with a --off the
primary plane_state will get its fb (and crtc) set to NULL.
The only difference is that for dpms off the helpers don't call
->cleanup_plane (and then ->prepare_plane on re-enabling), since the
framebuffers are supposed to stick around. Are you perchance doing some
modeset programming in there? That would be a bug in the atomic
implementation.
crtc_state->active should only be consulted from atomic_check callbacks.
I've added some pretty lengthy kerneldoc for this just recently, to
explain better how this is supposed to work.
See above, I believe that the code path which I point out changes
crtc_state->active without making any further changes.
I'm pretty new to all this, so I could be wrong, but this is what
I believe is happening.
Regards,
Hans
After this patch they become black instead.
Note somewhat related, Virtualbox does not allow closing a window from
within the guest, if the user wants to stop using an (extra) virtual monitor
it needs to be disabled in the VMs UI. Turning of a monitor through e.g.
"xrandr --output foo --off" just makes the window black.
Ok that's funny, but shouldn't be related to what's going on here.
-Daniel
Regards,
Hans
-Daniel
---
drivers/staging/vboxvideo/vbox_drv.h | 1 -
drivers/staging/vboxvideo/vbox_mode.c | 28 ++-------------------------
2 files changed, 2 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h
index fccb3851d6a3..9cc20c182df1 100644
--- a/drivers/staging/vboxvideo/vbox_drv.h
+++ b/drivers/staging/vboxvideo/vbox_drv.h
@@ -139,7 +139,6 @@ struct vbox_connector {
struct vbox_crtc {
struct drm_crtc base;
- bool blanked;
bool disconnected;
unsigned int crtc_id;
u32 fb_offset;
diff --git a/drivers/staging/vboxvideo/vbox_mode.c b/drivers/staging/vboxvideo/vbox_mode.c
index c4ec3fa49782..49ff9c4a6302 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -84,14 +84,13 @@ static void vbox_do_modeset(struct drm_crtc *crtc)
}
flags = VBVA_SCREEN_F_ACTIVE;
- flags |= (fb && !vbox_crtc->blanked) ? 0 : VBVA_SCREEN_F_BLANK;
+ flags |= (fb && crtc->state->active) ? 0 : VBVA_SCREEN_F_BLANK;
flags |= vbox_crtc->disconnected ? VBVA_SCREEN_F_DISABLED : 0;
hgsmi_process_display_info(vbox->guest_pool, vbox_crtc->crtc_id,
x_offset, y_offset,
vbox_crtc->x * bpp / 8 +
vbox_crtc->y * pitch,
- pitch, width, height,
- vbox_crtc->blanked ? 0 : bpp, flags);
+ pitch, width, height, bpp, flags);
}
static int vbox_set_view(struct drm_crtc *crtc)
@@ -128,27 +127,6 @@ static int vbox_set_view(struct drm_crtc *crtc)
return 0;
}
-static void vbox_crtc_dpms(struct drm_crtc *crtc, int mode)
-{
- struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
- struct vbox_private *vbox = crtc->dev->dev_private;
-
- switch (mode) {
- case DRM_MODE_DPMS_ON:
- vbox_crtc->blanked = false;
- break;
- case DRM_MODE_DPMS_STANDBY:
- case DRM_MODE_DPMS_SUSPEND:
- case DRM_MODE_DPMS_OFF:
- vbox_crtc->blanked = true;
- break;
- }
-
- mutex_lock(&vbox->hw_mutex);
- vbox_do_modeset(crtc);
- mutex_unlock(&vbox->hw_mutex);
-}
-
/*
* Try to map the layout of virtual screens to the range of the input device.
* Return true if we need to re-set the crtc modes due to screen offset
@@ -276,7 +254,6 @@ static void vbox_crtc_atomic_flush(struct drm_crtc *crtc,
}
static const struct drm_crtc_helper_funcs vbox_crtc_helper_funcs = {
- .dpms = vbox_crtc_dpms,
.disable = vbox_crtc_disable,
.commit = vbox_crtc_commit,
.atomic_flush = vbox_crtc_atomic_flush,
@@ -861,7 +838,6 @@ static const struct drm_connector_helper_funcs vbox_connector_helper_funcs = {
};
static const struct drm_connector_funcs vbox_connector_funcs = {
- .dpms = drm_helper_connector_dpms,
.detect = vbox_connector_detect,
.fill_modes = vbox_fill_modes,
.destroy = vbox_connector_destroy,
--
2.19.0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
