Commit 755b2a25 authored by Mark Yacoub's avatar Mark Yacoub Committed by Commit Bot

Revert "Disable Render Buffer Compression on external displays."

This reverts commit 28d8162b.

Reason for revert: Getting DRM connector causes a significant delay when
HDMI is connector to Kled. This code is not in use at the moment as
Rendering Buffer Compression is not currently enabled on Intel devices.

Original change's description:
>Disable Render Buffer Compression on external displays.
>
>When Getting format modifiers, exclude I915_FORMAT_MOD_Y*_TILED_CCS
>(Intel color control surface (CCS) for render compression) if it's not
>an internal display.
>This solves the bandwidth issue when driving more than 1 4K monitor with
>rbc enabled.
>
>BUG=996011,996036,994341,979736
>TEST=HardwareDisplayControllerTest.ModifiersWithConnectorType
>
>Change-Id: I31ce046fb20557eb66340bdf131efa1437bd7484
>Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1918941
>Commit-Queue: Mark Yacoub <markyacoub@google.com>
>Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
>Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
>Cr-Commit-Position: refs/heads/master@{#715717}

BUG=979736,b/146874982

Change-Id: I3083e08501ef91cb395c4e2f3940d0a5cbcfd1de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2159758Reviewed-by: default avatarDaniele Castagna <dcastagna@chromium.org>
Commit-Queue: Mark Yacoub <markyacoub@google.com>
Cr-Commit-Position: refs/heads/master@{#761960}
parent cb562e2f
......@@ -109,6 +109,28 @@ float GetRefreshRate(const drmModeModeInfo& mode) {
return (clock * 1000.0f) / (htotal * vtotal);
}
display::DisplayConnectionType GetDisplayType(drmModeConnector* connector) {
switch (connector->connector_type) {
case DRM_MODE_CONNECTOR_VGA:
return display::DISPLAY_CONNECTION_TYPE_VGA;
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_DVIA:
return display::DISPLAY_CONNECTION_TYPE_DVI;
case DRM_MODE_CONNECTOR_LVDS:
case DRM_MODE_CONNECTOR_eDP:
case DRM_MODE_CONNECTOR_DSI:
return display::DISPLAY_CONNECTION_TYPE_INTERNAL;
case DRM_MODE_CONNECTOR_DisplayPort:
return display::DISPLAY_CONNECTION_TYPE_DISPLAYPORT;
case DRM_MODE_CONNECTOR_HDMIA:
case DRM_MODE_CONNECTOR_HDMIB:
return display::DISPLAY_CONNECTION_TYPE_HDMI;
default:
return display::DISPLAY_CONNECTION_TYPE_UNKNOWN;
}
}
int GetDrmProperty(int fd,
drmModeConnector* connector,
const std::string& name,
......@@ -424,29 +446,6 @@ display::DisplaySnapshot::DisplayModeList ExtractDisplayModes(
return modes;
}
display::DisplayConnectionType GetDisplayType(
const drmModeConnector* connector) {
switch (connector->connector_type) {
case DRM_MODE_CONNECTOR_VGA:
return display::DISPLAY_CONNECTION_TYPE_VGA;
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_DVIA:
return display::DISPLAY_CONNECTION_TYPE_DVI;
case DRM_MODE_CONNECTOR_LVDS:
case DRM_MODE_CONNECTOR_eDP:
case DRM_MODE_CONNECTOR_DSI:
return display::DISPLAY_CONNECTION_TYPE_INTERNAL;
case DRM_MODE_CONNECTOR_DisplayPort:
return display::DISPLAY_CONNECTION_TYPE_DISPLAYPORT;
case DRM_MODE_CONNECTOR_HDMIA:
case DRM_MODE_CONNECTOR_HDMIB:
return display::DISPLAY_CONNECTION_TYPE_HDMI;
default:
return display::DISPLAY_CONNECTION_TYPE_UNKNOWN;
}
}
std::unique_ptr<display::DisplaySnapshot> CreateDisplaySnapshot(
HardwareDisplayControllerInfo* info,
int fd,
......
......@@ -71,9 +71,6 @@ display::DisplaySnapshot::DisplayModeList ExtractDisplayModes(
const display::DisplayMode** out_current_mode,
const display::DisplayMode** out_native_mode);
display::DisplayConnectionType GetDisplayType(
const drmModeConnector* connector);
// |info| provides the DRM information related to the display, |fd| is the
// connection to the DRM device.
std::unique_ptr<display::DisplaySnapshot> CreateDisplaySnapshot(
......
......@@ -8,9 +8,7 @@
#include "base/logging.h"
#include "base/time/time.h"
#include "ui/display/types/display_constants.h"
#include "ui/gfx/presentation_feedback.h"
#include "ui/ozone/platform/drm/common/drm_util.h"
#include "ui/ozone/platform/drm/gpu/drm_device.h"
#include "ui/ozone/platform/drm/gpu/drm_dumb_buffer.h"
#include "ui/ozone/platform/drm/gpu/drm_framebuffer.h"
......@@ -24,9 +22,7 @@ CrtcController::CrtcController(const scoped_refptr<DrmDevice>& drm,
uint32_t connector)
: drm_(drm),
crtc_(crtc),
connector_(connector),
internal_diplay_only_modifiers_(
{I915_FORMAT_MOD_Y_TILED_CCS, I915_FORMAT_MOD_Yf_TILED_CCS}) {}
connector_(connector) {}
CrtcController::~CrtcController() {
if (!is_disabled_) {
......@@ -102,21 +98,7 @@ bool CrtcController::AssignOverlayPlanes(HardwareDisplayPlaneList* plane_list,
}
std::vector<uint64_t> CrtcController::GetFormatModifiers(uint32_t format) {
std::vector<uint64_t> modifiers =
drm_->plane_manager()->GetFormatModifiers(crtc_, format);
display::DisplayConnectionType display_type =
ui::GetDisplayType(drm_->GetConnector(connector_).get());
// If this is an external display, remove the modifiers applicable to internal
// displays only.
if (display_type != display::DISPLAY_CONNECTION_TYPE_INTERNAL) {
for (auto modifier : internal_diplay_only_modifiers_) {
modifiers.erase(std::remove(modifiers.begin(), modifiers.end(), modifier),
modifiers.end());
}
}
return modifiers;
return drm_->plane_manager()->GetFormatModifiers(crtc_, format);
}
void CrtcController::SetCursor(uint32_t handle, const gfx::Size& size) {
......
......@@ -78,8 +78,6 @@ class CrtcController {
// TODO(dnicoara) Add support for hardware mirroring (multiple connectors).
const uint32_t connector_;
const std::vector<uint64_t> internal_diplay_only_modifiers_;
drmModeModeInfo mode_ = {};
scoped_refptr<DrmFramebuffer> modeset_framebuffer_;
......
......@@ -168,17 +168,8 @@ void HardwareDisplayControllerTest::InitializeDrmDevice(bool use_atomic) {
{/* .id = */ pair.first, /*.value = */ value});
};
drm_format_modifier y_css = {.formats = 1UL,
.modifier = I915_FORMAT_MOD_Y_TILED_CCS};
drm_format_modifier yf_css = {.formats = 1UL,
.modifier = I915_FORMAT_MOD_Yf_TILED_CCS};
drm_format_modifier x = {.formats = 1UL,
.modifier = I915_FORMAT_MOD_X_TILED};
drm_format_modifier linear = {.formats = 1UL,
.modifier = DRM_FORMAT_MOD_LINEAR};
drm_->SetPropertyBlob(ui::MockDrmDevice::AllocateInFormatsBlob(
kInFormatsBlobPropId, {DRM_FORMAT_XRGB8888},
{y_css, yf_css, x, linear}));
kInFormatsBlobPropId, {DRM_FORMAT_XRGB8888}, {}));
plane_properties.emplace_back(std::move(plane));
}
......@@ -234,86 +225,6 @@ TEST_F(HardwareDisplayControllerTest, CheckModesettingResult) {
EXPECT_FALSE(plane.buffer->HasOneRef());
}
TEST_F(HardwareDisplayControllerTest, ModifiersWithConnectorType) {
ui::DrmOverlayPlane plane(CreateBuffer(), nullptr);
// With internal displays, all modifiers including compressed (css) should be
// there.
drm_->set_connector_type(DRM_MODE_CONNECTOR_eDP);
std::vector<uint64_t> internal_modifiers =
controller_->GetFormatModifiers(DRM_FORMAT_XRGB8888);
ASSERT_FALSE(internal_modifiers.empty());
EXPECT_NE(std::find(internal_modifiers.begin(), internal_modifiers.end(),
I915_FORMAT_MOD_Y_TILED_CCS),
internal_modifiers.end());
EXPECT_NE(std::find(internal_modifiers.begin(), internal_modifiers.end(),
I915_FORMAT_MOD_Yf_TILED_CCS),
internal_modifiers.end());
EXPECT_NE(std::find(internal_modifiers.begin(), internal_modifiers.end(),
I915_FORMAT_MOD_X_TILED),
internal_modifiers.end());
EXPECT_NE(std::find(internal_modifiers.begin(), internal_modifiers.end(),
DRM_FORMAT_MOD_LINEAR),
internal_modifiers.end());
// With external displays, *_CSS modifiers (2 of them) should not exist.
drm_->set_connector_type(DRM_MODE_CONNECTOR_DisplayPort);
std::vector<uint64_t> external_modifiers =
controller_->GetFormatModifiers(DRM_FORMAT_XRGB8888);
ASSERT_FALSE(external_modifiers.empty());
EXPECT_EQ(external_modifiers.size(), internal_modifiers.size() - 2);
EXPECT_EQ(std::find(external_modifiers.begin(), external_modifiers.end(),
I915_FORMAT_MOD_Y_TILED_CCS),
external_modifiers.end());
EXPECT_EQ(std::find(external_modifiers.begin(), external_modifiers.end(),
I915_FORMAT_MOD_Yf_TILED_CCS),
external_modifiers.end());
EXPECT_NE(std::find(internal_modifiers.begin(), internal_modifiers.end(),
I915_FORMAT_MOD_X_TILED),
internal_modifiers.end());
EXPECT_NE(std::find(internal_modifiers.begin(), internal_modifiers.end(),
DRM_FORMAT_MOD_LINEAR),
internal_modifiers.end());
}
TEST_F(HardwareDisplayControllerTest, CheckDisableResetsProps) {
ui::DrmOverlayPlane plane1(CreateBuffer(), nullptr);
EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
ui::DrmOverlayPlane plane2(CreateBuffer(), nullptr);
std::vector<ui::DrmOverlayPlane> planes = {};
planes.push_back(plane2.Clone());
SchedulePageFlip(std::move(planes));
// Test props values after disabling.
controller_->Disable();
ui::DrmDevice::Property connector_prop_crtc_id;
ui::ScopedDrmObjectPropertyPtr connector_props =
drm_->GetObjectProperties(kConnectorIdBase, DRM_MODE_OBJECT_CONNECTOR);
ui::GetDrmPropertyForName(drm_.get(), connector_props.get(), "CRTC_ID",
&connector_prop_crtc_id);
EXPECT_EQ(0U, connector_prop_crtc_id.value);
ui::DrmDevice::Property crtc_prop_for_name;
ui::ScopedDrmObjectPropertyPtr crtc_props =
drm_->GetObjectProperties(kPrimaryCrtc, DRM_MODE_OBJECT_CRTC);
GetDrmPropertyForName(drm_.get(), crtc_props.get(), "ACTIVE",
&crtc_prop_for_name);
EXPECT_EQ(0U, crtc_prop_for_name.value);
crtc_props = drm_->GetObjectProperties(kPrimaryCrtc, DRM_MODE_OBJECT_CRTC);
GetDrmPropertyForName(drm_.get(), crtc_props.get(), "MODE_ID",
&crtc_prop_for_name);
EXPECT_EQ(0U, crtc_prop_for_name.value);
}
TEST_F(HardwareDisplayControllerTest, CheckStateAfterPageFlip) {
ui::DrmOverlayPlane plane1(CreateBuffer(), nullptr);
......
......@@ -239,11 +239,7 @@ bool MockDrmDevice::DisableCrtc(uint32_t crtc_id) {
}
ScopedDrmConnectorPtr MockDrmDevice::GetConnector(uint32_t connector_id) {
ScopedDrmConnectorPtr connector =
ScopedDrmConnectorPtr(DrmAllocator<drmModeConnector>());
connector->connector_id = connector_id;
connector->connector_type = connector_type_;
return connector;
return ScopedDrmConnectorPtr(DrmAllocator<drmModeConnector>());
}
bool MockDrmDevice::AddFramebuffer2(uint32_t width,
......
......@@ -98,8 +98,6 @@ class MockDrmDevice : public DrmDevice {
return it != crtc_cursor_map_.end() ? it->second : 0;
}
void set_connector_type(uint32_t type) { connector_type_ = type; }
void InitializeState(
const std::vector<CrtcProperties>& crtc_properties,
const std::vector<ConnectorProperties>& connector_properties,
......@@ -241,8 +239,6 @@ class MockDrmDevice : public DrmDevice {
std::set<uint32_t> allocated_property_blobs_;
uint32_t connector_type_ = DRM_MODE_CONNECTOR_eDP;
DISALLOW_COPY_AND_ASSIGN(MockDrmDevice);
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment