Den 06.02.2017 10.17, skrev Thierry Reding:
On Tue, Jan 31, 2017 at 05:03:13PM +0100, Noralf Trønnes wrote:
tinydrm provides helpers for very simple displays that can use
CMA backed framebuffers and need flushing on changes.
Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
---
Changes since version 2:
- Remove fbdev after drm unregister, not before.
Changes since version 1:
- Add tinydrm.rst
- Set tdev->fbdev_cma=NULL on unregister (lastclose is called after that).
- Remove some DRM_DEBUG*()
Documentation/gpu/index.rst | 1 +
Documentation/gpu/tinydrm.rst | 21 ++
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/tinydrm/Kconfig | 8 +
drivers/gpu/drm/tinydrm/Makefile | 1 +
drivers/gpu/drm/tinydrm/core/Makefile | 3 +
drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 377 ++++++++++++++++++++++++++++
drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 234 +++++++++++++++++
include/drm/tinydrm/tinydrm.h | 115 +++++++++
10 files changed, 763 insertions(+)
create mode 100644 Documentation/gpu/tinydrm.rst
create mode 100644 drivers/gpu/drm/tinydrm/Kconfig
create mode 100644 drivers/gpu/drm/tinydrm/Makefile
create mode 100644 drivers/gpu/drm/tinydrm/core/Makefile
create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c
create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
create mode 100644 include/drm/tinydrm/tinydrm.h
I realize this is totally subjective, but this feels somewhat too much
of a separation. Given the helper nature of TinyDRM, I think it would be
more appropriate to move the helpers themselves into drm_tiny.[ch] and
then maybe add a subdirectory drivers/gpu/drm/tiny that contains all the
drivers that use the helpers.
The separation above further shows in subsequent patches where helpers
are added to tinydrm that aren't specific to TinyDRM. So this make the
new helpers appear as more of a subsystem in DRM rather than a helper
library. It also makes things somewhat inconsistent with existing
infrastructure.
What I have done with tinydrm is to do as little as possible in the
core helper. The minimum required to pull together
drm_simple_display_pipe, cma helper + fbdev and framebuffer flushing.
Then I have added a set of functions that ease the writing of drivers
for RGB565 displays with optional backlight and regulator.
Added to that is a controller library that handles register access (will
use regmap for non-mipi) and framebuffer flushing (set the windowing
registers and write the buffer to the pixel register).
And at last there's the display driver that initializes the controller
to match a particular panel.
Maybe I should narrow tinydrm to _just_ support "RGB565 displays with
optional backlight and regulator". It looks like I split it up to much,
unless someone sees a need for the core of tinydrm elsewhere.
I think it's hard to avoid the subsystem smell here. These displays are
really simple, fbdev is more than enough to cover their needs.
But fbdev is closed.
I'm glad you pick on this, as getting the architecture right will save
me maintenance down the line. I did paint me into a corner with
staging/fbtft and I'm not keen on repeating that (to my defence it was
my first C code since university and I had 2 displays that had some
similarities which ended up as fbtft).
Noralf.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
