Skip to content

Commit

Permalink
platforms/atomic-kms: Make DisplaySink associated with exactly one KM…
Browse files Browse the repository at this point in the history 
…SOutput.

This is a different behaviour to `gbm-kms`. On `gbm-kms`, when outputs
have an overlapping view of the logical space they are grouped
together so that they share a single *physical* framebuffer.

When the overlap is great (such as a complete clone), this
should result in lower GPU memory usage (as we need only a single
set of framebuffers for all clones) at the cost of tying
the refresh rates of each clone together. (And some bugs, like
#3641).

When the overlap is *not* great, this potential memory saving
goes away (and may indeed result in *higher* GPU memory usage -
if outputs have different sizes, we may now need to have a
bunch of unused pixels, as the FB can only be rectangular).

For Atomic KMS, instead, we give each output its own physical
framebuffer, regardless of whether it overlaps.
  • Loading branch information
@RAOF
RAOF committed Oct 18, 2024
1 parent 99e65ba commit b09ddee
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 133 deletions.
78 changes: 39 additions & 39 deletions src/platforms/atomic-kms/server/kms/display.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,58 +332,58 @@ void mga::Display::configure_locked(
});
}

/* Set up used outputs */
OverlappingOutputGrouping grouping{kms_conf};
auto group_idx = 0;

grouping.for_each_group(
[&](OverlappingOutputGroup const& group)
size_t output_idx{0};
kms_conf.for_each_output(
[&](DisplayConfigurationOutput const& out)
{
auto bounding_rect = group.bounding_rectangle();
std::vector<std::shared_ptr<KMSOutput>> kms_outputs;
glm::mat2 transformation;
geom::Size current_mode_resolution;

group.for_each_output(
[&](DisplayConfigurationOutput const& conf_output)
{
auto kms_output = current_display_configuration.get_output_for(conf_output.id);

auto const mode_index = kms_conf.get_kms_mode_index(conf_output.id,
conf_output.current_mode_index);
kms_output->configure(conf_output.top_left - bounding_rect.top_left, mode_index);
if (!comp)
{
kms_output->set_power_mode(conf_output.power_mode);
kms_output->set_gamma(conf_output.gamma);
kms_outputs.push_back(std::move(kms_output));
}

/*
* Presently OverlappingOutputGroup guarantees all grouped
* outputs have the same transformation.
*/
transformation = conf_output.transformation();
if (conf_output.current_mode_index < conf_output.modes.size())
current_mode_resolution = conf_output.modes[conf_output.current_mode_index].size;
});
if (!out.connected || !out.used ||
(out.power_mode != mir_power_mode_on) ||
(out.current_mode_index >= out.modes.size()))
{
// We don't need to do anything for unconfigured outputs
return;
}

auto kms_output = kms_conf.get_output_for(out.id);
auto const mode_index = kms_conf.get_kms_mode_index(out.id, out.current_mode_index);

kms_output->configure({0, 0}, mode_index);

if (!comp)
{
kms_output->set_power_mode(out.power_mode);
kms_output->set_gamma(out.gamma);
}

auto const transform = out.transformation();

if (comp)
{
display_sinks[group_idx++]->set_transformation(transformation,
bounding_rect);
/* If we don't need a modeset we can just
* update our existing `DisplaySink`s.
*
* Note: this assumes that RealKMSDisplayConfiguration.for_each_output
* iterates over outputs in the same order each time.
*/
display_sinks[output_idx]->set_transformation(
transform,
out.extents());
++output_idx;
}
else
{
/* If we need a modeset we'll create a new set
* of `DisplaySink`s
*/
auto db = std::make_unique<DisplaySink>(
drm_fd,
gbm,
event_handler,
bypass_option,
listener,
kms_outputs,
bounding_rect,
transformation);
std::move(kms_output),
out.extents(),
transform);

display_buffers_new.push_back(std::move(db));
}
Expand Down
124 changes: 33 additions & 91 deletions src/platforms/atomic-kms/server/kms/display_buffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,30 +53,20 @@ mga::DisplaySink::DisplaySink(
std::shared_ptr<mgk::DRMEventHandler> event_handler,
mga::BypassOption,
std::shared_ptr<DisplayReport> const& listener,
std::vector<std::shared_ptr<KMSOutput>> const& outputs,
std::shared_ptr<KMSOutput> output,
geom::Rectangle const& area,
glm::mat2 const& transformation)
: gbm{std::move(gbm)},
listener(listener),
outputs(outputs),
output{std::move(output)},
event_handler{std::move(event_handler)},
area(area),
transform{transformation},
needs_set_crtc{false}
{
listener->report_successful_setup_of_native_resources();

// If any of the outputs have a CRTC mismatch, we will want to set all of them
// so that they're all showing the same buffer.
bool has_crtc_mismatch = false;
for (auto& output : outputs)
{
has_crtc_mismatch = output->has_crtc_mismatch();
if (has_crtc_mismatch)
break;
}

if (has_crtc_mismatch)
if (this->output->has_crtc_mismatch())
{
mir::log_info("Clearing screen due to differing encountered and target modes");
// TODO: Pull a supported format out of KMS rather than assuming XRGB8888
Expand All @@ -90,9 +80,7 @@ mga::DisplaySink::DisplaySink(
::memset(mapping->data(), 24, mapping->len());

visible_fb = std::move(initial_fb);
for (auto &output: outputs) {
output->set_crtc(*visible_fb);
}
this->output->set_crtc(*visible_fb);
listener->report_successful_drm_mode_set_crtc_on_construction();
}
listener->report_successful_display_construction();
Expand Down Expand Up @@ -151,20 +139,17 @@ void mga::DisplaySink::for_each_display_sink(std::function<void(graphics::Displa

void mga::DisplaySink::set_crtc(FBHandle const& forced_frame)
{
for (auto& output : outputs)
{
/*
* Note that failure to set the CRTC is not a fatal error. This can
* happen under normal conditions when resizing VirtualBox (which
* actually removes and replaces the virtual output each time so
* sometimes it's really not there). Xorg often reports similar
* errors, and it's not fatal.
*/
if (!output->set_crtc(forced_frame))
mir::log_error("Failed to set DRM CRTC. "
"Screen contents may be incomplete. "
"Try plugging the monitor in again.");
}
/*
* Note that failure to set the CRTC is not a fatal error. This can
* happen under normal conditions when resizing VirtualBox (which
* actually removes and replaces the virtual output each time so
* sometimes it's really not there). Xorg often reports similar
* errors, and it's not fatal.
*/
if (!output->set_crtc(forced_frame))
mir::log_error("Failed to set DRM CRTC. "
"Screen contents may be incomplete. "
"Try plugging the monitor in again.");
}

void mga::DisplaySink::post()
Expand Down Expand Up @@ -215,50 +200,16 @@ void mga::DisplaySink::post()
// Predicted worst case render time for the next frame...
auto predicted_render_time = 50ms;

if (holding_client_buffers)
{
/*
* For composited frames we defer wait_for_page_flip till just before
* the next frame, but not for bypass frames. Deferring the flip of
* bypass frames would increase the time we held
* visible_bypass_frame unacceptably, resulting in client stuttering
* unless we allocate more buffers (which I'm trying to avoid).
* Also, bypass does not need the deferred page flip because it has
* no compositing/rendering step for which to save time for.
*/
wait_for_page_flip();

// It's very likely the next frame will be bypassed like this one so
// we only need time for kernel page flip scheduling...
predicted_render_time = 5ms;
}
else
{
/*
* Not in clone mode? We can afford to wait for the page flip then,
* making us double-buffered (noticeably less laggy than the triple
* buffering that clone mode requires).
*/
if (outputs.size() == 1)
wait_for_page_flip();

/*
* TODO: If you're optimistic about your GPU performance and/or
* measure it carefully you may wish to set predicted_render_time
* to a lower value here for lower latency.
*
*predicted_render_time = 9ms; // e.g. about the same as Weston
*/
}
wait_for_page_flip();

/*
* TODO: Make a sensible predicited_render_time
*/

recommend_sleep = 0ms;
if (outputs.size() == 1)
{
auto const& output = outputs.front();
auto const min_frame_interval = 1000ms / output->max_refresh_rate();
if (predicted_render_time < min_frame_interval)
recommend_sleep = min_frame_interval - predicted_render_time;
}
auto const min_frame_interval = 1000ms / output->max_refresh_rate();
if (predicted_render_time < min_frame_interval)
recommend_sleep = min_frame_interval - predicted_render_time;
}

std::chrono::milliseconds mga::DisplaySink::recommended_sleep() const
Expand All @@ -272,33 +223,24 @@ bool mga::DisplaySink::schedule_page_flip(FBHandle const& bufobj)
* Schedule the current front buffer object for display. Note that
* the page flip is asynchronous and synchronized with vertical refresh.
*/
/* TODO: This works badly if *some* outputs successfully flipped and
* others did not. We should instead have exactly one KMSOutput per DisplaySink
*/
for (auto& output : outputs)
if (output->schedule_page_flip(bufobj))
{
if (output->schedule_page_flip(bufobj))
{
pending_flips.push_back(output.get());
}
page_flip_pending = true;
return true;
}

return !pending_flips.empty();
return false;
}

void mga::DisplaySink::wait_for_page_flip()
{
if (!pending_flips.empty())
if (page_flip_pending)
{
for (auto pending_flip : pending_flips)
{
pending_flip->wait_for_page_flip();
}
pending_flips.clear();
output->wait_for_page_flip();

// The previously-scheduled FB has been page-flipped, and is now visible
visible_fb = std::move(scheduled_fb);
scheduled_fb = nullptr;
page_flip_pending = false;
}
}

Expand All @@ -309,7 +251,7 @@ void mga::DisplaySink::schedule_set_crtc()

auto mga::DisplaySink::drm_fd() const -> mir::Fd
{
return mir::Fd{mir::IntOwnedFd{outputs.front()->drm_fd()}};
return mir::Fd{mir::IntOwnedFd{output->drm_fd()}};
}

auto mga::DisplaySink::gbm_device() const -> std::shared_ptr<struct gbm_device>
Expand Down Expand Up @@ -342,15 +284,15 @@ auto mga::DisplaySink::maybe_create_allocator(DisplayAllocator::Tag const& type_
{
if (!kms_allocator)
{
kms_allocator = kms::CPUAddressableDisplayAllocator::create_if_supported(drm_fd(), outputs.front()->size());
kms_allocator = kms::CPUAddressableDisplayAllocator::create_if_supported(drm_fd(), output->size());
}
return kms_allocator.get();
}
if (dynamic_cast<mg::GBMDisplayAllocator::Tag const*>(&type_tag))
{
if (!gbm_allocator)
{
gbm_allocator = std::make_unique<GBMDisplayAllocator>(drm_fd(), gbm, outputs.front()->size());
gbm_allocator = std::make_unique<GBMDisplayAllocator>(drm_fd(), gbm, output->size());
}
return gbm_allocator.get();
}
Expand Down
6 changes: 3 additions & 3 deletions src/platforms/atomic-kms/server/kms/display_sink.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class DisplaySink : public graphics::DisplaySink,
std::shared_ptr<kms::DRMEventHandler> event_handler,
BypassOption bypass_options,
std::shared_ptr<DisplayReport> const& listener,
std::vector<std::shared_ptr<KMSOutput>> const& outputs,
std::shared_ptr<KMSOutput> output,
geometry::Rectangle const& area,
glm::mat2 const& transformation);
~DisplaySink();
Expand Down Expand Up @@ -98,8 +98,8 @@ class DisplaySink : public graphics::DisplaySink,
std::shared_ptr<FBHandle const> bypass_bufobj{nullptr};
std::shared_ptr<DisplayReport> const listener;

std::vector<std::shared_ptr<KMSOutput>> const outputs;
std::vector<KMSOutput*> pending_flips;
std::shared_ptr<KMSOutput> const output;
bool page_flip_pending{false};

std::shared_ptr<kms::DRMEventHandler> const event_handler;

Expand Down

0 comments on commit b09ddee

Please sign in to comment.