From 4d4f4007cc3c1474da0ce0242d578fa4c4b5a66a Mon Sep 17 00:00:00 2001 From: Saurabh Shah Date: Tue, 26 Dec 2017 12:42:59 -0800 Subject: [PATCH 1/8] sdm: Unregister fb_ids after Validate HWC allows destruction of layers between Validate and Commit. Buffer fds could also get updated in between. Unregister fb_id after Validate to avoid carrying over stale fds as keys to Commit Change-Id: Ic26a7644ffb1fb4da26b24364a33ac9293031867 CRs-Fixed: 2157977 --- sdm/libs/core/drm/hw_device_drm.cpp | 25 ++++++++++++++++--------- sdm/libs/core/drm/hw_device_drm.h | 13 ++++++++----- sdm/libs/core/drm/hw_virtual_drm.cpp | 6 ++++-- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/sdm/libs/core/drm/hw_device_drm.cpp b/sdm/libs/core/drm/hw_device_drm.cpp index c2beb7a8..48bf02b3 100644 --- a/sdm/libs/core/drm/hw_device_drm.cpp +++ b/sdm/libs/core/drm/hw_device_drm.cpp @@ -241,7 +241,7 @@ HWDeviceDRM::Registry::~Registry() { delete [] hashmap_; } -void HWDeviceDRM::Registry::RegisterCurrent(HWLayers *hw_layers) { +void HWDeviceDRM::Registry::Register(HWLayers *hw_layers) { HWLayersInfo &hw_layer_info = hw_layers->info; uint32_t hw_layer_count = UINT32(hw_layer_info.hw_layers.size()); @@ -292,7 +292,11 @@ void HWDeviceDRM::Registry::MapBufferToFbId(LayerBuffer* buffer) { return; } -void HWDeviceDRM::Registry::UnregisterNext() { +void HWDeviceDRM::Registry::Next() { + current_index_ = (current_index_ + 1) % rmfb_delay_; +} + +void HWDeviceDRM::Registry::Unregister() { DRMMaster *master = nullptr; DRMMaster::GetInstance(&master); @@ -301,7 +305,6 @@ void HWDeviceDRM::Registry::UnregisterNext() { return; } - current_index_ = (current_index_ + 1) % rmfb_delay_; auto &curr_map = hashmap_[current_index_]; for (auto &pair : curr_map) { uint32_t fb_id = pair.second; @@ -316,7 +319,8 @@ void HWDeviceDRM::Registry::UnregisterNext() { void HWDeviceDRM::Registry::Clear() { for (int i = 0; i < rmfb_delay_; i++) { - UnregisterNext(); + Unregister(); + Next(); } current_index_ = 0; } @@ -987,24 +991,26 @@ void HWDeviceDRM::SetSolidfillStages() { DisplayError HWDeviceDRM::Validate(HWLayers *hw_layers) { DTRACE_SCOPED(); - registry_.RegisterCurrent(hw_layers); + DisplayError err = kErrorNone; + registry_.Register(hw_layers); SetupAtomic(hw_layers, true /* validate */); int ret = drm_atomic_intf_->Validate(); if (ret) { DLOGE("failed with error %d for %s", ret, device_name_); vrefresh_ = 0; - return kErrorHardware; + err = kErrorHardware; } - return kErrorNone; + registry_.Unregister(); + return err; } DisplayError HWDeviceDRM::Commit(HWLayers *hw_layers) { DTRACE_SCOPED(); DisplayError err = kErrorNone; - registry_.RegisterCurrent(hw_layers); + registry_.Register(hw_layers); if (default_mode_) { err = DefaultCommit(hw_layers); @@ -1012,7 +1018,8 @@ DisplayError HWDeviceDRM::Commit(HWLayers *hw_layers) { err = AtomicCommit(hw_layers); } - registry_.UnregisterNext(); + registry_.Next(); + registry_.Unregister(); return err; } diff --git a/sdm/libs/core/drm/hw_device_drm.h b/sdm/libs/core/drm/hw_device_drm.h index 5b7576ab..edb5d58d 100644 --- a/sdm/libs/core/drm/hw_device_drm.h +++ b/sdm/libs/core/drm/hw_device_drm.h @@ -137,11 +137,14 @@ class HWDeviceDRM : public HWInterface { public: explicit Registry(BufferAllocator *buffer_allocator); ~Registry(); - // Call on each validate and commit to register layer buffers - void RegisterCurrent(HWLayers *hw_layers); - // Call at the end of draw cycle to clear the next slot for business - void UnregisterNext(); - // Call on display disconnect to release all gem handles and fb_ids + // Called on each Validate and Commit to register layer buffers fds to the slot pointed to by + // current_index_ + void Register(HWLayers *hw_layers); + // Clears the slot pointed to by current_index_ + void Unregister(); + // Moves current_index_ to the next position + void Next(); + // Called on display disconnect to release all gem handles and fb_ids void Clear(); // Maps given fd to FB ID void MapBufferToFbId(LayerBuffer* buffer); diff --git a/sdm/libs/core/drm/hw_virtual_drm.cpp b/sdm/libs/core/drm/hw_virtual_drm.cpp index 28d81bb2..6f7b12b3 100644 --- a/sdm/libs/core/drm/hw_virtual_drm.cpp +++ b/sdm/libs/core/drm/hw_virtual_drm.cpp @@ -137,7 +137,7 @@ DisplayError HWVirtualDRM::Commit(HWLayers *hw_layers) { LayerBuffer *output_buffer = hw_layers->info.stack->output_buffer; DisplayError err = kErrorNone; - registry_.RegisterCurrent(hw_layers); + registry_.Register(hw_layers); registry_.MapBufferToFbId(output_buffer); uint32_t fb_id = registry_.GetFbId(output_buffer->planes[0].fd); @@ -149,7 +149,9 @@ DisplayError HWVirtualDRM::Commit(HWLayers *hw_layers) { if (err != kErrorNone) { DLOGE("Atomic commit failed for crtc_id %d conn_id %d", token_.crtc_id, token_.conn_id); } - registry_.UnregisterNext(); + + registry_.Next(); + registry_.Unregister(); return(err); } From 546f77bed5ea998572e8b492d8c39c05387c7ce6 Mon Sep 17 00:00:00 2001 From: Saurabh Shah Date: Wed, 13 Dec 2017 14:10:39 -0800 Subject: [PATCH 2/8] qdutils/hwc: Remove unused code surrounding fps calculations Remove unused code surrounding fps calculations from qdutils and hwc Change-Id: I1d78a26fdd582ae184fd7367cf692e472283fee1 CRs-fixed: 2157422 --- common.mk | 2 +- libqdutils/Android.mk | 3 +- libqdutils/profiler.cpp | 197 ------------------------- libqdutils/profiler.h | 108 -------------- sdm/libs/hwc2/hwc_session.cpp | 4 - sdm/libs/hwc2/hwc_session_services.cpp | 5 +- 6 files changed, 4 insertions(+), 315 deletions(-) delete mode 100644 libqdutils/profiler.cpp delete mode 100644 libqdutils/profiler.h diff --git a/common.mk b/common.mk index c003fdef..c1b13ec1 100644 --- a/common.mk +++ b/common.mk @@ -5,7 +5,7 @@ display_config_version := $(shell \ then echo DISPLAY_CONFIG_1_1; fi) #Common C flags -common_flags := -DDEBUG_CALC_FPS -Wno-missing-field-initializers +common_flags := -Wno-missing-field-initializers common_flags += -Wconversion -Wall -Werror -std=c++14 common_flags += -DUSE_GRALLOC1 ifeq ($(TARGET_IS_HEADLESS), true) diff --git a/libqdutils/Android.mk b/libqdutils/Android.mk index 5dac5810..820ed87c 100644 --- a/libqdutils/Android.mk +++ b/libqdutils/Android.mk @@ -13,8 +13,7 @@ LOCAL_CFLAGS := $(common_flags) -DLOG_TAG=\"qdutils\" -Wno-sign LOCAL_ADDITIONAL_DEPENDENCIES := $(common_deps) LOCAL_COPY_HEADERS_TO := $(common_header_export_path) LOCAL_COPY_HEADERS := display_config.h qd_utils.h -LOCAL_SRC_FILES := profiler.cpp \ - qd_utils.cpp \ +LOCAL_SRC_FILES := qd_utils.cpp \ display_config.cpp include $(BUILD_SHARED_LIBRARY) diff --git a/libqdutils/profiler.cpp b/libqdutils/profiler.cpp deleted file mode 100644 index 810b0198..00000000 --- a/libqdutils/profiler.cpp +++ /dev/null @@ -1,197 +0,0 @@ -/* - * Copyright (c) 2011-2012, The Linux Foundation. All rights reserved. - - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following - * disclaimer in the documentation and/or other materials provided - * with the distribution. - * * Neither the name of The Linux Foundation nor the names of its - * contributors may be used to endorse or promote products derived - * from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED - * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS - * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR - * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, - * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE - * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN - * IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#define LOG_NDDEBUG 0 -#define __STDC_FORMAT_MACROS 1 -#include - -#include "profiler.h" - -#ifdef DEBUG_CALC_FPS - - -ANDROID_SINGLETON_STATIC_INSTANCE(qdutils::CalcFps) ; - -namespace qdutils { - -CalcFps::CalcFps() { - debug_fps_level = 0; - Init(); -} - -CalcFps::~CalcFps() { -} - -void CalcFps::Init() { - char prop[PROPERTY_VALUE_MAX]; - property_get("debug.gr.calcfps", prop, "0"); - debug_fps_level = atoi(prop); - if (debug_fps_level > MAX_DEBUG_FPS_LEVEL) { - ALOGW("out of range value for debug.gr.calcfps, using 0"); - debug_fps_level = 0; - } - - ALOGD("DEBUG_CALC_FPS: %d", debug_fps_level); - populate_debug_fps_metadata(); -} - -void CalcFps::Fps() { - if (debug_fps_level > 0) - calc_fps(ns2us(systemTime())); -} - -void CalcFps::populate_debug_fps_metadata(void) -{ - char prop[PROPERTY_VALUE_MAX]; - - /*defaults calculation of fps to based on number of frames*/ - property_get("debug.gr.calcfps.type", prop, "0"); - debug_fps_metadata.type = (debug_fps_metadata_t::DfmType) atoi(prop); - - /*defaults to 1000ms*/ - property_get("debug.gr.calcfps.timeperiod", prop, "1000"); - debug_fps_metadata.time_period = atoi(prop); - - property_get("debug.gr.calcfps.period", prop, "10"); - debug_fps_metadata.period = atoi(prop); - - if (debug_fps_metadata.period > MAX_FPS_CALC_PERIOD_IN_FRAMES) { - debug_fps_metadata.period = MAX_FPS_CALC_PERIOD_IN_FRAMES; - } - - /* default ignorethresh_us: 500 milli seconds */ - property_get("debug.gr.calcfps.ignorethresh_us", prop, "500000"); - debug_fps_metadata.ignorethresh_us = atoi(prop); - - debug_fps_metadata.framearrival_steps = - (unsigned int)(debug_fps_metadata.ignorethresh_us / 16666); - - if (debug_fps_metadata.framearrival_steps > MAX_FRAMEARRIVAL_STEPS) { - debug_fps_metadata.framearrival_steps = MAX_FRAMEARRIVAL_STEPS; - debug_fps_metadata.ignorethresh_us = - debug_fps_metadata.framearrival_steps * 16666; - } - - /* 2ms margin of error for the gettimeofday */ - debug_fps_metadata.margin_us = 2000; - - for (unsigned int i = 0; i < MAX_FRAMEARRIVAL_STEPS; i++) - debug_fps_metadata.accum_framearrivals[i] = 0; - - debug_fps_metadata.curr_frame = 0; - - ALOGD("period: %u", debug_fps_metadata.period); - ALOGD("ignorethresh_us: %" PRId64, debug_fps_metadata.ignorethresh_us); -} - -void CalcFps::print_fps(float fps) -{ - if (debug_fps_metadata_t::DFM_FRAMES == debug_fps_metadata.type) - ALOGD("FPS for last %d frames: %3.2f", debug_fps_metadata.period, fps); - else - ALOGD("FPS for last (%f ms, %d frames): %3.2f", - debug_fps_metadata.time_elapsed, - debug_fps_metadata.curr_frame, fps); - - debug_fps_metadata.curr_frame = 0; - debug_fps_metadata.time_elapsed = 0.0; - - if (debug_fps_level > 1) { - ALOGD("Frame Arrival Distribution:"); - for (unsigned int i = 0; - i < ((debug_fps_metadata.framearrival_steps / 6) + 1); - i++) { - ALOGD("%" PRId64" %" PRId64" %" PRId64" %" PRId64" %" PRId64" %" PRId64, - debug_fps_metadata.accum_framearrivals[i*6], - debug_fps_metadata.accum_framearrivals[i*6+1], - debug_fps_metadata.accum_framearrivals[i*6+2], - debug_fps_metadata.accum_framearrivals[i*6+3], - debug_fps_metadata.accum_framearrivals[i*6+4], - debug_fps_metadata.accum_framearrivals[i*6+5]); - } - - /* We are done with displaying, now clear the stats */ - for (unsigned int i = 0; - i < debug_fps_metadata.framearrival_steps; - i++) - debug_fps_metadata.accum_framearrivals[i] = 0; - } - return; -} - -void CalcFps::calc_fps(nsecs_t currtime_us) -{ - static nsecs_t oldtime_us = 0; - - nsecs_t diff = currtime_us - oldtime_us; - - oldtime_us = currtime_us; - - if (debug_fps_metadata_t::DFM_FRAMES == debug_fps_metadata.type && - diff > debug_fps_metadata.ignorethresh_us) { - return; - } - - if (debug_fps_metadata.curr_frame < MAX_FPS_CALC_PERIOD_IN_FRAMES) { - debug_fps_metadata.framearrivals[debug_fps_metadata.curr_frame] = diff; - } - - debug_fps_metadata.curr_frame++; - - if (debug_fps_level > 1) { - unsigned int currstep = - (unsigned int)(diff + debug_fps_metadata.margin_us) / 16666; - - if (currstep < debug_fps_metadata.framearrival_steps) { - debug_fps_metadata.accum_framearrivals[currstep-1]++; - } - } - - if (debug_fps_metadata_t::DFM_FRAMES == debug_fps_metadata.type) { - if (debug_fps_metadata.curr_frame == debug_fps_metadata.period) { - /* time to calculate and display FPS */ - nsecs_t sum = 0; - for (unsigned int i = 0; i < debug_fps_metadata.period; i++) - sum += debug_fps_metadata.framearrivals[i]; - print_fps(float(float(debug_fps_metadata.period * 1000000) / - (float)sum)); - } - } - else if (debug_fps_metadata_t::DFM_TIME == debug_fps_metadata.type) { - debug_fps_metadata.time_elapsed += (float)((float)diff/1000.0); - if (debug_fps_metadata.time_elapsed >= debug_fps_metadata.time_period) { - float fps = float(1000.0 * debug_fps_metadata.curr_frame/ - debug_fps_metadata.time_elapsed); - print_fps(fps); - } - } - return; -} -};//namespace qomutils -#endif diff --git a/libqdutils/profiler.h b/libqdutils/profiler.h deleted file mode 100644 index 5f270b02..00000000 --- a/libqdutils/profiler.h +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright (c) 2011-2012, The Linux Foundation. All rights reserved. - - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following - * disclaimer in the documentation and/or other materials provided - * with the distribution. - * * Neither the name of The Linux Foundation nor the names of its - * contributors may be used to endorse or promote products derived - * from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED - * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS - * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR - * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, - * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE - * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN - * IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#ifndef INCLUDE_PROFILER -#define INCLUDE_PROFILER - -#include -#include -#include -#include - -#ifndef DEBUG_CALC_FPS -#define CALC_FPS() ((void)0) -#define CALC_INIT() ((void)0) -#else -#define CALC_FPS() qdutils::CalcFps::getInstance().Fps() -#define CALC_INIT() qdutils::CalcFps::getInstance().Init() -using namespace android; -namespace qdutils { -class CalcFps : public Singleton { - public: - CalcFps(); - ~CalcFps(); - - void Init(); - void Fps(); - - private: - static const unsigned int MAX_FPS_CALC_PERIOD_IN_FRAMES = 128; - static const unsigned int MAX_FRAMEARRIVAL_STEPS = 50; - static const unsigned int MAX_DEBUG_FPS_LEVEL = 2; - - struct debug_fps_metadata_t { - /*fps calculation based on time or number of frames*/ - enum DfmType { - DFM_FRAMES = 0, - DFM_TIME = 1, - }; - - DfmType type; - - /* indicates how much time do we wait till we calculate FPS */ - unsigned long time_period; - - /*indicates how much time elapsed since we report fps*/ - float time_elapsed; - - /* indicates how many frames do we wait till we calculate FPS */ - unsigned int period; - /* current frame, will go upto period, and then reset */ - unsigned int curr_frame; - /* frame will arrive at a multiple of 16666 us at the display. - This indicates how many steps to consider for our calculations. - For example, if framearrival_steps = 10, then the frame that arrived - after 166660 us or more will be ignored. - */ - unsigned int framearrival_steps; - /* ignorethresh_us = framearrival_steps * 16666 */ - nsecs_t ignorethresh_us; - /* used to calculate the actual frame arrival step, the times might not be - accurate - */ - unsigned int margin_us; - - /* actual data storage */ - nsecs_t framearrivals[MAX_FPS_CALC_PERIOD_IN_FRAMES]; - nsecs_t accum_framearrivals[MAX_FRAMEARRIVAL_STEPS]; - }; - - private: - void populate_debug_fps_metadata(void); - void print_fps(float fps); - void calc_fps(nsecs_t currtime_us); - - private: - debug_fps_metadata_t debug_fps_metadata; - unsigned int debug_fps_level; -}; -};//namespace qdutils -#endif - -#endif // INCLUDE_PROFILER diff --git a/sdm/libs/hwc2/hwc_session.cpp b/sdm/libs/hwc2/hwc_session.cpp index 4138cc6b..24a75000 100644 --- a/sdm/libs/hwc2/hwc_session.cpp +++ b/sdm/libs/hwc2/hwc_session.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -544,9 +543,6 @@ int32_t HWCSession::PresentDisplay(hwc2_device_t *device, hwc2_display_t display // TODO(user): Handle virtual display/HDMI concurrency if (hwc_session->hwc_display_[display]) { status = hwc_session->hwc_display_[display]->Present(out_retire_fence); - // This is only indicative of how many times SurfaceFlinger posts - // frames to the display. - CALC_FPS(); } } diff --git a/sdm/libs/hwc2/hwc_session_services.cpp b/sdm/libs/hwc2/hwc_session_services.cpp index 4cef8845..69847118 100644 --- a/sdm/libs/hwc2/hwc_session_services.cpp +++ b/sdm/libs/hwc2/hwc_session_services.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include "hwc_buffer_sync_handler.h" #include "hwc_session.h" @@ -42,8 +41,8 @@ namespace sdm { using ::android::hardware::Void; void HWCSession::StartServices() { - status_t status = IDisplayConfig::registerAsService(); - if (status != OK) { + android::status_t status = IDisplayConfig::registerAsService(); + if (status != android::OK) { ALOGW("%s::%s: Could not register IDisplayConfig as service (%d).", __CLASS__, __FUNCTION__, status); } else { From dbb572e6fbfb8d86601a55b1034ad29db23bd6b8 Mon Sep 17 00:00:00 2001 From: Saurabh Shah Date: Thu, 4 Jan 2018 15:38:01 -0800 Subject: [PATCH 3/8] gralloc: Add missing format HAL_PIXEL_FORMAT_YCrCb_420_SP_VENUS Add missing HAL_PIXEL_FORMAT_YCrCb_420_SP_VENUS in YUV format checks. Move IsYUVFormat utility to grallocutils. Change-Id: I8103ba2aead0d00aede21a0b43a59af741b2df95 CRs-fixed: 2166182 --- libgralloc1/gr_buf_mgr.cpp | 27 --------------------------- libgralloc1/gr_utils.cpp | 28 ++++++++++++++++++++++++++++ libgralloc1/gr_utils.h | 1 + 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/libgralloc1/gr_buf_mgr.cpp b/libgralloc1/gr_buf_mgr.cpp index c5d91d9b..189f19ef 100644 --- a/libgralloc1/gr_buf_mgr.cpp +++ b/libgralloc1/gr_buf_mgr.cpp @@ -829,33 +829,6 @@ gralloc1_error_t BufferManager::Perform(int operation, va_list args) { return GRALLOC1_ERROR_NONE; } -static bool IsYuvFormat(const private_handle_t *hnd) { - switch (hnd->format) { - case HAL_PIXEL_FORMAT_YCbCr_420_SP: - case HAL_PIXEL_FORMAT_YCbCr_422_SP: - case HAL_PIXEL_FORMAT_YCbCr_420_SP_VENUS: - case HAL_PIXEL_FORMAT_NV12_ENCODEABLE: // Same as YCbCr_420_SP_VENUS - case HAL_PIXEL_FORMAT_YCbCr_420_SP_VENUS_UBWC: - case HAL_PIXEL_FORMAT_YCrCb_420_SP: - case HAL_PIXEL_FORMAT_YCrCb_422_SP: - case HAL_PIXEL_FORMAT_YCrCb_420_SP_ADRENO: - case HAL_PIXEL_FORMAT_NV21_ZSL: - case HAL_PIXEL_FORMAT_RAW16: - case HAL_PIXEL_FORMAT_Y16: - case HAL_PIXEL_FORMAT_RAW12: - case HAL_PIXEL_FORMAT_RAW10: - case HAL_PIXEL_FORMAT_YV12: - case HAL_PIXEL_FORMAT_Y8: - case HAL_PIXEL_FORMAT_YCbCr_420_P010: - case HAL_PIXEL_FORMAT_YCbCr_420_TP10_UBWC: - case HAL_PIXEL_FORMAT_YCbCr_420_P010_UBWC: - case HAL_PIXEL_FORMAT_YCbCr_420_P010_VENUS: - return true; - default: - return false; - } -} - gralloc1_error_t BufferManager::GetNumFlexPlanes(const private_handle_t *hnd, uint32_t *out_num_planes) { if (!IsYuvFormat(hnd)) { diff --git a/libgralloc1/gr_utils.cpp b/libgralloc1/gr_utils.cpp index 6af5df60..480ad984 100644 --- a/libgralloc1/gr_utils.cpp +++ b/libgralloc1/gr_utils.cpp @@ -42,6 +42,34 @@ namespace gralloc1 { +bool IsYuvFormat(const private_handle_t *hnd) { + switch (hnd->format) { + case HAL_PIXEL_FORMAT_YCbCr_420_SP: + case HAL_PIXEL_FORMAT_YCbCr_422_SP: + case HAL_PIXEL_FORMAT_YCbCr_420_SP_VENUS: + case HAL_PIXEL_FORMAT_NV12_ENCODEABLE: // Same as YCbCr_420_SP_VENUS + case HAL_PIXEL_FORMAT_YCbCr_420_SP_VENUS_UBWC: + case HAL_PIXEL_FORMAT_YCrCb_420_SP: + case HAL_PIXEL_FORMAT_YCrCb_422_SP: + case HAL_PIXEL_FORMAT_YCrCb_420_SP_ADRENO: + case HAL_PIXEL_FORMAT_YCrCb_420_SP_VENUS: + case HAL_PIXEL_FORMAT_NV21_ZSL: + case HAL_PIXEL_FORMAT_RAW16: + case HAL_PIXEL_FORMAT_Y16: + case HAL_PIXEL_FORMAT_RAW12: + case HAL_PIXEL_FORMAT_RAW10: + case HAL_PIXEL_FORMAT_YV12: + case HAL_PIXEL_FORMAT_Y8: + case HAL_PIXEL_FORMAT_YCbCr_420_P010: + case HAL_PIXEL_FORMAT_YCbCr_420_TP10_UBWC: + case HAL_PIXEL_FORMAT_YCbCr_420_P010_UBWC: + case HAL_PIXEL_FORMAT_YCbCr_420_P010_VENUS: + return true; + default: + return false; + } +} + bool IsUncompressedRGBFormat(int format) { switch (format) { case HAL_PIXEL_FORMAT_RGBA_8888: diff --git a/libgralloc1/gr_utils.h b/libgralloc1/gr_utils.h index 4b2f1368..7fcb6c2c 100644 --- a/libgralloc1/gr_utils.h +++ b/libgralloc1/gr_utils.h @@ -60,6 +60,7 @@ inline Type1 ALIGN(Type1 x, Type2 align) { return (Type1)((x + (Type1)align - 1) & ~((Type1)align - 1)); } +bool IsYuvFormat(const private_handle_t *hnd); bool IsCompressedRGBFormat(int format); bool IsUncompressedRGBFormat(int format); uint32_t GetBppForUncompressedRGB(int format); From b338690ade9496abed6b353068f04a767d6fdc3d Mon Sep 17 00:00:00 2001 From: Ramkumar Radhakrishnan Date: Tue, 19 Dec 2017 17:14:13 -0800 Subject: [PATCH 4/8] hwc2: Fix acquire fence fd leak in SetLayerBuffer() Close acquire fence fd of the null buffer with client requested composition is DEVICE/CURSOR Change-Id: I40a4a0ddc413af797a04fff39a2f0b5c4c9396af CRs-Fixed: 2167175 --- sdm/libs/hwc2/hwc_layers.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdm/libs/hwc2/hwc_layers.cpp b/sdm/libs/hwc2/hwc_layers.cpp index 51384f93..0bcffc62 100644 --- a/sdm/libs/hwc2/hwc_layers.cpp +++ b/sdm/libs/hwc2/hwc_layers.cpp @@ -195,7 +195,9 @@ HWC2::Error HWCLayer::SetLayerBuffer(buffer_handle_t buffer, int32_t acquire_fen if (!buffer) { if (client_requested_ == HWC2::Composition::Device || client_requested_ == HWC2::Composition::Cursor) { - DLOGE("Invalid buffer handle: %p on layer: %d", buffer, id_); + DLOGE("Invalid buffer handle: %p on layer: %d client requested comp type %d", buffer, id_, + client_requested_); + ::close(acquire_fence); return HWC2::Error::BadParameter; } else { return HWC2::Error::None; From e769e8e7a1cd099b7ede60f2ff9530b4bafd7dc8 Mon Sep 17 00:00:00 2001 From: Ramkumar Radhakrishnan Date: Tue, 5 Dec 2017 01:20:02 -0800 Subject: [PATCH 5/8] sdm: Add virtual flag in HWPipeInfo to find pipe pairs Change-Id: Id2a867a10991341f7e52c78ca721e9db3e1685bf CRs-Fixed: 2154113 --- sdm/include/private/hw_info_types.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdm/include/private/hw_info_types.h b/sdm/include/private/hw_info_types.h index d2d4b0b7..5d311d93 100644 --- a/sdm/include/private/hw_info_types.h +++ b/sdm/include/private/hw_info_types.h @@ -1,5 +1,5 @@ /* -* Copyright (c) 2015-2017, The Linux Foundation. All rights reserved. +* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. * * Redistribution and use in source and binary forms, with or without modification, are permitted * provided that the following conditions are met: @@ -485,6 +485,7 @@ struct HWPipeInfo { uint32_t z_order = 0; uint8_t flags = 0; bool valid = false; + bool is_virtual = 0; void Reset() { *this = HWPipeInfo(); } }; From 00a40ba6e2f032f2054c4b024a6b0fbf04d38051 Mon Sep 17 00:00:00 2001 From: Saurabh Shah Date: Fri, 5 Jan 2018 11:05:35 -0800 Subject: [PATCH 6/8] sdm: Fix multiple VBlank registration Currently VBlank registration happens even if polling thread wakes up for other events. This causes multiple wake-ups on actual VBlank. This change registers for VBlank in VBlank handler and on VBlank enable, if it's not already registered. Change-Id: I4aada5a5bd28382d60c68865c20eaabda5325ccf CRs-fixed: 2167257 --- sdm/libs/core/drm/hw_events_drm.cpp | 30 ++++++++++++++++++----------- sdm/libs/core/drm/hw_events_drm.h | 5 ++++- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/sdm/libs/core/drm/hw_events_drm.cpp b/sdm/libs/core/drm/hw_events_drm.cpp index 7a3d3851..86a3f544 100644 --- a/sdm/libs/core/drm/hw_events_drm.cpp +++ b/sdm/libs/core/drm/hw_events_drm.cpp @@ -1,5 +1,5 @@ /* -* Copyright (c) 2017, The Linux Foundation. All rights reserved. +* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -182,7 +182,6 @@ DisplayError HWEventsDRM::Init(int display_type, HWEventHandler *event_handler, static_cast(hw_intf)->GetDRMDisplayToken(&token_); is_primary_ = static_cast(hw_intf)->IsPrimaryDisplay(); - vsync_enabled_ = is_primary_; DLOGI("Setup event handler for display %d, CRTC %d, Connector %d", display_type, token_.crtc_id, token_.conn_id); @@ -198,6 +197,11 @@ DisplayError HWEventsDRM::Init(int display_type, HWEventHandler *event_handler, return kErrorResources; } + if (is_primary_) { + RegisterVSync(); + vsync_registered_ = true; + } + RegisterPanelDead(true); RegisterIdleNotify(true); RegisterIdlePowerCollapse(true); @@ -220,15 +224,17 @@ DisplayError HWEventsDRM::Deinit() { DisplayError HWEventsDRM::SetEventState(HWEvent event, bool enable, void *arg) { switch (event) { - case HWEvent::VSYNC: + case HWEvent::VSYNC: { + std::lock_guard lock(vsync_mutex_); if (!is_primary_) { break; } vsync_enabled_ = enable; - if (enable) { - WakeUpEventThread(); + if (vsync_enabled_ && !vsync_registered_) { + RegisterVSync(); + vsync_registered_ = true; } - break; + } break; default: DLOGE("Event not supported"); return kErrorNotSupported; @@ -295,11 +301,6 @@ void *HWEventsDRM::DisplayEventHandler() { setpriority(PRIO_PROCESS, 0, kThreadPriorityUrgent); while (!exit_threads_) { - if (vsync_enabled_ && RegisterVSync() != kErrorNone) { - pthread_exit(0); - return nullptr; - } - int error = Sys::poll_(poll_fds_.data(), UINT32(poll_fds_.size()), -1); if (error <= 0) { DLOGW("poll failed. error = %s", strerror(errno)); @@ -469,6 +470,13 @@ void HWEventsDRM::HandleVSync(char *data) { if (error != 0) { DLOGE("drmHandleEvent failed: %i", error); } + + std::lock_guard lock(vsync_mutex_); + vsync_registered_ = false; + if (vsync_enabled_) { + RegisterVSync(); + vsync_registered_ = true; + } } void HWEventsDRM::HandlePanelDead(char *data) { diff --git a/sdm/libs/core/drm/hw_events_drm.h b/sdm/libs/core/drm/hw_events_drm.h index c54ac6fb..fafe606b 100644 --- a/sdm/libs/core/drm/hw_events_drm.h +++ b/sdm/libs/core/drm/hw_events_drm.h @@ -1,5 +1,5 @@ /* -* Copyright (c) 2017, The Linux Foundation. All rights reserved. +* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -94,6 +95,8 @@ class HWEventsDRM : public HWEventsInterface { bool exit_threads_ = false; uint32_t vsync_index_ = 0; bool vsync_enabled_ = false; + bool vsync_registered_ = false; + std::mutex vsync_mutex_; // To protect vsync_enabled_ and vsync_registered_ uint32_t idle_notify_index_ = 0; sde_drm::DRMDisplayToken token_ = {}; bool is_primary_ = false; From 9c72e07adcf66227426ab7508e5baa20b5e6e682 Mon Sep 17 00:00:00 2001 From: Saurabh Shah Date: Wed, 10 Jan 2018 18:27:13 -0800 Subject: [PATCH 7/8] hwc: Allow callback deregistration ComposerClient calls RegisterCallbacks() with nullptr to deregister callbacks when SF dies. Allow this operation to avoid crashes since HWC is still active. Change-Id: I431d232130ec8cfb06cc40e81ef4e733496aa442 CRs-fixed: 2165931 --- sdm/libs/hwc2/hwc_session.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdm/libs/hwc2/hwc_session.cpp b/sdm/libs/hwc2/hwc_session.cpp index 4138cc6b..2a84c0dc 100644 --- a/sdm/libs/hwc2/hwc_session.cpp +++ b/sdm/libs/hwc2/hwc_session.cpp @@ -577,14 +577,14 @@ int32_t HWCSession::PresentDisplay(hwc2_device_t *device, hwc2_display_t display int32_t HWCSession::RegisterCallback(hwc2_device_t *device, int32_t descriptor, hwc2_callback_data_t callback_data, hwc2_function_pointer_t pointer) { - if (!device || pointer == nullptr) { + if (!device) { return HWC2_ERROR_BAD_PARAMETER; } HWCSession *hwc_session = static_cast(device); SCOPE_LOCK(hwc_session->callbacks_lock_); auto desc = static_cast(descriptor); auto error = hwc_session->callbacks_.Register(desc, callback_data, pointer); - DLOGD("Registering callback: %s", to_string(desc).c_str()); + DLOGD("%s callback: %s", pointer ? "Registering" : "Deregistering", to_string(desc).c_str()); if (descriptor == HWC2_CALLBACK_HOTPLUG) { if (hwc_session->hwc_display_[HWC_DISPLAY_PRIMARY]) { hwc_session->callbacks_.Hotplug(HWC_DISPLAY_PRIMARY, HWC2::Connection::Connected); From 07254304fa7f457e1ba76105676e0a25027aa4be Mon Sep 17 00:00:00 2001 From: Ramkumar Radhakrishnan Date: Mon, 13 Nov 2017 16:18:22 -0800 Subject: [PATCH 8/8] sdm: Set multirect mode to driver via plane property Change-Id: Ia3b33e8761392ca211f210e6600a47dde9c768e0 CRs-Fixed: 2166036 --- libdrmutils/drm_interface.h | 13 +++++++++++++ sdm/libs/core/drm/hw_device_drm.cpp | 16 +++++++++++++++- sdm/libs/core/drm/hw_device_drm.h | 1 + sdm/libs/core/drm/hw_info_drm.cpp | 2 +- 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/libdrmutils/drm_interface.h b/libdrmutils/drm_interface.h index 01981ca8..97660ea9 100644 --- a/libdrmutils/drm_interface.h +++ b/libdrmutils/drm_interface.h @@ -143,6 +143,12 @@ enum struct DRMOps { * uint32_t* - pointer to csc type */ PLANE_SET_CSC_CONFIG, + /* + * Op: Sets multirect mode on this plane. + * Arg: uint32_t - Plane ID + * uint32_t - multirect mode + */ + PLANE_SET_MULTIRECT_MODE, /* * Op: Activate or deactivate a CRTC * Arg: uint32_t - CRTC ID @@ -429,6 +435,7 @@ struct DRMPlaneTypeInfo { uint64_t max_pipe_bandwidth; uint32_t cache_size; // cache size in bytes for inline rotation support. QSEEDStepVersion qseed3_version; + bool multirect_prop_present = false; }; // All DRM Planes as map listed from highest to lowest priority @@ -556,6 +563,12 @@ enum struct DRMSecurityLevel { SECURE_ONLY, }; +enum struct DRMMultiRectMode { + NONE = 0, + PARALLEL = 1, + SERIAL = 2, +}; + struct DRMSolidfillStage { DRMRect bounding_rect {}; bool is_exclusion_rect = false; diff --git a/sdm/libs/core/drm/hw_device_drm.cpp b/sdm/libs/core/drm/hw_device_drm.cpp index 48bf02b3..c734f4cd 100644 --- a/sdm/libs/core/drm/hw_device_drm.cpp +++ b/sdm/libs/core/drm/hw_device_drm.cpp @@ -98,6 +98,7 @@ using sde_drm::DRMPowerMode; using sde_drm::DRMSecureMode; using sde_drm::DRMSecurityLevel; using sde_drm::DRMCscType; +using sde_drm::DRMMultiRectMode; namespace sdm { @@ -901,6 +902,10 @@ void HWDeviceDRM::SetupAtomic(HWLayers *hw_layers, bool validate) { DRMCscType csc_type = DRMCscType::kCscTypeMax; SelectCscType(layer.input_buffer, &csc_type); drm_atomic_intf_->Perform(DRMOps::PLANE_SET_CSC_CONFIG, pipe_id, &csc_type); + + DRMMultiRectMode multirect_mode; + SetMultiRectMode(pipe_info->flags, &multirect_mode); + drm_atomic_intf_->Perform(DRMOps::PLANE_SET_MULTIRECT_MODE, pipe_id, multirect_mode); } } } @@ -1152,7 +1157,6 @@ void HWDeviceDRM::SetBlending(const LayerBlending &source, DRMBlendType *target) } } - void HWDeviceDRM::SetSrcConfig(const LayerBuffer &input_buffer, const HWRotatorMode &mode, uint32_t *config) { // In offline rotation case, rotator will handle deinterlacing. @@ -1535,4 +1539,14 @@ void HWDeviceDRM::SetTopology(sde_drm::DRMTopology drm_topology, HWTopology *hw_ } } +void HWDeviceDRM::SetMultiRectMode(const uint32_t flags, DRMMultiRectMode *target) { + *target = DRMMultiRectMode::NONE; + if (flags & kMultiRect) { + *target = DRMMultiRectMode::SERIAL; + if (flags & kMultiRectParallelMode) { + *target = DRMMultiRectMode::PARALLEL; + } + } +} + } // namespace sdm diff --git a/sdm/libs/core/drm/hw_device_drm.h b/sdm/libs/core/drm/hw_device_drm.h index edb5d58d..b08c0e66 100644 --- a/sdm/libs/core/drm/hw_device_drm.h +++ b/sdm/libs/core/drm/hw_device_drm.h @@ -132,6 +132,7 @@ class HWDeviceDRM : public HWInterface { sde_drm::DRMSecurityLevel *security_level); bool IsResolutionSwitchEnabled() const { return resolution_switch_enabled_; } void SetTopology(sde_drm::DRMTopology drm_topology, HWTopology *hw_topology); + void SetMultiRectMode(const uint32_t flags, sde_drm::DRMMultiRectMode *target); class Registry { public: diff --git a/sdm/libs/core/drm/hw_info_drm.cpp b/sdm/libs/core/drm/hw_info_drm.cpp index 269cc16a..5c69ed7b 100644 --- a/sdm/libs/core/drm/hw_info_drm.cpp +++ b/sdm/libs/core/drm/hw_info_drm.cpp @@ -399,7 +399,7 @@ void HWInfoDRM::GetHWPlanesInfo(HWResourceInfo *hw_resource) { } pipe_caps.id = pipe_obj.first; pipe_caps.master_pipe_id = pipe_obj.second.master_plane_id; - DLOGI("Adding %s Pipe : Id %d, master_pipe_id : Id %d", + DLOGI("Adding %s Pipe : Id %x, master_pipe_id : Id %x", name.c_str(), pipe_obj.first, pipe_obj.second.master_plane_id); hw_resource->hw_pipes.push_back(std::move(pipe_caps)); }