On Wed, Apr 11, 2018 at 04:22:25PM +0100, Alexandru Gheorghe wrote:
> ApplyFrame holds the lock just when it swaps the value of
> active_composition_, in a multithread context we could end up in a
> situation where something is shown on the screen, but something else
> is set in active_composition_. Fix it by holding the lock during
> CommitFrame.
>
> Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@xxxxxxx>
> ---
> drmdisplaycompositor.cpp | 40 +++++++++++++++++-----------------------
> drmdisplaycompositor.h | 2 +-
> 2 files changed, 18 insertions(+), 24 deletions(-)
>
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index afd3b05..576539b 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -791,11 +791,6 @@ std::tuple<int, uint32_t> DrmDisplayCompositor::CreateModeBlob(
> }
>
> void DrmDisplayCompositor::ClearDisplay() {
> - AutoLock lock(&lock_, "compositor");
> - int ret = lock.Lock();
> - if (ret)
> - return;
> -
> if (!active_composition_)
> return;
>
> @@ -808,11 +803,25 @@ void DrmDisplayCompositor::ClearDisplay() {
> }
>
> void DrmDisplayCompositor::ApplyFrame(
> - std::unique_ptr<DrmDisplayComposition> composition, int status) {
> + std::unique_ptr<DrmDisplayComposition> composition, int status,
> + bool writeback) {
The writeback argument addition seems unrelated to this change.
> + AutoLock lock(&lock_, __FUNCTION__);
> + if (lock.Lock())
> + return;
> int ret = status;
> -
> - if (!ret)
> + if (!ret) {
> + if (writeback && !CountdownExpired()) {
> + ALOGE("Abort playing back scene");
> + return;
> + }
> ret = CommitFrame(composition.get(), false);
> + if (!ret) {
> + ++dump_frames_composited_;
> + if (active_composition_)
> + active_composition_->SignalCompositionDone();
> + active_composition_.swap(composition);
Why move this stuff?
> + }
> + }
>
> if (ret) {
> ALOGE("Composite failed for display %d", display_);
> @@ -821,21 +830,6 @@ void DrmDisplayCompositor::ApplyFrame(
> ClearDisplay();
> return;
> }
> - ++dump_frames_composited_;
> -
> - if (active_composition_)
> - active_composition_->SignalCompositionDone();
> -
> - ret = pthread_mutex_lock(&lock_);
> - if (ret)
> - ALOGE("Failed to acquire lock for active_composition swap");
> -
> - active_composition_.swap(composition);
> -
> - if (!ret)
> - ret = pthread_mutex_unlock(&lock_);
> - if (ret)
> - ALOGE("Failed to release lock for active_composition swap");
> }
>
> int DrmDisplayCompositor::ApplyComposition(
> diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
> index 0f8daad..b35ef70 100644
> --- a/drmdisplaycompositor.h
> +++ b/drmdisplaycompositor.h
> @@ -127,7 +127,7 @@ class DrmDisplayCompositor {
>
> void ClearDisplay();
> void ApplyFrame(std::unique_ptr<DrmDisplayComposition> composition,
> - int status);
> + int status, bool writeback = false);
>
> std::tuple<int, uint32_t> CreateModeBlob(const DrmMode &mode);
>
> --
> 2.7.4
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
