Mir

Merge lp:~vanvugt/mir/scanout into lp:~mir-team/mir/trunk

Proposed by Daniel van Vugt
Status: Merged
Approved by: Alexandros Frantzis
Approved revision: no longer in the source branch.
Merged at revision: 1063
Proposed branch: lp:~vanvugt/mir/scanout
Merge into: lp:~mir-team/mir/trunk
Diff against target: 287 lines (+111/-3)
13 files modified
include/platform/mir/graphics/buffer_ipc_packer.h (+1/-0)
include/shared/mir_toolkit/mir_native_buffer.h (+10/-1)
include/test/mir_test_doubles/mock_buffer_packer.h (+1/-0)
src/client/mir_surface.cpp (+1/-0)
src/server/frontend/protobuf_buffer_packer.cpp (+6/-0)
src/server/frontend/protobuf_buffer_packer.h (+1/-0)
src/server/graphics/gbm/gbm_buffer.cpp (+1/-0)
src/server/graphics/gbm/gbm_platform.cpp (+1/-0)
src/shared/protobuf/mir_protobuf.proto (+2/-1)
tests/acceptance-tests/test_client_library.cpp (+71/-0)
tests/mir_test_framework/testing_server_options.cpp (+12/-1)
tests/unit-tests/frontend/test_protobuf_buffer_packer.cpp (+2/-0)
tests/unit-tests/graphics/gbm/test_gbm_platform.cpp (+2/-0)
To merge this branch: bzr merge lp:~vanvugt/mir/scanout
Reviewer Review Type Date Requested Status
Robert Ancell Approve
Kevin DuBois (community) Approve
Alan Griffiths Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+184961@code.launchpad.net

Commit message

Add a "flags" field to MirBufferPackage so that clients can find out if the
buffer they've been given is scanout-capable. This is normally something a
client should never need to know. However there are two specialized cases
where it's required to fix bugs in the intel and radeon X drivers:
  LP: #1218735, LP: #1218815
The intel fix (already landed) contains a hack which will be updated after
this branch lands.

Description of the change

The weird looking reshuffle in mir_native_buffer.h is necessary to avoid breaking the ABI for Mesa. It seems only Mesa is inflexible enough that we can't extend MirBufferPackage without breaking it. So we avoid growing the size of MirBufferPackage for now.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Looks messy but I guess it is justified in the description

review: Approve
Revision history for this message
Kevin DuBois (kdub) wrote :

the abi-preserving changes for the gbm buffer look good. Android has a flag on its 'usage' field in its native type that can indicate scanout surfaces

It would be nice to get rid of the #ifndef ANDROID around the test case... but looks okay otherwise.

review: Approve
Revision history for this message
Robert Ancell (robert-ancell) wrote :

The ABI preserving is a little bit unclear, it would be nice to add a big comment saying "we are stealing the last element of data[] and fd[] to use for flags - we can revert this once we bump ABI.

Otherwise makes sense to me

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/platform/mir/graphics/buffer_ipc_packer.h'
2--- include/platform/mir/graphics/buffer_ipc_packer.h 2013-08-28 03:41:48 +0000
3+++ include/platform/mir/graphics/buffer_ipc_packer.h 2013-09-11 08:52:46 +0000
4@@ -33,6 +33,7 @@
5 virtual void pack_fd(int) = 0;
6 virtual void pack_data(int) = 0;
7 virtual void pack_stride(geometry::Stride) = 0;
8+ virtual void pack_flags(unsigned int) = 0;
9
10 protected:
11 BufferIPCPacker() {}
12
13=== modified file 'include/shared/mir_toolkit/mir_native_buffer.h'
14--- include/shared/mir_toolkit/mir_native_buffer.h 2013-05-06 15:38:52 +0000
15+++ include/shared/mir_toolkit/mir_native_buffer.h 2013-09-11 08:52:46 +0000
16@@ -20,15 +20,24 @@
17 #ifndef MIR_CLIENT_MIR_NATIVE_BUFFER_H_
18 #define MIR_CLIENT_MIR_NATIVE_BUFFER_H_
19
20-enum { mir_buffer_package_max = 32 };
21+enum { mir_buffer_package_max = 31 };
22+
23+typedef enum
24+{
25+ mir_buffer_flag_can_scanout = 1
26+} MirBufferFlag;
27+
28 typedef struct MirBufferPackage
29 {
30 int data_items;
31 int fd_items;
32
33 int data[mir_buffer_package_max];
34+ int unused1; /* Retain ABI compatibility (avoid rebuilding Mesa) */
35+
36 int fd[mir_buffer_package_max];
37
38+ unsigned int flags; /* MirBufferFlag's */
39 int stride;
40 int age; /**< Number of frames submitted by the client since the client has rendered to this buffer. */
41 /**< This has the same semantics as the EGL_EXT_buffer_age extension */
42
43=== modified file 'include/test/mir_test_doubles/mock_buffer_packer.h'
44--- include/test/mir_test_doubles/mock_buffer_packer.h 2013-08-28 03:41:48 +0000
45+++ include/test/mir_test_doubles/mock_buffer_packer.h 2013-09-11 08:52:46 +0000
46@@ -36,6 +36,7 @@
47 MOCK_METHOD1(pack_fd, void(int));
48 MOCK_METHOD1(pack_data, void(int));
49 MOCK_METHOD1(pack_stride, void(geometry::Stride));
50+ MOCK_METHOD1(pack_flags, void(unsigned int));
51 };
52
53 }
54
55=== modified file 'src/client/mir_surface.cpp'
56--- src/client/mir_surface.cpp 2013-08-28 03:41:48 +0000
57+++ src/client/mir_surface.cpp 2013-09-11 08:52:46 +0000
58@@ -268,6 +268,7 @@
59 }
60
61 buffer_package.stride = buffer.stride();
62+ buffer_package.flags = buffer.flags();
63 }
64 else
65 {
66
67=== modified file 'src/server/frontend/protobuf_buffer_packer.cpp'
68--- src/server/frontend/protobuf_buffer_packer.cpp 2013-09-06 03:46:37 +0000
69+++ src/server/frontend/protobuf_buffer_packer.cpp 2013-09-11 08:52:46 +0000
70@@ -107,3 +107,9 @@
71 {
72 buffer_response->set_stride(stride.as_uint32_t());
73 }
74+
75+void mfd::ProtobufBufferPacker::pack_flags(unsigned int flags)
76+{
77+ buffer_response->set_flags(flags);
78+}
79+
80
81=== modified file 'src/server/frontend/protobuf_buffer_packer.h'
82--- src/server/frontend/protobuf_buffer_packer.h 2013-08-28 03:41:48 +0000
83+++ src/server/frontend/protobuf_buffer_packer.h 2013-09-11 08:52:46 +0000
84@@ -43,6 +43,7 @@
85 void pack_fd(int);
86 void pack_data(int);
87 void pack_stride(geometry::Stride);
88+ void pack_flags(unsigned int);
89 private:
90 protobuf::Buffer* buffer_response;
91 };
92
93=== modified file 'src/server/graphics/gbm/gbm_buffer.cpp'
94--- src/server/graphics/gbm/gbm_buffer.cpp 2013-08-28 03:41:48 +0000
95+++ src/server/graphics/gbm/gbm_buffer.cpp 2013-09-11 08:52:46 +0000
96@@ -130,6 +130,7 @@
97 temp->fd_items = 1;
98 temp->fd[0] = prime_fd;
99 temp->stride = stride().as_uint32_t();
100+ temp->flags = can_bypass() ? mir_buffer_flag_can_scanout : 0;
101 temp->bo = gbm_handle.get();
102
103 return temp;
104
105=== modified file 'src/server/graphics/gbm/gbm_platform.cpp'
106--- src/server/graphics/gbm/gbm_platform.cpp 2013-08-30 07:47:16 +0000
107+++ src/server/graphics/gbm/gbm_platform.cpp 2013-09-11 08:52:46 +0000
108@@ -146,6 +146,7 @@
109 }
110
111 packer->pack_stride(buffer->stride());
112+ packer->pack_flags(native_handle->flags);
113 }
114
115 void mgg::GBMPlatform::drm_auth_magic(drm_magic_t magic)
116
117=== modified file 'src/shared/protobuf/mir_protobuf.proto'
118--- src/shared/protobuf/mir_protobuf.proto 2013-09-06 03:46:37 +0000
119+++ src/shared/protobuf/mir_protobuf.proto 2013-09-11 08:52:46 +0000
120@@ -29,7 +29,8 @@
121 repeated sint32 fd = 2;
122 repeated int32 data = 3;
123 optional int32 fds_on_side_channel = 4;
124- optional int32 stride = 5;
125+ optional int32 stride = 5;
126+ optional uint32 flags = 6;
127
128 optional string error = 127;
129 }
130
131=== modified file 'tests/acceptance-tests/test_client_library.cpp'
132--- tests/acceptance-tests/test_client_library.cpp 2013-09-02 09:54:31 +0000
133+++ tests/acceptance-tests/test_client_library.cpp 2013-09-11 08:52:46 +0000
134@@ -456,6 +456,77 @@
135 launch_client_process(client_config);
136 }
137
138+#ifndef ANDROID
139+TEST_F(DefaultDisplayServerTestFixture, surface_scanout_flag_toggles)
140+{
141+ struct ClientConfig : ClientConfigCommon
142+ {
143+ void exec()
144+ {
145+ connection = mir_connect_sync(mir_test_socket, __PRETTY_FUNCTION__);
146+ ASSERT_TRUE(connection != NULL);
147+ EXPECT_TRUE(mir_connection_is_valid(connection));
148+ EXPECT_STREQ(mir_connection_get_error_message(connection), "");
149+
150+ MirSurfaceParameters parm =
151+ {
152+ __PRETTY_FUNCTION__,
153+ 1280, 1024,
154+ mir_pixel_format_abgr_8888,
155+ mir_buffer_usage_hardware,
156+ mir_display_output_id_invalid
157+ };
158+
159+ surface = mir_connection_create_surface_sync(connection, &parm);
160+ ASSERT_TRUE(mir_surface_is_valid(surface));
161+ MirNativeBuffer *native;
162+ mir_surface_get_current_buffer(surface, &native);
163+ EXPECT_TRUE(native->flags & mir_buffer_flag_can_scanout);
164+ mir_surface_swap_buffers_sync(surface);
165+ EXPECT_TRUE(native->flags & mir_buffer_flag_can_scanout);
166+ mir_surface_release_sync(surface);
167+
168+ parm.width = 100;
169+ parm.height = 100;
170+
171+ surface = mir_connection_create_surface_sync(connection, &parm);
172+ ASSERT_TRUE(mir_surface_is_valid(surface));
173+ mir_surface_get_current_buffer(surface, &native);
174+ EXPECT_FALSE(native->flags & mir_buffer_flag_can_scanout);
175+ mir_surface_swap_buffers_sync(surface);
176+ EXPECT_FALSE(native->flags & mir_buffer_flag_can_scanout);
177+ mir_surface_release_sync(surface);
178+
179+ parm.width = 800;
180+ parm.height = 600;
181+ parm.buffer_usage = mir_buffer_usage_software;
182+
183+ surface = mir_connection_create_surface_sync(connection, &parm);
184+ ASSERT_TRUE(mir_surface_is_valid(surface));
185+ mir_surface_get_current_buffer(surface, &native);
186+ EXPECT_FALSE(native->flags & mir_buffer_flag_can_scanout);
187+ mir_surface_swap_buffers_sync(surface);
188+ EXPECT_FALSE(native->flags & mir_buffer_flag_can_scanout);
189+ mir_surface_release_sync(surface);
190+
191+ parm.buffer_usage = mir_buffer_usage_hardware;
192+
193+ surface = mir_connection_create_surface_sync(connection, &parm);
194+ ASSERT_TRUE(mir_surface_is_valid(surface));
195+ mir_surface_get_current_buffer(surface, &native);
196+ EXPECT_TRUE(native->flags & mir_buffer_flag_can_scanout);
197+ mir_surface_swap_buffers_sync(surface);
198+ EXPECT_TRUE(native->flags & mir_buffer_flag_can_scanout);
199+ mir_surface_release_sync(surface);
200+
201+ mir_connection_release(connection);
202+ }
203+ } client_config;
204+
205+ launch_client_process(client_config);
206+}
207+#endif
208+
209 TEST_F(DefaultDisplayServerTestFixture, client_library_creates_multiple_surfaces)
210 {
211 int const n_surfaces = 13;
212
213=== modified file 'tests/mir_test_framework/testing_server_options.cpp'
214--- tests/mir_test_framework/testing_server_options.cpp 2013-09-06 03:46:37 +0000
215+++ tests/mir_test_framework/testing_server_options.cpp 2013-09-11 08:52:46 +0000
216@@ -65,7 +65,8 @@
217 {
218 public:
219 StubFDBuffer(mg::BufferProperties const& properties)
220- : StubBuffer(properties)
221+ : StubBuffer(properties),
222+ properties{properties}
223 {
224 fd = open("/dev/zero", O_RDONLY);
225 if (fd < 0)
226@@ -82,6 +83,14 @@
227 native_buffer->data[0] = 0xDEADBEEF;
228 native_buffer->fd_items = 1;
229 native_buffer->fd[0] = fd;
230+
231+ native_buffer->flags = 0;
232+ if (properties.size.width.as_int() >= 800 &&
233+ properties.size.height.as_int() >= 600 &&
234+ properties.usage == mg::BufferUsage::hardware)
235+ {
236+ native_buffer->flags |= mir_buffer_flag_can_scanout;
237+ }
238 return native_buffer;
239 #else
240 return std::shared_ptr<MirNativeBuffer>();
241@@ -94,6 +103,7 @@
242 }
243 private:
244 int fd;
245+ const mg::BufferProperties properties;
246 };
247
248 class StubGraphicBufferAllocator : public mg::GraphicBufferAllocator
249@@ -180,6 +190,7 @@
250 }
251
252 packer->pack_stride(buffer->stride());
253+ packer->pack_flags(native_handle->flags);
254 #else
255 (void)packer;
256 (void)buffer;
257
258=== modified file 'tests/unit-tests/frontend/test_protobuf_buffer_packer.cpp'
259--- tests/unit-tests/frontend/test_protobuf_buffer_packer.cpp 2013-08-28 03:41:48 +0000
260+++ tests/unit-tests/frontend/test_protobuf_buffer_packer.cpp 2013-09-11 08:52:46 +0000
261@@ -39,6 +39,7 @@
262 packer.pack_data(i);
263
264 packer.pack_stride(dummy_stride);
265+ packer.pack_flags(123);
266
267 EXPECT_EQ(num_fd, response.fd_size());
268 EXPECT_EQ(num_int, response.data_size());
269@@ -47,4 +48,5 @@
270 for (int i = 0; i < response.data_size(); ++i)
271 EXPECT_EQ(i, response.data(i));
272 EXPECT_EQ(dummy_stride.as_uint32_t(), static_cast<unsigned int>(response.stride()));
273+ EXPECT_EQ(123U, response.flags());
274 }
275
276=== modified file 'tests/unit-tests/graphics/gbm/test_gbm_platform.cpp'
277--- tests/unit-tests/graphics/gbm/test_gbm_platform.cpp 2013-08-28 03:41:48 +0000
278+++ tests/unit-tests/graphics/gbm/test_gbm_platform.cpp 2013-09-11 08:52:46 +0000
279@@ -171,6 +171,8 @@
280 }
281 EXPECT_CALL(*mock_packer, pack_stride(dummy_stride))
282 .Times(1);
283+ EXPECT_CALL(*mock_packer, pack_flags(testing::_))
284+ .Times(1);
285
286 platform->fill_ipc_package(mock_packer, mock_buffer);
287 }

Subscribers

People subscribed via source and target branches