backends/drm: don't take crtcs from dpms disabled outputs

Fixes a crash I have with dpms + suspend, which was caused by the udev
event for updating outputs being called before the output got enabled
again. When DrmGpu::updateOutputs got called it removed the crtc from
the inactive output and then disabled the output afterwards. Instead,
only remove crtcs if an output is really disabled.
This also allows to generalize the logic for lease outputs, and could
in the future allow for faster dpms on/off switching.
This commit is contained in:
Xaver Hugl 2021-11-16 11:41:15 +01:00
parent 47d5d50bdf
commit b6b0af727d
3 changed files with 20 additions and 22 deletions

View file

@ -268,6 +268,8 @@ bool DrmGpu::updateOutputs()
qCDebug(KWIN_DRM, "New %soutput on GPU %s: %s", conn->isNonDesktop() ? "non-desktop " : "", qPrintable(m_devNode), qPrintable(conn->modelName())); qCDebug(KWIN_DRM, "New %soutput on GPU %s: %s", conn->isNonDesktop() ? "non-desktop " : "", qPrintable(m_devNode), qPrintable(conn->modelName()));
m_pipelines << conn->pipeline(); m_pipelines << conn->pipeline();
if (conn->isNonDesktop()) { if (conn->isNonDesktop()) {
conn->pipeline()->pending.active = false;
conn->pipeline()->applyPendingChanges();
auto leaseOutput = new DrmLeaseOutput(conn->pipeline(), m_leaseDevice); auto leaseOutput = new DrmLeaseOutput(conn->pipeline(), m_leaseDevice);
m_leaseOutputs << leaseOutput; m_leaseOutputs << leaseOutput;
} else { } else {
@ -311,6 +313,7 @@ bool DrmGpu::updateOutputs()
for (const auto &pipeline : qAsConst(m_pipelines)) { for (const auto &pipeline : qAsConst(m_pipelines)) {
pipeline->applyPendingChanges(); pipeline->applyPendingChanges();
if (!pipeline->pending.crtc && pipeline->output()) { if (!pipeline->pending.crtc && pipeline->output()) {
pipeline->pending.enabled = false;
pipeline->output()->setEnabled(false); pipeline->output()->setEnabled(false);
} }
} }
@ -335,35 +338,28 @@ bool DrmGpu::checkCrtcAssignment(QVector<DrmConnector*> connectors, QVector<DrmC
qCWarning(KWIN_DRM) << "disabling connector" << conn->modelName() << "without a crtc"; qCWarning(KWIN_DRM) << "disabling connector" << conn->modelName() << "without a crtc";
conn->pipeline()->pending.crtc = nullptr; conn->pipeline()->pending.crtc = nullptr;
} }
// non-desktop outputs need to be tested when they would be enabled so that they can be used if leased // pipelines that are enabled but not active need to be activated for the test
QVector<DrmPipeline*> leasePipelines; QVector<DrmPipeline*> inactivePipelines;
for (const auto &output : qAsConst(m_leaseOutputs)) { for (const auto &pipeline : qAsConst(m_pipelines)) {
if (!output->lease()) { if (!pipeline->pending.active) {
output->pipeline()->pending.active = true; pipeline->pending.active = true;
leasePipelines << output->pipeline(); inactivePipelines << pipeline;
} }
} }
const auto unused = unusedObjects(); const auto unused = unusedObjects();
bool test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unused); bool test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unused);
if (!leasePipelines.isEmpty() && test) { // disable inactive pipelines again
// non-desktop outputs should be disabled for normal usage for (const auto &pipeline : qAsConst(inactivePipelines)) {
for (const auto &pipeline : qAsConst(leasePipelines)) { pipeline->pending.active = false;
pipeline->pending.active = false;
}
bool ret = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unused);
if (ret) {
for (const auto &pipeline : qAsConst(leasePipelines)) {
pipeline->applyPendingChanges();
}
}
return ret;
} else {
return test;
} }
if (!inactivePipelines.isEmpty() && test) {
test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unused);
}
return test;
} }
auto connector = connectors.takeFirst(); auto connector = connectors.takeFirst();
auto pipeline = connector->pipeline(); auto pipeline = connector->pipeline();
if (const auto &output = pipeline->output(); output && !pipeline->pending.active) { if (!pipeline->pending.enabled) {
// disabled pipelines don't need CRTCs // disabled pipelines don't need CRTCs
pipeline->pending.crtc = nullptr; pipeline->pending.crtc = nullptr;
return checkCrtcAssignment(connectors, crtcs); return checkCrtcAssignment(connectors, crtcs);

View file

@ -453,6 +453,7 @@ bool DrmOutput::queueChanges(const WaylandOutputConfig &config)
m_pipeline->pending.overscan = props->overscan; m_pipeline->pending.overscan = props->overscan;
m_pipeline->pending.rgbRange = props->rgbRange; m_pipeline->pending.rgbRange = props->rgbRange;
m_pipeline->pending.transformation = DrmPlane::Transformation::Rotate0; m_pipeline->pending.transformation = DrmPlane::Transformation::Rotate0;
m_pipeline->pending.enabled = props->enabled;
return true; return true;
} }

View file

@ -92,7 +92,8 @@ public:
struct State { struct State {
DrmCrtc *crtc = nullptr; DrmCrtc *crtc = nullptr;
bool active = true; bool active = true; // whether or not the pipeline should be currently used
bool enabled = true;// whether or not the pipeline needs a crtc
int modeIndex = 0; int modeIndex = 0;
uint32_t overscan = 0; uint32_t overscan = 0;
AbstractWaylandOutput::RgbRange rgbRange = AbstractWaylandOutput::RgbRange::Automatic; AbstractWaylandOutput::RgbRange rgbRange = AbstractWaylandOutput::RgbRange::Automatic;