1
0
Fork 0

drm_hwcomposer: Use AtomicRequest struct

Add AtomicRequest struct to wrap the pending DrmModeAtomicReq and
related kms objects and state for the pending atomic commit.

After the AtomicRequest's property set has been committed, update the
internal DrmAtomicStateManager state to reflect what has been committed.

Change-Id: I49d39649eb957d69f724b53b91577a347a75527e
This commit is contained in:
Drew Davenport 2025-09-05 12:36:14 -06:00
parent 88527cddf2
commit c4c008f63a
2 changed files with 108 additions and 83 deletions

View file

@ -93,9 +93,6 @@ bool DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) {
// NOLINTNEXTLINE(misc-const-correctness)
ATRACE_CALL();
args.new_frame_state = {};
args.used_kms_objects = {};
// Clear args.active if it's a no-op.
CheckDoubleSettingState(args);
@ -110,8 +107,8 @@ bool DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) {
args.active = true;
}
auto pset = GetAtomicModeReqForArgs(args);
if (!pset) {
auto atomic_request = GetAtomicModeReqForArgs(args);
if (!atomic_request) {
ALOGE("Failed to get property set");
return false;
}
@ -122,7 +119,8 @@ bool DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) {
auto *drm = pipe_->device;
if (args.test_only) {
auto err = drmModeAtomicCommit(*drm->GetFd(), pset.get(),
auto err = drmModeAtomicCommit(*drm->GetFd(),
atomic_request->property_set.get(),
flags | DRM_MODE_ATOMIC_TEST_ONLY, drm);
ALOGW_IF(err != 0, "Test-only seamless=%d ret=%d errno=%d strerror=%s\n",
@ -136,14 +134,16 @@ bool DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) {
bool nonblock = !args.blocking && !args.active;
flags |= nonblock ? DRM_MODE_ATOMIC_NONBLOCK : 0U;
auto err = drmModeAtomicCommit(*drm->GetFd(), pset.get(), flags, drm);
auto err = drmModeAtomicCommit(*drm->GetFd(),
atomic_request->property_set.get(), flags,
drm);
if (err != 0 && args.seamless) {
ALOGE(
"Seamless commit failed, retrying a full modeset (visual artifacts may "
"be observed). Error: %s",
strerror_r(errno, err_buf, error_buf_max_size));
err = drmModeAtomicCommit(*drm->GetFd(), pset.get(),
err = drmModeAtomicCommit(*drm->GetFd(), atomic_request->property_set.get(),
flags | DRM_MODE_ATOMIC_ALLOW_MODESET, drm);
}
@ -153,14 +153,15 @@ bool DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) {
return false;
}
args.out_fence = MakeSharedFd(args.out_fence_address);
args.out_fence = MakeSharedFd(atomic_request->out_fence_address);
// Store the writeback fence if this operation used a writeback connector
if (pipe_->writeback_connector && args.writeback_fb) {
args.out_writeback_complete_fence = MakeSharedFd(args.wb_fence_address);
args.out_writeback_complete_fence = MakeSharedFd(
atomic_request->wb_fence_address);
}
committed_frame_state_ = std::move(args.new_frame_state);
committed_frame_state_ = std::move(atomic_request->new_frame_state);
if (args.display_mode) {
auto raw_mode = args.display_mode.value().GetRawMode();
@ -171,7 +172,7 @@ bool DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) {
{
const std::lock_guard lock(mutex_);
last_present_fence_ = args.out_fence;
frame_objects_.emplace(std::move(args.used_kms_objects));
frame_objects_.emplace(std::move(atomic_request->used_kms_objects));
frames_staged_++;
}
cv_.notify_all();
@ -179,7 +180,7 @@ bool DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) {
const std::lock_guard lock(mutex_);
last_present_fence_ = {};
frame_objects_ = {};
frame_objects_.emplace(std::move(args.used_kms_objects));
frame_objects_.emplace(std::move(atomic_request->used_kms_objects));
}
return true;
@ -193,7 +194,7 @@ void DrmAtomicStateManager::CheckDoubleSettingState(
}
}
bool DrmAtomicStateManager::SetWriteBackFenceIfNeeded(drmModeAtomicReq *pset,
bool DrmAtomicStateManager::SetWriteBackFenceIfNeeded(AtomicRequest &request,
AtomicCommitArgs &args) {
if (!pipe_->writeback_connector || !args.writeback_fb) {
return true;
@ -202,21 +203,22 @@ bool DrmAtomicStateManager::SetWriteBackFenceIfNeeded(drmModeAtomicReq *pset,
if (!pipe_->writeback_connector->Get()
->GetCrtcIdProperty()
.AtomicSet(*pset, crtc->GetId())) {
.AtomicSet(*request.property_set, crtc->GetId())) {
ALOGE("DrmAtomicStateManager: Failed to set writeback CRTC_ID property");
return false;
}
if (!pipe_->writeback_connector->Get()
->GetWritebackFbIdProperty()
.AtomicSet(*pset, args.writeback_fb->GetFbId())) {
.AtomicSet(*request.property_set, args.writeback_fb->GetFbId())) {
ALOGE("DrmAtomicStateManager: Failed to set writeback FB_ID property");
return false;
}
if (!pipe_->writeback_connector->Get()
->GetWritebackOutFenceProperty()
.AtomicSet(*pset, uint64_t(&args.wb_fence_address))) {
.AtomicSet(*request.property_set,
uint64_t(&request.wb_fence_address))) {
ALOGE(
"DrmAtomicStateManager: Failed to set writeback OUT_FENCE_PTR "
"property");
@ -232,30 +234,30 @@ bool DrmAtomicStateManager::SetWriteBackFenceIfNeeded(drmModeAtomicReq *pset,
return true;
}
bool DrmAtomicStateManager::SetOutputFence(drmModeAtomicReq *pset,
AtomicCommitArgs &args) {
bool DrmAtomicStateManager::SetOutputFence(AtomicRequest &request) {
auto *crtc = pipe_->crtc->Get();
return crtc->GetOutFencePtrProperty().AtomicSet(*pset,
uint64_t(
&args.out_fence_address));
return crtc->GetOutFencePtrProperty()
.AtomicSet(*request.property_set, uint64_t(&request.out_fence_address));
}
bool DrmAtomicStateManager::SetActiveIfNeeded(drmModeAtomicReq *pset,
bool DrmAtomicStateManager::SetActiveIfNeeded(AtomicRequest &request,
AtomicCommitArgs &args) {
if (!args.active) {
return true;
}
auto *crtc = pipe_->crtc->Get();
auto *connector = pipe_->connector->Get();
args.new_frame_state.crtc_active_state = *args.active;
if (!crtc->GetActiveProperty().AtomicSet(*pset, *args.active ? 1 : 0) ||
!connector->GetCrtcIdProperty().AtomicSet(*pset, crtc->GetId())) {
request.new_frame_state.crtc_active_state = *args.active;
if (!crtc->GetActiveProperty().AtomicSet(*request.property_set,
*args.active ? 1 : 0) ||
!connector->GetCrtcIdProperty().AtomicSet(*request.property_set,
crtc->GetId())) {
return false;
}
if (!*args.active && args.teardown) {
if (!connector->GetCrtcIdProperty().AtomicSet(*pset, 0) ||
!crtc->GetModeProperty().AtomicSet(*pset, 0)) {
if (!connector->GetCrtcIdProperty().AtomicSet(*request.property_set, 0) ||
!crtc->GetModeProperty().AtomicSet(*request.property_set, 0)) {
return false;
}
}
@ -263,7 +265,7 @@ bool DrmAtomicStateManager::SetActiveIfNeeded(drmModeAtomicReq *pset,
return true;
}
bool DrmAtomicStateManager::SetDisplayModeIfNeeded(drmModeAtomicReq *pset,
bool DrmAtomicStateManager::SetDisplayModeIfNeeded(AtomicRequest &request,
AtomicCommitArgs &args) {
if (!args.display_mode) {
return true;
@ -278,14 +280,14 @@ bool DrmAtomicStateManager::SetDisplayModeIfNeeded(drmModeAtomicReq *pset,
}
auto *crtc = pipe_->crtc->Get();
if (!crtc->GetModeProperty().AtomicSet(*pset, *mode_blob)) {
if (!crtc->GetModeProperty().AtomicSet(*request.property_set, *mode_blob)) {
return false;
}
args.used_kms_objects.blobs.emplace_back(std::move(mode_blob));
request.used_kms_objects.blobs.emplace_back(std::move(mode_blob));
return true;
}
bool DrmAtomicStateManager::SetCtmIfNeeded(drmModeAtomicReq *pset,
bool DrmAtomicStateManager::SetCtmIfNeeded(AtomicRequest &request,
AtomicCommitArgs &args) {
auto *crtc = pipe_->crtc->Get();
if (!args.color_matrix || !crtc->GetCtmProperty()) {
@ -300,15 +302,15 @@ bool DrmAtomicStateManager::SetCtmIfNeeded(drmModeAtomicReq *pset,
return false;
}
if (!crtc->GetCtmProperty().AtomicSet(*pset, *ctm_blob)) {
if (!crtc->GetCtmProperty().AtomicSet(*request.property_set, *ctm_blob)) {
return false;
}
args.used_kms_objects.blobs.emplace_back(std::move(ctm_blob));
request.used_kms_objects.blobs.emplace_back(std::move(ctm_blob));
return true;
}
bool DrmAtomicStateManager::SetColorSpaceIfNeeded(drmModeAtomicReq *pset,
bool DrmAtomicStateManager::SetColorSpaceIfNeeded(AtomicRequest &request,
AtomicCommitArgs &args) {
auto *connector = pipe_->connector->Get();
if (!args.colorspace || !connector->GetColorspaceProperty()) {
@ -316,22 +318,22 @@ bool DrmAtomicStateManager::SetColorSpaceIfNeeded(drmModeAtomicReq *pset,
}
return connector->GetColorspaceProperty()
.AtomicSet(*pset,
.AtomicSet(*request.property_set,
connector->GetColorspacePropertyValue(*args.colorspace));
}
bool DrmAtomicStateManager::SetContentTypeIfNeeded(drmModeAtomicReq *pset,
bool DrmAtomicStateManager::SetContentTypeIfNeeded(AtomicRequest &request,
AtomicCommitArgs &args) {
auto *connector = pipe_->connector->Get();
if (!args.content_type || !connector->GetContentTypeProperty()) {
return true;
}
return connector->GetContentTypeProperty().AtomicSet(*pset,
return connector->GetContentTypeProperty().AtomicSet(*request.property_set,
static_cast<uint64_t>(
*args.content_type));
}
bool DrmAtomicStateManager::SetHdrMetadataIfNeeded(drmModeAtomicReq *pset,
bool DrmAtomicStateManager::SetHdrMetadataIfNeeded(AtomicRequest &request,
AtomicCommitArgs &args) {
auto *connector = pipe_->connector->Get();
if (!args.hdr_metadata || !connector->GetHdrOutputMetadataProperty()) {
@ -348,15 +350,15 @@ bool DrmAtomicStateManager::SetHdrMetadataIfNeeded(drmModeAtomicReq *pset,
}
if (!connector->GetHdrOutputMetadataProperty()
.AtomicSet(*pset, *hdr_metadata_blob)) {
.AtomicSet(*request.property_set, *hdr_metadata_blob)) {
return false;
}
args.used_kms_objects.blobs.emplace_back(std::move(hdr_metadata_blob));
request.used_kms_objects.blobs.emplace_back(std::move(hdr_metadata_blob));
return true;
}
bool DrmAtomicStateManager::SetMinBpcIfNeeded(drmModeAtomicReq *pset,
bool DrmAtomicStateManager::SetMinBpcIfNeeded(AtomicRequest &request,
AtomicCommitArgs &args) {
auto *connector = pipe_->connector->Get();
if (!args.min_bpc || !connector->GetMinBpcProperty()) {
@ -380,10 +382,11 @@ bool DrmAtomicStateManager::SetMinBpcIfNeeded(drmModeAtomicReq *pset,
int32_t min_bpc_val = std::max(args.min_bpc.value(),
static_cast<int32_t>(range_min));
min_bpc_val = std::min(min_bpc_val, static_cast<int32_t>(range_max));
return connector->GetMinBpcProperty().AtomicSet(*pset, min_bpc_val);
return connector->GetMinBpcProperty().AtomicSet(*request.property_set,
min_bpc_val);
}
bool DrmAtomicStateManager::SetCompositionIfNeeded(drmModeAtomicReq *pset,
bool DrmAtomicStateManager::SetCompositionIfNeeded(AtomicRequest &request,
AtomicCommitArgs &args) {
if (!args.composition) {
return true;
@ -397,8 +400,8 @@ bool DrmAtomicStateManager::SetCompositionIfNeeded(drmModeAtomicReq *pset,
DrmPlane *plane = joining.plane->Get();
LayerData &layer = joining.layer;
args.used_kms_objects.framebuffers.emplace_back(layer.fb);
args.new_frame_state.used_planes.emplace_back(joining.plane);
request.used_kms_objects.framebuffers.emplace_back(layer.fb);
request.new_frame_state.used_planes.emplace_back(joining.plane);
/* Remove from 'unused' list, since plane is re-used */
auto &v = unused_planes;
@ -413,81 +416,84 @@ bool DrmAtomicStateManager::SetCompositionIfNeeded(drmModeAtomicReq *pset,
display_rect_info.i_rect = {0, 0, raw_mode.hdisplay, raw_mode.vdisplay};
}
if (plane->AtomicSetState(*pset, layer, joining.z_pos, crtc->GetId(),
display_rect_info, damage_blob) != 0) {
if (plane->AtomicSetState(*request.property_set, layer, joining.z_pos,
crtc->GetId(), display_rect_info,
damage_blob) != 0) {
return false;
}
args.used_kms_objects.blobs.emplace_back(std::move(damage_blob));
request.used_kms_objects.blobs.emplace_back(std::move(damage_blob));
}
// Disable all planes that were used in the previous commit which are no
// longer being used.
return std::all_of(unused_planes.begin(), unused_planes.end(),
[&pset](auto &plane) {
return plane->Get()->AtomicDisablePlane(*pset) == 0;
[&request](auto &plane) {
return plane->Get()->AtomicDisablePlane(
*request.property_set) == 0;
});
}
DrmModeAtomicReqUnique DrmAtomicStateManager::GetAtomicModeReqForArgs(
std::unique_ptr<AtomicRequest> DrmAtomicStateManager::GetAtomicModeReqForArgs(
AtomicCommitArgs &args) {
ATRACE_CALL();
auto pset = MakeDrmModeAtomicReqUnique();
if (!pset) {
auto atomic_request = std::make_unique<AtomicRequest>();
atomic_request->property_set = MakeDrmModeAtomicReqUnique();
if (!atomic_request->property_set) {
ALOGE("Failed to allocate property set");
return nullptr;
}
if (!SetWriteBackFenceIfNeeded(pset.get(), args)) {
if (!SetWriteBackFenceIfNeeded(*atomic_request, args)) {
ALOGE("Failed to set writeback fence");
return nullptr;
}
if (!SetOutputFence(pset.get(), args)) {
if (!SetOutputFence(*atomic_request)) {
ALOGE("Failed to set output fence");
return nullptr;
}
if (!SetActiveIfNeeded(pset.get(), args)) {
if (!SetActiveIfNeeded(*atomic_request, args)) {
ALOGE("Failed to set active");
return nullptr;
}
if (!SetDisplayModeIfNeeded(pset.get(), args)) {
if (!SetDisplayModeIfNeeded(*atomic_request, args)) {
ALOGE("Failed to set display mode");
return nullptr;
}
if (!SetCtmIfNeeded(pset.get(), args)) {
if (!SetCtmIfNeeded(*atomic_request, args)) {
ALOGE("Failed to set CTM blob");
return nullptr;
}
if (!SetColorSpaceIfNeeded(pset.get(), args)) {
if (!SetColorSpaceIfNeeded(*atomic_request, args)) {
ALOGE("Failed to set color space");
return nullptr;
}
if (!SetContentTypeIfNeeded(pset.get(), args)) {
if (!SetContentTypeIfNeeded(*atomic_request, args)) {
ALOGE("Failed to set content type");
return nullptr;
}
if (!SetHdrMetadataIfNeeded(pset.get(), args)) {
if (!SetHdrMetadataIfNeeded(*atomic_request, args)) {
ALOGE("Failed to set HDR metadata");
return nullptr;
}
if (!SetMinBpcIfNeeded(pset.get(), args)) {
if (!SetMinBpcIfNeeded(*atomic_request, args)) {
ALOGE("Failed to set min BPC");
return nullptr;
}
if (!SetCompositionIfNeeded(pset.get(), args)) {
if (!SetCompositionIfNeeded(*atomic_request, args)) {
ALOGE("Failed to set composition");
return nullptr;
}
return pset;
return atomic_request;
}
void DrmAtomicStateManager::ThreadFn() {

View file

@ -68,15 +68,8 @@ struct AtomicCommitArgs {
SharedFd writeback_release_fence;
/* out */
KmsState new_frame_state;
KmsObjects used_kms_objects;
SharedFd out_writeback_complete_fence;
SharedFd out_fence;
// Shared FD can't be initiallized to an invalid value, for now we keep
// the address separate from the FD for initialization.
// TODO: look into adding support for invalid fences.
int wb_fence_address = -1;
int out_fence_address = -1;
/* helpers */
auto HasInputs() const -> bool {
@ -84,6 +77,31 @@ struct AtomicCommitArgs {
}
};
// State for a pending atomic request. Includes the pending kms objects and
// resulting state of the driver. Since the AtomicRequest might include
// properties that reference memory addresses, this struct must not be moved
// or copied.
struct AtomicRequest {
AtomicRequest() = default;
DrmModeAtomicReqUnique property_set;
// Properties in the property set may reference the memory addresses of
// these struct members.
int wb_fence_address = -1;
int out_fence_address = -1;
KmsObjects used_kms_objects;
KmsState new_frame_state;
// Make this struct non-copyable and non-movable to avoid dangling
// references to struct member addresses.
AtomicRequest(const AtomicRequest &) = delete;
AtomicRequest &operator=(const AtomicRequest &) = delete;
AtomicRequest(AtomicRequest &&) = delete;
AtomicRequest &operator=(AtomicRequest &&) = delete;
};
class DrmAtomicStateManager {
public:
static auto CreateInstance(DrmDisplayPipeline *pipe)
@ -119,19 +137,20 @@ class DrmAtomicStateManager {
DstRectInfo whole_display_rect_{};
void WaitLastFrame();
bool SetWriteBackFenceIfNeeded(drmModeAtomicReq *pset,
bool SetWriteBackFenceIfNeeded(AtomicRequest &request,
AtomicCommitArgs &args);
bool SetOutputFence(drmModeAtomicReq *pset, AtomicCommitArgs &args);
bool SetActiveIfNeeded(drmModeAtomicReq *pset, AtomicCommitArgs &args);
bool SetDisplayModeIfNeeded(drmModeAtomicReq *pset, AtomicCommitArgs &args);
bool SetCtmIfNeeded(drmModeAtomicReq *pset, AtomicCommitArgs &args);
bool SetColorSpaceIfNeeded(drmModeAtomicReq *pset, AtomicCommitArgs &args);
bool SetContentTypeIfNeeded(drmModeAtomicReq *pset, AtomicCommitArgs &args);
bool SetHdrMetadataIfNeeded(drmModeAtomicReq *pset, AtomicCommitArgs &args);
bool SetMinBpcIfNeeded(drmModeAtomicReq *pset, AtomicCommitArgs &args);
bool SetCompositionIfNeeded(drmModeAtomicReq *pset, AtomicCommitArgs &args);
bool SetOutputFence(AtomicRequest &request);
bool SetActiveIfNeeded(AtomicRequest &request, AtomicCommitArgs &args);
bool SetDisplayModeIfNeeded(AtomicRequest &request, AtomicCommitArgs &args);
bool SetCtmIfNeeded(AtomicRequest &request, AtomicCommitArgs &args);
bool SetColorSpaceIfNeeded(AtomicRequest &request, AtomicCommitArgs &args);
bool SetContentTypeIfNeeded(AtomicRequest &request, AtomicCommitArgs &args);
bool SetHdrMetadataIfNeeded(AtomicRequest &request, AtomicCommitArgs &args);
bool SetMinBpcIfNeeded(AtomicRequest &request, AtomicCommitArgs &args);
bool SetCompositionIfNeeded(AtomicRequest &request, AtomicCommitArgs &args);
DrmModeAtomicReqUnique GetAtomicModeReqForArgs(AtomicCommitArgs &args);
std::unique_ptr<AtomicRequest> GetAtomicModeReqForArgs(
AtomicCommitArgs &args);
void CheckDoubleSettingState(AtomicCommitArgs &args) const;
std::thread thread_;