From c92db29cf08b886cdd50811c6ecc0eead1cc4154 Mon Sep 17 00:00:00 2001 From: Anthony DiGirolamo Date: Wed, 14 Aug 2024 19:12:45 +0000 Subject: [PATCH] pw_display: Create module directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Create the empty pw_display module with docs.rst - Update seed 104 to reflect the planned one module multiple library structure. Change-Id: Ia8c81fc546bf03c54d9643d43a49a2db3e7a8e33 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/229712 Lint: Lint 🤖 Commit-Queue: Anthony DiGirolamo Reviewed-by: Kayce Basques --- CMakeLists.txt | 1 + PIGWEED_MODULES | 1 + docs/module_metadata.json | 5 + docs/modules.rst | 1 + pw_build/generated_pigweed_modules_lists.gni | 4 + pw_display/BUILD.bazel | 17 +++ pw_display/BUILD.gn | 28 +++++ pw_display/CMakeLists.txt | 15 +++ pw_display/OWNERS | 2 + pw_display/docs.rst | 10 ++ seed/0104-display-support.rst | 124 +++++++++---------- 11 files changed, 143 insertions(+), 65 deletions(-) create mode 100644 pw_display/BUILD.bazel create mode 100644 pw_display/BUILD.gn create mode 100644 pw_display/CMakeLists.txt create mode 100644 pw_display/OWNERS create mode 100644 pw_display/docs.rst diff --git a/CMakeLists.txt b/CMakeLists.txt index 57d05fe7f3..3542b5638b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -96,6 +96,7 @@ add_subdirectory(pw_containers EXCLUDE_FROM_ALL) add_subdirectory(pw_cpu_exception EXCLUDE_FROM_ALL) add_subdirectory(pw_cpu_exception_cortex_m EXCLUDE_FROM_ALL) add_subdirectory(pw_digital_io EXCLUDE_FROM_ALL) +add_subdirectory(pw_display EXCLUDE_FROM_ALL) add_subdirectory(pw_file EXCLUDE_FROM_ALL) add_subdirectory(pw_function EXCLUDE_FROM_ALL) add_subdirectory(pw_fuzzer EXCLUDE_FROM_ALL) diff --git a/PIGWEED_MODULES b/PIGWEED_MODULES index 339e053479..d4db6d7112 100644 --- a/PIGWEED_MODULES +++ b/PIGWEED_MODULES @@ -55,6 +55,7 @@ pw_digital_io pw_digital_io_linux pw_digital_io_mcuxpresso pw_digital_io_rp2040 +pw_display pw_dma_mcuxpresso pw_docgen pw_doctor diff --git a/docs/module_metadata.json b/docs/module_metadata.json index eaa9113bdf..a1040a634b 100644 --- a/docs/module_metadata.json +++ b/docs/module_metadata.json @@ -238,6 +238,11 @@ "C++17" ] }, + "pw_display": { + "tagline": "Graphic display support and framebuffer management", + "status": "experimental", + "languages": [] + }, "pw_dma_mcuxpresso": { "tagline": "DMA helpers for MCUXpresso SDK", "status": "unstable", diff --git a/docs/modules.rst b/docs/modules.rst index 3195707c12..6f4f0b48c5 100644 --- a/docs/modules.rst +++ b/docs/modules.rst @@ -47,6 +47,7 @@ Libraries (modules) pw_cpu_exception/docs pw_crypto/docs pw_digital_io/docs + pw_display/docs pw_dma_mcuxpresso/docs pw_docgen/docs pw_doctor/docs diff --git a/pw_build/generated_pigweed_modules_lists.gni b/pw_build/generated_pigweed_modules_lists.gni index 2a541fafc6..4949f3cf1d 100644 --- a/pw_build/generated_pigweed_modules_lists.gni +++ b/pw_build/generated_pigweed_modules_lists.gni @@ -91,6 +91,7 @@ declare_args() { dir_pw_digital_io_mcuxpresso = get_path_info("../pw_digital_io_mcuxpresso", "abspath") dir_pw_digital_io_rp2040 = get_path_info("../pw_digital_io_rp2040", "abspath") + dir_pw_display = get_path_info("../pw_display", "abspath") dir_pw_dma_mcuxpresso = get_path_info("../pw_dma_mcuxpresso", "abspath") dir_pw_docgen = get_path_info("../pw_docgen", "abspath") dir_pw_doctor = get_path_info("../pw_doctor", "abspath") @@ -280,6 +281,7 @@ declare_args() { dir_pw_digital_io_linux, dir_pw_digital_io_mcuxpresso, dir_pw_digital_io_rp2040, + dir_pw_display, dir_pw_dma_mcuxpresso, dir_pw_docgen, dir_pw_doctor, @@ -457,6 +459,7 @@ declare_args() { "$dir_pw_digital_io_linux:tests", "$dir_pw_digital_io_mcuxpresso:tests", "$dir_pw_digital_io_rp2040:tests", + "$dir_pw_display:tests", "$dir_pw_dma_mcuxpresso:tests", "$dir_pw_docgen:tests", "$dir_pw_doctor:tests", @@ -634,6 +637,7 @@ declare_args() { "$dir_pw_digital_io_linux:docs", "$dir_pw_digital_io_mcuxpresso:docs", "$dir_pw_digital_io_rp2040:docs", + "$dir_pw_display:docs", "$dir_pw_dma_mcuxpresso:docs", "$dir_pw_docgen:docs", "$dir_pw_doctor:docs", diff --git a/pw_display/BUILD.bazel b/pw_display/BUILD.bazel new file mode 100644 index 0000000000..e3ed04fe68 --- /dev/null +++ b/pw_display/BUILD.bazel @@ -0,0 +1,17 @@ +# Copyright 2024 The Pigweed Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not +# use this file except in compliance with the License. You may obtain a copy of +# the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations under +# the License. + +package(default_visibility = ["//visibility:public"]) + +licenses(["notice"]) diff --git a/pw_display/BUILD.gn b/pw_display/BUILD.gn new file mode 100644 index 0000000000..54b0f34784 --- /dev/null +++ b/pw_display/BUILD.gn @@ -0,0 +1,28 @@ +# Copyright 2024 The Pigweed Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not +# use this file except in compliance with the License. You may obtain a copy of +# the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations under +# the License. + +import("//build_overrides/pigweed.gni") + +import("$dir_pw_build/target_types.gni") +import("$dir_pw_docgen/docs.gni") +import("$dir_pw_sync/backend.gni") +import("$dir_pw_unit_test/test.gni") + +pw_test_group("tests") { + tests = [] +} + +pw_doc_group("docs") { + sources = [ "docs.rst" ] +} diff --git a/pw_display/CMakeLists.txt b/pw_display/CMakeLists.txt new file mode 100644 index 0000000000..aae99b7e4e --- /dev/null +++ b/pw_display/CMakeLists.txt @@ -0,0 +1,15 @@ +# Copyright 2024 The Pigweed Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not +# use this file except in compliance with the License. You may obtain a copy of +# the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations under +# the License. + +include($ENV{PW_ROOT}/pw_build/pigweed.cmake) diff --git a/pw_display/OWNERS b/pw_display/OWNERS new file mode 100644 index 0000000000..55936259d3 --- /dev/null +++ b/pw_display/OWNERS @@ -0,0 +1,2 @@ +amontanez@google.com +tonymd@google.com diff --git a/pw_display/docs.rst b/pw_display/docs.rst new file mode 100644 index 0000000000..5f6ce5e472 --- /dev/null +++ b/pw_display/docs.rst @@ -0,0 +1,10 @@ +.. _module-pw_display: + +========== +pw_display +========== +.. pigweed-module:: + :name: pw_display + +.. attention:: + This module is undergoing implementation as part of SEED :ref:`seed-0104`. diff --git a/seed/0104-display-support.rst b/seed/0104-display-support.rst index 56d183510d..124a5f7e01 100644 --- a/seed/0104-display-support.rst +++ b/seed/0104-display-support.rst @@ -32,71 +32,71 @@ STM32F429I. This enables developers to quickly and easily add display support for supported devices and an implementation to model when adding new device support. ---------------- +-------- Proposal ---------------- -This proposes no changes to existing modules, but suggests several new modules -that together define a framework for rendering to displays. - +-------- +This proposes no changes to existing modules, but suggests several new libraries +all within a single new module titled ``pw_display`` that together define a +framework for rendering to displays. -New Modules ------------ +New Libraries +============= .. list-table:: :widths: 5 45 :header-rows: 1 - * - Module + * - Library - Function - * - pw_display + * - pw_display/display - Manage draw thread, framebuffers, and driver - * - pw_display_driver + * - pw_display/driver - Display driver interface definition - * - pw_display_driver_ili9341 + * - pw_display/pixel_pusher + - Transport of pixel data to display controller + + * - pw_display/drivers/ili9341 - Display driver for the ILI9341 display controller - * - pw_display_driver_imgui + * - pw_display/drivers/imgui - Host display driver using `Dear ImGui `_ - * - pw_display_driver_mipi + * - pw_display/drivers/mipi - Display driver for `MIPI DSI `_ controllers - * - pw_display_driver_null + * - pw_display/drivers/null - Null display driver for headless devices - * - pw_display_driver_st7735 + * - pw_display/drivers/st7735 - Display driver for the `ST7735 `_ display controller - * - pw_display_driver_st7789 + * - pw_display/drivers/st7789 - Display driver for the ST7789 display controller - * - pw_simple_draw - - Very basic drawing library for test purposes + * - pw_display/draw + - Very basic drawing library for test and bring-up purposes - * - pw_framebuffer + * - pw_display/framebuffer - Manage access to pixel buffer. - * - pw_framebuffer_mcuxpresso + * - pw_display/framebuffer_mcuxpresso - Specialization of the framebuffer for the MCUxpresso devices - * - pw_geometry + * - pw_display/geometry - Basic shared math types such as 2D vectors, etc. - * - pw_pixel_pusher - - Transport of pixel data to display controller - -Math ----- -``pw_geometry`` contains two helper structures for common values usually used as -a pair. +Geometry +======== +``pw_display/geometry`` contains two helper structures for common values usually +used as a pair. .. code-block:: cpp - namespace pw::math { + namespace pw::display { template struct Size { @@ -110,18 +110,18 @@ a pair. T y; }; - } // namespace pw::math + } // namespace pw::display Framebuffer ------------ +=========== A framebuffer is a small class that provides access to a pixel buffer. It keeps a copy of the pixel buffer metadata and provides accessor methods for those values. .. code-block:: cpp - namespace pw::framebuffer { + namespace pw::display { enum class PixelFormat { None, @@ -156,7 +156,7 @@ those values. uint16_t row_bytes() const; }; - } // namespace pw::framebuffer + } // namespace pw::display FrameBuffer is a moveable class that is intended to signify read/write privileges to the underlying pixel data. This makes it easier to track when the @@ -190,8 +190,7 @@ can be given the pixel buffer pointer retrieved by calling ``data()``. DrawScreen(&fb); FramebufferPool ---------------- - +=============== The FramebufferPool is intended to simplify the use of multiple framebuffers when multi-buffered rendering is being used. It is a collection of framebuffers which can be retrieved, used, and then returned to the pool for reuse. All @@ -202,7 +201,7 @@ until it has been returned by calling ``ReleaseFramebuffer()``. .. code-block:: cpp - namespace pw::framebuffer_pool { + namespace pw::display { class FramebufferPool { public: @@ -240,7 +239,7 @@ until it has been returned by calling ``ReleaseFramebuffer()``. virtual Status ReleaseFramebuffer(pw::framebuffer::Framebuffer framebuffer); }; - } // namespace pw::framebuffer + } // namespace pw::display An example use of the framebuffer pool is: @@ -257,8 +256,7 @@ An example use of the framebuffer pool is: framebuffer_pool.ReleaseFramebuffer(std::move(fb)); DisplayDriver -------------- - +============= A DisplayDriver is usually the sole class responsible for communicating with the display controller. Its primary responsibilities are the display controller initialization, and the writing of pixel data when a display update is needed. @@ -273,7 +271,7 @@ Because of this approach the DisplayDriver is defined as an interface: .. code-block:: cpp - namespace pw::display_driver { + namespace pw::display { class DisplayDriver { public: @@ -290,14 +288,14 @@ Because of this approach the DisplayDriver is defined as an interface: virtual pw::math::Size size() const = 0; }; - } // namespace pw::display_driver + } // namespace pw::display Each driver then provides a concrete implementation of the driver. Below is the definition of the display driver for the ILI9341: .. code-block:: cpp - namespace pw::display_driver { + namespace pw::display { class DisplayDriverILI9341 : public DisplayDriver { public: @@ -338,7 +336,7 @@ definition of the display driver for the ILI9341: const Command& command); }; - } // namespace pw::display_driver + } // namespace pw::display Here is an example retrieving a framebuffer from the framebuffer pool, drawing into the framebuffer, using the display driver to write the pixel data, and then @@ -374,7 +372,7 @@ may use a background write and the write callback is called when the write is complete. The write callback **may be called during an interrupt**. PixelPusher ------------ +=========== Pixel data for Simple SPI based display controllers can be written to the display controller using ``pw_spi``. There are some controllers which use other interfaces (RGB, MIPI, etc.). Also, some vendors provide an API for @@ -387,7 +385,7 @@ a framebuffer to the display controller. Specializations of this will use .. code-block:: cpp - namespace pw::pixel_pusher { + namespace pw::display { class PixelPusher { public: @@ -402,11 +400,10 @@ a framebuffer to the display controller. Specializations of this will use WriteCallback complete_callback) = 0; }; - } // namespace pw::pixel_pusher + } // namespace pw::display Display -------- - +======= Each display has: 1. One and only one display driver. @@ -466,17 +463,16 @@ A simplified application rendering loop would resemble: // controller by the display's display driver. display.ReleaseFramebuffer(std::move(fb), [](Status){}); -Simple Drawing Module ---------------------- - -``pw_simple_draw`` was created for testing and verification purposes only. It is +Drawing Library +=============== +``pw_display/draw`` was created for testing and verification purposes only. It is not intended to be feature rich or performant in any way. This is small collection of basic drawing primitives not intended to be used by shipping applications. .. code-block:: cpp - namespace pw::draw { + namespace pw::display { void DrawLine(pw::framebuffer::Framebuffer& fb, int x1, @@ -541,11 +537,10 @@ applications. const FontSet& font, pw::framebuffer::Framebuffer& framebuffer); - } // namespace pw::draw + } // namespace pw::display Class Interaction Diagram -------------------------- - +========================= .. mermaid:: :alt: Framebuffer Classes :align: center @@ -646,7 +641,6 @@ be easily interfaced with those libraries. --------------- Detailed design --------------- - This proposal suggests no changes to existing APIs. All changes introduce new modules that leverage the existing API. It supports static allocation of the pixel buffers and all display framework objects. Additionally pixel buffers @@ -685,7 +679,7 @@ is available. This unburdens the application drawing loop from the task of managing framebuffers or tracking screen update completion. Testing -------- +======= All classes will be accompanied by a robust set of unit tests. These can be run on the host or the device. Test applications will be able to run on a workstation (i.e. not an MCU) in order to enable tests that depend on @@ -696,7 +690,7 @@ based tests. Desktop tests will use them to be run in a headless continuous integration environment. Performance ------------ +=========== Display support will include performance tests. Although this proposal does not include a rendering library, it will include support for specific platforms that will utilize means of transferring pixel data to the display controller @@ -705,7 +699,6 @@ in the background. ------------ Alternatives ------------ - One alternative is to create the necessary port/HAL, the terminology varies by library, for the popular embedded graphics libraries. This would make it easier for Pigweed applications to add display support - bot only for those supported @@ -723,7 +716,7 @@ Open questions -------------- Parameter Configuration ------------------------ +======================= One open question is what parameters to specify in initialization parameters to a driver ``Init()`` function, which to set in build flags via ``config(...)`` in GN, and which to hard-code into the driver. The most ideal, from the @@ -751,19 +744,20 @@ with the various display controller APIs as possible to increase the likelihood of a well crafted API. Module Hierarchy ----------------- +================ At present Pigweed's module structure is flat and at the project root level. There are currently 134 top level ``pw_*`` directories. This proposal could significantly increase this count as each new display driver will be a new module. This might be a good time to consider putting modules into a hierarchy. Pixel Pusher ------------- +============ ``PixelPusher`` was created to remove the details of writing pixels from the display driver. Many displays support multiple ways to send pixel data. For example the ILI9341 supports SPI and a parallel bus for pixel transport. The `STM32F429I-DISC1 `_ -also has a display controller (`LTDC `_) +also has a display controller (`LTDC +`_) which uses an STM proprietary API. The ``PixelPusher`` was essentially created to allow that driver to use the LTDC API without the need to be coupled in any way to a vendor API. @@ -775,7 +769,7 @@ be cleaner to move the command writes into the ``PixelPusher`` and remove any should be renamed. Copyrighted SDKs ----------------- +================ Some vendors have copyrighted SDKs which cannot be included in the Pigweed source code unless the project is willing to have the source covered by more than one license. Additionally some SDKs have no simple download link and the