summaryrefslogtreecommitdiffstats
path: root/video
diff options
context:
space:
mode:
authorAnton Kindestam <antonki@kth.se>2018-06-02 12:53:54 +0200
committerJan Ekström <jeebjp@gmail.com>2018-07-09 02:17:47 +0300
commit7beee68f8d8617557990a5f860d9d1f99c4ba009 (patch)
tree592bd1673b1edd902746e4bf39d9c7406d69f987 /video
parent1298b9d201fc61cfffc6f82d78ae27fc294655b7 (diff)
downloadmpv-7beee68f8d8617557990a5f860d9d1f99c4ba009.tar.bz2
mpv-7beee68f8d8617557990a5f860d9d1f99c4ba009.tar.xz
context_drm_egl: Fix CRTC setup and release code when using atomic
The previous code did not save enough information about the old state, and could end up changing what plane the fbcon:s FB got attached to, or in worse case causing a blank screen (observed in some multi-screen setups on Sandy Bridge). In addition refactor the handling of drmModeModeInfo property blobs to not leak, as well as enable reuse of already created blobs.
Diffstat (limited to 'video')
-rw-r--r--video/out/drm_atomic.c144
-rw-r--r--video/out/drm_atomic.h42
-rw-r--r--video/out/drm_common.c8
-rw-r--r--video/out/drm_common.h2
-rw-r--r--video/out/opengl/context_drm_egl.c56
-rw-r--r--video/out/vo_drm.c6
6 files changed, 217 insertions, 41 deletions
diff --git a/video/out/drm_atomic.c b/video/out/drm_atomic.c
index 8e68cdb074..5c6b3bbe21 100644
--- a/video/out/drm_atomic.c
+++ b/video/out/drm_atomic.c
@@ -290,9 +290,153 @@ fail:
void drm_atomic_destroy_context(struct drm_atomic_context *ctx)
{
+ drm_mode_destroy_blob(ctx->fd, &ctx->old_state.crtc.mode);
drm_object_free(ctx->crtc);
drm_object_free(ctx->connector);
drm_object_free(ctx->osd_plane);
drm_object_free(ctx->video_plane);
talloc_free(ctx);
}
+
+static bool drm_atomic_save_plane_state(struct drm_object *plane,
+ struct drm_atomic_plane_state *plane_state)
+{
+ bool ret = true;
+
+ if (0 > drm_object_get_property(plane, "FB_ID", &plane_state->fb_id))
+ ret = false;
+ if (0 > drm_object_get_property(plane, "CRTC_ID", &plane_state->crtc_id))
+ ret = false;
+ if (0 > drm_object_get_property(plane, "SRC_X", &plane_state->src_x))
+ ret = false;
+ if (0 > drm_object_get_property(plane, "SRC_Y", &plane_state->src_y))
+ ret = false;
+ if (0 > drm_object_get_property(plane, "SRC_W", &plane_state->src_w))
+ ret = false;
+ if (0 > drm_object_get_property(plane, "SRC_H", &plane_state->src_h))
+ ret = false;
+ if (0 > drm_object_get_property(plane, "CRTC_X", &plane_state->crtc_x))
+ ret = false;
+ if (0 > drm_object_get_property(plane, "CRTC_Y", &plane_state->crtc_y))
+ ret = false;
+ if (0 > drm_object_get_property(plane, "CRTC_W", &plane_state->crtc_w))
+ ret = false;
+ if (0 > drm_object_get_property(plane, "CRTC_H", &plane_state->crtc_h))
+ ret = false;
+ // ZPOS might not exist, so ignore whether or not this succeeds
+ drm_object_get_property(plane, "ZPOS", &plane_state->zpos);
+
+ return ret;
+}
+
+static bool drm_atomic_restore_plane_state(drmModeAtomicReq *request,
+ struct drm_object *plane,
+ const struct drm_atomic_plane_state *plane_state)
+{
+ bool ret = true;
+
+ if (0 > drm_object_set_property(request, plane, "FB_ID", plane_state->fb_id))
+ ret = false;
+ if (0 > drm_object_set_property(request, plane, "CRTC_ID", plane_state->crtc_id))
+ ret = false;
+ if (0 > drm_object_set_property(request, plane, "SRC_X", plane_state->src_x))
+ ret = false;
+ if (0 > drm_object_set_property(request, plane, "SRC_Y", plane_state->src_y))
+ ret = false;
+ if (0 > drm_object_set_property(request, plane, "SRC_W", plane_state->src_w))
+ ret = false;
+ if (0 > drm_object_set_property(request, plane, "SRC_H", plane_state->src_h))
+ ret = false;
+ if (0 > drm_object_set_property(request, plane, "CRTC_X", plane_state->crtc_x))
+ ret = false;
+ if (0 > drm_object_set_property(request, plane, "CRTC_Y", plane_state->crtc_y))
+ ret = false;
+ if (0 > drm_object_set_property(request, plane, "CRTC_W", plane_state->crtc_w))
+ ret = false;
+ if (0 > drm_object_set_property(request, plane, "CRTC_H", plane_state->crtc_h))
+ ret = false;
+ // ZPOS might not exist, so ignore whether or not this succeeds
+ drm_object_set_property(request, plane, "ZPOS", plane_state->zpos);
+
+ return ret;
+}
+
+bool drm_atomic_save_old_state(struct drm_atomic_context *ctx)
+{
+ if (ctx->old_state.saved)
+ return false;
+
+ bool ret = true;
+
+ drmModeCrtc *crtc = drmModeGetCrtc(ctx->fd, ctx->crtc->id);
+ if (crtc == NULL)
+ return false;
+ ctx->old_state.crtc.mode.mode = crtc->mode;
+ drmModeFreeCrtc(crtc);
+
+ if (0 > drm_object_get_property(ctx->crtc, "ACTIVE", &ctx->old_state.crtc.active))
+ ret = false;
+
+ if (0 > drm_object_get_property(ctx->connector, "CRTC_ID", &ctx->old_state.connector.crtc_id))
+ ret = false;
+
+ if (!drm_atomic_save_plane_state(ctx->osd_plane, &ctx->old_state.osd_plane))
+ ret = false;
+ if (!drm_atomic_save_plane_state(ctx->video_plane, &ctx->old_state.video_plane))
+ ret = false;
+
+ ctx->old_state.saved = true;
+
+ return ret;
+}
+
+bool drm_atomic_restore_old_state(drmModeAtomicReqPtr request, struct drm_atomic_context *ctx)
+{
+ if (!ctx->old_state.saved)
+ return false;
+
+ bool ret = true;
+
+ if (0 > drm_object_set_property(request, ctx->connector, "CRTC_ID", ctx->old_state.connector.crtc_id))
+ ret = false;
+
+ if (!drm_mode_ensure_blob(ctx->fd, &ctx->old_state.crtc.mode))
+ ret = false;
+ if (0 > drm_object_set_property(request, ctx->crtc, "MODE_ID", ctx->old_state.crtc.mode.blob_id))
+ ret = false;
+ if (0 > drm_object_set_property(request, ctx->crtc, "ACTIVE", ctx->old_state.crtc.active))
+ ret = false;
+
+ if (!drm_atomic_restore_plane_state(request, ctx->osd_plane, &ctx->old_state.osd_plane))
+ ret = false;
+ if (!drm_atomic_restore_plane_state(request, ctx->video_plane, &ctx->old_state.video_plane))
+ ret = false;
+
+ ctx->old_state.saved = false;
+
+ return ret;
+}
+
+bool drm_mode_ensure_blob(int fd, struct drm_mode *mode)
+{
+ int ret = 0;
+
+ if (!mode->blob_id) {
+ ret = drmModeCreatePropertyBlob(fd, &mode->mode, sizeof(drmModeModeInfo),
+ &mode->blob_id);
+ }
+
+ return (ret == 0);
+}
+
+bool drm_mode_destroy_blob(int fd, struct drm_mode *mode)
+{
+ int ret = 0;
+
+ if (mode->blob_id) {
+ ret = drmModeDestroyPropertyBlob(fd, mode->blob_id);
+ mode->blob_id = 0;
+ }
+
+ return (ret == 0);
+}
diff --git a/video/out/drm_atomic.h b/video/out/drm_atomic.h
index 01246baf36..cd0252a767 100644
--- a/video/out/drm_atomic.h
+++ b/video/out/drm_atomic.h
@@ -19,11 +19,45 @@
#define MP_DRMATOMIC_H
#include <stdlib.h>
+#include <stdbool.h>
#include <xf86drm.h>
#include <xf86drmMode.h>
#include "common/msg.h"
+struct drm_mode {
+ drmModeModeInfo mode;
+ uint32_t blob_id;
+};
+
+struct drm_atomic_plane_state {
+ uint64_t fb_id;
+ uint64_t crtc_id;
+ uint64_t src_x;
+ uint64_t src_y;
+ uint64_t src_w;
+ uint64_t src_h;
+ uint64_t crtc_x;
+ uint64_t crtc_y;
+ uint64_t crtc_w;
+ uint64_t crtc_h;
+ uint64_t zpos;
+};
+
+// Used to store the restore state for VT switching and uninit
+struct drm_atomic_state {
+ bool saved;
+ struct {
+ uint64_t crtc_id;
+ } connector;
+ struct {
+ struct drm_mode mode;
+ uint64_t active;
+ } crtc;
+ struct drm_atomic_plane_state osd_plane;
+ struct drm_atomic_plane_state video_plane;
+};
+
struct drm_object {
int fd;
uint32_t id;
@@ -41,6 +75,8 @@ struct drm_atomic_context {
struct drm_object *video_plane;
drmModeAtomicReq *request;
+
+ struct drm_atomic_state old_state;
};
@@ -56,4 +92,10 @@ struct drm_atomic_context *drm_atomic_create_context(struct mp_log *log, int fd,
int osd_plane_id, int video_plane_id);
void drm_atomic_destroy_context(struct drm_atomic_context *ctx);
+bool drm_atomic_save_old_state(struct drm_atomic_context *ctx);
+bool drm_atomic_restore_old_state(drmModeAtomicReq *request, struct drm_atomic_context *ctx);
+
+bool drm_mode_ensure_blob(int fd, struct drm_mode *mode);
+bool drm_mode_destroy_blob(int fd, struct drm_mode *mode);
+
#endif // MP_DRMATOMIC_H
diff --git a/video/out/drm_common.c b/video/out/drm_common.c
index df06744fbe..aa3d0998c4 100644
--- a/video/out/drm_common.c
+++ b/video/out/drm_common.c
@@ -237,7 +237,7 @@ static bool setup_mode(struct kms *kms, int mode_id)
return false;
}
- kms->mode = kms->connector->modes[mode_id];
+ kms->mode.mode = kms->connector->modes[mode_id];
return true;
}
@@ -281,7 +281,7 @@ struct kms *kms_create(struct mp_log *log, const char *connector_spec,
.fd = open_card(card_no),
.connector = NULL,
.encoder = NULL,
- .mode = { 0 },
+ .mode = {{0}},
.crtc_id = -1,
.card_no = card_no,
};
@@ -324,7 +324,6 @@ struct kms *kms_create(struct mp_log *log, const char *connector_spec,
}
}
-
drmModeFreeResources(res);
return kms;
@@ -341,6 +340,7 @@ void kms_destroy(struct kms *kms)
{
if (!kms)
return;
+ drm_mode_destroy_blob(kms->fd, &kms->mode);
if (kms->connector) {
drmModeFreeConnector(kms->connector);
kms->connector = NULL;
@@ -425,7 +425,7 @@ void kms_show_available_cards_and_connectors(struct mp_log *log)
double kms_get_display_fps(const struct kms *kms)
{
- return mode_get_Hz(&kms->mode);
+ return mode_get_Hz(&kms->mode.mode);
}
int drm_validate_connector_opt(struct mp_log *log, const struct m_option *opt,
diff --git a/video/out/drm_common.h b/video/out/drm_common.h
index 4d1e2a9e3f..3f144102ec 100644
--- a/video/out/drm_common.h
+++ b/video/out/drm_common.h
@@ -32,7 +32,7 @@ struct kms {
int fd;
drmModeConnector *connector;
drmModeEncoder *encoder;
- drmModeModeInfo mode;
+ struct drm_mode mode;
uint32_t crtc_id;
int card_no;
struct drm_atomic_context *atomic_context;
diff --git a/video/out/opengl/context_drm_egl.c b/video/out/opengl/context_drm_egl.c
index c101a5b809..52774d8170 100644
--- a/video/out/opengl/context_drm_egl.c
+++ b/video/out/opengl/context_drm_egl.c
@@ -252,6 +252,10 @@ static bool crtc_setup_atomic(struct ra_ctx *ctx)
struct priv *p = ctx->priv;
struct drm_atomic_context *atomic_ctx = p->kms->atomic_context;
+ if (!drm_atomic_save_old_state(atomic_ctx)) {
+ MP_WARN(ctx->vo, "Failed to save old DRM atomic state\n");
+ }
+
drmModeAtomicReqPtr request = drmModeAtomicAlloc();
if (!request) {
MP_ERR(ctx->vo, "Failed to allocate drm atomic request\n");
@@ -263,14 +267,11 @@ static bool crtc_setup_atomic(struct ra_ctx *ctx)
return false;
}
- uint32_t blob_id = 0;
- if (drmModeCreatePropertyBlob(p->kms->fd, &p->kms->mode, sizeof(drmModeModeInfo),
- &blob_id) != 0) {
+ if (!drm_mode_ensure_blob(p->kms->fd, &p->kms->mode)) {
MP_ERR(ctx->vo, "Failed to create DRM mode blob\n");
- blob_id = 0;
goto err;
}
- if (drm_object_set_property(request, atomic_ctx->crtc, "MODE_ID", blob_id) < 0) {
+ if (drm_object_set_property(request, atomic_ctx->crtc, "MODE_ID", p->kms->mode.blob_id) < 0) {
MP_ERR(ctx->vo, "Could not set MODE_ID on crtc\n");
goto err;
}
@@ -287,8 +288,8 @@ static bool crtc_setup_atomic(struct ra_ctx *ctx)
drm_object_set_property(request, atomic_ctx->osd_plane, "SRC_H", p->osd_size.height << 16);
drm_object_set_property(request, atomic_ctx->osd_plane, "CRTC_X", 0);
drm_object_set_property(request, atomic_ctx->osd_plane, "CRTC_Y", 0);
- drm_object_set_property(request, atomic_ctx->osd_plane, "CRTC_W", p->kms->mode.hdisplay);
- drm_object_set_property(request, atomic_ctx->osd_plane, "CRTC_H", p->kms->mode.vdisplay);
+ drm_object_set_property(request, atomic_ctx->osd_plane, "CRTC_W", p->kms->mode.mode.hdisplay);
+ drm_object_set_property(request, atomic_ctx->osd_plane, "CRTC_H", p->kms->mode.mode.vdisplay);
int ret = drmModeAtomicCommit(p->kms->fd, request, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
if (ret)
@@ -298,8 +299,6 @@ static bool crtc_setup_atomic(struct ra_ctx *ctx)
return ret == 0;
err:
- if (blob_id)
- drmModeDestroyPropertyBlob(p->kms->fd, blob_id);
drmModeAtomicFree(request);
return false;
}
@@ -314,17 +313,10 @@ static bool crtc_release_atomic(struct ra_ctx *ctx)
MP_ERR(ctx->vo, "Failed to allocate drm atomic request\n");
return false;
}
- drm_object_set_property(request, atomic_ctx->connector, "CRTC_ID", p->old_crtc->crtc_id);
- uint32_t blob_id;
- if (drmModeCreatePropertyBlob(p->kms->fd, &p->old_crtc->mode, sizeof(drmModeModeInfo),
- &blob_id) != 0) {
- MP_ERR(ctx->vo, "Failed to create DRM mode blob\n");
- goto err;
+ if (!drm_atomic_restore_old_state(request, atomic_ctx)) {
+ MP_WARN(ctx->vo, "Got error while restoring old state\n");
}
- drm_object_set_property(request, atomic_ctx->crtc, "MODE_ID", blob_id);
- drm_object_set_property(request, atomic_ctx->crtc, "ACTIVE", 1);
- drm_object_set_property(request, atomic_ctx->osd_plane, "FB_ID", p->old_crtc->buffer_id);
int ret = drmModeAtomicCommit(p->kms->fd, request, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
@@ -333,10 +325,6 @@ static bool crtc_release_atomic(struct ra_ctx *ctx)
drmModeAtomicFree(request);
return ret == 0;
-
- err:
- drmModeAtomicFree(request);
- return false;
}
static bool crtc_setup(struct ra_ctx *ctx)
@@ -344,16 +332,16 @@ static bool crtc_setup(struct ra_ctx *ctx)
struct priv *p = ctx->priv;
if (p->active)
return true;
- p->old_crtc = drmModeGetCrtc(p->kms->fd, p->kms->crtc_id);
if (p->kms->atomic_context) {
int ret = crtc_setup_atomic(ctx);
p->active = true;
return ret;
} else {
+ p->old_crtc = drmModeGetCrtc(p->kms->fd, p->kms->crtc_id);
int ret = drmModeSetCrtc(p->kms->fd, p->kms->crtc_id, p->fb->id,
0, 0, &p->kms->connector->connector_id, 1,
- &p->kms->mode);
+ &p->kms->mode.mode);
p->active = true;
return ret == 0;
}
@@ -376,19 +364,21 @@ static void crtc_release(struct ra_ctx *ctx)
}
}
- if (p->old_crtc) {
- if (p->kms->atomic_context) {
+ if (p->kms->atomic_context) {
+ if (p->kms->atomic_context->old_state.saved) {
if (!crtc_release_atomic(ctx))
MP_ERR(ctx->vo, "Failed to restore previous mode\n");
- } else {
+ }
+ } else {
+ if (p->old_crtc) {
drmModeSetCrtc(p->kms->fd,
p->old_crtc->crtc_id, p->old_crtc->buffer_id,
p->old_crtc->x, p->old_crtc->y,
&p->kms->connector->connector_id, 1,
&p->old_crtc->mode);
+ drmModeFreeCrtc(p->old_crtc);
+ p->old_crtc = NULL;
}
- drmModeFreeCrtc(p->old_crtc);
- p->old_crtc = NULL;
}
}
@@ -606,13 +596,13 @@ static bool drm_egl_init(struct ra_ctx *ctx)
p->osd_size.width = ctx->vo->opts->drm_opts->drm_osd_size.w;
p->osd_size.height = ctx->vo->opts->drm_opts->drm_osd_size.h;
} else {
- p->osd_size.width = p->kms->mode.hdisplay;
- p->osd_size.height = p->kms->mode.vdisplay;
+ p->osd_size.width = p->kms->mode.mode.hdisplay;
+ p->osd_size.height = p->kms->mode.mode.vdisplay;
MP_WARN(ctx, "Setting OSD size is only available with DRM atomic, defaulting to screen resolution\n");
}
} else {
- p->osd_size.width = p->kms->mode.hdisplay;
- p->osd_size.height = p->kms->mode.vdisplay;
+ p->osd_size.width = p->kms->mode.mode.hdisplay;
+ p->osd_size.height = p->kms->mode.mode.vdisplay;
}
uint32_t argb_format;
diff --git a/video/out/vo_drm.c b/video/out/vo_drm.c
index ed41d2aab0..7f5290110f 100644
--- a/video/out/vo_drm.c
+++ b/video/out/vo_drm.c
@@ -153,8 +153,8 @@ static bool fb_setup_double_buffering(struct vo *vo)
p->front_buf = 0;
for (unsigned int i = 0; i < 2; i++) {
- p->bufs[i].width = p->kms->mode.hdisplay;
- p->bufs[i].height = p->kms->mode.vdisplay;
+ p->bufs[i].width = p->kms->mode.mode.hdisplay;
+ p->bufs[i].height = p->kms->mode.mode.vdisplay;
}
for (unsigned int i = 0; i < BUF_COUNT; i++) {
@@ -186,7 +186,7 @@ static bool crtc_setup(struct vo *vo)
int ret = drmModeSetCrtc(p->kms->fd, p->kms->crtc_id,
p->bufs[MOD(p->front_buf - 1, BUF_COUNT)].fb,
0, 0, &p->kms->connector->connector_id, 1,
- &p->kms->mode);
+ &p->kms->mode.mode);
p->active = true;
return ret == 0;
}