linux-rockchip.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
@ 2022-04-13 22:19 Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 01/16] drm: Add a helper library for EPD drivers Samuel Holland
                   ` (17 more replies)
  0 siblings, 18 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

This series adds a DRM driver for the electrophoretic display controller
found in a few different Rockchip SoCs, specifically the RK3566/RK3568
variant[0] used by the PineNote tablet[1].

This is my first real involvement with the DRM subsystem, so please let
me know where I am misunderstanding things.

This is now the second SoC-integrated EPD controller with a DRM driver
submission -- the first one being the i.MX6 EPDC[2]. I want to thank
Andreas for sending that series, and for his advice while writing this
driver.

One goal I have with sending this series is to discuss how to support
EPDs more generally within the DRM subsystem, so the interfaces with
panels and PMICs and waveform LUTs can be controller-independent.

My understanding is that the i.MX6 EPDC series is at least partly based
on the downstream vendor driver. This driver is a clean-sheet design for
hardware with different (read: fewer) capabilities, so we took some
different design paths, but we ran into many of the same sharp edges.

Here are some of the areas I would like input on:

Panel Lifecycle
===============
Panels use prepare/unprepare callbacks for their power supply. EPDs
should only be powered up when the display contents are changed. Should
the controller call both drm_panel_(un)prepare during each atomic update
when the framebuffer is dirty?

Similarly, panel enable/disable callbacks are tied to backlight state.
For an EPD, it makes sense to have the backlight enabled while the panel
is powered down (because the contents are static). Is it acceptable to
call drm_panel_{en,dis}able while the panel is not prepared?

With panel_bridge, the "normal" callback ordering is enforced, and tied
to the atomic state, so neither of these is possible.

As a result, neither the backlight nor the voltage regulators are tied
to the panel. The panel's regulators are consumed by the EBC itself.

Panel Timing Parameters
=======================
EPDs have more timing parameters than LCDs, and there are several
different ways of labeling these parameters. See for example the timing
diagrams on pp. 2237-2239 of the RK3568 TRM[0], the descriptions in the
ED103TC2 panel datasheet[3], and the submitted EPDC bindings[2].

Both the EPDC and EBC vendor drivers put all of the timing parameters in
the controller's OF node. There is no panel device/node.

I was able to squeeze everything needed for my specific case into a
struct drm_display_mode (see patches 5 and 14), but I don't know if this
is an acceptable use of those fields, or if will work with other
controllers. Is adding more fields to drm_display_mode an option?

See also the discussion of "dumb" LCD TCONs below.

Panel Connector Type / Media Bus Format
=======================================
The EBC supports either an 8-bit or 16-bit wide data bus, where each
pair of data lines represents the source driver polarity (positive,
negative, or neutral) for a pixel.

The only effect of the data bus width is the number of pixels that are
transferred per clock cycle. It has no impact on the number of possible
grayscale levels.

How does that translate to DRM_MODE_CONNECTOR_* or MEDIA_BUS_FMT_*?

Panel Reflection
================
The ED103TC2 panel scans from right to left. Currently, there is no API
or OF property to represent this. I can add something similar to
drm_panel_orientation.

Should this be exposed to userspace? It is acceptable for the kernel
driver to flip the image when blitting from the framebuffer?

CRTC "active" and "enabled" states
==================================
What do these states mean in the context of an EPD? Currently, the
driver ignores "active" and clears the screen to solid white when the
CRTC is disabled.

The vendor drivers can switch to a user-provided image when the CRTC is
disabled. Is this something that can/should be supported upstream? If
so, how? Would userspace provide the image to the kernel, or just tell
the kernel not to clear the screen?

VBLANK Events and Asynchronous Commits
======================================
When should the VBLANK event complete? When the pixels have been blitted
to the kernel's shadow buffer? When the first frame of the waveform is
sent to the panel? When the last frame is sent to the panel?

Currently, the driver is taking the first option, letting
drm_atomic_helper_fake_vblank() send the VBLANK event without waiting on
the refresh thread. This is the only way I was able to get good
performance with existing userspace.

Waveform Loading
================
Waveform files are calibrated for each batch of panels. So while a
single waveform file may be "good enough" for all panels of a certain
model, the correctly-calibrated file will have better image quality.

I don't know of a good way to choose the calibrated file. Even the
board's compatible string may not be specific enough, if the board is
manufactured with multiple batches of panels.

Maybe the filename should just be the panel compatible, and the user is
responsible for putting the right file there? In that case, how should I
get the compatible string from the panel_bridge? Traverse the OF graph
myself?

There is also the issue that different controllers need the waveform
data in different formats. ".wbf" appears to be the format provided by
PVI/eInk, the panel manufacturer. The Rockchip EBC hardware expects a
single waveform in a flat array, so the driver has to extract/decompress
that from the .wbf file (this is done in patch 1). On the other hand,
the i.MX EPDC expects a ".wrf" file containing multiple waveforms[8].

I propose that the waveform file on disk should always be what was
shipped with the panel -- the .wbf file -- and any extracting or
reformatting is done in the kernel.

Waveform Selection From Userspace
=================================
EPDs use different waveforms for different purposes: high-quality
grayscale vs. monochrome text vs. dithered monochrome video. How can
userspace select which waveform to use? Should this be a plane property?

It is also likely that userspace will want to use different waveforms at
the same time for different parts of the screen, for example a fast
monochrome waveform for the drawing area of a note-taking app, but a
grayscale waveform for surrounding UI and window manager.

I believe the i.MX6 EPDC supports multiple planes, each with their own
waveform choice. That seems like a good abstraction, but the EBC only
supports one plane in hardware. So using this abstraction with the EBC
would require blending pixels and doing waveform lookups in software.

Blitting/Blending in Software
=============================
There are multiple layers to this topic (pun slightly intended):
 1) Today's userspace does not expect a grayscale framebuffer.
    Currently, the driver advertises XRGB8888 and converts to Y4
    in software. This seems to match other drivers (e.g. repaper).

 2) Ignoring what userspace "wants", the closest existing format is
    DRM_FORMAT_R8. Geert sent a series[4] adding DRM_FORMAT_R1 through
    DRM_FORMAT_R4 (patch 9), which I believe are the "correct" formats
    to use.

 3) The RK356x SoCs have an "RGA" hardware block that can do the
    RGB-to-grayscale conversion, and also RGB-to-dithered-monochrome
    which is needed for animation/video. Currently this is exposed with
    a V4L2 platform driver. Can this be inserted into the pipeline in a
    way that is transparent to userspace? Or must some userspace library
    be responsible for setting up the RGA => EBC pipeline?

 4) Supporting multiple planes (for multiple concurrent waveforms)
    implies blending in software. Is that acceptable?

 5) Thoughts on SIMD-optimized blitting and waveform lookup functions?

 5) Currently the driver uses kmalloc() and dma_sync_single_for_device()
    for its buffers, because it needs both fast reads and fast writes to
    several of them. Maybe cma_alloc() or dma_alloc_from_contiguous()
    would be more appropriate, but I don't see any drivers using those
    directly.

EPDs connected to "dumb" LCD TCONs
==================================
This topic is mostly related to my first patch. Some boards exist that
hook up an EPD to a normal LCD TCON, not a dedicated EPD controller. For
example, there's the reMarkable 2[5] and some PocketBook models[6][7].

I have some concerns about this:
 1) If we put EPD panel timings in panel drivers (e.g. panel-simple),
    can the same timings work with LCD TCONs and EPD controllers?

    For example: one cycle of the 16-bit data bus is "one pixel" to an
    LCD controller, but is "8 pixels" to an EPD controller. So there is
    a factor-of-8 difference in horizontal resolution depending on your
    perspective. Should we have the "number of pixel clock cycles" or
    "number of pixels" in .hdisplay/.htotal in the panel timings?

    Patch 14 adds a panel with "number of pixels" horizontal resolution,
    so the correct resolution is reported to userspace, but the existing
    eink_vb3300_kca_timing in panel-simple.c appears to use "number of
    pixel clocks" for its horizontal resolution. This makes the panel
    timing definitions incompatible across controllers.

 2) Using fbdev/fbcon with an EPD hooked up to an LCD TCON will have
    unintended consequences, and possibly damage the panel. Currently,
    there is no way to mark the framebuffer as expecting "source driver
    polarity waveforms" and not pixel data. Is there a specific
    DRM_FORMAT_* we should use for these cases to prevent accidental use
    by userspace?

    Or should we disallow this entirely, and have some wrapper layer to
    do the waveform lookups in kernelspace?

    I like the wrapper layer idea because it allows normal userspace and
    fbcon to work. It would not be much new code, especially since this
    driver already supports doing the whole pipeline in software. So
    that's why I wrote a separate helper library; I hope this code can
    be reused.

Thanks for any input!
Samuel

[0]: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf
[1]: https://wiki.pine64.org/wiki/PineNote
[2]: https://lore.kernel.org/lkml/20220206080016.796556-1-andreas@kemnade.info/T/
[3]: https://files.pine64.org/doc/quartz64/Eink%20P-511-828-V1_ED103TC2%20Formal%20Spec%20V1.0_20190514.pdf
[4]: https://lore.kernel.org/lkml/cover.1646683502.git.geert@linux-m68k.org/T/
[5]: https://lore.kernel.org/lkml/20220330094126.30252-1-alistair@alistair23.me/T/
[6]: https://github.com/megous/linux/commits/pb-5.17
[7]: https://github.com/megous/linux/commit/3cdf84388959
[8]: https://github.com/fread-ink/inkwave


Samuel Holland (16):
  drm: Add a helper library for EPD drivers
  dt-bindings: display: rockchip: Add EBC binding
  drm/rockchip: Add EBC platform driver skeleton
  drm/rockchip: ebc: Add DRM driver skeleton
  drm/rockchip: ebc: Add CRTC mode setting
  drm/rockchip: ebc: Add CRTC refresh thread
  drm/rockchip: ebc: Add CRTC buffer management
  drm/rockchip: ebc: Add LUT loading
  drm/rockchip: ebc: Implement global refreshes
  drm/rockchip: ebc: Implement partial refreshes
  drm/rockchip: ebc: Enable diff mode for partial refreshes
  drm/rockchip: ebc: Add support for direct mode
  drm/rockchip: ebc: Add a panel reflection option
  drm/panel-simple: Add eInk ED103TC2
  arm64: dts: rockchip: rk356x: Add EBC node
  [DO NOT MERGE] arm64: dts: rockchip: pinenote: Enable EBC display
    pipeline

 .../display/rockchip/rockchip,rk3568-ebc.yaml |  106 ++
 .../boot/dts/rockchip/rk3566-pinenote.dtsi    |   80 +
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      |   14 +
 drivers/gpu/drm/Kconfig                       |    6 +
 drivers/gpu/drm/Makefile                      |    4 +-
 drivers/gpu/drm/drm_epd_helper.c              |  663 +++++++
 drivers/gpu/drm/panel/panel-simple.c          |   31 +
 drivers/gpu/drm/rockchip/Kconfig              |   12 +
 drivers/gpu/drm/rockchip/Makefile             |    2 +
 drivers/gpu/drm/rockchip/rockchip_ebc.c       | 1586 +++++++++++++++++
 include/drm/drm_epd_helper.h                  |  104 ++
 11 files changed, 2607 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip,rk3568-ebc.yaml
 create mode 100644 drivers/gpu/drm/drm_epd_helper.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_ebc.c
 create mode 100644 include/drm/drm_epd_helper.h

-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RFC PATCH 01/16] drm: Add a helper library for EPD drivers
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding Samuel Holland
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

Currently, this library is focused on LUTs and LUT files, specifically
the PVI wbf-format files shipped with E-Ink displays. It provides
helpers to load and validate LUT files, and extract LUTs from them.

Since EPD controllers need the LUTs in various formats, this library
allows drivers to declare their desired format. It will then convert
LUTs to that format before returning them.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/Kconfig          |   6 +
 drivers/gpu/drm/Makefile         |   2 +
 drivers/gpu/drm/drm_epd_helper.c | 663 +++++++++++++++++++++++++++++++
 include/drm/drm_epd_helper.h     | 104 +++++
 4 files changed, 775 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_epd_helper.c
 create mode 100644 include/drm/drm_epd_helper.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f1422bee3dcc..ad96cf605444 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -198,6 +198,12 @@ config DRM_DP_CEC
 	  Note: not all adapters support this feature, and even for those
 	  that do support this they often do not hook up the CEC pin.
 
+config DRM_EPD_HELPER
+	tristate
+	depends on DRM
+	help
+	  Choose this if you need the EPD (LUT, etc.) helper functions
+
 config DRM_TTM
 	tristate
 	depends on DRM && MMU
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index c2ef5f9fce54..49380ccfe9d6 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -33,6 +33,8 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o drm_privacy_screen_x86.
 
 obj-$(CONFIG_DRM_NOMODESET) += drm_nomodeset.o
 
+obj-$(CONFIG_DRM_EPD_HELPER) += drm_epd_helper.o
+
 drm_cma_helper-y := drm_gem_cma_helper.o
 drm_cma_helper-$(CONFIG_DRM_KMS_HELPER) += drm_fb_cma_helper.o
 obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
diff --git a/drivers/gpu/drm/drm_epd_helper.c b/drivers/gpu/drm/drm_epd_helper.c
new file mode 100644
index 000000000000..433a6728ef3e
--- /dev/null
+++ b/drivers/gpu/drm/drm_epd_helper.c
@@ -0,0 +1,663 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Electrophoretic Display Helper Library
+ *
+ * Copyright (C) 2022 Samuel Holland <samuel@sholland.org>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/vmalloc.h>
+
+#include <drm/drm_device.h>
+#include <drm/drm_epd_helper.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_print.h>
+
+/**
+ * DOC: overview
+ *
+ * This library provides functions for working with the lookup tables (LUTs)
+ * used by electrophoretic displays (EPDs). It fills in a LUT buffer based on
+ * the selected waveform, the panel temperature, and the buffer format needed
+ * by the driver.
+ */
+
+struct pvi_wbf_offset {
+	u8			b[3];
+};
+
+struct pvi_wbf_pointer {
+	struct pvi_wbf_offset	offset;
+	u8			checksum;
+};
+
+struct pvi_wbf_file_header {
+	__le32			checksum;		// 0x00
+	__le32			file_size;		// 0x04
+	__le32			serial;			// 0x08
+	u8			run_type;		// 0x0c
+	u8			fpl_platform;		// 0x0d
+	__le16			fpl_lot;		// 0x0e
+	u8			mode_version;		// 0x10
+	u8			wf_version;		// 0x11
+	u8			wf_subversion;		// 0x12
+	u8			wf_type;		// 0x13
+	u8			panel_size;		// 0x14
+	u8			amepd_part_number;	// 0x15
+	u8			wf_rev;			// 0x16
+	u8			frame_rate_bcd;		// 0x17
+	u8			frame_rate_hex;		// 0x18
+	u8			vcom_offset;		// 0x19
+	u8			unknown[2];		// 0x1a
+	struct pvi_wbf_offset	xwia;			// 0x1c
+	u8			cs1;			// 0x1f
+	struct pvi_wbf_offset	wmta;			// 0x20
+	u8			fvsn;			// 0x23
+	u8			luts;			// 0x24
+	u8			mode_count;		// 0x25
+	u8			temp_range_count;	// 0x26
+	u8			advanced_wf_flags;	// 0x27
+	u8			eb;			// 0x28
+	u8			sb;			// 0x29
+	u8			reserved[5];		// 0x2a
+	u8			cs2;			// 0x2f
+	u8			temp_range_table[];	// 0x30
+};
+static_assert(sizeof(struct pvi_wbf_file_header) == 0x30);
+
+struct pvi_wbf_mode_info {
+	u8			versions[2];
+	u8			format;
+	u8			modes[DRM_EPD_WF_MAX];
+};
+
+static const struct pvi_wbf_mode_info pvi_wbf_mode_info_table[] = {
+	{
+		.versions = {
+			0x09,
+		},
+		.format = DRM_EPD_LUT_4BIT_PACKED,
+		.modes = {
+			[DRM_EPD_WF_RESET]	= 0,
+			[DRM_EPD_WF_DU]		= 1,
+			[DRM_EPD_WF_DU4]	= 1,
+			[DRM_EPD_WF_GC16]	= 2,
+			[DRM_EPD_WF_GL16]	= 3,
+			[DRM_EPD_WF_GLR16]	= 3,
+			[DRM_EPD_WF_GLD16]	= 3,
+			[DRM_EPD_WF_A2]		= 4,
+			[DRM_EPD_WF_GCC16]	= 3,
+		},
+	},
+	{
+		.versions = {
+			0x12,
+		},
+		.format = DRM_EPD_LUT_4BIT_PACKED,
+		.modes = {
+			[DRM_EPD_WF_RESET]	= 0,
+			[DRM_EPD_WF_DU]		= 1,
+			[DRM_EPD_WF_DU4]	= 7,
+			[DRM_EPD_WF_GC16]	= 3,
+			[DRM_EPD_WF_GL16]	= 3,
+			[DRM_EPD_WF_GLR16]	= 5,
+			[DRM_EPD_WF_GLD16]	= 6,
+			[DRM_EPD_WF_A2]		= 4,
+			[DRM_EPD_WF_GCC16]	= 5,
+		},
+	},
+	{
+		.versions = {
+			0x16,
+		},
+		.format = DRM_EPD_LUT_5BIT_PACKED,
+		.modes = {
+			[DRM_EPD_WF_RESET]	= 0,
+			[DRM_EPD_WF_DU]		= 1,
+			[DRM_EPD_WF_DU4]	= 1,
+			[DRM_EPD_WF_GC16]	= 2,
+			[DRM_EPD_WF_GL16]	= 3,
+			[DRM_EPD_WF_GLR16]	= 4,
+			[DRM_EPD_WF_GLD16]	= 4,
+			[DRM_EPD_WF_A2]		= 6,
+			[DRM_EPD_WF_GCC16]	= 5,
+		},
+	},
+	{
+		.versions = {
+			0x18,
+			0x20,
+		},
+		.format = DRM_EPD_LUT_5BIT_PACKED,
+		.modes = {
+			[DRM_EPD_WF_RESET]	= 0,
+			[DRM_EPD_WF_DU]		= 1,
+			[DRM_EPD_WF_DU4]	= 1,
+			[DRM_EPD_WF_GC16]	= 2,
+			[DRM_EPD_WF_GL16]	= 3,
+			[DRM_EPD_WF_GLR16]	= 4,
+			[DRM_EPD_WF_GLD16]	= 5,
+			[DRM_EPD_WF_A2]		= 6,
+			[DRM_EPD_WF_GCC16]	= 4,
+		},
+	},
+	{
+		.versions = {
+			0x19,
+			0x43,
+		},
+		.format = DRM_EPD_LUT_5BIT_PACKED,
+		.modes = {
+			[DRM_EPD_WF_RESET]	= 0,
+			[DRM_EPD_WF_DU]		= 1,
+			[DRM_EPD_WF_DU4]	= 7,
+			[DRM_EPD_WF_GC16]	= 2,
+			[DRM_EPD_WF_GL16]	= 3,
+			[DRM_EPD_WF_GLR16]	= 4,
+			[DRM_EPD_WF_GLD16]	= 5,
+			[DRM_EPD_WF_A2]		= 6,
+			[DRM_EPD_WF_GCC16]	= 4,
+		},
+	},
+	{
+		.versions = {
+			0x23,
+		},
+		.format = DRM_EPD_LUT_4BIT_PACKED,
+		.modes = {
+			[DRM_EPD_WF_RESET]	= 0,
+			[DRM_EPD_WF_DU]		= 1,
+			[DRM_EPD_WF_DU4]	= 5,
+			[DRM_EPD_WF_GC16]	= 2,
+			[DRM_EPD_WF_GL16]	= 3,
+			[DRM_EPD_WF_GLR16]	= 3,
+			[DRM_EPD_WF_GLD16]	= 3,
+			[DRM_EPD_WF_A2]		= 4,
+			[DRM_EPD_WF_GCC16]	= 3,
+		},
+	},
+	{
+		.versions = {
+			0x54,
+		},
+		.format = DRM_EPD_LUT_4BIT_PACKED,
+		.modes = {
+			[DRM_EPD_WF_RESET]	= 0,
+			[DRM_EPD_WF_DU]		= 1,
+			[DRM_EPD_WF_DU4]	= 1,
+			[DRM_EPD_WF_GC16]	= 2,
+			[DRM_EPD_WF_GL16]	= 3,
+			[DRM_EPD_WF_GLR16]	= 4,
+			[DRM_EPD_WF_GLD16]	= 4,
+			[DRM_EPD_WF_A2]		= 5,
+			[DRM_EPD_WF_GCC16]	= 4,
+		},
+	},
+};
+
+static const void *pvi_wbf_apply_offset(const struct drm_epd_lut_file *file,
+					const struct pvi_wbf_offset *offset)
+{
+	u32 bytes = offset->b[0] | offset->b[1] << 8 | offset->b[2] << 16;
+
+	if (bytes >= file->fw->size)
+		return NULL;
+
+	return (const void *)file->header + bytes;
+}
+
+static const void *pvi_wbf_dereference(const struct drm_epd_lut_file *file,
+				       const struct pvi_wbf_pointer *ptr)
+{
+	u8 sum = ptr->offset.b[0] + ptr->offset.b[1] + ptr->offset.b[2];
+
+	if (ptr->checksum != sum)
+		return NULL;
+
+	return pvi_wbf_apply_offset(file, &ptr->offset);
+}
+
+static int pvi_wbf_get_mode_index(const struct drm_epd_lut_file *file,
+				  enum drm_epd_waveform waveform)
+{
+	if (waveform >= DRM_EPD_WF_MAX)
+		return -EINVAL;
+
+	return file->mode_info->modes[waveform];
+}
+
+static int pvi_wbf_get_mode_info(struct drm_epd_lut_file *file)
+{
+	u8 mode_version = file->header->mode_version;
+	const struct pvi_wbf_mode_info *mode_info;
+	int i, v;
+
+	for (i = 0; i < ARRAY_SIZE(pvi_wbf_mode_info_table); i++) {
+		mode_info = &pvi_wbf_mode_info_table[i];
+
+		for (v = 0; v < ARRAY_SIZE(mode_info->versions); v++) {
+			if (mode_info->versions[v] == mode_version) {
+				file->mode_info = mode_info;
+				return 0;
+			}
+		}
+	}
+
+	drm_err(file->dev, "Unknown PVI waveform version 0x%02x\n",
+		mode_version);
+
+	return -EOPNOTSUPP;
+}
+
+static int pvi_wbf_get_temp_index(const struct drm_epd_lut_file *file,
+				  int temperature)
+{
+	const struct pvi_wbf_file_header *header = file->header;
+	int i;
+
+	for (i = 0; i < header->temp_range_count; i++)
+		if (temperature < header->temp_range_table[i])
+			return i - 1;
+
+	return header->temp_range_count - 1;
+}
+
+static int pvi_wbf_validate_header(struct drm_epd_lut_file *file)
+{
+	const struct pvi_wbf_file_header *header = file->header;
+	int ret;
+
+	if (le32_to_cpu(header->file_size) > file->fw->size)
+		return -EINVAL;
+
+	ret = pvi_wbf_get_mode_info(file);
+	if (ret)
+		return ret;
+
+	drm_info(file->dev, "Loaded %d-bit PVI waveform version 0x%02x\n",
+		 file->mode_info->format == DRM_EPD_LUT_5BIT_PACKED ? 5 : 4,
+		 header->mode_version);
+
+	return 0;
+}
+
+static void drm_epd_lut_file_free(struct drm_device *dev, void *res)
+{
+	struct drm_epd_lut_file *file = res;
+
+	release_firmware(file->fw);
+}
+
+/**
+ * drmm_epd_lut_file_init - Initialize a managed EPD LUT file
+ *
+ * @dev: The DRM device owning this LUT file
+ * @file: The LUT file to initialize
+ * @file_name: The filesystem name of the LUT file
+ *
+ * Return: negative errno on failure, 0 otherwise
+ */
+int drmm_epd_lut_file_init(struct drm_device *dev,
+			   struct drm_epd_lut_file *file,
+			   const char *file_name)
+{
+	int ret;
+
+	ret = request_firmware(&file->fw, file_name, dev->dev);
+	if (ret)
+		return ret;
+
+	ret = drmm_add_action_or_reset(dev, drm_epd_lut_file_free, file);
+	if (ret)
+		return ret;
+
+	file->dev = dev;
+	file->header = (const void *)file->fw->data;
+
+	ret = pvi_wbf_validate_header(file);
+	if (ret)
+		return ret;
+
+	/* Only 5-bit waveform files are supported by drm_epd_lut_convert. */
+	if (file->mode_info->format != DRM_EPD_LUT_5BIT_PACKED)
+		return -EOPNOTSUPP;
+
+	return 0;
+}
+EXPORT_SYMBOL(drmm_epd_lut_file_init);
+
+/**
+ * drm_epd_lut_size_shift - Get the size of a LUT phase in power-of-2 bytes
+ *
+ * @format: One of the LUT buffer formats
+ *
+ * Return: buffer size shift amount
+ */
+static int drm_epd_lut_size_shift(enum drm_epd_lut_format format)
+{
+	switch (format) {
+	case DRM_EPD_LUT_4BIT:
+		/* (4 bits/pixel)^2 / 1 pixel/byte */
+		return 4 + 4;
+	case DRM_EPD_LUT_4BIT_PACKED:
+		/* (4 bits/pixel)^2 / 4 pixels/byte */
+		return 4 + 4 - 2;
+	case DRM_EPD_LUT_5BIT:
+		/* (5 bits/pixel)^2 / 1 pixel/byte */
+		return 5 + 5;
+	case DRM_EPD_LUT_5BIT_PACKED:
+		/* (5 bits/pixel)^2 / 4 pixels/byte */
+		return 5 + 5 - 2;
+	}
+
+	unreachable();
+}
+
+static int pvi_wbf_decode_lut(struct drm_epd_lut *lut, const u8 *lut_data)
+{
+
+	unsigned int copies, max_bytes, size_shift, state, total_bytes;
+	struct drm_device *dev = lut->file->dev;
+	const u8 *in = lut_data;
+	u8 *out = lut->buf;
+	u8 token;
+
+	size_shift = drm_epd_lut_size_shift(lut->file->mode_info->format);
+	max_bytes = lut->max_phases << size_shift;
+	total_bytes = 0;
+	state = 1;
+
+	/* Read tokens until reaching the end-of-input marker. */
+	while ((token = *in++) != 0xff) {
+		/* Special handling for the state switch token. */
+		if (token == 0xfc) {
+			state = !state;
+			token = *in++;
+		}
+
+		/*
+		 * State 0 is a sequence of data bytes.
+		 * State 1 is a sequence of [data byte, extra copies] pairs.
+		 */
+		copies = 1 + (state ? *in++ : 0);
+
+		total_bytes += copies;
+		if (total_bytes > max_bytes) {
+			drm_err(dev, "LUT contains too many phases\n");
+			lut->num_phases = 0;
+			return -EILSEQ;
+		}
+
+		while (copies--)
+			*out++ = token;
+	}
+
+	lut->num_phases = total_bytes >> size_shift;
+	if (total_bytes != lut->num_phases << size_shift) {
+		drm_err(dev, "LUT contains a partial phase\n");
+		lut->num_phases = 0;
+		return -EILSEQ;
+	}
+
+	drm_dbg_core(dev, "LUT contains %d phases (%ld => %ld bytes)\n",
+		     lut->num_phases, in - lut_data, out - lut->buf);
+
+	return 0;
+}
+
+static int pvi_wbf_get_lut(struct drm_epd_lut *lut,
+			   int mode_index, int temp_index)
+{
+	const struct pvi_wbf_pointer *mode_table, *temp_table;
+	const struct drm_epd_lut_file *file = lut->file;
+	const u8 *lut_data;
+	int ret;
+
+	mode_table = pvi_wbf_apply_offset(file, &file->header->wmta);
+	if (!mode_table)
+		return -EFAULT;
+
+	temp_table = pvi_wbf_dereference(file, &mode_table[mode_index]);
+	if (!temp_table)
+		return -EFAULT;
+
+	lut_data = pvi_wbf_dereference(file, &temp_table[temp_index]);
+	if (!lut_data)
+		return -EFAULT;
+
+	ret = pvi_wbf_decode_lut(lut, lut_data);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void drm_epd_lut_convert(const struct drm_epd_lut *lut)
+{
+	enum drm_epd_lut_format from = lut->file->mode_info->format;
+	enum drm_epd_lut_format to = lut->format;
+	u8 *buf = lut->buf;
+	size_t x, y;
+
+	if (to == from)
+		return;
+
+	switch (to) {
+	case DRM_EPD_LUT_4BIT:
+		for (y = 0; y < 16 * lut->num_phases; ++y) {
+			for (x = 8; x--;) {
+				u8 byte = buf[16 * y + x];
+
+				buf[16 * y + 2 * x + 0] = (byte >> 0) & 0x03;
+				buf[16 * y + 2 * x + 1] = (byte >> 4) & 0x03;
+			}
+		}
+		for (; y < 16 * lut->max_phases; ++y) {
+			for (x = 8; x--;) {
+				buf[16 * y + 2 * x + 0] = 0;
+				buf[16 * y + 2 * x + 1] = 0;
+			}
+		}
+		break;
+	case DRM_EPD_LUT_4BIT_PACKED:
+		for (y = 0; y < 16 * lut->num_phases; ++y) {
+			for (x = 4; x--;) {
+				u8 lo_byte = buf[16 * y + 2 * x + 0] & 0x33;
+				u8 hi_byte = buf[16 * y + 2 * x + 1] & 0x33;
+
+				/* Copy bits 4:5 => bits 2:3. */
+				lo_byte |= lo_byte >> 2;
+				hi_byte |= hi_byte >> 2;
+
+				buf[4 * y + x] = (lo_byte & 0xf) |
+						 (hi_byte << 4);
+			}
+		}
+		for (; y < 16 * lut->max_phases; ++y) {
+			for (x = 4; x--;)
+				buf[4 * y + x] = 0;
+		}
+		break;
+	case DRM_EPD_LUT_5BIT:
+		memset(buf + 256 * lut->num_phases, 0,
+		       256 * (lut->max_phases - lut->num_phases));
+		for (x = 256 * lut->num_phases; x--;) {
+			u8 byte = buf[x];
+
+			buf[4 * x + 0] = (byte >> 0) & 0x03;
+			buf[4 * x + 1] = (byte >> 2) & 0x03;
+			buf[4 * x + 2] = (byte >> 4) & 0x03;
+			buf[4 * x + 3] = (byte >> 6) & 0x03;
+		}
+		break;
+	case DRM_EPD_LUT_5BIT_PACKED:
+		/* Nothing to do. */
+		break;
+	}
+}
+
+static int drm_epd_lut_update(struct drm_epd_lut *lut,
+			      int mode_index, int temp_index)
+{
+	int ret;
+
+	ret = pvi_wbf_get_lut(lut, mode_index, temp_index);
+	if (ret)
+		return ret;
+
+	drm_epd_lut_convert(lut);
+
+	return 0;
+}
+
+/**
+ * drm_epd_lut_set_temperature - Update the LUT due to panel temperature change
+ *
+ * @lut: The LUT structure
+ * @temperateure: The current panel temperature in degrees Celsius
+ *
+ * Return: negative errno on failure, 1 if LUT was changed, 0 otherwise
+ */
+int drm_epd_lut_set_temperature(struct drm_epd_lut *lut,
+				int temperature)
+{
+	int temp_index;
+	int ret;
+
+	temp_index = pvi_wbf_get_temp_index(lut->file, temperature);
+	if (temp_index < 0)
+		return -ENOENT;
+
+	if (temp_index == lut->temp_index)
+		return 0;
+
+	drm_dbg_core(lut->file->dev, "LUT temperature changed (%d)\n",
+		     temperature);
+
+	ret = drm_epd_lut_update(lut, lut->mode_index, temp_index);
+	if (ret)
+		return ret;
+
+	lut->temp_index = temp_index;
+
+	return 1;
+}
+EXPORT_SYMBOL(drm_epd_lut_set_temperature);
+
+/**
+ * drm_epd_lut_set_waveform - Update the LUT due to waveform selection change
+ *
+ * @lut: The LUT structure
+ * @waveform: The desired waveform
+ *
+ * Return: negative errno on failure, 1 if LUT was changed, 0 otherwise
+ */
+int drm_epd_lut_set_waveform(struct drm_epd_lut *lut,
+			     enum drm_epd_waveform waveform)
+{
+	int mode_index;
+	int ret;
+
+	mode_index = pvi_wbf_get_mode_index(lut->file, waveform);
+	if (mode_index < 0)
+		return -ENOENT;
+
+	if (mode_index == lut->mode_index)
+		return 0;
+
+	drm_dbg_core(lut->file->dev, "LUT waveform changed (%d)\n",
+		     waveform);
+
+	ret = drm_epd_lut_update(lut, mode_index, lut->temp_index);
+	if (ret)
+		return ret;
+
+	lut->mode_index = mode_index;
+
+	return 1;
+}
+EXPORT_SYMBOL(drm_epd_lut_set_waveform);
+
+static void drm_epd_lut_free(struct drm_device *dev, void *res)
+{
+	struct drm_epd_lut *lut = res;
+
+	vfree(lut->buf);
+}
+
+/**
+ * drmm_epd_lut_init - Initialize a managed EPD LUT from a LUT file
+ *
+ * @dev: The DRM device owning this LUT
+ * @lut: The LUT to initialize
+ * @file_name: The file name of the waveform firmware
+ * @format: The LUT buffer format needed by the driver
+ * @max_phases: The maximum number of waveform phases supported by the driver
+ *
+ * Return: negative errno on failure, 0 otherwise
+ */
+int drmm_epd_lut_init(struct drm_epd_lut_file *file,
+		      struct drm_epd_lut *lut,
+		      enum drm_epd_lut_format format,
+		      unsigned int max_phases)
+{
+	size_t max_order;
+	int ret;
+
+	/* Allocate a buffer large enough to convert the LUT in place. */
+	max_order = max(drm_epd_lut_size_shift(file->mode_info->format),
+			drm_epd_lut_size_shift(format));
+	lut->buf = vmalloc(max_phases << max_order);
+	if (!lut->buf)
+		return -ENOMEM;
+
+	ret = drmm_add_action_or_reset(file->dev, drm_epd_lut_free, lut);
+	if (ret)
+		return ret;
+
+	lut->file	= file;
+	lut->format	= format;
+	lut->max_phases	= max_phases;
+	lut->num_phases	= 0;
+
+	/* Set sane initial values for the waveform and temperature. */
+	lut->mode_index = pvi_wbf_get_mode_index(file, DRM_EPD_WF_RESET);
+	if (lut->mode_index < 0)
+		return -ENOENT;
+
+	lut->temp_index = pvi_wbf_get_temp_index(file, 25);
+	if (lut->temp_index < 0)
+		return -ENOENT;
+
+	ret = drm_epd_lut_update(lut, lut->mode_index, lut->temp_index);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL(drmm_epd_lut_init);
+
+MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
+MODULE_DESCRIPTION("DRM EPD waveform LUT library");
+MODULE_LICENSE("Dual MIT/GPL");
diff --git a/include/drm/drm_epd_helper.h b/include/drm/drm_epd_helper.h
new file mode 100644
index 000000000000..290f5d48c0cb
--- /dev/null
+++ b/include/drm/drm_epd_helper.h
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+
+#ifndef __DRM_EPD_HELPER_H__
+#define __DRM_EPD_HELPER_H__
+
+#define DRM_EPD_DEFAULT_TEMPERATURE	25
+
+struct drm_device;
+struct firmware;
+struct pvi_wbf_file_header;
+struct pvi_wbf_mode_info;
+
+/**
+ * enum drm_epd_waveform - Identifiers for waveforms used to drive EPD pixels
+ *
+ * @DRM_EPD_WF_RESET: Used to initialize the panel, ends with white
+ * @DRM_EPD_WF_A2: Fast transitions between black and white only
+ * @DRM_EPD_WF_DU: Transitions 16-level grayscale to monochrome
+ * @DRM_EPD_WF_DU4: Transitions 16-level grayscale to 4-level grayscale
+ * @DRM_EPD_WF_GC16: High-quality but flashy 16-level grayscale
+ * @DRM_EPD_WF_GCC16: Less flashy 16-level grayscale
+ * @DRM_EPD_WF_GL16: Less flashy 16-level grayscale
+ * @DRM_EPD_WF_GLR16: Less flashy 16-level grayscale, plus anti-ghosting
+ * @DRM_EPD_WF_GLD16: Less flashy 16-level grayscale, plus anti-ghosting
+ */
+enum drm_epd_waveform {
+	DRM_EPD_WF_RESET,
+	DRM_EPD_WF_A2,
+	DRM_EPD_WF_DU,
+	DRM_EPD_WF_DU4,
+	DRM_EPD_WF_GC16,
+	DRM_EPD_WF_GCC16,
+	DRM_EPD_WF_GL16,
+	DRM_EPD_WF_GLR16,
+	DRM_EPD_WF_GLD16,
+	DRM_EPD_WF_MAX
+};
+
+/**
+ * enum drm_epd_lut_format - EPD LUT buffer format
+ *
+ * @DRM_EPD_LUT_4BIT: 4-bit grayscale indexes, 1 byte per element
+ * @DRM_EPD_LUT_4BIT_PACKED: 4-bit grayscale indexes, 2 bits per element
+ * @DRM_EPD_LUT_5BIT: 5-bit grayscale indexes, 1 byte per element
+ * @DRM_EPD_LUT_5BIT_PACKED: 5-bit grayscale indexes, 2 bits per element
+ */
+enum drm_epd_lut_format {
+	DRM_EPD_LUT_4BIT,
+	DRM_EPD_LUT_4BIT_PACKED,
+	DRM_EPD_LUT_5BIT,
+	DRM_EPD_LUT_5BIT_PACKED,
+};
+
+/**
+ * struct drm_epd_lut_file - Describes a file containing EPD LUTs
+ *
+ * @dev: The DRM device owning this LUT file
+ * @fw: The firmware object holding the raw file contents
+ * @header: Vendor-specific LUT file header
+ * @mode_info: Vendor-specific information about available waveforms
+ */
+struct drm_epd_lut_file {
+	struct drm_device			*dev;
+	const struct firmware			*fw;
+	const struct pvi_wbf_file_header	*header;
+	const struct pvi_wbf_mode_info		*mode_info;
+};
+
+/**
+ * struct drm_epd_lut - Describes a particular LUT buffer
+ *
+ * @buf: The LUT, in the format requested by the driver
+ * @file: The file where this LUT was loaded from
+ * @format: The LUT buffer format needed by the driver
+ * @max_phases: The maximum number of waveform phases supported by the driver
+ * @num_phases: The number of waveform phases in the current LUT
+ * @mode_index: Private identifier for the current waveform
+ * @temp_index: Private identifier for the current temperature
+ */
+struct drm_epd_lut {
+	u8				*buf;
+	const struct drm_epd_lut_file	*file;
+	enum drm_epd_lut_format		format;
+	unsigned int			max_phases;
+	unsigned int			num_phases;
+	int				mode_index;
+	int				temp_index;
+};
+
+int drmm_epd_lut_file_init(struct drm_device *dev,
+			   struct drm_epd_lut_file *file,
+			   const char *file_name);
+
+int drmm_epd_lut_init(struct drm_epd_lut_file *file,
+		      struct drm_epd_lut *lut,
+		      enum drm_epd_lut_format format,
+		      unsigned int max_phases);
+
+int drm_epd_lut_set_temperature(struct drm_epd_lut *lut,
+				int temperature);
+int drm_epd_lut_set_waveform(struct drm_epd_lut *lut,
+			     enum drm_epd_waveform waveform);
+
+#endif /* __DRM_EPD_HELPER_H__ */
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 01/16] drm: Add a helper library for EPD drivers Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-14  8:15   ` Andreas Kemnade
  2022-04-13 22:19 ` [RFC PATCH 03/16] drm/rockchip: Add EBC platform driver skeleton Samuel Holland
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

The Rockchip E-Book Controller (EBC) is a controller for Electrophoretic
Displays (EPDs). It is self-contained; it does not interact directly
with the VOP or the RGA.

While two of the regulator consumers here actually power the display
panel, not the EBC hardware, they are consumed here because they are
only needed during display refreshes. They do not match the normal
panel prepare/enable lifecycle.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 .../display/rockchip/rockchip,rk3568-ebc.yaml | 106 ++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip,rk3568-ebc.yaml

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3568-ebc.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3568-ebc.yaml
new file mode 100644
index 000000000000..957ca874ab02
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3568-ebc.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/rockchip/rockchip,rk3568-ebc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip SoC E-Book Controller (EBC)
+
+description:
+  Rockchip EBC is a controller for Electrophoretic Displays (EPDs).
+
+maintainers:
+  - Samuel Holland <samuel@sholland.org>
+
+properties:
+  compatible:
+    enum:
+      - rockchip,rk3568-ebc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: AHB register clock
+      - description: Pixel clock
+
+  clock-names:
+    items:
+      - const: hclk
+      - const: dclk
+
+  resets:
+    items:
+      - description: hclk domain reset
+      - description: dclk domain reset
+
+  reset-names:
+    items:
+      - const: hclk
+      - const: dclk
+
+  io-channels:
+    maxItems: 1
+    description: I/O channel for panel temperature measurement
+
+  panel-supply:
+    description: Regulator supplying the panel's logic voltage
+
+  power-domains:
+    maxItems: 1
+
+  vcom-supply:
+    description: Regulator supplying the panel's compensation voltage
+
+  vdrive-supply:
+    description: Regulator supplying the panel's gate and source drivers
+
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description: OF graph port for the attached display panel
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - power-domains
+  - panel-supply
+  - vcom-supply
+  - vdrive-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rk3568-cru.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/rk3568-power.h>
+
+    ebc: ebc@fdec0000 {
+      compatible = "rockchip,rk3568-ebc";
+      reg = <0x0 0xfdec0000 0x0 0x5000>;
+      interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&cru HCLK_EBC>, <&cru DCLK_EBC>;
+      clock-names = "hclk", "dclk";
+      resets = <&cru SRST_H_EBC>, <&cru SRST_D_EBC>;
+      reset-names = "hclk", "dclk";
+      power-domains = <&power RK3568_PD_RGA>;
+
+      panel-supply = <&v3p3>;
+      vcom-supply = <&vcom>;
+      vdrive-supply = <&vdrive>;
+
+      port {
+        ebc_out_panel: endpoint {
+          remote-endpoint = <&panel_in_ebc>;
+        };
+      };
+    };
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 03/16] drm/rockchip: Add EBC platform driver skeleton
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 01/16] drm: Add a helper library for EPD drivers Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 04/16] drm/rockchip: ebc: Add DRM " Samuel Holland
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

The Rockchip E-Book Controller (EBC) is a timing controller (TCON)
responsible for sending timing signals and pixel update waveforms to an
electrophoretic display (EPD). The EBC has several modes of operation.
In direct mode, it reads precomputed source driver polarity data from a
series of buffers in RAM. In the other modes, it reads pixel luminance
data from RAM, and uses a lookup table (LUT) to compute the source
driver polarity for each phase within the waveform.

This commit adds a platform driver skeleton for the EBC, containing the
IRQ handler and runtime PM hooks. The EBC only needs to be powered up
when the display is actively being refreshed. regcache is used to allow
configuration changes (i.e. modeset) while the EBC is powered down.

While two of the regulator consumers here actually power the display
panel, not the EBC hardware, they are consumed here because again they
are only needed during display refreshes. They do not match the normal
panel prepare/enable lifecycle.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/Makefile                |   2 +-
 drivers/gpu/drm/rockchip/Kconfig        |  11 +
 drivers/gpu/drm/rockchip/Makefile       |   2 +
 drivers/gpu/drm/rockchip/rockchip_ebc.c | 324 ++++++++++++++++++++++++
 4 files changed, 338 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_ebc.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 49380ccfe9d6..e940f81a2acf 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -93,7 +93,7 @@ obj-$(CONFIG_DRM_VGEM)	+= vgem/
 obj-$(CONFIG_DRM_VKMS)	+= vkms/
 obj-$(CONFIG_DRM_NOUVEAU) +=nouveau/
 obj-$(CONFIG_DRM_EXYNOS) +=exynos/
-obj-$(CONFIG_DRM_ROCKCHIP) +=rockchip/
+obj-y			+=rockchip/
 obj-$(CONFIG_DRM_GMA500) += gma500/
 obj-$(CONFIG_DRM_UDL) += udl/
 obj-$(CONFIG_DRM_AST) += ast/
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index fa5cfda4e90e..9d3273a5fd97 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -91,3 +91,14 @@ config ROCKCHIP_RK3066_HDMI
 	  for the RK3066 HDMI driver. If you want to enable
 	  HDMI on RK3066 based SoC, you should select this option.
 endif
+
+config DRM_ROCKCHIP_EBC
+	tristate "DRM Support for Rockchip EBC"
+	depends on DRM
+	select DRM_GEM_SHMEM_HELPER
+	select DRM_KMS_HELPER
+	help
+	  This selects DRM/KMS support for the Rockchip E-Book Controller (EBC).
+	  Choose this option if you have a Rockchip SoC and an electrophoretic
+	  display. This hardware and driver is separate from the normal Rockchip
+	  display hardware and DRM driver.
diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
index 1a56f696558c..e3accc526438 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -16,3 +16,5 @@ rockchipdrm-$(CONFIG_ROCKCHIP_RGB) += rockchip_rgb.o
 rockchipdrm-$(CONFIG_ROCKCHIP_RK3066_HDMI) += rk3066_hdmi.o
 
 obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
+
+obj-$(CONFIG_DRM_ROCKCHIP_EBC) += rockchip_ebc.o
diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
new file mode 100644
index 000000000000..5ed66c6cd2f0
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -0,0 +1,324 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021-2022 Samuel Holland <samuel@sholland.org>
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+#define EBC_DSP_START			0x0000
+#define EBC_DSP_START_DSP_OUT_LOW		BIT(31)
+#define EBC_DSP_START_DSP_SDCE_WIDTH(x)		((x) << 16)
+#define EBC_DSP_START_DSP_EINK_MODE		BIT(13)
+#define EBC_DSP_START_SW_BURST_CTRL		BIT(12)
+#define EBC_DSP_START_DSP_FRM_TOTAL(x)		((x) << 2)
+#define EBC_DSP_START_DSP_RST			BIT(1)
+#define EBC_DSP_START_DSP_FRM_START		BIT(0)
+#define EBC_EPD_CTRL			0x0004
+#define EBC_EPD_CTRL_EINK_MODE_SWAP		BIT(31)
+#define EBC_EPD_CTRL_DSP_GD_END(x)		((x) << 16)
+#define EBC_EPD_CTRL_DSP_GD_ST(x)		((x) << 8)
+#define EBC_EPD_CTRL_DSP_THREE_WIN_MODE		BIT(7)
+#define EBC_EPD_CTRL_DSP_SDDW_MODE		BIT(6)
+#define EBC_EPD_CTRL_EPD_AUO			BIT(5)
+#define EBC_EPD_CTRL_EPD_PWR(x)			((x) << 2)
+#define EBC_EPD_CTRL_EPD_GDRL			BIT(1)
+#define EBC_EPD_CTRL_EPD_SDSHR			BIT(0)
+#define EBC_DSP_CTRL			0x0008
+#define EBC_DSP_CTRL_DSP_SWAP_MODE(x)		((x) << 30)
+#define EBC_DSP_CTRL_DSP_DIFF_MODE		BIT(29)
+#define EBC_DSP_CTRL_DSP_LUT_MODE		BIT(28)
+#define EBC_DSP_CTRL_DSP_VCOM_MODE		BIT(27)
+#define EBC_DSP_CTRL_DSP_GDOE_POL		BIT(26)
+#define EBC_DSP_CTRL_DSP_GDSP_POL		BIT(25)
+#define EBC_DSP_CTRL_DSP_GDCLK_POL		BIT(24)
+#define EBC_DSP_CTRL_DSP_SDCE_POL		BIT(23)
+#define EBC_DSP_CTRL_DSP_SDOE_POL		BIT(22)
+#define EBC_DSP_CTRL_DSP_SDLE_POL		BIT(21)
+#define EBC_DSP_CTRL_DSP_SDCLK_POL		BIT(20)
+#define EBC_DSP_CTRL_DSP_SDCLK_DIV(x)		((x) << 16)
+#define EBC_DSP_CTRL_DSP_BACKGROUND(x)		((x) << 0)
+#define EBC_DSP_HTIMING0		0x000c
+#define EBC_DSP_HTIMING0_DSP_HTOTAL(x)		((x) << 16)
+#define EBC_DSP_HTIMING0_DSP_HS_END(x)		((x) << 0)
+#define EBC_DSP_HTIMING1		0x0010
+#define EBC_DSP_HTIMING1_DSP_HACT_END(x)	((x) << 16)
+#define EBC_DSP_HTIMING1_DSP_HACT_ST(x)		((x) << 0)
+#define EBC_DSP_VTIMING0		0x0014
+#define EBC_DSP_VTIMING0_DSP_VTOTAL(x)		((x) << 16)
+#define EBC_DSP_VTIMING0_DSP_VS_END(x)		((x) << 0)
+#define EBC_DSP_VTIMING1		0x0018
+#define EBC_DSP_VTIMING1_DSP_VACT_END(x)	((x) << 16)
+#define EBC_DSP_VTIMING1_DSP_VACT_ST(x)		((x) << 0)
+#define EBC_DSP_ACT_INFO		0x001c
+#define EBC_DSP_ACT_INFO_DSP_HEIGHT(x)		((x) << 16)
+#define EBC_DSP_ACT_INFO_DSP_WIDTH(x)		((x) << 0)
+#define EBC_WIN_CTRL			0x0020
+#define EBC_WIN_CTRL_WIN2_FIFO_THRESHOLD(x)	((x) << 19)
+#define EBC_WIN_CTRL_WIN_EN			BIT(18)
+#define EBC_WIN_CTRL_AHB_INCR_NUM_REG(x)	((x) << 13)
+#define EBC_WIN_CTRL_AHB_BURST_REG(x)		((x) << 10)
+#define EBC_WIN_CTRL_WIN_FIFO_THRESHOLD(x)	((x) << 2)
+#define EBC_WIN_CTRL_WIN_FMT_Y4			(0x0 << 0)
+#define EBC_WIN_CTRL_WIN_FMT_Y8			(0x1 << 0)
+#define EBC_WIN_CTRL_WIN_FMT_XRGB8888		(0x2 << 0)
+#define EBC_WIN_CTRL_WIN_FMT_RGB565		(0x3 << 0)
+#define EBC_WIN_MST0			0x0024
+#define EBC_WIN_MST1			0x0028
+#define EBC_WIN_VIR			0x002c
+#define EBC_WIN_VIR_WIN_VIR_HEIGHT(x)		((x) << 16)
+#define EBC_WIN_VIR_WIN_VIR_WIDTH(x)		((x) << 0)
+#define EBC_WIN_ACT			0x0030
+#define EBC_WIN_ACT_WIN_ACT_HEIGHT(x)		((x) << 16)
+#define EBC_WIN_ACT_WIN_ACT_WIDTH(x)		((x) << 0)
+#define EBC_WIN_DSP			0x0034
+#define EBC_WIN_DSP_WIN_DSP_HEIGHT(x)		((x) << 16)
+#define EBC_WIN_DSP_WIN_DSP_WIDTH(x)		((x) << 0)
+#define EBC_WIN_DSP_ST			0x0038
+#define EBC_WIN_DSP_ST_WIN_DSP_YST(x)		((x) << 16)
+#define EBC_WIN_DSP_ST_WIN_DSP_XST(x)		((x) << 0)
+#define EBC_INT_STATUS			0x003c
+#define EBC_INT_STATUS_DSP_FRM_INT_NUM(x)	((x) << 12)
+#define EBC_INT_STATUS_LINE_FLAG_INT_CLR	BIT(11)
+#define EBC_INT_STATUS_DSP_FRM_INT_CLR		BIT(10)
+#define EBC_INT_STATUS_DSP_END_INT_CLR		BIT(9)
+#define EBC_INT_STATUS_FRM_END_INT_CLR		BIT(8)
+#define EBC_INT_STATUS_LINE_FLAG_INT_MSK	BIT(7)
+#define EBC_INT_STATUS_DSP_FRM_INT_MSK		BIT(6)
+#define EBC_INT_STATUS_DSP_END_INT_MSK		BIT(5)
+#define EBC_INT_STATUS_FRM_END_INT_MSK		BIT(4)
+#define EBC_INT_STATUS_LINE_FLAG_INT_ST		BIT(3)
+#define EBC_INT_STATUS_DSP_FRM_INT_ST		BIT(2)
+#define EBC_INT_STATUS_DSP_END_INT_ST		BIT(1)
+#define EBC_INT_STATUS_FRM_END_INT_ST		BIT(0)
+#define EBC_VCOM0			0x0040
+#define EBC_VCOM1			0x0044
+#define EBC_VCOM2			0x0048
+#define EBC_VCOM3			0x004c
+#define EBC_CONFIG_DONE			0x0050
+#define EBC_CONFIG_DONE_REG_CONFIG_DONE		BIT(0)
+#define EBC_VNUM			0x0054
+#define EBC_VNUM_DSP_VCNT(x)			((x) << 16)
+#define EBC_VNUM_LINE_FLAG_NUM(x)		((x) << 0)
+#define EBC_WIN_MST2			0x0058
+#define EBC_LUT_DATA			0x1000
+
+#define EBC_NUM_LUT_REGS		0x1000
+#define EBC_NUM_SUPPLIES		3
+
+#define EBC_SUSPEND_DELAY_MS		2000
+
+struct rockchip_ebc {
+	struct clk			*dclk;
+	struct clk			*hclk;
+	struct completion		display_end;
+	struct regmap			*regmap;
+	struct regulator_bulk_data	supplies[EBC_NUM_SUPPLIES];
+};
+
+static int rockchip_ebc_runtime_suspend(struct device *dev)
+{
+	struct rockchip_ebc *ebc = dev_get_drvdata(dev);
+
+	regcache_cache_only(ebc->regmap, true);
+
+	clk_disable_unprepare(ebc->dclk);
+	clk_disable_unprepare(ebc->hclk);
+	regulator_bulk_disable(EBC_NUM_SUPPLIES, ebc->supplies);
+
+	return 0;
+}
+
+static int rockchip_ebc_runtime_resume(struct device *dev)
+{
+	struct rockchip_ebc *ebc = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regulator_bulk_enable(EBC_NUM_SUPPLIES, ebc->supplies);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(ebc->hclk);
+	if (ret)
+		goto err_disable_supplies;
+
+	ret = clk_prepare_enable(ebc->dclk);
+	if (ret)
+		goto err_disable_hclk;
+
+	regcache_cache_only(ebc->regmap, false);
+	regcache_mark_dirty(ebc->regmap);
+	regcache_sync(ebc->regmap);
+
+	regmap_write(ebc->regmap, EBC_INT_STATUS,
+		     EBC_INT_STATUS_DSP_END_INT_CLR |
+		     EBC_INT_STATUS_LINE_FLAG_INT_MSK |
+		     EBC_INT_STATUS_DSP_FRM_INT_MSK |
+		     EBC_INT_STATUS_FRM_END_INT_MSK);
+
+	return 0;
+
+err_disable_hclk:
+	clk_disable_unprepare(ebc->hclk);
+err_disable_supplies:
+	regulator_bulk_disable(EBC_NUM_SUPPLIES, ebc->supplies);
+
+	return ret;
+}
+
+static const struct dev_pm_ops rockchip_ebc_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(rockchip_ebc_runtime_suspend,
+			   rockchip_ebc_runtime_resume, NULL)
+};
+
+static bool rockchip_ebc_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case EBC_DSP_START:
+	case EBC_INT_STATUS:
+	case EBC_CONFIG_DONE:
+	case EBC_VNUM:
+		return true;
+	default:
+		/* Do not cache the LUT registers. */
+		return reg > EBC_WIN_MST2;
+	}
+}
+
+static const struct regmap_config rockchip_ebc_regmap_config = {
+	.reg_bits	= 32,
+	.reg_stride	= 4,
+	.val_bits	= 32,
+	.volatile_reg	= rockchip_ebc_volatile_reg,
+	.max_register	= 0x4ffc, /* end of EBC_LUT_DATA */
+	.cache_type	= REGCACHE_FLAT,
+};
+
+static const char *const rockchip_ebc_supplies[EBC_NUM_SUPPLIES] = {
+	"panel",
+	"vcom",
+	"vdrive",
+};
+
+static irqreturn_t rockchip_ebc_irq(int irq, void *dev_id)
+{
+	struct rockchip_ebc *ebc = dev_id;
+	unsigned int status;
+
+	regmap_read(ebc->regmap, EBC_INT_STATUS, &status);
+
+	if (status & EBC_INT_STATUS_DSP_END_INT_ST) {
+		status |= EBC_INT_STATUS_DSP_END_INT_CLR;
+		complete(&ebc->display_end);
+	}
+
+	regmap_write(ebc->regmap, EBC_INT_STATUS, status);
+
+	return IRQ_HANDLED;
+}
+
+static int rockchip_ebc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rockchip_ebc *ebc;
+	void __iomem *base;
+	int i, ret;
+
+	ebc = devm_kzalloc(dev, sizeof(*ebc), GFP_KERNEL);
+	if (!ebc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, ebc);
+	init_completion(&ebc->display_end);
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	ebc->regmap = devm_regmap_init_mmio(dev, base,
+					     &rockchip_ebc_regmap_config);
+	if (IS_ERR(ebc->regmap))
+		return PTR_ERR(ebc->regmap);
+
+	regcache_cache_only(ebc->regmap, true);
+
+	ebc->dclk = devm_clk_get(dev, "dclk");
+	if (IS_ERR(ebc->dclk))
+		return dev_err_probe(dev, PTR_ERR(ebc->dclk),
+				     "Failed to get dclk\n");
+
+	ebc->hclk = devm_clk_get(dev, "hclk");
+	if (IS_ERR(ebc->hclk))
+		return dev_err_probe(dev, PTR_ERR(ebc->hclk),
+				     "Failed to get hclk\n");
+
+	for (i = 0; i < EBC_NUM_SUPPLIES; i++)
+		ebc->supplies[i].supply = rockchip_ebc_supplies[i];
+
+	ret = devm_regulator_bulk_get(dev, EBC_NUM_SUPPLIES, ebc->supplies);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get supplies\n");
+
+	ret = devm_request_irq(dev, platform_get_irq(pdev, 0),
+			       rockchip_ebc_irq, 0, dev_name(dev), ebc);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
+
+	pm_runtime_set_autosuspend_delay(dev, EBC_SUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
+	if (!pm_runtime_enabled(dev)) {
+		ret = rockchip_ebc_runtime_resume(&pdev->dev);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int rockchip_ebc_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+
+	pm_runtime_disable(dev);
+	if (!pm_runtime_status_suspended(dev))
+		rockchip_ebc_runtime_suspend(dev);
+
+	return 0;
+}
+
+static void rockchip_ebc_shutdown(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+
+	if (!pm_runtime_status_suspended(dev))
+		rockchip_ebc_runtime_suspend(dev);
+}
+
+static const struct of_device_id rockchip_ebc_of_match[] = {
+	{ .compatible = "rockchip,rk3568-ebc" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rockchip_ebc_of_match);
+
+static struct platform_driver rockchip_ebc_driver = {
+	.probe		= rockchip_ebc_probe,
+	.remove		= rockchip_ebc_remove,
+	.shutdown	= rockchip_ebc_shutdown,
+	.driver		= {
+		.name		= "rockchip-ebc",
+		.of_match_table	= rockchip_ebc_of_match,
+		.pm		= &rockchip_ebc_dev_pm_ops,
+	},
+};
+module_platform_driver(rockchip_ebc_driver);
+
+MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
+MODULE_DESCRIPTION("Rockchip EBC driver");
+MODULE_LICENSE("GPL");
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 04/16] drm/rockchip: ebc: Add DRM driver skeleton
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (2 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 03/16] drm/rockchip: Add EBC platform driver skeleton Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 05/16] drm/rockchip: ebc: Add CRTC mode setting Samuel Holland
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

The Rockchip E-Book Controller (EBC) has a relatively simple and self-
contained display pipeline. The pipeline consists of a single CRTC,
encoder, and bridge, with the bridge normally attached to a panel.

Initially, there is also a single plane. Since all blitting is done in
software, the driver could eventually support any number of planes. For
example, it could expose one plane for each supported waveform.

However, EPD controller hardware has some unique properties which
complicate using drm_simple_display_pipe:
  - EPDs operate on relative pixel values, not absolute pixel values.
    This requires the driver to maintain multiple shadow buffers for the
    "previous" and "next" framebuffer contents.
  - It also means that disabling the CRTC (i.e. clearing the screen)
    requires access to these shadow buffers, as it requires knowing the
    previous contents of the framebuffer. And of course it requires a
    buffer for the blank image.
  - Atomically managing these shadow buffers needs reference counting in
    .atomic_check. However, drm_simple_display_pipe_funcs::check is only
    called while the plane is visible, complicating this.
  - Furthermore, because all plane blitting/blending must be done in
    software, the number and location of these planes is arbitrary.
    drm_simple_display_pipe enforces an unnecessary limitation that a
    single plane covers the entire CRTC.

For these reasons, drm_simple_display_pipe is not used.

This commit adds the structure for this pipeline. The atomic helper
callbacks are left empty. They will be filled in incrementally by the
next several commits.

Both the CRTC and the pipe need extra state information, so this commit
adds the state hook boilerplate. Additionally, the plane takes advantage
of the shadow plane helpers.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/rockchip/rockchip_ebc.c | 359 +++++++++++++++++++++++-
 1 file changed, 356 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
index 5ed66c6cd2f0..f75fd23adda2 100644
--- a/drivers/gpu/drm/rockchip/rockchip_ebc.c
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -12,6 +12,17 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_plane_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+
 #define EBC_DSP_START			0x0000
 #define EBC_DSP_START_DSP_OUT_LOW		BIT(31)
 #define EBC_DSP_START_DSP_SDCE_WIDTH(x)		((x) << 16)
@@ -118,10 +129,332 @@ struct rockchip_ebc {
 	struct clk			*dclk;
 	struct clk			*hclk;
 	struct completion		display_end;
+	struct drm_crtc			crtc;
+	struct drm_device		drm;
+	struct drm_encoder		encoder;
+	struct drm_plane		plane;
 	struct regmap			*regmap;
 	struct regulator_bulk_data	supplies[EBC_NUM_SUPPLIES];
 };
 
+DEFINE_DRM_GEM_FOPS(rockchip_ebc_fops);
+
+static const struct drm_driver rockchip_ebc_drm_driver = {
+	.lastclose		= drm_fb_helper_lastclose,
+	DRM_GEM_SHMEM_DRIVER_OPS,
+	.major			= 0,
+	.minor			= 3,
+	.name			= "rockchip-ebc",
+	.desc			= "Rockchip E-Book Controller",
+	.date			= "20220303",
+	.driver_features	= DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
+	.fops			= &rockchip_ebc_fops,
+};
+
+static const struct drm_mode_config_funcs rockchip_ebc_mode_config_funcs = {
+	.fb_create		= drm_gem_fb_create_with_dirty,
+	.atomic_check		= drm_atomic_helper_check,
+	.atomic_commit		= drm_atomic_helper_commit,
+};
+
+/*
+ * CRTC
+ */
+
+struct ebc_crtc_state {
+	struct drm_crtc_state		base;
+};
+
+static inline struct ebc_crtc_state *
+to_ebc_crtc_state(struct drm_crtc_state *crtc_state)
+{
+	return container_of(crtc_state, struct ebc_crtc_state, base);
+}
+
+static inline struct rockchip_ebc *crtc_to_ebc(struct drm_crtc *crtc)
+{
+	return container_of(crtc, struct rockchip_ebc, crtc);
+}
+
+static void rockchip_ebc_crtc_mode_set_nofb(struct drm_crtc *crtc)
+{
+}
+
+static int rockchip_ebc_crtc_atomic_check(struct drm_crtc *crtc,
+					  struct drm_atomic_state *state)
+{
+	return 0;
+}
+
+static void rockchip_ebc_crtc_atomic_flush(struct drm_crtc *crtc,
+					   struct drm_atomic_state *state)
+{
+}
+
+static void rockchip_ebc_crtc_atomic_enable(struct drm_crtc *crtc,
+					    struct drm_atomic_state *state)
+{
+}
+
+static void rockchip_ebc_crtc_atomic_disable(struct drm_crtc *crtc,
+					     struct drm_atomic_state *state)
+{
+}
+
+static const struct drm_crtc_helper_funcs rockchip_ebc_crtc_helper_funcs = {
+	.mode_set_nofb		= rockchip_ebc_crtc_mode_set_nofb,
+	.atomic_check		= rockchip_ebc_crtc_atomic_check,
+	.atomic_flush		= rockchip_ebc_crtc_atomic_flush,
+	.atomic_enable		= rockchip_ebc_crtc_atomic_enable,
+	.atomic_disable		= rockchip_ebc_crtc_atomic_disable,
+};
+
+static void rockchip_ebc_crtc_destroy_state(struct drm_crtc *crtc,
+					    struct drm_crtc_state *crtc_state);
+
+static void rockchip_ebc_crtc_reset(struct drm_crtc *crtc)
+{
+	struct ebc_crtc_state *ebc_crtc_state;
+
+	if (crtc->state)
+		rockchip_ebc_crtc_destroy_state(crtc, crtc->state);
+
+	ebc_crtc_state = kzalloc(sizeof(*ebc_crtc_state), GFP_KERNEL);
+	if (!ebc_crtc_state)
+		return;
+
+	__drm_atomic_helper_crtc_reset(crtc, &ebc_crtc_state->base);
+}
+
+static struct drm_crtc_state *
+rockchip_ebc_crtc_duplicate_state(struct drm_crtc *crtc)
+{
+	struct ebc_crtc_state *ebc_crtc_state;
+
+	if (!crtc->state)
+		return NULL;
+
+	ebc_crtc_state = kzalloc(sizeof(*ebc_crtc_state), GFP_KERNEL);
+	if (!ebc_crtc_state)
+		return NULL;
+
+	__drm_atomic_helper_crtc_duplicate_state(crtc, &ebc_crtc_state->base);
+
+	return &ebc_crtc_state->base;
+}
+
+static void rockchip_ebc_crtc_destroy_state(struct drm_crtc *crtc,
+					    struct drm_crtc_state *crtc_state)
+{
+	struct ebc_crtc_state *ebc_crtc_state = to_ebc_crtc_state(crtc_state);
+
+	__drm_atomic_helper_crtc_destroy_state(&ebc_crtc_state->base);
+
+	kfree(ebc_crtc_state);
+}
+
+static const struct drm_crtc_funcs rockchip_ebc_crtc_funcs = {
+	.reset			= rockchip_ebc_crtc_reset,
+	.destroy		= drm_crtc_cleanup,
+	.set_config		= drm_atomic_helper_set_config,
+	.page_flip		= drm_atomic_helper_page_flip,
+	.atomic_duplicate_state	= rockchip_ebc_crtc_duplicate_state,
+	.atomic_destroy_state	= rockchip_ebc_crtc_destroy_state,
+};
+
+/*
+ * Plane
+ */
+
+struct ebc_plane_state {
+	struct drm_shadow_plane_state	base;
+};
+
+static inline struct ebc_plane_state *
+to_ebc_plane_state(struct drm_plane_state *plane_state)
+{
+	return container_of(plane_state, struct ebc_plane_state, base.base);
+}
+
+static inline struct rockchip_ebc *plane_to_ebc(struct drm_plane *plane)
+{
+	return container_of(plane, struct rockchip_ebc, plane);
+}
+
+static int rockchip_ebc_plane_atomic_check(struct drm_plane *plane,
+					   struct drm_atomic_state *state)
+{
+	struct drm_plane_state *plane_state;
+	struct drm_crtc_state *crtc_state;
+	int ret;
+
+	plane_state = drm_atomic_get_new_plane_state(state, plane);
+	if (!plane_state->crtc)
+		return 0;
+
+	crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc);
+	ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  true, true);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void rockchip_ebc_plane_atomic_update(struct drm_plane *plane,
+					     struct drm_atomic_state *state)
+{
+}
+
+static const struct drm_plane_helper_funcs rockchip_ebc_plane_helper_funcs = {
+	.prepare_fb		= drm_gem_prepare_shadow_fb,
+	.cleanup_fb		= drm_gem_cleanup_shadow_fb,
+	.atomic_check		= rockchip_ebc_plane_atomic_check,
+	.atomic_update		= rockchip_ebc_plane_atomic_update,
+};
+
+static void rockchip_ebc_plane_destroy_state(struct drm_plane *plane,
+					     struct drm_plane_state *plane_state);
+
+static void rockchip_ebc_plane_reset(struct drm_plane *plane)
+{
+	struct ebc_plane_state *ebc_plane_state;
+
+	if (plane->state)
+		rockchip_ebc_plane_destroy_state(plane, plane->state);
+
+	ebc_plane_state = kzalloc(sizeof(*ebc_plane_state), GFP_KERNEL);
+	if (!ebc_plane_state)
+		return;
+
+	__drm_gem_reset_shadow_plane(plane, &ebc_plane_state->base);
+}
+
+static struct drm_plane_state *
+rockchip_ebc_plane_duplicate_state(struct drm_plane *plane)
+{
+	struct ebc_plane_state *ebc_plane_state;
+
+	if (!plane->state)
+		return NULL;
+
+	ebc_plane_state = kzalloc(sizeof(*ebc_plane_state), GFP_KERNEL);
+	if (!ebc_plane_state)
+		return NULL;
+
+	__drm_gem_duplicate_shadow_plane_state(plane, &ebc_plane_state->base);
+
+	return &ebc_plane_state->base.base;
+}
+
+static void rockchip_ebc_plane_destroy_state(struct drm_plane *plane,
+					     struct drm_plane_state *plane_state)
+{
+	struct ebc_plane_state *ebc_plane_state = to_ebc_plane_state(plane_state);
+
+	__drm_gem_destroy_shadow_plane_state(&ebc_plane_state->base);
+
+	kfree(ebc_plane_state);
+}
+
+static const struct drm_plane_funcs rockchip_ebc_plane_funcs = {
+	.update_plane		= drm_atomic_helper_update_plane,
+	.disable_plane		= drm_atomic_helper_disable_plane,
+	.destroy		= drm_plane_cleanup,
+	.reset			= rockchip_ebc_plane_reset,
+	.atomic_duplicate_state	= rockchip_ebc_plane_duplicate_state,
+	.atomic_destroy_state	= rockchip_ebc_plane_destroy_state,
+};
+
+static const u32 rockchip_ebc_plane_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+static const u64 rockchip_ebc_plane_format_modifiers[] = {
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID
+};
+
+static int rockchip_ebc_drm_init(struct rockchip_ebc *ebc)
+{
+	struct drm_device *drm = &ebc->drm;
+	struct drm_bridge *bridge;
+	int ret;
+
+	ret = drmm_mode_config_init(drm);
+	if (ret)
+		return ret;
+
+	drm->mode_config.max_width = DRM_SHADOW_PLANE_MAX_WIDTH;
+	drm->mode_config.max_height = DRM_SHADOW_PLANE_MAX_HEIGHT;
+	drm->mode_config.funcs = &rockchip_ebc_mode_config_funcs;
+	drm->mode_config.quirk_addfb_prefer_host_byte_order = true;
+
+	drm_plane_helper_add(&ebc->plane, &rockchip_ebc_plane_helper_funcs);
+	ret = drm_universal_plane_init(drm, &ebc->plane, 0,
+				       &rockchip_ebc_plane_funcs,
+				       rockchip_ebc_plane_formats,
+				       ARRAY_SIZE(rockchip_ebc_plane_formats),
+				       rockchip_ebc_plane_format_modifiers,
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret)
+		return ret;
+
+	drm_plane_enable_fb_damage_clips(&ebc->plane);
+
+	drm_crtc_helper_add(&ebc->crtc, &rockchip_ebc_crtc_helper_funcs);
+	ret = drm_crtc_init_with_planes(drm, &ebc->crtc, &ebc->plane, NULL,
+					&rockchip_ebc_crtc_funcs, NULL);
+	if (ret)
+		return ret;
+
+	ebc->encoder.possible_crtcs = drm_crtc_mask(&ebc->crtc);
+	ret = drm_simple_encoder_init(drm, &ebc->encoder, DRM_MODE_ENCODER_NONE);
+	if (ret)
+		return ret;
+
+	bridge = devm_drm_of_get_bridge(drm->dev, drm->dev->of_node, 0, 0);
+	if (IS_ERR(bridge))
+		return PTR_ERR(bridge);
+
+	ret = drm_bridge_attach(&ebc->encoder, bridge, NULL, 0);
+	if (ret)
+		return ret;
+
+	drm_mode_config_reset(drm);
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	drm_fbdev_generic_setup(drm, 0);
+
+	return 0;
+}
+
+static int __maybe_unused rockchip_ebc_suspend(struct device *dev)
+{
+	struct rockchip_ebc *ebc = dev_get_drvdata(dev);
+	int ret;
+
+	ret = drm_mode_config_helper_suspend(&ebc->drm);
+	if (ret)
+		return ret;
+
+	return pm_runtime_force_suspend(dev);
+}
+
+static int __maybe_unused rockchip_ebc_resume(struct device *dev)
+{
+	struct rockchip_ebc *ebc = dev_get_drvdata(dev);
+
+	pm_runtime_force_resume(dev);
+
+	return drm_mode_config_helper_resume(&ebc->drm);
+}
+
 static int rockchip_ebc_runtime_suspend(struct device *dev)
 {
 	struct rockchip_ebc *ebc = dev_get_drvdata(dev);
@@ -173,6 +506,7 @@ static int rockchip_ebc_runtime_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops rockchip_ebc_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(rockchip_ebc_suspend, rockchip_ebc_resume)
 	SET_RUNTIME_PM_OPS(rockchip_ebc_runtime_suspend,
 			   rockchip_ebc_runtime_resume, NULL)
 };
@@ -230,9 +564,10 @@ static int rockchip_ebc_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int i, ret;
 
-	ebc = devm_kzalloc(dev, sizeof(*ebc), GFP_KERNEL);
-	if (!ebc)
-		return -ENOMEM;
+	ebc = devm_drm_dev_alloc(dev, &rockchip_ebc_drm_driver,
+				 struct rockchip_ebc, drm);
+	if (IS_ERR(ebc))
+		return PTR_ERR(ebc);
 
 	platform_set_drvdata(pdev, ebc);
 	init_completion(&ebc->display_end);
@@ -279,13 +614,28 @@ static int rockchip_ebc_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	ret = rockchip_ebc_drm_init(ebc);
+	if (ret)
+		goto err_disable_pm;
+
 	return 0;
+
+err_disable_pm:
+	pm_runtime_disable(dev);
+	if (!pm_runtime_status_suspended(dev))
+		rockchip_ebc_runtime_suspend(dev);
+
+	return ret;
 }
 
 static int rockchip_ebc_remove(struct platform_device *pdev)
 {
+	struct rockchip_ebc *ebc = platform_get_drvdata(pdev);
 	struct device *dev = &pdev->dev;
 
+	drm_dev_unregister(&ebc->drm);
+	drm_atomic_helper_shutdown(&ebc->drm);
+
 	pm_runtime_disable(dev);
 	if (!pm_runtime_status_suspended(dev))
 		rockchip_ebc_runtime_suspend(dev);
@@ -295,8 +645,11 @@ static int rockchip_ebc_remove(struct platform_device *pdev)
 
 static void rockchip_ebc_shutdown(struct platform_device *pdev)
 {
+	struct rockchip_ebc *ebc = platform_get_drvdata(pdev);
 	struct device *dev = &pdev->dev;
 
+	drm_atomic_helper_shutdown(&ebc->drm);
+
 	if (!pm_runtime_status_suspended(dev))
 		rockchip_ebc_runtime_suspend(dev);
 }
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 05/16] drm/rockchip: ebc: Add CRTC mode setting
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (3 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 04/16] drm/rockchip: ebc: Add DRM " Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 06/16] drm/rockchip: ebc: Add CRTC refresh thread Samuel Holland
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

EPDs require some additional timing data beyond what is normally
provided by drm_display_mode, as that struct is designed for CRTs/LCDs.
For example, EPDs care about the width and position of the gate driver
(vertical) clock pulse within a line.

EPDs also update some number of pixels in parallel, based on the
interface width, which of course varies by panel. Only two data bits are
used for each pixel, to choose between driving it positive, negative, or
neither direction. Color depth is thus not limited by interface width,
but by time (the number of phases in the active waveform).

This additional timing information is packed inside drm_display_mode as
hskew and DRM_MODE_FLAG_CLKDIV2. This allows getting the complete mode
from a DRM bridge.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/rockchip/rockchip_ebc.c | 102 ++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
index f75fd23adda2..5f9502313657 100644
--- a/drivers/gpu/drm/rockchip/rockchip_ebc.c
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -135,6 +135,7 @@ struct rockchip_ebc {
 	struct drm_plane		plane;
 	struct regmap			*regmap;
 	struct regulator_bulk_data	supplies[EBC_NUM_SUPPLIES];
+	u32				dsp_start;
 };
 
 DEFINE_DRM_GEM_FOPS(rockchip_ebc_fops);
@@ -178,11 +179,112 @@ static inline struct rockchip_ebc *crtc_to_ebc(struct drm_crtc *crtc)
 
 static void rockchip_ebc_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
+	struct rockchip_ebc *ebc = crtc_to_ebc(crtc);
+	struct drm_display_mode mode = crtc->state->adjusted_mode;
+	struct drm_display_mode sdck;
+	u16 hsync_width, vsync_width;
+	u16 hact_start, vact_start;
+	u16 pixels_per_sdck;
+	bool bus_16bit;
+
+	/*
+	 * Hardware needs horizontal timings in SDCK (source driver clock)
+	 * cycles, not pixels. Bus width is either 8 bits (normal) or 16 bits
+	 * (DRM_MODE_FLAG_CLKDIV2), and each pixel uses two data bits.
+	 */
+	bus_16bit = !!(mode.flags & DRM_MODE_FLAG_CLKDIV2);
+	pixels_per_sdck = bus_16bit ? 8 : 4;
+	sdck.hdisplay = mode.hdisplay / pixels_per_sdck;
+	sdck.hsync_start = mode.hsync_start / pixels_per_sdck;
+	sdck.hsync_end = mode.hsync_end / pixels_per_sdck;
+	sdck.htotal = mode.htotal / pixels_per_sdck;
+	sdck.hskew = mode.hskew / pixels_per_sdck;
+
+	/*
+	 * Linux timing order is display/fp/sync/bp. Hardware timing order is
+	 * sync/bp/display/fp, aka sync/start/display/end.
+	 */
+	hact_start = sdck.htotal - sdck.hsync_start;
+	vact_start = mode.vtotal - mode.vsync_start;
+
+	hsync_width = sdck.hsync_end - sdck.hsync_start;
+	vsync_width = mode.vsync_end - mode.vsync_start;
+
+	clk_set_rate(ebc->dclk, mode.clock * 1000);
+
+	ebc->dsp_start = EBC_DSP_START_DSP_SDCE_WIDTH(sdck.hdisplay) |
+			 EBC_DSP_START_SW_BURST_CTRL;
+	regmap_write(ebc->regmap, EBC_EPD_CTRL,
+		     EBC_EPD_CTRL_DSP_GD_END(sdck.htotal - sdck.hskew) |
+		     EBC_EPD_CTRL_DSP_GD_ST(hsync_width + sdck.hskew) |
+		     EBC_EPD_CTRL_DSP_SDDW_MODE * bus_16bit);
+	regmap_write(ebc->regmap, EBC_DSP_CTRL,
+		     /* no swap */
+		     EBC_DSP_CTRL_DSP_SWAP_MODE(bus_16bit ? 2 : 3) |
+		     EBC_DSP_CTRL_DSP_SDCLK_DIV(pixels_per_sdck - 1));
+	regmap_write(ebc->regmap, EBC_DSP_HTIMING0,
+		     EBC_DSP_HTIMING0_DSP_HTOTAL(sdck.htotal) |
+		     /* sync end == sync width */
+		     EBC_DSP_HTIMING0_DSP_HS_END(hsync_width));
+	regmap_write(ebc->regmap, EBC_DSP_HTIMING1,
+		     EBC_DSP_HTIMING1_DSP_HACT_END(hact_start + sdck.hdisplay) |
+		     /* minus 1 for fixed delay in timing sequence */
+		     EBC_DSP_HTIMING1_DSP_HACT_ST(hact_start - 1));
+	regmap_write(ebc->regmap, EBC_DSP_VTIMING0,
+		     EBC_DSP_VTIMING0_DSP_VTOTAL(mode.vtotal) |
+		     /* sync end == sync width */
+		     EBC_DSP_VTIMING0_DSP_VS_END(vsync_width));
+	regmap_write(ebc->regmap, EBC_DSP_VTIMING1,
+		     EBC_DSP_VTIMING1_DSP_VACT_END(vact_start + mode.vdisplay) |
+		     EBC_DSP_VTIMING1_DSP_VACT_ST(vact_start));
+	regmap_write(ebc->regmap, EBC_DSP_ACT_INFO,
+		     EBC_DSP_ACT_INFO_DSP_HEIGHT(mode.vdisplay) |
+		     EBC_DSP_ACT_INFO_DSP_WIDTH(mode.hdisplay));
+	regmap_write(ebc->regmap, EBC_WIN_CTRL,
+		     /* FIFO depth - 16 */
+		     EBC_WIN_CTRL_WIN2_FIFO_THRESHOLD(496) |
+		     EBC_WIN_CTRL_WIN_EN |
+		     /* INCR16 */
+		     EBC_WIN_CTRL_AHB_BURST_REG(7) |
+		     /* FIFO depth - 16 */
+		     EBC_WIN_CTRL_WIN_FIFO_THRESHOLD(240) |
+		     EBC_WIN_CTRL_WIN_FMT_Y4);
+
+	/* To keep things simple, always use a window size matching the CRTC. */
+	regmap_write(ebc->regmap, EBC_WIN_VIR,
+		     EBC_WIN_VIR_WIN_VIR_HEIGHT(mode.vdisplay) |
+		     EBC_WIN_VIR_WIN_VIR_WIDTH(mode.hdisplay));
+	regmap_write(ebc->regmap, EBC_WIN_ACT,
+		     EBC_WIN_ACT_WIN_ACT_HEIGHT(mode.vdisplay) |
+		     EBC_WIN_ACT_WIN_ACT_WIDTH(mode.hdisplay));
+	regmap_write(ebc->regmap, EBC_WIN_DSP,
+		     EBC_WIN_DSP_WIN_DSP_HEIGHT(mode.vdisplay) |
+		     EBC_WIN_DSP_WIN_DSP_WIDTH(mode.hdisplay));
+	regmap_write(ebc->regmap, EBC_WIN_DSP_ST,
+		     EBC_WIN_DSP_ST_WIN_DSP_YST(vact_start) |
+		     EBC_WIN_DSP_ST_WIN_DSP_XST(hact_start));
 }
 
 static int rockchip_ebc_crtc_atomic_check(struct drm_crtc *crtc,
 					  struct drm_atomic_state *state)
 {
+	struct rockchip_ebc *ebc = crtc_to_ebc(crtc);
+	struct drm_crtc_state *crtc_state;
+
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	if (!crtc_state->mode_changed)
+		return 0;
+
+	if (crtc_state->enable) {
+		struct drm_display_mode *mode = &crtc_state->adjusted_mode;
+		long rate = mode->clock * 1000;
+
+		rate = clk_round_rate(ebc->dclk, rate);
+		if (rate < 0)
+			return rate;
+		mode->clock = rate / 1000;
+	}
+
 	return 0;
 }
 
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 06/16] drm/rockchip: ebc: Add CRTC refresh thread
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (4 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 05/16] drm/rockchip: ebc: Add CRTC mode setting Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 07/16] drm/rockchip: ebc: Add CRTC buffer management Samuel Holland
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

EPD refreshes are extremely slow; they take anywhere between hundreds of
milliseconds and several seconds. To avoid blocking userspace, perform
these refreshes on a separate thread. The thread will also take care of
initializing the display before first use and clearing it when the CRTC
is disabled.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/rockchip/rockchip_ebc.c | 82 ++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
index 5f9502313657..ebe60d5e011a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_ebc.c
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -6,6 +6,7 @@
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/irq.h>
+#include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/of_platform.h>
 #include <linux/pm_runtime.h>
@@ -135,9 +136,15 @@ struct rockchip_ebc {
 	struct drm_plane		plane;
 	struct regmap			*regmap;
 	struct regulator_bulk_data	supplies[EBC_NUM_SUPPLIES];
+	struct task_struct		*refresh_thread;
 	u32				dsp_start;
+	bool				reset_complete;
 };
 
+static bool skip_reset;
+module_param(skip_reset, bool, 0444);
+MODULE_PARM_DESC(skip_reset, "skip the initial display reset");
+
 DEFINE_DRM_GEM_FOPS(rockchip_ebc_fops);
 
 static const struct drm_driver rockchip_ebc_drm_driver = {
@@ -172,6 +179,42 @@ to_ebc_crtc_state(struct drm_crtc_state *crtc_state)
 	return container_of(crtc_state, struct ebc_crtc_state, base);
 }
 
+static int rockchip_ebc_refresh_thread(void *data)
+{
+	struct rockchip_ebc *ebc = data;
+
+	while (!kthread_should_stop()) {
+		/*
+		 * LUTs use both the old and the new pixel values as inputs.
+		 * However, the initial contents of the display are unknown.
+		 * The special RESET waveform will initialize the display to
+		 * known contents (white) regardless of its current contents.
+		 */
+		if (!ebc->reset_complete) {
+			ebc->reset_complete = true;
+			drm_dbg(&ebc->drm, "display reset\n");
+		}
+
+		while (!kthread_should_park()) {
+			drm_dbg(&ebc->drm, "display update\n");
+
+			set_current_state(TASK_IDLE);
+			schedule();
+			__set_current_state(TASK_RUNNING);
+		}
+
+		/*
+		 * Clear the display before disabling the CRTC. Use the
+		 * highest-quality waveform to minimize visible artifacts.
+		 */
+		drm_dbg(&ebc->drm, "display clear\n");
+
+		kthread_parkme();
+	}
+
+	return 0;
+}
+
 static inline struct rockchip_ebc *crtc_to_ebc(struct drm_crtc *crtc)
 {
 	return container_of(crtc, struct rockchip_ebc, crtc);
@@ -296,11 +339,23 @@ static void rockchip_ebc_crtc_atomic_flush(struct drm_crtc *crtc,
 static void rockchip_ebc_crtc_atomic_enable(struct drm_crtc *crtc,
 					    struct drm_atomic_state *state)
 {
+	struct rockchip_ebc *ebc = crtc_to_ebc(crtc);
+	struct drm_crtc_state *crtc_state;
+
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	if (crtc_state->mode_changed)
+		kthread_unpark(ebc->refresh_thread);
 }
 
 static void rockchip_ebc_crtc_atomic_disable(struct drm_crtc *crtc,
 					     struct drm_atomic_state *state)
 {
+	struct rockchip_ebc *ebc = crtc_to_ebc(crtc);
+	struct drm_crtc_state *crtc_state;
+
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	if (crtc_state->mode_changed)
+		kthread_park(ebc->refresh_thread);
 }
 
 static const struct drm_crtc_helper_funcs rockchip_ebc_crtc_helper_funcs = {
@@ -408,6 +463,14 @@ static int rockchip_ebc_plane_atomic_check(struct drm_plane *plane,
 static void rockchip_ebc_plane_atomic_update(struct drm_plane *plane,
 					     struct drm_atomic_state *state)
 {
+	struct rockchip_ebc *ebc = plane_to_ebc(plane);
+	struct drm_plane_state *plane_state;
+
+	plane_state = drm_atomic_get_new_plane_state(state, plane);
+	if (!plane_state->crtc)
+		return;
+
+	wake_up_process(ebc->refresh_thread);
 }
 
 static const struct drm_plane_helper_funcs rockchip_ebc_plane_helper_funcs = {
@@ -673,6 +736,7 @@ static int rockchip_ebc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, ebc);
 	init_completion(&ebc->display_end);
+	ebc->reset_complete = skip_reset;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
@@ -716,12 +780,26 @@ static int rockchip_ebc_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	ebc->refresh_thread = kthread_create(rockchip_ebc_refresh_thread,
+					     ebc, "ebc-refresh/%s",
+					     dev_name(dev));
+	if (IS_ERR(ebc->refresh_thread)) {
+		ret = dev_err_probe(dev, PTR_ERR(ebc->refresh_thread),
+				    "Failed to start refresh thread\n");
+		goto err_disable_pm;
+	}
+
+	kthread_park(ebc->refresh_thread);
+	sched_set_fifo(ebc->refresh_thread);
+
 	ret = rockchip_ebc_drm_init(ebc);
 	if (ret)
-		goto err_disable_pm;
+		goto err_stop_kthread;
 
 	return 0;
 
+err_stop_kthread:
+	kthread_stop(ebc->refresh_thread);
 err_disable_pm:
 	pm_runtime_disable(dev);
 	if (!pm_runtime_status_suspended(dev))
@@ -738,6 +816,8 @@ static int rockchip_ebc_remove(struct platform_device *pdev)
 	drm_dev_unregister(&ebc->drm);
 	drm_atomic_helper_shutdown(&ebc->drm);
 
+	kthread_stop(ebc->refresh_thread);
+
 	pm_runtime_disable(dev);
 	if (!pm_runtime_status_suspended(dev))
 		rockchip_ebc_runtime_suspend(dev);
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 07/16] drm/rockchip: ebc: Add CRTC buffer management
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (5 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 06/16] drm/rockchip: ebc: Add CRTC refresh thread Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 08/16] drm/rockchip: ebc: Add LUT loading Samuel Holland
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

This commit adds the "context" structure which holds all buffers needed
to refresh the display. It is allocated separately from the CRTC state
because it is reused as long as no mode change occurs.

There are three buffers holding Y4 (grayscale 4 bits/pixel) pixel data:
  - "prev" - contents of the display at the beginning of a refresh.
  - "next" - contents of the display at the end of that refresh. When a
    refresh finishes, the "next" buffer is copied into "prev".
  - "final" - contents of the display at the end of the final refresh.
    This buffer is necessary because a refresh waveform cannot be
    modified or interrupted once it is started. If a pixel's value is
    changed while it is already being refreshed, the "next" buffer
    cannot be updated until the first waveform completes. At that time,
    the "final" buffer is copied into "next", and another refresh is
    started. The name "final" refers to the write-combining behavior of
    this buffer; any number of plane updates can change this buffer
    while waiting for the current refresh to complete.

Then there are two buffers holding "phase" data. These are only used
during partial refreshes. The phase represents the time component (the X
coordinate) of the waveform. Since the EBC supports a maximum of 256
phases in a waveform, the phase number requires one byte per pixel. The
driver swaps between two buffers to minimize the delay between phases,
as these buffers must be updated for every phase in the waveform.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/rockchip/rockchip_ebc.c | 166 +++++++++++++++++++++++-
 1 file changed, 163 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
index ebe60d5e011a..095d66e67c2f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_ebc.c
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -17,6 +17,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_epd_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -141,6 +142,10 @@ struct rockchip_ebc {
 	bool				reset_complete;
 };
 
+static int default_waveform = DRM_EPD_WF_GC16;
+module_param(default_waveform, int, 0644);
+MODULE_PARM_DESC(default_waveform, "waveform to use for display updates");
+
 static bool skip_reset;
 module_param(skip_reset, bool, 0444);
 MODULE_PARM_DESC(skip_reset, "skip the initial display reset");
@@ -165,12 +170,86 @@ static const struct drm_mode_config_funcs rockchip_ebc_mode_config_funcs = {
 	.atomic_commit		= drm_atomic_helper_commit,
 };
 
+/**
+ * struct rockchip_ebc_ctx - context for performing display refreshes
+ *
+ * @kref: Reference count, maintained as part of the CRTC's atomic state
+ * @prev: Display contents (Y4) before this refresh
+ * @next: Display contents (Y4) after this refresh
+ * @final: Display contents (Y4) after all pending refreshes
+ * @phase: Buffers for selecting a phase from the EBC's LUT, 1 byte/pixel
+ * @gray4_pitch: Horizontal line length of a Y4 pixel buffer in bytes
+ * @gray4_size: Size of a Y4 pixel buffer in bytes
+ * @phase_pitch: Horizontal line length of a phase buffer in bytes
+ * @phase_size: Size of a phase buffer in bytes
+ */
+struct rockchip_ebc_ctx {
+	struct kref			kref;
+	u8				*prev;
+	u8				*next;
+	u8				*final;
+	u8				*phase[2];
+	u32				gray4_pitch;
+	u32				gray4_size;
+	u32				phase_pitch;
+	u32				phase_size;
+};
+
+static void rockchip_ebc_ctx_free(struct rockchip_ebc_ctx *ctx)
+{
+	kfree(ctx->prev);
+	kfree(ctx->next);
+	kfree(ctx->final);
+	kfree(ctx->phase[0]);
+	kfree(ctx->phase[1]);
+	kfree(ctx);
+}
+
+static struct rockchip_ebc_ctx *rockchip_ebc_ctx_alloc(u32 width, u32 height)
+{
+	u32 gray4_size = width * height / 2;
+	u32 phase_size = width * height;
+	struct rockchip_ebc_ctx *ctx;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return NULL;
+
+	ctx->prev = kmalloc(gray4_size, GFP_KERNEL);
+	ctx->next = kmalloc(gray4_size, GFP_KERNEL);
+	ctx->final = kmalloc(gray4_size, GFP_KERNEL);
+	ctx->phase[0] = kmalloc(phase_size, GFP_KERNEL);
+	ctx->phase[1] = kmalloc(phase_size, GFP_KERNEL);
+	if (!ctx->prev || !ctx->next || !ctx->final ||
+	    !ctx->phase[0] || !ctx->phase[1]) {
+		rockchip_ebc_ctx_free(ctx);
+		return NULL;
+	}
+
+	kref_init(&ctx->kref);
+	ctx->gray4_pitch = width / 2;
+	ctx->gray4_size  = gray4_size;
+	ctx->phase_pitch = width;
+	ctx->phase_size  = phase_size;
+
+	return ctx;
+}
+
+static void rockchip_ebc_ctx_release(struct kref *kref)
+{
+	struct rockchip_ebc_ctx *ctx =
+		container_of(kref, struct rockchip_ebc_ctx, kref);
+
+	return rockchip_ebc_ctx_free(ctx);
+}
+
 /*
  * CRTC
  */
 
 struct ebc_crtc_state {
 	struct drm_crtc_state		base;
+	struct rockchip_ebc_ctx		*ctx;
 };
 
 static inline struct ebc_crtc_state *
@@ -179,11 +258,70 @@ to_ebc_crtc_state(struct drm_crtc_state *crtc_state)
 	return container_of(crtc_state, struct ebc_crtc_state, base);
 }
 
+static void rockchip_ebc_global_refresh(struct rockchip_ebc *ebc,
+					const struct rockchip_ebc_ctx *ctx)
+{
+	struct drm_device *drm = &ebc->drm;
+	u32 gray4_size = ctx->gray4_size;
+
+	drm_dbg(drm, "global refresh\n");
+
+	memcpy(ctx->prev, ctx->next, gray4_size);
+}
+
+static void rockchip_ebc_partial_refresh(struct rockchip_ebc *ebc,
+					 struct rockchip_ebc_ctx *ctx)
+{
+	struct drm_device *drm = &ebc->drm;
+
+	drm_dbg(drm, "partial refresh\n");
+}
+
+static void rockchip_ebc_refresh(struct rockchip_ebc *ebc,
+				 struct rockchip_ebc_ctx *ctx,
+				 bool global_refresh,
+				 enum drm_epd_waveform waveform)
+{
+	if (global_refresh)
+		rockchip_ebc_global_refresh(ebc, ctx);
+	else
+		rockchip_ebc_partial_refresh(ebc, ctx);
+}
+
 static int rockchip_ebc_refresh_thread(void *data)
 {
 	struct rockchip_ebc *ebc = data;
+	struct rockchip_ebc_ctx *ctx;
 
 	while (!kthread_should_stop()) {
+		/* The context will change each time the thread is unparked. */
+		ctx = to_ebc_crtc_state(READ_ONCE(ebc->crtc.state))->ctx;
+
+		/*
+		 * Initialize the buffers before use. This is deferred to the
+		 * kthread to avoid slowing down atomic_check.
+		 *
+		 * ctx->prev and ctx->next are set to 0xff, all white, because:
+		 *  1) the display is set to white by the reset waveform, and
+		 *  2) the driver maintains the invariant that the display is
+		 *     all white whenever the CRTC is disabled.
+		 *
+		 * ctx->final is initialized by the first plane update.
+		 *
+		 * ctx->phase is set to 0xff, the number of the last possible
+		 * phase, because the LUT for that phase is known to be all
+		 * zeroes. (The last real phase in a waveform is zero in order
+		 * to discharge the display, and unused phases in the LUT are
+		 * zeroed out.) This prevents undesired driving of the display
+		 * in 3-window mode between when the framebuffer is blitted
+		 * (and thus prev != next) and when the refresh thread starts
+		 * counting phases for that region.
+		 */
+		memset(ctx->prev, 0xff, ctx->gray4_size);
+		memset(ctx->next, 0xff, ctx->gray4_size);
+		memset(ctx->phase[0], 0xff, ctx->phase_size);
+		memset(ctx->phase[1], 0xff, ctx->phase_size);
+
 		/*
 		 * LUTs use both the old and the new pixel values as inputs.
 		 * However, the initial contents of the display are unknown.
@@ -192,11 +330,11 @@ static int rockchip_ebc_refresh_thread(void *data)
 		 */
 		if (!ebc->reset_complete) {
 			ebc->reset_complete = true;
-			drm_dbg(&ebc->drm, "display reset\n");
+			rockchip_ebc_refresh(ebc, ctx, true, DRM_EPD_WF_RESET);
 		}
 
 		while (!kthread_should_park()) {
-			drm_dbg(&ebc->drm, "display update\n");
+			rockchip_ebc_refresh(ebc, ctx, false, default_waveform);
 
 			set_current_state(TASK_IDLE);
 			schedule();
@@ -207,7 +345,8 @@ static int rockchip_ebc_refresh_thread(void *data)
 		 * Clear the display before disabling the CRTC. Use the
 		 * highest-quality waveform to minimize visible artifacts.
 		 */
-		drm_dbg(&ebc->drm, "display clear\n");
+		memset(ctx->next, 0xff, ctx->gray4_size);
+		rockchip_ebc_refresh(ebc, ctx, true, DRM_EPD_WF_GC16);
 
 		kthread_parkme();
 	}
@@ -312,7 +451,9 @@ static int rockchip_ebc_crtc_atomic_check(struct drm_crtc *crtc,
 					  struct drm_atomic_state *state)
 {
 	struct rockchip_ebc *ebc = crtc_to_ebc(crtc);
+	struct ebc_crtc_state *ebc_crtc_state;
 	struct drm_crtc_state *crtc_state;
+	struct rockchip_ebc_ctx *ctx;
 
 	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 	if (!crtc_state->mode_changed)
@@ -320,14 +461,26 @@ static int rockchip_ebc_crtc_atomic_check(struct drm_crtc *crtc,
 
 	if (crtc_state->enable) {
 		struct drm_display_mode *mode = &crtc_state->adjusted_mode;
+
 		long rate = mode->clock * 1000;
 
 		rate = clk_round_rate(ebc->dclk, rate);
 		if (rate < 0)
 			return rate;
 		mode->clock = rate / 1000;
+
+		ctx = rockchip_ebc_ctx_alloc(mode->hdisplay, mode->vdisplay);
+		if (!ctx)
+			return -ENOMEM;
+	} else {
+		ctx = NULL;
 	}
 
+	ebc_crtc_state = to_ebc_crtc_state(crtc_state);
+	if (ebc_crtc_state->ctx)
+		kref_put(&ebc_crtc_state->ctx->kref, rockchip_ebc_ctx_release);
+	ebc_crtc_state->ctx = ctx;
+
 	return 0;
 }
 
@@ -397,6 +550,10 @@ rockchip_ebc_crtc_duplicate_state(struct drm_crtc *crtc)
 
 	__drm_atomic_helper_crtc_duplicate_state(crtc, &ebc_crtc_state->base);
 
+	ebc_crtc_state->ctx = to_ebc_crtc_state(crtc->state)->ctx;
+	if (ebc_crtc_state->ctx)
+		kref_get(&ebc_crtc_state->ctx->kref);
+
 	return &ebc_crtc_state->base;
 }
 
@@ -405,6 +562,9 @@ static void rockchip_ebc_crtc_destroy_state(struct drm_crtc *crtc,
 {
 	struct ebc_crtc_state *ebc_crtc_state = to_ebc_crtc_state(crtc_state);
 
+	if (ebc_crtc_state->ctx)
+		kref_put(&ebc_crtc_state->ctx->kref, rockchip_ebc_ctx_release);
+
 	__drm_atomic_helper_crtc_destroy_state(&ebc_crtc_state->base);
 
 	kfree(ebc_crtc_state);
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 08/16] drm/rockchip: ebc: Add LUT loading
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (6 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 07/16] drm/rockchip: ebc: Add CRTC buffer management Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 09/16] drm/rockchip: ebc: Implement global refreshes Samuel Holland
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

The EBC contains a 16 KiB SRAM which stores the current LUT. It needs to
be programmed any time the LUT changes or the hardware block is enabled.
Since both of these triggers can happen at the same time, use a flag to
avoid writing the LUT twice.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/rockchip/Kconfig        |  3 +-
 drivers/gpu/drm/rockchip/rockchip_ebc.c | 76 +++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 9d3273a5fd97..efe4476e336d 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -94,7 +94,8 @@ endif
 
 config DRM_ROCKCHIP_EBC
 	tristate "DRM Support for Rockchip EBC"
-	depends on DRM
+	depends on DRM && IIO
+	select DRM_EPD_HELPER
 	select DRM_GEM_SHMEM_HELPER
 	select DRM_KMS_HELPER
 	help
diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
index 095d66e67c2f..ca3173b28d1c 100644
--- a/drivers/gpu/drm/rockchip/rockchip_ebc.c
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -5,6 +5,7 @@
 
 #include <linux/clk.h>
 #include <linux/completion.h>
+#include <linux/iio/consumer.h>
 #include <linux/irq.h>
 #include <linux/kthread.h>
 #include <linux/module.h>
@@ -122,6 +123,7 @@
 #define EBC_WIN_MST2			0x0058
 #define EBC_LUT_DATA			0x1000
 
+#define EBC_MAX_PHASES			256
 #define EBC_NUM_LUT_REGS		0x1000
 #define EBC_NUM_SUPPLIES		3
 
@@ -134,11 +136,15 @@ struct rockchip_ebc {
 	struct drm_crtc			crtc;
 	struct drm_device		drm;
 	struct drm_encoder		encoder;
+	struct drm_epd_lut		lut;
+	struct drm_epd_lut_file		lut_file;
 	struct drm_plane		plane;
+	struct iio_channel		*temperature_channel;
 	struct regmap			*regmap;
 	struct regulator_bulk_data	supplies[EBC_NUM_SUPPLIES];
 	struct task_struct		*refresh_thread;
 	u32				dsp_start;
+	bool				lut_changed;
 	bool				reset_complete;
 };
 
@@ -282,10 +288,59 @@ static void rockchip_ebc_refresh(struct rockchip_ebc *ebc,
 				 bool global_refresh,
 				 enum drm_epd_waveform waveform)
 {
+	struct drm_device *drm = &ebc->drm;
+	struct device *dev = drm->dev;
+	int ret, temperature;
+
+	/* Resume asynchronously while preparing to refresh. */
+	ret = pm_runtime_get(dev);
+	if (ret < 0) {
+		drm_err(drm, "Failed to request resume: %d\n", ret);
+		return;
+	}
+
+	ret = iio_read_channel_processed(ebc->temperature_channel, &temperature);
+	if (ret < 0) {
+		drm_err(drm, "Failed to get temperature: %d\n", ret);
+	} else {
+		/* Convert from millicelsius to celsius. */
+		temperature /= 1000;
+
+		ret = drm_epd_lut_set_temperature(&ebc->lut, temperature);
+		if (ret < 0)
+			drm_err(drm, "Failed to set LUT temperature: %d\n", ret);
+		else if (ret)
+			ebc->lut_changed = true;
+	}
+
+	ret = drm_epd_lut_set_waveform(&ebc->lut, waveform);
+	if (ret < 0)
+		drm_err(drm, "Failed to set LUT waveform: %d\n", ret);
+	else if (ret)
+		ebc->lut_changed = true;
+
+	/* Wait for the resume to complete before writing any registers. */
+	ret = pm_runtime_resume(dev);
+	if (ret < 0) {
+		drm_err(drm, "Failed to resume: %d\n", ret);
+		pm_runtime_put(dev);
+		return;
+	}
+
+	/* This flag may have been set above, or by the runtime PM callback. */
+	if (ebc->lut_changed) {
+		ebc->lut_changed = false;
+		regmap_bulk_write(ebc->regmap, EBC_LUT_DATA,
+				  ebc->lut.buf, EBC_NUM_LUT_REGS);
+	}
+
 	if (global_refresh)
 		rockchip_ebc_global_refresh(ebc, ctx);
 	else
 		rockchip_ebc_partial_refresh(ebc, ctx);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 }
 
 static int rockchip_ebc_refresh_thread(void *data)
@@ -708,6 +763,15 @@ static int rockchip_ebc_drm_init(struct rockchip_ebc *ebc)
 	struct drm_bridge *bridge;
 	int ret;
 
+	ret = drmm_epd_lut_file_init(drm, &ebc->lut_file, "rockchip/ebc.wbf");
+	if (ret)
+		return ret;
+
+	ret = drmm_epd_lut_init(&ebc->lut_file, &ebc->lut,
+				DRM_EPD_LUT_4BIT_PACKED, EBC_MAX_PHASES);
+	if (ret)
+		return ret;
+
 	ret = drmm_mode_config_init(drm);
 	if (ret)
 		return ret;
@@ -810,6 +874,13 @@ static int rockchip_ebc_runtime_resume(struct device *dev)
 	if (ret)
 		goto err_disable_hclk;
 
+	/*
+	 * Do not restore the LUT registers here, because the temperature or
+	 * waveform may have changed since the last refresh. Instead, have the
+	 * refresh thread program the LUT during the next refresh.
+	 */
+	ebc->lut_changed = true;
+
 	regcache_cache_only(ebc->regmap, false);
 	regcache_mark_dirty(ebc->regmap);
 	regcache_sync(ebc->regmap);
@@ -919,6 +990,11 @@ static int rockchip_ebc_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(ebc->hclk),
 				     "Failed to get hclk\n");
 
+	ebc->temperature_channel = devm_iio_channel_get(dev, NULL);
+	if (IS_ERR(ebc->temperature_channel))
+		return dev_err_probe(dev, PTR_ERR(ebc->temperature_channel),
+				     "Failed to get temperature I/O channel\n");
+
 	for (i = 0; i < EBC_NUM_SUPPLIES; i++)
 		ebc->supplies[i].supply = rockchip_ebc_supplies[i];
 
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 09/16] drm/rockchip: ebc: Implement global refreshes
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (7 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 08/16] drm/rockchip: ebc: Add LUT loading Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 10/16] drm/rockchip: ebc: Implement partial refreshes Samuel Holland
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

The global refresh mode is used to initialize and clear the screen.
It is the most efficient refresh mode. It uses two pixel buffers (old
and new) and a frame count. The frame count is set to the number of
phases in the waveform. The hardware then looks up the combination of
(old pixel value, new pixel value, frame number) in the LUT and sends
the resulting polarity value to the display.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/rockchip/rockchip_ebc.c | 48 ++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
index ca3173b28d1c..cb6dc567e94c 100644
--- a/drivers/gpu/drm/rockchip/rockchip_ebc.c
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -127,6 +127,7 @@
 #define EBC_NUM_LUT_REGS		0x1000
 #define EBC_NUM_SUPPLIES		3
 
+#define EBC_REFRESH_TIMEOUT		msecs_to_jiffies(3000)
 #define EBC_SUSPEND_DELAY_MS		2000
 
 struct rockchip_ebc {
@@ -269,8 +270,23 @@ static void rockchip_ebc_global_refresh(struct rockchip_ebc *ebc,
 {
 	struct drm_device *drm = &ebc->drm;
 	u32 gray4_size = ctx->gray4_size;
+	struct device *dev = drm->dev;
 
-	drm_dbg(drm, "global refresh\n");
+	dma_sync_single_for_device(dev, virt_to_phys(ctx->next),
+				   gray4_size, DMA_TO_DEVICE);
+	dma_sync_single_for_device(dev, virt_to_phys(ctx->prev),
+				   gray4_size, DMA_TO_DEVICE);
+
+	reinit_completion(&ebc->display_end);
+	regmap_write(ebc->regmap, EBC_CONFIG_DONE,
+		     EBC_CONFIG_DONE_REG_CONFIG_DONE);
+	regmap_write(ebc->regmap, EBC_DSP_START,
+		     ebc->dsp_start |
+		     EBC_DSP_START_DSP_FRM_TOTAL(ebc->lut.num_phases - 1) |
+		     EBC_DSP_START_DSP_FRM_START);
+	if (!wait_for_completion_timeout(&ebc->display_end,
+					 EBC_REFRESH_TIMEOUT))
+		drm_err(drm, "Refresh timed out!\n");
 
 	memcpy(ctx->prev, ctx->next, gray4_size);
 }
@@ -289,6 +305,7 @@ static void rockchip_ebc_refresh(struct rockchip_ebc *ebc,
 				 enum drm_epd_waveform waveform)
 {
 	struct drm_device *drm = &ebc->drm;
+	u32 dsp_ctrl = 0, epd_ctrl = 0;
 	struct device *dev = drm->dev;
 	int ret, temperature;
 
@@ -334,11 +351,40 @@ static void rockchip_ebc_refresh(struct rockchip_ebc *ebc,
 				  ebc->lut.buf, EBC_NUM_LUT_REGS);
 	}
 
+	regmap_write(ebc->regmap, EBC_DSP_START,
+		     ebc->dsp_start);
+
+	/*
+	 * The hardware has a separate bit for each mode, with some priority
+	 * scheme between them. For clarity, only set one bit at a time.
+	 */
+	if (global_refresh) {
+		dsp_ctrl |= EBC_DSP_CTRL_DSP_LUT_MODE;
+	} else {
+		epd_ctrl |= EBC_EPD_CTRL_DSP_THREE_WIN_MODE;
+	}
+	regmap_update_bits(ebc->regmap, EBC_EPD_CTRL,
+			   EBC_EPD_CTRL_DSP_THREE_WIN_MODE,
+			   epd_ctrl);
+	regmap_update_bits(ebc->regmap, EBC_DSP_CTRL,
+			   EBC_DSP_CTRL_DSP_LUT_MODE,
+			   dsp_ctrl);
+
+	regmap_write(ebc->regmap, EBC_WIN_MST0,
+		     virt_to_phys(ctx->next));
+	regmap_write(ebc->regmap, EBC_WIN_MST1,
+		     virt_to_phys(ctx->prev));
+
 	if (global_refresh)
 		rockchip_ebc_global_refresh(ebc, ctx);
 	else
 		rockchip_ebc_partial_refresh(ebc, ctx);
 
+	/* Drive the output pins low once the refresh is complete. */
+	regmap_write(ebc->regmap, EBC_DSP_START,
+		     ebc->dsp_start |
+		     EBC_DSP_START_DSP_OUT_LOW);
+
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
 }
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 10/16] drm/rockchip: ebc: Implement partial refreshes
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (8 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 09/16] drm/rockchip: ebc: Implement global refreshes Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 11/16] drm/rockchip: ebc: Enable diff mode for " Samuel Holland
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

Several areas of the display can be refreshed concurrently, but only if
they do not overlap. This commit adds a queue of damaged areas, and
schedules them for refresh based on collision with other areas. While
the queue is unbounded, there is logic to quickly drop duplicate areas.

Because three-window mode disables the hardware's frame counter, a
separate buffer is required for each frame. (In other words, there is no
automatic increment.) To minimize overhead, swap between two buffers for
phase numbers. This requires extending the loop for one extra frame to
clear the phase numbers in both buffers when an area completes. (This
extra frame is a no-op and is not sent to the hardware.)

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/rockchip/rockchip_ebc.c | 346 +++++++++++++++++++++++-
 1 file changed, 344 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
index cb6dc567e94c..c3e4b65bdee6 100644
--- a/drivers/gpu/drm/rockchip/rockchip_ebc.c
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -17,6 +17,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_epd_helper.h>
 #include <drm/drm_fb_helper.h>
@@ -123,10 +124,14 @@
 #define EBC_WIN_MST2			0x0058
 #define EBC_LUT_DATA			0x1000
 
+#define EBC_FRAME_PENDING		(-1U)
+
 #define EBC_MAX_PHASES			256
+
 #define EBC_NUM_LUT_REGS		0x1000
 #define EBC_NUM_SUPPLIES		3
 
+#define EBC_FRAME_TIMEOUT		msecs_to_jiffies(25)
 #define EBC_REFRESH_TIMEOUT		msecs_to_jiffies(3000)
 #define EBC_SUSPEND_DELAY_MS		2000
 
@@ -177,10 +182,25 @@ static const struct drm_mode_config_funcs rockchip_ebc_mode_config_funcs = {
 	.atomic_commit		= drm_atomic_helper_commit,
 };
 
+/**
+ * struct rockchip_ebc_area - describes a damaged area of the display
+ *
+ * @list: Used to put this area in the state/context/refresh thread list
+ * @clip: The rectangular clip of this damage area
+ * @frame_begin: The frame number when this damage area starts being refreshed
+ */
+struct rockchip_ebc_area {
+	struct list_head		list;
+	struct drm_rect			clip;
+	u32				frame_begin;
+};
+
 /**
  * struct rockchip_ebc_ctx - context for performing display refreshes
  *
  * @kref: Reference count, maintained as part of the CRTC's atomic state
+ * @queue: Queue of damaged areas to be refreshed
+ * @queue_lock: Lock protecting access to @queue
  * @prev: Display contents (Y4) before this refresh
  * @next: Display contents (Y4) after this refresh
  * @final: Display contents (Y4) after all pending refreshes
@@ -192,6 +212,8 @@ static const struct drm_mode_config_funcs rockchip_ebc_mode_config_funcs = {
  */
 struct rockchip_ebc_ctx {
 	struct kref			kref;
+	struct list_head		queue;
+	spinlock_t			queue_lock;
 	u8				*prev;
 	u8				*next;
 	u8				*final;
@@ -204,6 +226,10 @@ struct rockchip_ebc_ctx {
 
 static void rockchip_ebc_ctx_free(struct rockchip_ebc_ctx *ctx)
 {
+	struct rockchip_ebc_area *area;
+
+	list_for_each_entry(area, &ctx->queue, list)
+		kfree(area);
 	kfree(ctx->prev);
 	kfree(ctx->next);
 	kfree(ctx->final);
@@ -234,6 +260,8 @@ static struct rockchip_ebc_ctx *rockchip_ebc_ctx_alloc(u32 width, u32 height)
 	}
 
 	kref_init(&ctx->kref);
+	INIT_LIST_HEAD(&ctx->queue);
+	spin_lock_init(&ctx->queue_lock);
 	ctx->gray4_pitch = width / 2;
 	ctx->gray4_size  = gray4_size;
 	ctx->phase_pitch = width;
@@ -291,12 +319,204 @@ static void rockchip_ebc_global_refresh(struct rockchip_ebc *ebc,
 	memcpy(ctx->prev, ctx->next, gray4_size);
 }
 
+static bool rockchip_ebc_schedule_area(struct list_head *areas,
+				       struct rockchip_ebc_area *area,
+				       u32 current_frame, u32 num_phases)
+{
+	struct rockchip_ebc_area *other;
+	u32 frame_begin = current_frame;
+
+	list_for_each_entry(other, areas, list) {
+		struct drm_rect intersection;
+		u32 other_end;
+
+		/* Only consider areas before this one in the list. */
+		if (other == area)
+			break;
+
+		/* Skip areas that finish refresh before this area begins. */
+		other_end = other->frame_begin + num_phases;
+		if (other_end <= frame_begin)
+			continue;
+
+		/* If there is no collision, the areas are independent. */
+		intersection = area->clip;
+		if (!drm_rect_intersect(&intersection, &other->clip))
+			continue;
+
+		/* If the other area already started, wait until it finishes. */
+		if (other->frame_begin < current_frame) {
+			frame_begin = other_end;
+			continue;
+		}
+
+		/*
+		 * If the other area has not started yet, and completely
+		 * contains this area, then this area is redundant.
+		 */
+		if (drm_rect_equals(&area->clip, &intersection))
+			return false;
+
+		/* Otherwise, start at the same time as the other area. */
+		frame_begin = other->frame_begin;
+	}
+
+	area->frame_begin = frame_begin;
+
+	return true;
+}
+
+static void rockchip_ebc_blit_phase(const struct rockchip_ebc_ctx *ctx,
+				    u8 *dst, u8 phase,
+				    const struct drm_rect *clip)
+{
+	unsigned int pitch = ctx->phase_pitch;
+	unsigned int width = clip->x2 - clip->x1;
+	unsigned int y;
+	u8 *dst_line;
+
+	dst_line = dst + clip->y1 * pitch + clip->x1;
+
+	for (y = clip->y1; y < clip->y2; y++) {
+		memset(dst_line, phase, width);
+
+		dst_line += pitch;
+	}
+}
+
+static void rockchip_ebc_blit_pixels(const struct rockchip_ebc_ctx *ctx,
+				     u8 *dst, const u8 *src,
+				     const struct drm_rect *clip)
+{
+	unsigned int x1_bytes = clip->x1 / 2;
+	unsigned int x2_bytes = clip->x2 / 2;
+	unsigned int pitch = ctx->gray4_pitch;
+	unsigned int width = x2_bytes - x1_bytes;
+	const u8 *src_line;
+	unsigned int y;
+	u8 *dst_line;
+
+	dst_line = dst + clip->y1 * pitch + x1_bytes;
+	src_line = src + clip->y1 * pitch + x1_bytes;
+
+	for (y = clip->y1; y < clip->y2; y++) {
+		memcpy(dst_line, src_line, width);
+
+		dst_line += pitch;
+		src_line += pitch;
+	}
+}
+
 static void rockchip_ebc_partial_refresh(struct rockchip_ebc *ebc,
 					 struct rockchip_ebc_ctx *ctx)
 {
+	dma_addr_t next_handle = virt_to_phys(ctx->next);
+	dma_addr_t prev_handle = virt_to_phys(ctx->prev);
+	struct rockchip_ebc_area *area, *next_area;
+	u32 last_phase = ebc->lut.num_phases - 1;
 	struct drm_device *drm = &ebc->drm;
+	u32 gray4_size = ctx->gray4_size;
+	struct device *dev = drm->dev;
+	LIST_HEAD(areas);
+	u32 frame;
+
+	for (frame = 0;; frame++) {
+		/* Swap phase buffers to minimize latency between frames. */
+		u8 *phase_buffer = ctx->phase[frame % 2];
+		dma_addr_t phase_handle = virt_to_phys(phase_buffer);
+		bool sync_next = false;
+		bool sync_prev = false;
+
+		/* Move the queued damage areas to the local list. */
+		spin_lock(&ctx->queue_lock);
+		list_splice_tail_init(&ctx->queue, &areas);
+		spin_unlock(&ctx->queue_lock);
+
+		list_for_each_entry_safe(area, next_area, &areas, list) {
+			s32 frame_delta;
+			u32 phase;
+
+			/*
+			 * Determine when this area can start its refresh.
+			 * If the area is redundant, drop it immediately.
+			 */
+			if (area->frame_begin == EBC_FRAME_PENDING &&
+			    !rockchip_ebc_schedule_area(&areas, area, frame,
+							ebc->lut.num_phases)) {
+				list_del(&area->list);
+				kfree(area);
+				continue;
+			}
+
+			frame_delta = frame - area->frame_begin;
+			if (frame_delta < 0)
+				continue;
+
+			/* Copy ctx->final to ctx->next on the first frame. */
+			if (frame_delta == 0) {
+				rockchip_ebc_blit_pixels(ctx, ctx->next,
+							 ctx->final,
+							 &area->clip);
+				sync_next = true;
+			}
+
+			/*
+			 * Take advantage of the fact that the last phase in a
+			 * waveform is always zero (neutral polarity). Instead
+			 * of writing the actual phase number, write 0xff (the
+			 * last possible phase number), which is guaranteed to
+			 * be neutral for every waveform.
+			 */
+			phase = frame_delta >= last_phase ? 0xff : frame_delta;
+			rockchip_ebc_blit_phase(ctx, phase_buffer, phase,
+						&area->clip);
+
+			/*
+			 * Copy ctx->next to ctx->prev after the last phase.
+			 * Technically, this races with the hardware computing
+			 * the last phase, but the last phase is all zeroes
+			 * anyway, regardless of prev/next (see above).
+			 *
+			 * Keeping the area in the list for one extra frame
+			 * also ensures both phase buffers get set to 0xff.
+			 */
+			if (frame_delta > last_phase) {
+				rockchip_ebc_blit_pixels(ctx, ctx->prev,
+							 ctx->next,
+							 &area->clip);
+				sync_prev = true;
+
+				list_del(&area->list);
+				kfree(area);
+			}
+		}
+
+		if (sync_next)
+			dma_sync_single_for_device(dev, next_handle,
+						   gray4_size, DMA_TO_DEVICE);
+		if (sync_prev)
+			dma_sync_single_for_device(dev, prev_handle,
+						   gray4_size, DMA_TO_DEVICE);
+		dma_sync_single_for_device(dev, phase_handle,
+					   ctx->phase_size, DMA_TO_DEVICE);
+
+		if (frame) {
+			if (!wait_for_completion_timeout(&ebc->display_end,
+							 EBC_FRAME_TIMEOUT))
+				drm_err(drm, "Frame %d timed out!\n", frame);
+		}
 
-	drm_dbg(drm, "partial refresh\n");
+		if (list_empty(&areas))
+			break;
+
+		regmap_write(ebc->regmap, EBC_WIN_MST2,
+			     phase_handle);
+		regmap_write(ebc->regmap, EBC_CONFIG_DONE,
+			     EBC_CONFIG_DONE_REG_CONFIG_DONE);
+		regmap_write(ebc->regmap, EBC_DSP_START,
+			     ebc->dsp_start |
+			     EBC_DSP_START_DSP_FRM_START);
+	}
 }
 
 static void rockchip_ebc_refresh(struct rockchip_ebc *ebc,
@@ -438,7 +658,8 @@ static int rockchip_ebc_refresh_thread(void *data)
 			rockchip_ebc_refresh(ebc, ctx, false, default_waveform);
 
 			set_current_state(TASK_IDLE);
-			schedule();
+			if (list_empty(&ctx->queue))
+				schedule();
 			__set_current_state(TASK_RUNNING);
 		}
 
@@ -686,6 +907,7 @@ static const struct drm_crtc_funcs rockchip_ebc_crtc_funcs = {
 
 struct ebc_plane_state {
 	struct drm_shadow_plane_state	base;
+	struct list_head		areas;
 };
 
 static inline struct ebc_plane_state *
@@ -702,8 +924,13 @@ static inline struct rockchip_ebc *plane_to_ebc(struct drm_plane *plane)
 static int rockchip_ebc_plane_atomic_check(struct drm_plane *plane,
 					   struct drm_atomic_state *state)
 {
+	struct drm_atomic_helper_damage_iter iter;
+	struct ebc_plane_state *ebc_plane_state;
+	struct drm_plane_state *old_plane_state;
 	struct drm_plane_state *plane_state;
 	struct drm_crtc_state *crtc_state;
+	struct rockchip_ebc_area *area;
+	struct drm_rect clip;
 	int ret;
 
 	plane_state = drm_atomic_get_new_plane_state(state, plane);
@@ -718,19 +945,126 @@ static int rockchip_ebc_plane_atomic_check(struct drm_plane *plane,
 	if (ret)
 		return ret;
 
+	ebc_plane_state = to_ebc_plane_state(plane_state);
+	old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
+	drm_atomic_for_each_plane_damage(&iter, &clip) {
+		area = kmalloc(sizeof(*area), GFP_KERNEL);
+		if (!area)
+			return -ENOMEM;
+
+		area->frame_begin = EBC_FRAME_PENDING;
+		area->clip = clip;
+
+		list_add_tail(&area->list, &ebc_plane_state->areas);
+	}
+
 	return 0;
 }
 
+static bool rockchip_ebc_blit_fb(const struct rockchip_ebc_ctx *ctx,
+				 const struct drm_rect *dst_clip,
+				 const void *vaddr,
+				 const struct drm_framebuffer *fb,
+				 const struct drm_rect *src_clip)
+{
+	unsigned int dst_pitch = ctx->gray4_pitch;
+	unsigned int src_pitch = fb->pitches[0];
+	unsigned int x, y;
+	const void *src;
+	u8 changed = 0;
+	void *dst;
+
+	dst = ctx->final + dst_clip->y1 * dst_pitch + dst_clip->x1 / 2;
+	src = vaddr + src_clip->y1 * src_pitch + src_clip->x1 * fb->format->cpp[0];
+
+	for (y = src_clip->y1; y < src_clip->y2; y++) {
+		const u32 *sbuf = src;
+		u8 *dbuf = dst;
+
+		for (x = src_clip->x1; x < src_clip->x2; x += 2) {
+			u32 rgb0 = *sbuf++;
+			u32 rgb1 = *sbuf++;
+			u8 gray;
+
+			/* Truncate the RGB values to 5 bits each. */
+			rgb0 &= 0x00f8f8f8U; rgb1 &= 0x00f8f8f8U;
+			/* Put the sum 2R+5G+B in bits 24-31. */
+			rgb0 *= 0x0020a040U; rgb1 *= 0x0020a040U;
+			/* Unbias the value for rounding to 4 bits. */
+			rgb0 += 0x07000000U; rgb1 += 0x07000000U;
+
+			gray = rgb0 >> 28 | rgb1 >> 28 << 4;
+			changed |= gray ^ *dbuf;
+			*dbuf++ = gray;
+		}
+
+		dst += dst_pitch;
+		src += src_pitch;
+	}
+
+	return !!changed;
+}
+
 static void rockchip_ebc_plane_atomic_update(struct drm_plane *plane,
 					     struct drm_atomic_state *state)
 {
 	struct rockchip_ebc *ebc = plane_to_ebc(plane);
+	struct rockchip_ebc_area *area, *next_area;
+	struct ebc_plane_state *ebc_plane_state;
 	struct drm_plane_state *plane_state;
+	struct drm_crtc_state *crtc_state;
+	struct rockchip_ebc_ctx *ctx;
+	int translate_x, translate_y;
+	struct drm_rect src;
+	const void *vaddr;
 
 	plane_state = drm_atomic_get_new_plane_state(state, plane);
 	if (!plane_state->crtc)
 		return;
 
+	crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc);
+	ctx = to_ebc_crtc_state(crtc_state)->ctx;
+
+	drm_rect_fp_to_int(&src, &plane_state->src);
+	translate_x = plane_state->dst.x1 - src.x1;
+	translate_y = plane_state->dst.y1 - src.y1;
+
+	ebc_plane_state = to_ebc_plane_state(plane_state);
+	vaddr = ebc_plane_state->base.data[0].vaddr;
+
+	list_for_each_entry_safe(area, next_area, &ebc_plane_state->areas, list) {
+		struct drm_rect *dst_clip = &area->clip;
+		struct drm_rect src_clip = area->clip;
+		int adjust;
+
+		/* Convert from plane coordinates to CRTC coordinates. */
+		drm_rect_translate(dst_clip, translate_x, translate_y);
+
+		/* Adjust the clips to always process full bytes (2 pixels). */
+		adjust = dst_clip->x1 & 1;
+		dst_clip->x1 -= adjust;
+		src_clip.x1  -= adjust;
+
+		adjust = dst_clip->x2 & 1;
+		dst_clip->x2 += adjust;
+		src_clip.x2  += adjust;
+
+		if (!rockchip_ebc_blit_fb(ctx, dst_clip, vaddr,
+					  plane_state->fb, &src_clip)) {
+			/* Drop the area if the FB didn't actually change. */
+			list_del(&area->list);
+			kfree(area);
+		}
+	}
+
+	if (list_empty(&ebc_plane_state->areas))
+		return;
+
+	spin_lock(&ctx->queue_lock);
+	list_splice_tail_init(&ebc_plane_state->areas, &ctx->queue);
+	spin_unlock(&ctx->queue_lock);
+
 	wake_up_process(ebc->refresh_thread);
 }
 
@@ -756,6 +1090,8 @@ static void rockchip_ebc_plane_reset(struct drm_plane *plane)
 		return;
 
 	__drm_gem_reset_shadow_plane(plane, &ebc_plane_state->base);
+
+	INIT_LIST_HEAD(&ebc_plane_state->areas);
 }
 
 static struct drm_plane_state *
@@ -772,6 +1108,8 @@ rockchip_ebc_plane_duplicate_state(struct drm_plane *plane)
 
 	__drm_gem_duplicate_shadow_plane_state(plane, &ebc_plane_state->base);
 
+	INIT_LIST_HEAD(&ebc_plane_state->areas);
+
 	return &ebc_plane_state->base.base;
 }
 
@@ -779,6 +1117,10 @@ static void rockchip_ebc_plane_destroy_state(struct drm_plane *plane,
 					     struct drm_plane_state *plane_state)
 {
 	struct ebc_plane_state *ebc_plane_state = to_ebc_plane_state(plane_state);
+	struct rockchip_ebc_area *area, *next_area;
+
+	list_for_each_entry_safe(area, next_area, &ebc_plane_state->areas, list)
+		kfree(area);
 
 	__drm_gem_destroy_shadow_plane_state(&ebc_plane_state->base);
 
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 11/16] drm/rockchip: ebc: Enable diff mode for partial refreshes
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (9 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 10/16] drm/rockchip: ebc: Implement partial refreshes Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 12/16] drm/rockchip: ebc: Add support for direct mode Samuel Holland
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

Some waveforms, such as GC16, cause the display to flash even when the
previous and next pixel values are the same. This can be helpful,
because it produces more consistent brightness, but usually it is more
distracting. Add an option, enabled by default, for the hardware to
ignore the LUT and always send zeroes for pixels with unchanged values.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/rockchip/rockchip_ebc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
index c3e4b65bdee6..dcd8c8e8208e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_ebc.c
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -158,6 +158,10 @@ static int default_waveform = DRM_EPD_WF_GC16;
 module_param(default_waveform, int, 0644);
 MODULE_PARM_DESC(default_waveform, "waveform to use for display updates");
 
+static bool diff_mode = true;
+module_param(diff_mode, bool, 0644);
+MODULE_PARM_DESC(diff_mode, "only compute waveforms for changed pixels");
+
 static bool skip_reset;
 module_param(skip_reset, bool, 0444);
 MODULE_PARM_DESC(skip_reset, "skip the initial display reset");
@@ -582,11 +586,14 @@ static void rockchip_ebc_refresh(struct rockchip_ebc *ebc,
 		dsp_ctrl |= EBC_DSP_CTRL_DSP_LUT_MODE;
 	} else {
 		epd_ctrl |= EBC_EPD_CTRL_DSP_THREE_WIN_MODE;
+		if (diff_mode)
+			dsp_ctrl |= EBC_DSP_CTRL_DSP_DIFF_MODE;
 	}
 	regmap_update_bits(ebc->regmap, EBC_EPD_CTRL,
 			   EBC_EPD_CTRL_DSP_THREE_WIN_MODE,
 			   epd_ctrl);
 	regmap_update_bits(ebc->regmap, EBC_DSP_CTRL,
+			   EBC_DSP_CTRL_DSP_DIFF_MODE |
 			   EBC_DSP_CTRL_DSP_LUT_MODE,
 			   dsp_ctrl);
 
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 12/16] drm/rockchip: ebc: Add support for direct mode
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (10 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 11/16] drm/rockchip: ebc: Enable diff mode for " Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 13/16] drm/rockchip: ebc: Add a panel reflection option Samuel Holland
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

Currently, 3-window mode causes some artifacting. Until the cause is
determined, provide an option to use direct mode instead. Direct mode
does the waveform lookups in software, so it has much higher CPU usage.
This limits the frame rate below the panel's ideal 85 Hz, so it leads to
slightly lower brightness accuracy. On the other hand, it doesn't leave
random lines all over the screen.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/rockchip/rockchip_ebc.c | 97 ++++++++++++++++++++++---
 1 file changed, 88 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
index dcd8c8e8208e..93d689ff0933 100644
--- a/drivers/gpu/drm/rockchip/rockchip_ebc.c
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -162,6 +162,10 @@ static bool diff_mode = true;
 module_param(diff_mode, bool, 0644);
 MODULE_PARM_DESC(diff_mode, "only compute waveforms for changed pixels");
 
+static bool direct_mode = true;
+module_param(direct_mode, bool, 0444);
+MODULE_PARM_DESC(direct_mode, "compute waveforms in software (software LUT)");
+
 static bool skip_reset;
 module_param(skip_reset, bool, 0444);
 MODULE_PARM_DESC(skip_reset, "skip the initial display reset");
@@ -370,6 +374,59 @@ static bool rockchip_ebc_schedule_area(struct list_head *areas,
 	return true;
 }
 
+static void rockchip_ebc_blit_direct(const struct rockchip_ebc_ctx *ctx,
+				     u8 *dst, u8 phase,
+				     const struct drm_epd_lut *lut,
+				     const struct drm_rect *clip)
+{
+	const u32 *phase_lut = (const u32 *)lut->buf + 16 * phase;
+	unsigned int dst_pitch = ctx->phase_pitch / 4;
+	unsigned int src_pitch = ctx->gray4_pitch;
+	unsigned int x, y;
+	u8 *dst_line;
+	u32 src_line;
+
+	dst_line = dst + clip->y1 * dst_pitch + clip->x1 / 4;
+	src_line = clip->y1 * src_pitch + clip->x1 / 2;
+
+	for (y = clip->y1; y < clip->y2; y++) {
+		u32 src_offset = src_line;
+		u8 *dbuf = dst_line;
+
+		for (x = clip->x1; x < clip->x2; x += 4) {
+			u8 prev0 = ctx->prev[src_offset];
+			u8 next0 = ctx->next[src_offset++];
+			u8 prev1 = ctx->prev[src_offset];
+			u8 next1 = ctx->next[src_offset++];
+
+			/*
+			 * The LUT is 256 phases * 16 next * 16 previous levels.
+			 * Each value is two bits, so the last dimension neatly
+			 * fits in a 32-bit word.
+			 */
+			u8 data = ((phase_lut[next0 & 0xf] >> ((prev0 & 0xf) << 1)) & 0x3) << 0 |
+				  ((phase_lut[next0 >>  4] >> ((prev0 >>  4) << 1)) & 0x3) << 2 |
+				  ((phase_lut[next1 & 0xf] >> ((prev1 & 0xf) << 1)) & 0x3) << 4 |
+				  ((phase_lut[next1 >>  4] >> ((prev1 >>  4) << 1)) & 0x3) << 6;
+
+			/* Diff mode ignores pixels that did not change brightness. */
+			if (diff_mode) {
+				u8 mask = ((next0 ^ prev0) & 0x0f ? 0x03 : 0) |
+					  ((next0 ^ prev0) & 0xf0 ? 0x0c : 0) |
+					  ((next1 ^ prev1) & 0x0f ? 0x30 : 0) |
+					  ((next1 ^ prev1) & 0xf0 ? 0xc0 : 0);
+
+				data &= mask;
+			}
+
+			*dbuf++ = data;
+		}
+
+		dst_line += dst_pitch;
+		src_line += src_pitch;
+	}
+}
+
 static void rockchip_ebc_blit_phase(const struct rockchip_ebc_ctx *ctx,
 				    u8 *dst, u8 phase,
 				    const struct drm_rect *clip)
@@ -472,8 +529,13 @@ static void rockchip_ebc_partial_refresh(struct rockchip_ebc *ebc,
 			 * be neutral for every waveform.
 			 */
 			phase = frame_delta >= last_phase ? 0xff : frame_delta;
-			rockchip_ebc_blit_phase(ctx, phase_buffer, phase,
-						&area->clip);
+			if (direct_mode)
+				rockchip_ebc_blit_direct(ctx, phase_buffer,
+							 phase, &ebc->lut,
+							 &area->clip);
+			else
+				rockchip_ebc_blit_phase(ctx, phase_buffer,
+							phase, &area->clip);
 
 			/*
 			 * Copy ctx->next to ctx->prev after the last phase.
@@ -513,7 +575,8 @@ static void rockchip_ebc_partial_refresh(struct rockchip_ebc *ebc,
 		if (list_empty(&areas))
 			break;
 
-		regmap_write(ebc->regmap, EBC_WIN_MST2,
+		regmap_write(ebc->regmap,
+			     direct_mode ? EBC_WIN_MST0 : EBC_WIN_MST2,
 			     phase_handle);
 		regmap_write(ebc->regmap, EBC_CONFIG_DONE,
 			     EBC_CONFIG_DONE_REG_CONFIG_DONE);
@@ -581,10 +644,12 @@ static void rockchip_ebc_refresh(struct rockchip_ebc *ebc,
 	/*
 	 * The hardware has a separate bit for each mode, with some priority
 	 * scheme between them. For clarity, only set one bit at a time.
+	 *
+	 * NOTE: In direct mode, no mode bits are set.
 	 */
 	if (global_refresh) {
 		dsp_ctrl |= EBC_DSP_CTRL_DSP_LUT_MODE;
-	} else {
+	} else if (!direct_mode) {
 		epd_ctrl |= EBC_EPD_CTRL_DSP_THREE_WIN_MODE;
 		if (diff_mode)
 			dsp_ctrl |= EBC_DSP_CTRL_DSP_DIFF_MODE;
@@ -647,8 +712,12 @@ static int rockchip_ebc_refresh_thread(void *data)
 		 */
 		memset(ctx->prev, 0xff, ctx->gray4_size);
 		memset(ctx->next, 0xff, ctx->gray4_size);
-		memset(ctx->phase[0], 0xff, ctx->phase_size);
-		memset(ctx->phase[1], 0xff, ctx->phase_size);
+		/*
+		 * NOTE: In direct mode, the phase buffers are repurposed for
+		 * source driver polarity data, where the no-op value is 0.
+		 */
+		memset(ctx->phase[0], direct_mode ? 0 : 0xff, ctx->phase_size);
+		memset(ctx->phase[1], direct_mode ? 0 : 0xff, ctx->phase_size);
 
 		/*
 		 * LUTs use both the old and the new pixel values as inputs.
@@ -1048,12 +1117,22 @@ static void rockchip_ebc_plane_atomic_update(struct drm_plane *plane,
 		/* Convert from plane coordinates to CRTC coordinates. */
 		drm_rect_translate(dst_clip, translate_x, translate_y);
 
-		/* Adjust the clips to always process full bytes (2 pixels). */
-		adjust = dst_clip->x1 & 1;
+		/*
+		 * Adjust the clips to always process full bytes (2 pixels).
+		 *
+		 * NOTE: in direct mode, the minimum block size is 4 pixels.
+		 */
+		if (direct_mode)
+			adjust = dst_clip->x1 & 3;
+		else
+			adjust = dst_clip->x1 & 1;
 		dst_clip->x1 -= adjust;
 		src_clip.x1  -= adjust;
 
-		adjust = dst_clip->x2 & 1;
+		if (direct_mode)
+			adjust = ((dst_clip->x2 + 3) ^ 3) & 3;
+		else
+			adjust = dst_clip->x2 & 1;
 		dst_clip->x2 += adjust;
 		src_clip.x2  += adjust;
 
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 13/16] drm/rockchip: ebc: Add a panel reflection option
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (11 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 12/16] drm/rockchip: ebc: Add support for direct mode Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 14/16] drm/panel-simple: Add eInk ED103TC2 Samuel Holland
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

Some panels, like the one in the PineNote, are reflected. Since the
driver already has to copy pixels, the driver can handle this with
little additional overhead.

Currently, there is no devicetree binding for this situation, so control
the behavior via a module parameter.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/rockchip/rockchip_ebc.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_ebc.c b/drivers/gpu/drm/rockchip/rockchip_ebc.c
index 93d689ff0933..9d0b2cdc5fdc 100644
--- a/drivers/gpu/drm/rockchip/rockchip_ebc.c
+++ b/drivers/gpu/drm/rockchip/rockchip_ebc.c
@@ -166,6 +166,10 @@ static bool direct_mode = true;
 module_param(direct_mode, bool, 0444);
 MODULE_PARM_DESC(direct_mode, "compute waveforms in software (software LUT)");
 
+static bool panel_reflection = true;
+module_param(panel_reflection, bool, 0644);
+MODULE_PARM_DESC(panel_reflection, "reflect the image horizontally");
+
 static bool skip_reset;
 module_param(skip_reset, bool, 0444);
 MODULE_PARM_DESC(skip_reset, "skip the initial display reset");
@@ -1046,23 +1050,29 @@ static bool rockchip_ebc_blit_fb(const struct rockchip_ebc_ctx *ctx,
 {
 	unsigned int dst_pitch = ctx->gray4_pitch;
 	unsigned int src_pitch = fb->pitches[0];
-	unsigned int x, y;
+	unsigned int start_x, x, y;
 	const void *src;
 	u8 changed = 0;
+	int delta_x;
 	void *dst;
 
+	delta_x = panel_reflection ? -1 : 1;
+	start_x = panel_reflection ? src_clip->x2 - 1 : src_clip->x1;
+
 	dst = ctx->final + dst_clip->y1 * dst_pitch + dst_clip->x1 / 2;
-	src = vaddr + src_clip->y1 * src_pitch + src_clip->x1 * fb->format->cpp[0];
+	src = vaddr + src_clip->y1 * src_pitch + start_x * fb->format->cpp[0];
 
 	for (y = src_clip->y1; y < src_clip->y2; y++) {
 		const u32 *sbuf = src;
 		u8 *dbuf = dst;
 
 		for (x = src_clip->x1; x < src_clip->x2; x += 2) {
-			u32 rgb0 = *sbuf++;
-			u32 rgb1 = *sbuf++;
+			u32 rgb0, rgb1;
 			u8 gray;
 
+			rgb0 = *sbuf; sbuf += delta_x;
+			rgb1 = *sbuf; sbuf += delta_x;
+
 			/* Truncate the RGB values to 5 bits each. */
 			rgb0 &= 0x00f8f8f8U; rgb1 &= 0x00f8f8f8U;
 			/* Put the sum 2R+5G+B in bits 24-31. */
@@ -1136,6 +1146,13 @@ static void rockchip_ebc_plane_atomic_update(struct drm_plane *plane,
 		dst_clip->x2 += adjust;
 		src_clip.x2  += adjust;
 
+		if (panel_reflection) {
+			int x1 = dst_clip->x1, x2 = dst_clip->x2;
+
+			dst_clip->x1 = plane_state->dst.x2 - x2;
+			dst_clip->x2 = plane_state->dst.x2 - x1;
+		}
+
 		if (!rockchip_ebc_blit_fb(ctx, dst_clip, vaddr,
 					  plane_state->fb, &src_clip)) {
 			/* Drop the area if the FB didn't actually change. */
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 14/16] drm/panel-simple: Add eInk ED103TC2
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (12 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 13/16] drm/rockchip: ebc: Add a panel reflection option Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 15/16] arm64: dts: rockchip: rk356x: Add EBC node Samuel Holland
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

ED103TC2 is a 10.3" 1872x1404 eInk panel which supports up to 16 levels
of grayscale and an 85 Hz frame rate. The timings and polarities here
were taken from the manufacturer's datasheet.

Since this panel is an electrophoretic display (EPD), the color depth is
independent from the bus width. Instead, it is largely determined by the
number of frames in the selected waveform. Each pixel uses two parallel
data lines to specify one of only three states each frame: positive,
negative, or no voltage.

This specific panel has a 16-bit data bus, allowing it to update 8
pixels during each source driver (horizontal) clock cycle. As a result,
the horizontal timings given in the datasheet were all multiplied by 8
to convert the units from clock cycles to pixels.

Since the 16-bit data bus is double the width of the usual 8-bit bus
used by eInk panels, the source driver clock will be half the usual
frequency. This is signified by the DRM_MODE_FLAG_CLKDIV2 flag.

The hskew parameter provides the spacing between the horizontal sync
puls and the gate driver (vertical) clock pulse. This spacing is
symmetrical on both sides, so it can be used to compute the gate
driver clock pulse width.

Datasheet: https://files.pine64.org/doc/quartz64/Eink%20P-511-828-V1_ED103TC2%20Formal%20Spec%20V1.0_20190514.pdf
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/panel/panel-simple.c | 31 ++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index a34f4198a534..c6b104ba01ee 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1686,6 +1686,34 @@ static const struct panel_desc edt_etmv570g2dhu = {
 	.connector_type = DRM_MODE_CONNECTOR_DPI,
 };
 
+static const struct drm_display_mode eink_ed103tc2_mode = {
+	.clock = 266693,
+	.hdisplay = 1872,
+	.hsync_start = 1872 + 184,
+	.hsync_end = 1872 + 184 + 88,
+	.htotal = 1872 + 184 + 88 + 64,
+	.hskew = 136,
+	.vdisplay = 1404,
+	.vsync_start = 1404 + 12,
+	.vsync_end = 1404 + 12 + 1,
+	.vtotal = 1404 + 12 + 1 + 4,
+	.flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC |
+		 DRM_MODE_FLAG_HSKEW | DRM_MODE_FLAG_CLKDIV2,
+};
+
+static const struct panel_desc eink_ed103tc2 = {
+	.modes = &eink_ed103tc2_mode,
+	.num_modes = 1,
+	.bpc = 4,
+	.size = {
+		.width = 210,
+		.height = 157,
+	},
+	.bus_format = MEDIA_BUS_FMT_FIXED,
+	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+	.connector_type = DRM_MODE_CONNECTOR_DPI,
+};
+
 static const struct display_timing eink_vb3300_kca_timing = {
 	.pixelclock = { 40000000, 40000000, 40000000 },
 	.hactive = { 334, 334, 334 },
@@ -3807,6 +3835,9 @@ static const struct of_device_id platform_of_match[] = {
 	}, {
 		.compatible = "edt,etmv570g2dhu",
 		.data = &edt_etmv570g2dhu,
+	}, {
+		.compatible = "eink,ed103tc2",
+		.data = &eink_ed103tc2,
 	}, {
 		.compatible = "eink,vb3300-kca",
 		.data = &eink_vb3300_kca,
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 15/16] arm64: dts: rockchip: rk356x: Add EBC node
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (13 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 14/16] drm/panel-simple: Add eInk ED103TC2 Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-13 22:19 ` [RFC PATCH 16/16] [DO NOT MERGE] arm64: dts: rockchip: pinenote: Enable EBC display pipeline Samuel Holland
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

The RK356x SoCs contain an EBC. Add its node.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 7cdef800cb3c..58c26f240af0 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -508,6 +508,20 @@ gpu: gpu@fde60000 {
 		status = "disabled";
 	};
 
+	ebc: ebc@fdec0000 {
+		compatible = "rockchip,rk3568-ebc";
+		reg = <0x0 0xfdec0000 0x0 0x5000>;
+		interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru HCLK_EBC>, <&cru DCLK_EBC>;
+		clock-names = "hclk", "dclk";
+		pinctrl-0 = <&ebc_pins>;
+		pinctrl-names = "default";
+		power-domains = <&power RK3568_PD_RGA>;
+		resets = <&cru SRST_H_EBC>, <&cru SRST_D_EBC>;
+		reset-names = "hclk", "dclk";
+		status = "disabled";
+	};
+
 	sdmmc2: mmc@fe000000 {
 		compatible = "rockchip,rk3568-dw-mshc", "rockchip,rk3288-dw-mshc";
 		reg = <0x0 0xfe000000 0x0 0x4000>;
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 16/16] [DO NOT MERGE] arm64: dts: rockchip: pinenote: Enable EBC display pipeline
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (14 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 15/16] arm64: dts: rockchip: rk356x: Add EBC node Samuel Holland
@ 2022-04-13 22:19 ` Samuel Holland
  2022-04-14  8:50 ` [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Maxime Ripard
  2022-04-21  6:43 ` Andreas Kemnade
  17 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2022-04-13 22:19 UTC (permalink / raw)
  To: Heiko Stübner, Sandy Huang, dri-devel
  Cc: linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Samuel Holland, Krzysztof Kozlowski, Liang Chen,
	Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

The PineNote contains an eInk ED103TC2 panel connected to the EBC,
powered by a TI TPS651851 PMIC.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 .../boot/dts/rockchip/rk3566-pinenote.dtsi    | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi
index fea748adfa90..4a53931c3f92 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi
@@ -72,6 +72,16 @@ led-0 {
 		};
 	};
 
+	panel {
+		compatible = "eink,ed103tc2";
+
+		port {
+			panel_in_ebc: endpoint {
+				remote-endpoint = <&ebc_out_panel>;
+			};
+		};
+	};
+
 	sdio_pwrseq: sdio-pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		clocks = <&rk817 1>;
@@ -213,6 +223,20 @@ &cpu3 {
 	cpu-supply = <&vdd_cpu>;
 };
 
+&ebc {
+	io-channels = <&ebc_pmic 0>;
+	panel-supply = <&v3p3>;
+	vcom-supply = <&vcom>;
+	vdrive-supply = <&vdrive>;
+	status = "okay";
+
+	port {
+		ebc_out_panel: endpoint {
+			remote-endpoint = <&panel_in_ebc>;
+		};
+	};
+};
+
 &i2c0 {
 	status = "okay";
 
@@ -466,6 +490,47 @@ led@1 {
 			default-brightness = <0>;
 		};
 	};
+
+	/* TODO: write binding */
+	ebc_pmic: pmic@68 {
+		compatible = "ti,tps65185";
+		reg = <0x68>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <RK_PA6 IRQ_TYPE_LEVEL_LOW>;
+		#io-channel-cells = <1>;
+		pinctrl-0 = <&ebc_pmic_pins>;
+		pinctrl-names = "default";
+		powerup-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_HIGH>;
+		pwr_good-gpios = <&gpio3 RK_PA7 GPIO_ACTIVE_HIGH>;
+		vcom_ctrl-gpios = <&gpio4 RK_PB2 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&vcc_bat>;
+		vin3p3-supply = <&vcc_3v3>;
+		wakeup-gpios = <&gpio3 RK_PA5 GPIO_ACTIVE_HIGH>;
+		ti,up-sequence = <1>, <0>, <2>, <3>;
+		ti,up-delay-ms = <3>, <3>, <3>, <3>;
+		ti,down-sequence = <2>, <3>, <1>, <0>;
+		ti,down-delay-ms = <3>, <6>, <6>, <6>;
+
+		regulators {
+			v3p3: v3p3 {
+				regulator-name = "v3p3";
+				regulator-always-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+			};
+
+			vcom: vcom {
+				regulator-name = "vcom";
+				/* voltage range is board-specific */
+			};
+
+			vdrive: vdrive {
+				regulator-name = "vdrive";
+				regulator-min-microvolt = <15000000>;
+				regulator-max-microvolt = <15000000>;
+			};
+		};
+	};
 };
 
 &i2s1_8ch {
@@ -508,6 +573,21 @@ bt_wake_h: bt-wake-h {
 		};
 	};
 
+	ebc-pmic {
+		ebc_pmic_pins: ebc-pmic-pins {
+			rockchip,pins = /* wakeup */
+					<3 RK_PA5 RK_FUNC_GPIO &pcfg_pull_none>,
+					/* int */
+					<3 RK_PA6 RK_FUNC_GPIO &pcfg_pull_up>,
+					/* pwr_good */
+					<3 RK_PA7 RK_FUNC_GPIO &pcfg_pull_none>,
+					/* pwrup */
+					<3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>,
+					/* vcom_ctrl */
+					<4 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	led {
 		led_pin: led-pin {
 			rockchip,pins = <3 RK_PC5 RK_FUNC_GPIO &pcfg_pull_none>;
-- 
2.35.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding
  2022-04-13 22:19 ` [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding Samuel Holland
@ 2022-04-14  8:15   ` Andreas Kemnade
  2022-04-15  3:00     ` Samuel Holland
  0 siblings, 1 reply; 28+ messages in thread
From: Andreas Kemnade @ 2022-04-14  8:15 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Heiko Stübner, Sandy Huang, dri-devel, linux-rockchip,
	Alistair Francis, Ondřej Jirman, Daniel Vetter,
	David Airlie, Geert Uytterhoeven, Krzysztof Kozlowski,
	Liang Chen, Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

Hi Samuel,

for comparison, here is my submission for the IMX EPDC bindings:

https://lore.kernel.org/linux-devicetree/20220206080016.796556-2-andreas@kemnade.info/

On Wed, 13 Apr 2022 17:19:02 -0500
Samuel Holland <samuel@sholland.org> wrote:

[...]
we have sy7636a driver in kernel which should be suitable for powering a EPD
and temperature measurement. So I would expect that to be 
> +  io-channels:
> +    maxItems: 1
> +    description: I/O channel for panel temperature measurement
> +
so how would I reference the hwmon/thermal(-zone) of the sy7636a here?

> +  panel-supply:
> +    description: Regulator supplying the panel's logic voltage
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  vcom-supply:
> +    description: Regulator supplying the panel's compensation voltage
> +
> +  vdrive-supply:
> +    description: Regulator supplying the panel's gate and source drivers
> +
SY7636a has only one logical regulator in kernel for for the latter two.

If we have a separate panel node, than maybe these regulators should go
there as they belong to the panel as they are powering the panel and
not the EBC.

> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description: OF graph port for the attached display panel
> +
In my approach for the IMX EPDC, (I will send a better commented one
soon) I have no separate subnode to avoid messing with additional
display parameters. Not sure what is really better here.

> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - power-domains
> +  - panel-supply
> +  - vcom-supply
> +  - vdrive-supply

If things differ how the different phyiscally existing regulators are
mapped into logical ones (even the vdrive supply is still a bunch of
physical regulators mapped into one logical one), then not everything
can be required.

Regards,
Andreas

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (15 preceding siblings ...)
  2022-04-13 22:19 ` [RFC PATCH 16/16] [DO NOT MERGE] arm64: dts: rockchip: pinenote: Enable EBC display pipeline Samuel Holland
@ 2022-04-14  8:50 ` Maxime Ripard
  2022-05-25 17:18   ` Daniel Vetter
  2022-04-21  6:43 ` Andreas Kemnade
  17 siblings, 1 reply; 28+ messages in thread
From: Maxime Ripard @ 2022-04-14  8:50 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Heiko Stübner, Sandy Huang, dri-devel, linux-rockchip,
	Alistair Francis, Ondřej Jirman, Andreas Kemnade,
	Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Krzysztof Kozlowski, Liang Chen, Maarten Lankhorst,
	Michael Riesch, Nicolas Frattaroli, Peter Geis, Rob Herring,
	Sam Ravnborg, Thierry Reding, Thomas Zimmermann, devicetree,
	linux-arm-kernel, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 14428 bytes --]

On Wed, Apr 13, 2022 at 05:19:00PM -0500, Samuel Holland wrote:
> This series adds a DRM driver for the electrophoretic display controller
> found in a few different Rockchip SoCs, specifically the RK3566/RK3568
> variant[0] used by the PineNote tablet[1].
> 
> This is my first real involvement with the DRM subsystem, so please let
> me know where I am misunderstanding things.
> 
> This is now the second SoC-integrated EPD controller with a DRM driver
> submission -- the first one being the i.MX6 EPDC[2]. I want to thank
> Andreas for sending that series, and for his advice while writing this
> driver.
> 
> One goal I have with sending this series is to discuss how to support
> EPDs more generally within the DRM subsystem, so the interfaces with
> panels and PMICs and waveform LUTs can be controller-independent.
> 
> My understanding is that the i.MX6 EPDC series is at least partly based
> on the downstream vendor driver. This driver is a clean-sheet design for
> hardware with different (read: fewer) capabilities, so we took some
> different design paths, but we ran into many of the same sharp edges.
> 
> Here are some of the areas I would like input on:
> 
> Panel Lifecycle
> ===============
> Panels use prepare/unprepare callbacks for their power supply. EPDs
> should only be powered up when the display contents are changed. Should
> the controller call both drm_panel_(un)prepare during each atomic update
> when the framebuffer is dirty?
> 
> Similarly, panel enable/disable callbacks are tied to backlight state.
> For an EPD, it makes sense to have the backlight enabled while the panel
> is powered down (because the contents are static). Is it acceptable to
> call drm_panel_{en,dis}able while the panel is not prepared?
> 
> With panel_bridge, the "normal" callback ordering is enforced, and tied
> to the atomic state, so neither of these is possible.
> 
> As a result, neither the backlight nor the voltage regulators are tied
> to the panel. The panel's regulators are consumed by the EBC itself.

At least to manage the power state, that looks fairly similar to what we
have already to enter / exit from panel self refresh, so maybe we can
leverage that infrastructure?

And thus we would have something like enabling the backlight when we
prepare the panel, but only enable / disable the regulator when we exit
/ enter PSR mode?

Would that make sense?

> Panel Timing Parameters
> =======================
> EPDs have more timing parameters than LCDs, and there are several
> different ways of labeling these parameters. See for example the timing
> diagrams on pp. 2237-2239 of the RK3568 TRM[0], the descriptions in the
> ED103TC2 panel datasheet[3], and the submitted EPDC bindings[2].
> 
> Both the EPDC and EBC vendor drivers put all of the timing parameters in
> the controller's OF node. There is no panel device/node.
> 
> I was able to squeeze everything needed for my specific case into a
> struct drm_display_mode (see patches 5 and 14), but I don't know if this
> is an acceptable use of those fields, or if will work with other
> controllers. Is adding more fields to drm_display_mode an option?
> 
> See also the discussion of "dumb" LCD TCONs below.

Reading that datasheet and patch series, it's not clear to me whether
it's just a set of generic parameters for E-ink display, or if it's some
hardware specific representation of those timings.

Generally speaking, drm_display_mode is an approximation of what the
timings are. The exact clock rate for example will be widely different
between RGB, HDMI or MIPI-DSI (with or without burst). I think that as
long as you can derive a drm_display_mode from those parameters, and can
infer those parameters from a drm_display_mode, you can definitely reuse
it.

> Panel Connector Type / Media Bus Format
> =======================================
> The EBC supports either an 8-bit or 16-bit wide data bus, where each
> pair of data lines represents the source driver polarity (positive,
> negative, or neutral) for a pixel.
> 
> The only effect of the data bus width is the number of pixels that are
> transferred per clock cycle. It has no impact on the number of possible
> grayscale levels.
> 
> How does that translate to DRM_MODE_CONNECTOR_* or MEDIA_BUS_FMT_*?

We'll probably want a separate connector mode, but you could add a
parameter on the OF-graph endpoint to set the media bus format.

> Panel Reflection
> ================
> The ED103TC2 panel scans from right to left. Currently, there is no API
> or OF property to represent this. I can add something similar to
> drm_panel_orientation.

Yeah, leveraging DRM_MODE_REFLECT_X into something similar to
drm_panel_orientation makes sense

> Should this be exposed to userspace? It is acceptable for the kernel
> driver to flip the image when blitting from the framebuffer?

I'm not sure about whether or not we should expose it to userspace. I'd
say yes, but I'll leave it to others :)

> CRTC "active" and "enabled" states
> ==================================
> What do these states mean in the context of an EPD? Currently, the
> driver ignores "active" and clears the screen to solid white when the
> CRTC is disabled.
> 
> The vendor drivers can switch to a user-provided image when the CRTC is
> disabled. Is this something that can/should be supported upstream? If
> so, how? Would userspace provide the image to the kernel, or just tell
> the kernel not to clear the screen?

I think the semantics are that whenever the CRTC is disabled, the panel
is expected to be blank.

Leaving an image on after it's been disabled would have a bunch of
side-effects we probably don't want. For example, let's assume we have
that support, an application sets a "disabled image" and quits. Should
we leave the content on? If so, for how long exactly?

Either way, this is likely to be doable with PSR as well, so I think
it's a bit out of scope of this series for now.

> VBLANK Events and Asynchronous Commits
> ======================================
> When should the VBLANK event complete? When the pixels have been blitted
> to the kernel's shadow buffer? When the first frame of the waveform is
> sent to the panel? When the last frame is sent to the panel?
> 
> Currently, the driver is taking the first option, letting
> drm_atomic_helper_fake_vblank() send the VBLANK event without waiting on
> the refresh thread. This is the only way I was able to get good
> performance with existing userspace.

I've been having the same kind of discussions in private lately, so I'm
interested by the answer as well :)

It would be worth looking into the SPI/I2C panels for this, since it's
basically the same case.

> Waveform Loading
> ================
> Waveform files are calibrated for each batch of panels. So while a
> single waveform file may be "good enough" for all panels of a certain
> model, the correctly-calibrated file will have better image quality.
> 
> I don't know of a good way to choose the calibrated file. Even the
> board's compatible string may not be specific enough, if the board is
> manufactured with multiple batches of panels.
> 
> Maybe the filename should just be the panel compatible, and the user is
> responsible for putting the right file there? In that case, how should I
> get the compatible string from the panel_bridge? Traverse the OF graph
> myself?

It's not really clear to me what panel_bridge has to do with it? I'm
assuming that file has to be uploaded some way or another to the
encoder?

If so, yeah, you should just follow through the OF-graph and use the
panel compatible. We have a similar case already with panel-mipi-dbi
(even though it's standalone)

> There is also the issue that different controllers need the waveform
> data in different formats. ".wbf" appears to be the format provided by
> PVI/eInk, the panel manufacturer. The Rockchip EBC hardware expects a
> single waveform in a flat array, so the driver has to extract/decompress
> that from the .wbf file (this is done in patch 1). On the other hand,
> the i.MX EPDC expects a ".wrf" file containing multiple waveforms[8].
> 
> I propose that the waveform file on disk should always be what was
> shipped with the panel -- the .wbf file -- and any extracting or
> reformatting is done in the kernel.

Any kind of parsing in the kernel from a file you have no control over
always irks me :)

Why and how are those files different in the first place?

> Waveform Selection From Userspace
> =================================
> EPDs use different waveforms for different purposes: high-quality
> grayscale vs. monochrome text vs. dithered monochrome video. How can
> userspace select which waveform to use? Should this be a plane property?
>
> It is also likely that userspace will want to use different waveforms at
> the same time for different parts of the screen, for example a fast
> monochrome waveform for the drawing area of a note-taking app, but a
> grayscale waveform for surrounding UI and window manager.
> 
> I believe the i.MX6 EPDC supports multiple planes, each with their own
> waveform choice. That seems like a good abstraction,

I agree

> but the EBC only supports one plane in hardware. So using this
> abstraction with the EBC would require blending pixels and doing
> waveform lookups in software.

Not really? You'd have a single plane available, with only one waveform
pick for that plane?

> Blitting/Blending in Software
> =============================
> There are multiple layers to this topic (pun slightly intended):
>  1) Today's userspace does not expect a grayscale framebuffer.
>     Currently, the driver advertises XRGB8888 and converts to Y4
>     in software. This seems to match other drivers (e.g. repaper).
>
>  2) Ignoring what userspace "wants", the closest existing format is
>     DRM_FORMAT_R8. Geert sent a series[4] adding DRM_FORMAT_R1 through
>     DRM_FORMAT_R4 (patch 9), which I believe are the "correct" formats
>     to use.
> 
>  3) The RK356x SoCs have an "RGA" hardware block that can do the
>     RGB-to-grayscale conversion, and also RGB-to-dithered-monochrome
>     which is needed for animation/video. Currently this is exposed with
>     a V4L2 platform driver. Can this be inserted into the pipeline in a
>     way that is transparent to userspace? Or must some userspace library
>     be responsible for setting up the RGA => EBC pipeline?

I'm very interested in this answer as well :)

I think the current consensus is that it's up to userspace to set this
up though.

>  4) Supporting multiple planes (for multiple concurrent waveforms)
>     implies blending in software. Is that acceptable?
> 
>  5) Thoughts on SIMD-optimized blitting and waveform lookup functions?
> 
>  5) Currently the driver uses kmalloc() and dma_sync_single_for_device()
>     for its buffers, because it needs both fast reads and fast writes to
>     several of them. Maybe cma_alloc() or dma_alloc_from_contiguous()
>     would be more appropriate, but I don't see any drivers using those
>     directly.

cma_alloc isn't meant to be used directly by drivers anyway, one of the
main reason being that CMA might not be available (or desirable) in the
first place on the platform the code will run.

The most common option would be dma_alloc_coherent. It often means that
the buffer will be mapped non-cacheable, so it kills the access
performances. So it completely depends on your access patterns whether
it makes sense in your driver or not. kmalloc + dma_sync_single or
dma_map_single is also a valid option.

> EPDs connected to "dumb" LCD TCONs
> ==================================
> This topic is mostly related to my first patch. Some boards exist that
> hook up an EPD to a normal LCD TCON, not a dedicated EPD controller. For
> example, there's the reMarkable 2[5] and some PocketBook models[6][7].
> 
> I have some concerns about this:
>  1) If we put EPD panel timings in panel drivers (e.g. panel-simple),
>     can the same timings work with LCD TCONs and EPD controllers?

I'll think we'll need a separate panel driver for this anyway

>     For example: one cycle of the 16-bit data bus is "one pixel" to an
>     LCD controller, but is "8 pixels" to an EPD controller. So there is
>     a factor-of-8 difference in horizontal resolution depending on your
>     perspective. Should we have the "number of pixel clock cycles" or
>     "number of pixels" in .hdisplay/.htotal in the panel timings?
> 
>     Patch 14 adds a panel with "number of pixels" horizontal resolution,
>     so the correct resolution is reported to userspace, but the existing
>     eink_vb3300_kca_timing in panel-simple.c appears to use "number of
>     pixel clocks" for its horizontal resolution. This makes the panel
>     timing definitions incompatible across controllers.
> 
>  2) Using fbdev/fbcon with an EPD hooked up to an LCD TCON will have
>     unintended consequences, and possibly damage the panel. Currently,
>     there is no way to mark the framebuffer as expecting "source driver
>     polarity waveforms" and not pixel data. Is there a specific
>     DRM_FORMAT_* we should use for these cases to prevent accidental use
>     by userspace?
> 
>     Or should we disallow this entirely, and have some wrapper layer to
>     do the waveform lookups in kernelspace?
> 
>     I like the wrapper layer idea because it allows normal userspace and
>     fbcon to work. It would not be much new code, especially since this
>     driver already supports doing the whole pipeline in software. So
>     that's why I wrote a separate helper library; I hope this code can
>     be reused.

If exposing the panel as a KMS connector can damage the display, I don't
think we should expose it at all. Even a property or something won't
work, because older applications won't know about that property and will
try to use it anyway.

So whatever the solution is, it can't be "you have to know that this
device is special, or else...". The default, trivial, case where an
application just comes up and tries to display something should somewhat
work (even if it might be a bit absurd, like ignoring non_desktop)

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding
  2022-04-14  8:15   ` Andreas Kemnade
@ 2022-04-15  3:00     ` Samuel Holland
  2022-04-15 11:45       ` Andreas Kemnade
  0 siblings, 1 reply; 28+ messages in thread
From: Samuel Holland @ 2022-04-15  3:00 UTC (permalink / raw)
  To: Andreas Kemnade
  Cc: Heiko Stübner, Sandy Huang, dri-devel, linux-rockchip,
	Alistair Francis, Ondřej Jirman, Daniel Vetter,
	David Airlie, Geert Uytterhoeven, Krzysztof Kozlowski,
	Liang Chen, Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

Hi Andreas,

Thanks for the comments.

On 4/14/22 3:15 AM, Andreas Kemnade wrote:
> Hi Samuel,
> 
> for comparison, here is my submission for the IMX EPDC bindings:
> 
> https://lore.kernel.org/linux-devicetree/20220206080016.796556-2-andreas@kemnade.info/
> 
> On Wed, 13 Apr 2022 17:19:02 -0500
> Samuel Holland <samuel@sholland.org> wrote:
> 
> [...]
> we have sy7636a driver in kernel which should be suitable for powering a EPD
> and temperature measurement. So I would expect that to be 
>> +  io-channels:
>> +    maxItems: 1
>> +    description: I/O channel for panel temperature measurement
>> +
> so how would I reference the hwmon/thermal(-zone) of the sy7636a here?

It seems the consensus is to use a thermal zone for panel temperature, so I will
need to change this.

I think it's best to reference the thermal zone by phandle, not by name, even if
it requires extending the thermal zone API to support this.

>> +  panel-supply:
>> +    description: Regulator supplying the panel's logic voltage
>> +
>> +  power-domains:
>> +    maxItems: 1
>> +
>> +  vcom-supply:
>> +    description: Regulator supplying the panel's compensation voltage
>> +
>> +  vdrive-supply:
>> +    description: Regulator supplying the panel's gate and source drivers
>> +
> SY7636a has only one logical regulator in kernel for for the latter two.

Both properties could point to the same regulator node if there are more
consumers than regulators. I don't know of a clean way to handle the opposite
situation.

The other benefit of separating out VCOM is that the controller or panel driver
can set a calibrated voltage from e.g. NVMEM or the panel's DT node.

> If we have a separate panel node, than maybe these regulators should go
> there as they belong to the panel as they are powering the panel and
> not the EBC.

I agree on this. It doesn't work with panel-simple, but as Maxime points out, we
have more flexibility with a custom panel driver.

>> +  port:
>> +    $ref: /schemas/graph.yaml#/properties/port
>> +    description: OF graph port for the attached display panel
>> +
> In my approach for the IMX EPDC, (I will send a better commented one
> soon) I have no separate subnode to avoid messing with additional
> display parameters. Not sure what is really better here.

I tried to match the existing abstractions as much as possible, and I saw there
was already an "eink,vb3300-kca" display in panel-simple. I believe that one was
added for the reMarkable 2, where the existing LCD controller driver already
depends on the DRM panel code (although I have concerns about hooking that up to
a driver that doesn't understand EPDs).

My thought here is that the timings for a given panel should be the same across
controllers, both dedicated EPD controllers and LCD controllers. Or at least it
should be possible to derive the timings from some common set of parameters.

The panel node also usually hooks up to the backlight, although I am not sure
that is the right thing to do for EPDs. (And the PineNote has a separate issue
of having two backlights [warm/cool] for one display.)

Regards,
Samuel

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding
  2022-04-15  3:00     ` Samuel Holland
@ 2022-04-15 11:45       ` Andreas Kemnade
  0 siblings, 0 replies; 28+ messages in thread
From: Andreas Kemnade @ 2022-04-15 11:45 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Heiko Stübner, Sandy Huang, dri-devel, linux-rockchip,
	Alistair Francis, Ondřej Jirman, Daniel Vetter,
	David Airlie, Geert Uytterhoeven, Krzysztof Kozlowski,
	Liang Chen, Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

On Thu, 14 Apr 2022 22:00:09 -0500
Samuel Holland <samuel@sholland.org> wrote:

> Hi Andreas,
> 
> Thanks for the comments.
> 
> On 4/14/22 3:15 AM, Andreas Kemnade wrote:
> > Hi Samuel,
> > 
> > for comparison, here is my submission for the IMX EPDC bindings:
> > 
> > https://lore.kernel.org/linux-devicetree/20220206080016.796556-2-andreas@kemnade.info/
> > 
> > On Wed, 13 Apr 2022 17:19:02 -0500
> > Samuel Holland <samuel@sholland.org> wrote:
> > 
> > [...]
> > we have sy7636a driver in kernel which should be suitable for powering a EPD
> > and temperature measurement. So I would expect that to be   
> >> +  io-channels:
> >> +    maxItems: 1
> >> +    description: I/O channel for panel temperature measurement
> >> +  
> > so how would I reference the hwmon/thermal(-zone) of the sy7636a here?  
> 
> It seems the consensus is to use a thermal zone for panel temperature, so I will
> need to change this.
> 
I am open to anything here as long as it fits together.

> I think it's best to reference the thermal zone by phandle, not by name, even if
> it requires extending the thermal zone API to support this.
> 
maybe referencing the hwmon might be interesting, or we add a hwmon_iio
adaptor. The other way round it is there. The thermal zone stuff is
only needed because hwmon cannot referenced directly.

Regards,
Andreas

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
  2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
                   ` (16 preceding siblings ...)
  2022-04-14  8:50 ` [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Maxime Ripard
@ 2022-04-21  6:43 ` Andreas Kemnade
  2022-04-21  7:10   ` Nicolas Frattaroli
  17 siblings, 1 reply; 28+ messages in thread
From: Andreas Kemnade @ 2022-04-21  6:43 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Heiko Stübner, Sandy Huang, dri-devel, linux-rockchip,
	Alistair Francis, Ondřej Jirman, Daniel Vetter,
	David Airlie, Geert Uytterhoeven, Krzysztof Kozlowski,
	Liang Chen, Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Nicolas Frattaroli, Peter Geis, Rob Herring, Sam Ravnborg,
	Thierry Reding, Thomas Zimmermann, devicetree, linux-arm-kernel,
	linux-kernel

On Wed, 13 Apr 2022 17:19:00 -0500
Samuel Holland <samuel@sholland.org> wrote:

[...]
> Waveform Selection From Userspace
> =================================
> EPDs use different waveforms for different purposes: high-quality
> grayscale vs. monochrome text vs. dithered monochrome video. How can
> userspace select which waveform to use? Should this be a plane property?
> 
Or does userspace rather select a QoS, like low-latency vs. high
quality. Or this will not change for a longer time: like doing full
refreshes.

> It is also likely that userspace will want to use different waveforms at
> the same time for different parts of the screen, for example a fast
> monochrome waveform for the drawing area of a note-taking app, but a
> grayscale waveform for surrounding UI and window manager.
> 

> I believe the i.MX6 EPDC supports multiple planes, each with their own
> waveform choice. That seems like a good abstraction, but the EBC only
> supports one plane in hardware. So using this abstraction with the EBC
> would require blending pixels and doing waveform lookups in software.
> 
The iMX6 EPDC has one working buffer containing the old+new state of
the pixel. That is 16bpp. Then for each update you can specify a
rectangle in an independant 8bpp buffer as a source. For now I am just
using a single buffer. But yes, that construction could be used to do
some multi plane stuff.

> Blitting/Blending in Software
> =============================
> There are multiple layers to this topic (pun slightly intended):
>  1) Today's userspace does not expect a grayscale framebuffer.
>     Currently, the driver advertises XRGB8888 and converts to Y4
>     in software. This seems to match other drivers (e.g. repaper).
> 
>  2) Ignoring what userspace "wants", the closest existing format is
>     DRM_FORMAT_R8. Geert sent a series[4] adding DRM_FORMAT_R1 through
>     DRM_FORMAT_R4 (patch 9), which I believe are the "correct" formats
>     to use.
>
hmm R=red? That sounds strange. I am unsure whether doing things with
lower bit depths actually really helps. 

>  3) The RK356x SoCs have an "RGA" hardware block that can do the
>     RGB-to-grayscale conversion, and also RGB-to-dithered-monochrome
>     which is needed for animation/video. Currently this is exposed with
>     a V4L2 platform driver. Can this be inserted into the pipeline in a
>     way that is transparent to userspace? Or must some userspace library
>     be responsible for setting up the RGA => EBC pipeline?

hmm, we have other drivers with some hardware block doing rotation, but
in that cases it is not exposed as v4l2 mem2mem device.

On IMX6 there is also the PXP doing RGB-to-grayscale and rotation but
exposed as v4l2 device. But it can also be used to do undocumented
stuff writing to the 16bpp working buffer. So basically it is similar.
But I would do thoso things in a second step and just have the basic
stuff upstreamed

Regards,
Andreas

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
  2022-04-21  6:43 ` Andreas Kemnade
@ 2022-04-21  7:10   ` Nicolas Frattaroli
  0 siblings, 0 replies; 28+ messages in thread
From: Nicolas Frattaroli @ 2022-04-21  7:10 UTC (permalink / raw)
  To: Samuel Holland, Andreas Kemnade
  Cc: Heiko Stübner, Sandy Huang, dri-devel, linux-rockchip,
	Alistair Francis, Ondřej Jirman, Daniel Vetter,
	David Airlie, Geert Uytterhoeven, Krzysztof Kozlowski,
	Liang Chen, Maarten Lankhorst, Maxime Ripard, Michael Riesch,
	Peter Geis, Rob Herring, Sam Ravnborg, Thierry Reding,
	Thomas Zimmermann, devicetree, linux-arm-kernel, linux-kernel

On Donnerstag, 21. April 2022 08:43:38 CEST Andreas Kemnade wrote:
> On Wed, 13 Apr 2022 17:19:00 -0500
> Samuel Holland <samuel@sholland.org> wrote:
> > Blitting/Blending in Software
> > =============================
> > There are multiple layers to this topic (pun slightly intended):
> >  1) Today's userspace does not expect a grayscale framebuffer.
> >     Currently, the driver advertises XRGB8888 and converts to Y4
> >     in software. This seems to match other drivers (e.g. repaper).
> > 
> >  2) Ignoring what userspace "wants", the closest existing format is
> >     DRM_FORMAT_R8. Geert sent a series[4] adding DRM_FORMAT_R1 through
> >     DRM_FORMAT_R4 (patch 9), which I believe are the "correct" formats
> >     to use.
> >
> hmm R=red? That sounds strange. I am unsure whether doing things with
> lower bit depths actually really helps. 

Hi,

for single-component formats, the name of the component plays
practically no role. Even if said component was really red,
it makes little difference to either side.

For example, the OpenGL straight up refers to all single component
image formats as only using the red component:

	OpenGL only allows "R", "RG", "RGB", or "RGBA"; other
	combinations are not allowed as internal image formats.

from https://www.khronos.org/opengl/wiki/Image_Format

In truth it would of course be nice if the world agreed on
not making the name of data structures imply some way they
are to be processed, but humanity hasn't gotten there yet.

> 
> Regards,
> Andreas
> 

Regards,
Nicolas Frattaroli



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
  2022-04-14  8:50 ` [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Maxime Ripard
@ 2022-05-25 17:18   ` Daniel Vetter
  2022-05-31  8:58     ` Maxime Ripard
  0 siblings, 1 reply; 28+ messages in thread
From: Daniel Vetter @ 2022-05-25 17:18 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Samuel Holland, Heiko Stübner, Sandy Huang, dri-devel,
	linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, Daniel Vetter, David Airlie, Geert Uytterhoeven,
	Krzysztof Kozlowski, Liang Chen, Maarten Lankhorst,
	Michael Riesch, Nicolas Frattaroli, Peter Geis, Rob Herring,
	Sam Ravnborg, Thierry Reding, Thomas Zimmermann, devicetree,
	linux-arm-kernel, linux-kernel

Some comments on this from my side too, not sure how good they are when it
comes more to the hw side of things :-)

On Thu, Apr 14, 2022 at 10:50:18AM +0200, Maxime Ripard wrote:
> On Wed, Apr 13, 2022 at 05:19:00PM -0500, Samuel Holland wrote:
> > This series adds a DRM driver for the electrophoretic display controller
> > found in a few different Rockchip SoCs, specifically the RK3566/RK3568
> > variant[0] used by the PineNote tablet[1].
> > 
> > This is my first real involvement with the DRM subsystem, so please let
> > me know where I am misunderstanding things.
> > 
> > This is now the second SoC-integrated EPD controller with a DRM driver
> > submission -- the first one being the i.MX6 EPDC[2]. I want to thank
> > Andreas for sending that series, and for his advice while writing this
> > driver.
> > 
> > One goal I have with sending this series is to discuss how to support
> > EPDs more generally within the DRM subsystem, so the interfaces with
> > panels and PMICs and waveform LUTs can be controller-independent.
> > 
> > My understanding is that the i.MX6 EPDC series is at least partly based
> > on the downstream vendor driver. This driver is a clean-sheet design for
> > hardware with different (read: fewer) capabilities, so we took some
> > different design paths, but we ran into many of the same sharp edges.
> > 
> > Here are some of the areas I would like input on:
> > 
> > Panel Lifecycle
> > ===============
> > Panels use prepare/unprepare callbacks for their power supply. EPDs
> > should only be powered up when the display contents are changed. Should
> > the controller call both drm_panel_(un)prepare during each atomic update
> > when the framebuffer is dirty?
> > 
> > Similarly, panel enable/disable callbacks are tied to backlight state.
> > For an EPD, it makes sense to have the backlight enabled while the panel
> > is powered down (because the contents are static). Is it acceptable to
> > call drm_panel_{en,dis}able while the panel is not prepared?
> > 
> > With panel_bridge, the "normal" callback ordering is enforced, and tied
> > to the atomic state, so neither of these is possible.
> > 
> > As a result, neither the backlight nor the voltage regulators are tied
> > to the panel. The panel's regulators are consumed by the EBC itself.
> 
> At least to manage the power state, that looks fairly similar to what we
> have already to enter / exit from panel self refresh, so maybe we can
> leverage that infrastructure?
> 
> And thus we would have something like enabling the backlight when we
> prepare the panel, but only enable / disable the regulator when we exit
> / enter PSR mode?
> 
> Would that make sense?
> 
> > Panel Timing Parameters
> > =======================
> > EPDs have more timing parameters than LCDs, and there are several
> > different ways of labeling these parameters. See for example the timing
> > diagrams on pp. 2237-2239 of the RK3568 TRM[0], the descriptions in the
> > ED103TC2 panel datasheet[3], and the submitted EPDC bindings[2].
> > 
> > Both the EPDC and EBC vendor drivers put all of the timing parameters in
> > the controller's OF node. There is no panel device/node.
> > 
> > I was able to squeeze everything needed for my specific case into a
> > struct drm_display_mode (see patches 5 and 14), but I don't know if this
> > is an acceptable use of those fields, or if will work with other
> > controllers. Is adding more fields to drm_display_mode an option?
> > 
> > See also the discussion of "dumb" LCD TCONs below.
> 
> Reading that datasheet and patch series, it's not clear to me whether
> it's just a set of generic parameters for E-ink display, or if it's some
> hardware specific representation of those timings.
> 
> Generally speaking, drm_display_mode is an approximation of what the
> timings are. The exact clock rate for example will be widely different
> between RGB, HDMI or MIPI-DSI (with or without burst). I think that as
> long as you can derive a drm_display_mode from those parameters, and can
> infer those parameters from a drm_display_mode, you can definitely reuse
> it.
> 
> > Panel Connector Type / Media Bus Format
> > =======================================
> > The EBC supports either an 8-bit or 16-bit wide data bus, where each
> > pair of data lines represents the source driver polarity (positive,
> > negative, or neutral) for a pixel.
> > 
> > The only effect of the data bus width is the number of pixels that are
> > transferred per clock cycle. It has no impact on the number of possible
> > grayscale levels.
> > 
> > How does that translate to DRM_MODE_CONNECTOR_* or MEDIA_BUS_FMT_*?
> 
> We'll probably want a separate connector mode, but you could add a
> parameter on the OF-graph endpoint to set the media bus format.
> 
> > Panel Reflection
> > ================
> > The ED103TC2 panel scans from right to left. Currently, there is no API
> > or OF property to represent this. I can add something similar to
> > drm_panel_orientation.
> 
> Yeah, leveraging DRM_MODE_REFLECT_X into something similar to
> drm_panel_orientation makes sense

Yeah

> > Should this be exposed to userspace? It is acceptable for the kernel
> > driver to flip the image when blitting from the framebuffer?
> 
> I'm not sure about whether or not we should expose it to userspace. I'd
> say yes, but I'll leave it to others :)

Same. I'm very grumpily accepting that we need sw conversion tools from
xrgb8888 to more unusual framebuffer formats, but everything else should
be userspace problems imo.

It's a bit more awkard than a wrongly rotate screen if it's mirrored, but
I guess that's it.

What is surprising is that your hw really doesn't have any hw support to
mirror things, since that's generally really easy to implement.

For the blitter I guess that would be a v4l mem2mem device?

> > CRTC "active" and "enabled" states
> > ==================================
> > What do these states mean in the context of an EPD? Currently, the
> > driver ignores "active" and clears the screen to solid white when the
> > CRTC is disabled.
> > 
> > The vendor drivers can switch to a user-provided image when the CRTC is
> > disabled. Is this something that can/should be supported upstream? If
> > so, how? Would userspace provide the image to the kernel, or just tell
> > the kernel not to clear the screen?
> 
> I think the semantics are that whenever the CRTC is disabled, the panel
> is expected to be blank.
> 
> Leaving an image on after it's been disabled would have a bunch of
> side-effects we probably don't want. For example, let's assume we have
> that support, an application sets a "disabled image" and quits. Should
> we leave the content on? If so, for how long exactly?
> 
> Either way, this is likely to be doable with PSR as well, so I think
> it's a bit out of scope of this series for now.

active is hw state

enabled is a pure sw state on top, to make sure that all the hw resources
you need are still reserved. E.g. when you have 2 crtc and you enable one,
but keep it off (i.e. active = false), then the clocks, memory bw and all
that are still reserved. This is to be able to guarantee that dpms off ->
on transitions always work.

Iow, in atomic_check you need to look at enabled, in atomic commit you
need to look at active.

With a single crtc there should never be any issue here really, since
there's no other crtc where you can steal clocks or similar things from.

Note that kerneldoc should explain this all, pls double check and if it's
not clear submit a patch please.

> > VBLANK Events and Asynchronous Commits
> > ======================================
> > When should the VBLANK event complete? When the pixels have been blitted
> > to the kernel's shadow buffer? When the first frame of the waveform is
> > sent to the panel? When the last frame is sent to the panel?
> > 
> > Currently, the driver is taking the first option, letting
> > drm_atomic_helper_fake_vblank() send the VBLANK event without waiting on
> > the refresh thread. This is the only way I was able to get good
> > performance with existing userspace.
> 
> I've been having the same kind of discussions in private lately, so I'm
> interested by the answer as well :)
> 
> It would be worth looking into the SPI/I2C panels for this, since it's
> basically the same case.

So it's maybe a bit misnamed and maybe kerneldocs aren't super clear (pls
help improve them), but there's two modes:

- drivers which have vblank, which might be somewhat variable (VRR) or
  become simulated (self-refresh panels), but otherwise is a more-or-less
  regular clock. For this case the atomic commit event must match the
  vblank events exactly (frame count and timestamp)

- drivers which don't have vblank at all, mostly these are i2c/spi panels
  or virtual hw and stuff like that. In this case the event simply happens
  when the driver is done with refresh/upload, and the frame count should
  be zero (since it's meaningless).

Unfortuantely the helper to dtrt has fake_vblank in it's name, maybe
should be renamed to no_vblank or so (the various flags that control it
are a bit better named).

Again the docs should explain it all, but maybe we should clarify them or
perhaps rename that helper to be more meaningful.

> > Waveform Loading
> > ================
> > Waveform files are calibrated for each batch of panels. So while a
> > single waveform file may be "good enough" for all panels of a certain
> > model, the correctly-calibrated file will have better image quality.
> > 
> > I don't know of a good way to choose the calibrated file. Even the
> > board's compatible string may not be specific enough, if the board is
> > manufactured with multiple batches of panels.
> > 
> > Maybe the filename should just be the panel compatible, and the user is
> > responsible for putting the right file there? In that case, how should I
> > get the compatible string from the panel_bridge? Traverse the OF graph
> > myself?
> 
> It's not really clear to me what panel_bridge has to do with it? I'm
> assuming that file has to be uploaded some way or another to the
> encoder?
> 
> If so, yeah, you should just follow through the OF-graph and use the
> panel compatible. We have a similar case already with panel-mipi-dbi
> (even though it's standalone)

Yeah if there's really on difference then I guess the best we can do is
"make sure you put the right file into the firmware directory". Sucks but
anything else isn't really better.

> > There is also the issue that different controllers need the waveform
> > data in different formats. ".wbf" appears to be the format provided by
> > PVI/eInk, the panel manufacturer. The Rockchip EBC hardware expects a
> > single waveform in a flat array, so the driver has to extract/decompress
> > that from the .wbf file (this is done in patch 1). On the other hand,
> > the i.MX EPDC expects a ".wrf" file containing multiple waveforms[8].
> > 
> > I propose that the waveform file on disk should always be what was
> > shipped with the panel -- the .wbf file -- and any extracting or
> > reformatting is done in the kernel.
> 
> Any kind of parsing in the kernel from a file you have no control over
> always irks me :)
> 
> Why and how are those files different in the first place?
> 
> > Waveform Selection From Userspace
> > =================================
> > EPDs use different waveforms for different purposes: high-quality
> > grayscale vs. monochrome text vs. dithered monochrome video. How can
> > userspace select which waveform to use? Should this be a plane property?
> >
> > It is also likely that userspace will want to use different waveforms at
> > the same time for different parts of the screen, for example a fast
> > monochrome waveform for the drawing area of a note-taking app, but a
> > grayscale waveform for surrounding UI and window manager.
> > 
> > I believe the i.MX6 EPDC supports multiple planes, each with their own
> > waveform choice. That seems like a good abstraction,
> 
> I agree
> 
> > but the EBC only supports one plane in hardware. So using this
> > abstraction with the EBC would require blending pixels and doing
> > waveform lookups in software.
> 
> Not really? You'd have a single plane available, with only one waveform
> pick for that plane?
> 
> > Blitting/Blending in Software
> > =============================
> > There are multiple layers to this topic (pun slightly intended):
> >  1) Today's userspace does not expect a grayscale framebuffer.
> >     Currently, the driver advertises XRGB8888 and converts to Y4
> >     in software. This seems to match other drivers (e.g. repaper).
> >
> >  2) Ignoring what userspace "wants", the closest existing format is
> >     DRM_FORMAT_R8. Geert sent a series[4] adding DRM_FORMAT_R1 through
> >     DRM_FORMAT_R4 (patch 9), which I believe are the "correct" formats
> >     to use.
> > 
> >  3) The RK356x SoCs have an "RGA" hardware block that can do the
> >     RGB-to-grayscale conversion, and also RGB-to-dithered-monochrome
> >     which is needed for animation/video. Currently this is exposed with
> >     a V4L2 platform driver. Can this be inserted into the pipeline in a
> >     way that is transparent to userspace? Or must some userspace library
> >     be responsible for setting up the RGA => EBC pipeline?
> 
> I'm very interested in this answer as well :)
> 
> I think the current consensus is that it's up to userspace to set this
> up though.

Yeah I think v4l mem2mem device is the answer for these, and then
userspace gets to set it all up.

> >  4) Supporting multiple planes (for multiple concurrent waveforms)
> >     implies blending in software. Is that acceptable?
> > 
> >  5) Thoughts on SIMD-optimized blitting and waveform lookup functions?
> > 
> >  5) Currently the driver uses kmalloc() and dma_sync_single_for_device()
> >     for its buffers, because it needs both fast reads and fast writes to
> >     several of them. Maybe cma_alloc() or dma_alloc_from_contiguous()
> >     would be more appropriate, but I don't see any drivers using those
> >     directly.
> 
> cma_alloc isn't meant to be used directly by drivers anyway, one of the
> main reason being that CMA might not be available (or desirable) in the
> first place on the platform the code will run.
> 
> The most common option would be dma_alloc_coherent. It often means that
> the buffer will be mapped non-cacheable, so it kills the access
> performances. So it completely depends on your access patterns whether
> it makes sense in your driver or not. kmalloc + dma_sync_single or
> dma_map_single is also a valid option.
> 
> > EPDs connected to "dumb" LCD TCONs
> > ==================================
> > This topic is mostly related to my first patch. Some boards exist that
> > hook up an EPD to a normal LCD TCON, not a dedicated EPD controller. For
> > example, there's the reMarkable 2[5] and some PocketBook models[6][7].
> > 
> > I have some concerns about this:
> >  1) If we put EPD panel timings in panel drivers (e.g. panel-simple),
> >     can the same timings work with LCD TCONs and EPD controllers?
> 
> I'll think we'll need a separate panel driver for this anyway
> 
> >     For example: one cycle of the 16-bit data bus is "one pixel" to an
> >     LCD controller, but is "8 pixels" to an EPD controller. So there is
> >     a factor-of-8 difference in horizontal resolution depending on your
> >     perspective. Should we have the "number of pixel clock cycles" or
> >     "number of pixels" in .hdisplay/.htotal in the panel timings?
> > 
> >     Patch 14 adds a panel with "number of pixels" horizontal resolution,
> >     so the correct resolution is reported to userspace, but the existing
> >     eink_vb3300_kca_timing in panel-simple.c appears to use "number of
> >     pixel clocks" for its horizontal resolution. This makes the panel
> >     timing definitions incompatible across controllers.

Yeah that sounds bad. And I guess this really should be "number of pixels"
and the drivers need to be adjusted/fixed to be consistent.

> > 
> >  2) Using fbdev/fbcon with an EPD hooked up to an LCD TCON will have
> >     unintended consequences, and possibly damage the panel. Currently,
> >     there is no way to mark the framebuffer as expecting "source driver
> >     polarity waveforms" and not pixel data. Is there a specific
> >     DRM_FORMAT_* we should use for these cases to prevent accidental use
> >     by userspace?
> > 
> >     Or should we disallow this entirely, and have some wrapper layer to
> >     do the waveform lookups in kernelspace?
> > 
> >     I like the wrapper layer idea because it allows normal userspace and
> >     fbcon to work. It would not be much new code, especially since this
> >     driver already supports doing the whole pipeline in software. So
> >     that's why I wrote a separate helper library; I hope this code can
> >     be reused.
> 
> If exposing the panel as a KMS connector can damage the display, I don't
> think we should expose it at all. Even a property or something won't
> work, because older applications won't know about that property and will
> try to use it anyway.
> 
> So whatever the solution is, it can't be "you have to know that this
> device is special, or else...". The default, trivial, case where an
> application just comes up and tries to display something should somewhat
> work (even if it might be a bit absurd, like ignoring non_desktop)

Yeah I think if you can wreak the panel that's no good, and should be
hidden I guess. So I guess for these the kernel gets to apply the waveform
stuff internally, which I really don't like but oh well. We have plenty of
cpu slicing and dicing in other spi/i2c/usb drivers too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
  2022-05-25 17:18   ` Daniel Vetter
@ 2022-05-31  8:58     ` Maxime Ripard
  2022-06-01 12:35       ` Daniel Vetter
  0 siblings, 1 reply; 28+ messages in thread
From: Maxime Ripard @ 2022-05-31  8:58 UTC (permalink / raw)
  To: Samuel Holland, Heiko Stübner, Sandy Huang, dri-devel,
	linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, David Airlie, Geert Uytterhoeven,
	Krzysztof Kozlowski, Liang Chen, Maarten Lankhorst,
	Michael Riesch, Nicolas Frattaroli, Peter Geis, Rob Herring,
	Sam Ravnborg, Thierry Reding, Thomas Zimmermann, devicetree,
	linux-arm-kernel, linux-kernel

Hi Daniel,

Thanks for your feedback

On Wed, May 25, 2022 at 07:18:07PM +0200, Daniel Vetter wrote:
> > > VBLANK Events and Asynchronous Commits
> > > ======================================
> > > When should the VBLANK event complete? When the pixels have been blitted
> > > to the kernel's shadow buffer? When the first frame of the waveform is
> > > sent to the panel? When the last frame is sent to the panel?
> > > 
> > > Currently, the driver is taking the first option, letting
> > > drm_atomic_helper_fake_vblank() send the VBLANK event without waiting on
> > > the refresh thread. This is the only way I was able to get good
> > > performance with existing userspace.
> > 
> > I've been having the same kind of discussions in private lately, so I'm
> > interested by the answer as well :)
> > 
> > It would be worth looking into the SPI/I2C panels for this, since it's
> > basically the same case.
> 
> So it's maybe a bit misnamed and maybe kerneldocs aren't super clear (pls
> help improve them), but there's two modes:
> 
> - drivers which have vblank, which might be somewhat variable (VRR) or
>   become simulated (self-refresh panels), but otherwise is a more-or-less
>   regular clock. For this case the atomic commit event must match the
>   vblank events exactly (frame count and timestamp)

Part of my interrogation there is do we have any kind of expectation
on whether or not, when we commit, the next vblank is going to be the
one matching that commit or we're allowed to defer it by an arbitrary
number of frames (provided that the frame count and timestamps are
correct) ?

> - drivers which don't have vblank at all, mostly these are i2c/spi panels
>   or virtual hw and stuff like that. In this case the event simply happens
>   when the driver is done with refresh/upload, and the frame count should
>   be zero (since it's meaningless).
> 
> Unfortuantely the helper to dtrt has fake_vblank in it's name, maybe
> should be renamed to no_vblank or so (the various flags that control it
> are a bit better named).
> 
> Again the docs should explain it all, but maybe we should clarify them or
> perhaps rename that helper to be more meaningful.
> 
> > > Blitting/Blending in Software
> > > =============================
> > > There are multiple layers to this topic (pun slightly intended):
> > >  1) Today's userspace does not expect a grayscale framebuffer.
> > >     Currently, the driver advertises XRGB8888 and converts to Y4
> > >     in software. This seems to match other drivers (e.g. repaper).
> > >
> > >  2) Ignoring what userspace "wants", the closest existing format is
> > >     DRM_FORMAT_R8. Geert sent a series[4] adding DRM_FORMAT_R1 through
> > >     DRM_FORMAT_R4 (patch 9), which I believe are the "correct" formats
> > >     to use.
> > > 
> > >  3) The RK356x SoCs have an "RGA" hardware block that can do the
> > >     RGB-to-grayscale conversion, and also RGB-to-dithered-monochrome
> > >     which is needed for animation/video. Currently this is exposed with
> > >     a V4L2 platform driver. Can this be inserted into the pipeline in a
> > >     way that is transparent to userspace? Or must some userspace library
> > >     be responsible for setting up the RGA => EBC pipeline?
> > 
> > I'm very interested in this answer as well :)
> > 
> > I think the current consensus is that it's up to userspace to set this
> > up though.
> 
> Yeah I think v4l mem2mem device is the answer for these, and then
> userspace gets to set it all up.

I think the question wasn't really about where that driver should be,
but more about who gets to set it up, and if the kernel could have
some component to expose the formats supported by the converter, but
whenever a commit is being done pipe that to the v4l2 device before
doing a page flip.

We have a similar use-case for the RaspberryPi where the hardware
codec will produce a framebuffer format that isn't standard. That
format is understood by the display pipeline, and it can do
writeback.

However, some people are using a separate display (like a SPI display
supported by tinydrm) and we would still like to be able to output the
decoded frames there.

Is there some way we could plumb things to "route" that buffer through
the writeback engine to perform a format conversion before sending it
over to the SPI display automatically?

Maxime

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
  2022-05-31  8:58     ` Maxime Ripard
@ 2022-06-01 12:35       ` Daniel Vetter
  2022-06-08 14:48         ` Maxime Ripard
  0 siblings, 1 reply; 28+ messages in thread
From: Daniel Vetter @ 2022-06-01 12:35 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Samuel Holland, Heiko Stübner, Sandy Huang, dri-devel,
	linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, David Airlie, Geert Uytterhoeven,
	Krzysztof Kozlowski, Liang Chen, Maarten Lankhorst,
	Michael Riesch, Nicolas Frattaroli, Peter Geis, Rob Herring,
	Sam Ravnborg, Thierry Reding, Thomas Zimmermann, devicetree,
	linux-arm-kernel, linux-kernel

On Tue, May 31, 2022 at 10:58:35AM +0200, Maxime Ripard wrote:
> Hi Daniel,
> 
> Thanks for your feedback
> 
> On Wed, May 25, 2022 at 07:18:07PM +0200, Daniel Vetter wrote:
> > > > VBLANK Events and Asynchronous Commits
> > > > ======================================
> > > > When should the VBLANK event complete? When the pixels have been blitted
> > > > to the kernel's shadow buffer? When the first frame of the waveform is
> > > > sent to the panel? When the last frame is sent to the panel?
> > > > 
> > > > Currently, the driver is taking the first option, letting
> > > > drm_atomic_helper_fake_vblank() send the VBLANK event without waiting on
> > > > the refresh thread. This is the only way I was able to get good
> > > > performance with existing userspace.
> > > 
> > > I've been having the same kind of discussions in private lately, so I'm
> > > interested by the answer as well :)
> > > 
> > > It would be worth looking into the SPI/I2C panels for this, since it's
> > > basically the same case.
> > 
> > So it's maybe a bit misnamed and maybe kerneldocs aren't super clear (pls
> > help improve them), but there's two modes:
> > 
> > - drivers which have vblank, which might be somewhat variable (VRR) or
> >   become simulated (self-refresh panels), but otherwise is a more-or-less
> >   regular clock. For this case the atomic commit event must match the
> >   vblank events exactly (frame count and timestamp)
> 
> Part of my interrogation there is do we have any kind of expectation
> on whether or not, when we commit, the next vblank is going to be the
> one matching that commit or we're allowed to defer it by an arbitrary
> number of frames (provided that the frame count and timestamps are
> correct) ?

In general yes, but there's no guarantee. The only guarante we give for
drivers with vblank counters is that if you receive a vblank event (flip
complete or vblank event) for frame #n, then an immediate flip/atomic
ioctl call will display earliest for frame #n+1.

Also usually you should be able to hit #n+1, but even today with fun stuff
like self refresh panels getting out of self refresh mode might take a bit
more than a few frames, and so you might end up being late. But otoh if
you just do a page flip loop then on average (after the crtc is fully
resumed) you should be able to update at vrefresh rate exactly.

> > - drivers which don't have vblank at all, mostly these are i2c/spi panels
> >   or virtual hw and stuff like that. In this case the event simply happens
> >   when the driver is done with refresh/upload, and the frame count should
> >   be zero (since it's meaningless).
> > 
> > Unfortuantely the helper to dtrt has fake_vblank in it's name, maybe
> > should be renamed to no_vblank or so (the various flags that control it
> > are a bit better named).
> > 
> > Again the docs should explain it all, but maybe we should clarify them or
> > perhaps rename that helper to be more meaningful.
> > 
> > > > Blitting/Blending in Software
> > > > =============================
> > > > There are multiple layers to this topic (pun slightly intended):
> > > >  1) Today's userspace does not expect a grayscale framebuffer.
> > > >     Currently, the driver advertises XRGB8888 and converts to Y4
> > > >     in software. This seems to match other drivers (e.g. repaper).
> > > >
> > > >  2) Ignoring what userspace "wants", the closest existing format is
> > > >     DRM_FORMAT_R8. Geert sent a series[4] adding DRM_FORMAT_R1 through
> > > >     DRM_FORMAT_R4 (patch 9), which I believe are the "correct" formats
> > > >     to use.
> > > > 
> > > >  3) The RK356x SoCs have an "RGA" hardware block that can do the
> > > >     RGB-to-grayscale conversion, and also RGB-to-dithered-monochrome
> > > >     which is needed for animation/video. Currently this is exposed with
> > > >     a V4L2 platform driver. Can this be inserted into the pipeline in a
> > > >     way that is transparent to userspace? Or must some userspace library
> > > >     be responsible for setting up the RGA => EBC pipeline?
> > > 
> > > I'm very interested in this answer as well :)
> > > 
> > > I think the current consensus is that it's up to userspace to set this
> > > up though.
> > 
> > Yeah I think v4l mem2mem device is the answer for these, and then
> > userspace gets to set it all up.
> 
> I think the question wasn't really about where that driver should be,
> but more about who gets to set it up, and if the kernel could have
> some component to expose the formats supported by the converter, but
> whenever a commit is being done pipe that to the v4l2 device before
> doing a page flip.
> 
> We have a similar use-case for the RaspberryPi where the hardware
> codec will produce a framebuffer format that isn't standard. That
> format is understood by the display pipeline, and it can do
> writeback.
> 
> However, some people are using a separate display (like a SPI display
> supported by tinydrm) and we would still like to be able to output the
> decoded frames there.
> 
> Is there some way we could plumb things to "route" that buffer through
> the writeback engine to perform a format conversion before sending it
> over to the SPI display automatically?

Currently not transparently. Or at least no one has done that, and I'm not
sure that's really a great idea. With big gpus all that stuff is done with
separate command submission to the render side of things, and you can
fully pipeline all that with in/out-fences.

Doing that in the kms driver side in the kernel feels very wrong to me :-/
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
  2022-06-01 12:35       ` Daniel Vetter
@ 2022-06-08 14:48         ` Maxime Ripard
  2022-06-08 15:34           ` Daniel Vetter
  0 siblings, 1 reply; 28+ messages in thread
From: Maxime Ripard @ 2022-06-08 14:48 UTC (permalink / raw)
  To: Samuel Holland, Heiko Stübner, Sandy Huang, dri-devel,
	linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, David Airlie, Geert Uytterhoeven,
	Krzysztof Kozlowski, Liang Chen, Maarten Lankhorst,
	Michael Riesch, Nicolas Frattaroli, Peter Geis, Rob Herring,
	Sam Ravnborg, Thierry Reding, Thomas Zimmermann, devicetree,
	linux-arm-kernel, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 6561 bytes --]

On Wed, Jun 01, 2022 at 02:35:35PM +0200, Daniel Vetter wrote:
> On Tue, May 31, 2022 at 10:58:35AM +0200, Maxime Ripard wrote:
> > Hi Daniel,
> > 
> > Thanks for your feedback
> > 
> > On Wed, May 25, 2022 at 07:18:07PM +0200, Daniel Vetter wrote:
> > > > > VBLANK Events and Asynchronous Commits
> > > > > ======================================
> > > > > When should the VBLANK event complete? When the pixels have been blitted
> > > > > to the kernel's shadow buffer? When the first frame of the waveform is
> > > > > sent to the panel? When the last frame is sent to the panel?
> > > > > 
> > > > > Currently, the driver is taking the first option, letting
> > > > > drm_atomic_helper_fake_vblank() send the VBLANK event without waiting on
> > > > > the refresh thread. This is the only way I was able to get good
> > > > > performance with existing userspace.
> > > > 
> > > > I've been having the same kind of discussions in private lately, so I'm
> > > > interested by the answer as well :)
> > > > 
> > > > It would be worth looking into the SPI/I2C panels for this, since it's
> > > > basically the same case.
> > > 
> > > So it's maybe a bit misnamed and maybe kerneldocs aren't super clear (pls
> > > help improve them), but there's two modes:
> > > 
> > > - drivers which have vblank, which might be somewhat variable (VRR) or
> > >   become simulated (self-refresh panels), but otherwise is a more-or-less
> > >   regular clock. For this case the atomic commit event must match the
> > >   vblank events exactly (frame count and timestamp)
> > 
> > Part of my interrogation there is do we have any kind of expectation
> > on whether or not, when we commit, the next vblank is going to be the
> > one matching that commit or we're allowed to defer it by an arbitrary
> > number of frames (provided that the frame count and timestamps are
> > correct) ?
> 
> In general yes, but there's no guarantee. The only guarante we give for
> drivers with vblank counters is that if you receive a vblank event (flip
> complete or vblank event) for frame #n, then an immediate flip/atomic
> ioctl call will display earliest for frame #n+1.
> 
> Also usually you should be able to hit #n+1, but even today with fun stuff
> like self refresh panels getting out of self refresh mode might take a bit
> more than a few frames, and so you might end up being late. But otoh if
> you just do a page flip loop then on average (after the crtc is fully
> resumed) you should be able to update at vrefresh rate exactly.

I had more the next item in mind there: if we were to write something in
the kernel that would transparently behave like a full-blown KMS driver,
but would pipe the commits through a KMS writeback driver before sending
them to our SPI panel, we would always be at best two vblanks late.

So this would mean that userspace would do a page flip, get a first
vblank, but the actual vblank for that commit would be the next one (at
best), consistently.

> > > - drivers which don't have vblank at all, mostly these are i2c/spi panels
> > >   or virtual hw and stuff like that. In this case the event simply happens
> > >   when the driver is done with refresh/upload, and the frame count should
> > >   be zero (since it's meaningless).
> > > 
> > > Unfortuantely the helper to dtrt has fake_vblank in it's name, maybe
> > > should be renamed to no_vblank or so (the various flags that control it
> > > are a bit better named).
> > > 
> > > Again the docs should explain it all, but maybe we should clarify them or
> > > perhaps rename that helper to be more meaningful.
> > > 
> > > > > Blitting/Blending in Software
> > > > > =============================
> > > > > There are multiple layers to this topic (pun slightly intended):
> > > > >  1) Today's userspace does not expect a grayscale framebuffer.
> > > > >     Currently, the driver advertises XRGB8888 and converts to Y4
> > > > >     in software. This seems to match other drivers (e.g. repaper).
> > > > >
> > > > >  2) Ignoring what userspace "wants", the closest existing format is
> > > > >     DRM_FORMAT_R8. Geert sent a series[4] adding DRM_FORMAT_R1 through
> > > > >     DRM_FORMAT_R4 (patch 9), which I believe are the "correct" formats
> > > > >     to use.
> > > > > 
> > > > >  3) The RK356x SoCs have an "RGA" hardware block that can do the
> > > > >     RGB-to-grayscale conversion, and also RGB-to-dithered-monochrome
> > > > >     which is needed for animation/video. Currently this is exposed with
> > > > >     a V4L2 platform driver. Can this be inserted into the pipeline in a
> > > > >     way that is transparent to userspace? Or must some userspace library
> > > > >     be responsible for setting up the RGA => EBC pipeline?
> > > > 
> > > > I'm very interested in this answer as well :)
> > > > 
> > > > I think the current consensus is that it's up to userspace to set this
> > > > up though.
> > > 
> > > Yeah I think v4l mem2mem device is the answer for these, and then
> > > userspace gets to set it all up.
> > 
> > I think the question wasn't really about where that driver should be,
> > but more about who gets to set it up, and if the kernel could have
> > some component to expose the formats supported by the converter, but
> > whenever a commit is being done pipe that to the v4l2 device before
> > doing a page flip.
> > 
> > We have a similar use-case for the RaspberryPi where the hardware
> > codec will produce a framebuffer format that isn't standard. That
> > format is understood by the display pipeline, and it can do
> > writeback.
> > 
> > However, some people are using a separate display (like a SPI display
> > supported by tinydrm) and we would still like to be able to output the
> > decoded frames there.
> > 
> > Is there some way we could plumb things to "route" that buffer through
> > the writeback engine to perform a format conversion before sending it
> > over to the SPI display automatically?
> 
> Currently not transparently. Or at least no one has done that, and I'm not
> sure that's really a great idea. With big gpus all that stuff is done with
> separate command submission to the render side of things, and you can
> fully pipeline all that with in/out-fences.
> 
> Doing that in the kms driver side in the kernel feels very wrong to me :-/

So I guess what you're saying is that there's a close to 0% chance of it
being accepted if we were to come up with such an architecture?

Thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
  2022-06-08 14:48         ` Maxime Ripard
@ 2022-06-08 15:34           ` Daniel Vetter
  0 siblings, 0 replies; 28+ messages in thread
From: Daniel Vetter @ 2022-06-08 15:34 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Samuel Holland, Heiko Stübner, Sandy Huang, dri-devel,
	linux-rockchip, Alistair Francis, Ondřej Jirman,
	Andreas Kemnade, David Airlie, Geert Uytterhoeven,
	Krzysztof Kozlowski, Liang Chen, Maarten Lankhorst,
	Michael Riesch, Nicolas Frattaroli, Peter Geis, Rob Herring,
	Sam Ravnborg, Thierry Reding, Thomas Zimmermann, devicetree,
	linux-arm-kernel, linux-kernel

On Wed, Jun 08, 2022 at 04:48:47PM +0200, Maxime Ripard wrote:
> On Wed, Jun 01, 2022 at 02:35:35PM +0200, Daniel Vetter wrote:
> > On Tue, May 31, 2022 at 10:58:35AM +0200, Maxime Ripard wrote:
> > > Hi Daniel,
> > > 
> > > Thanks for your feedback
> > > 
> > > On Wed, May 25, 2022 at 07:18:07PM +0200, Daniel Vetter wrote:
> > > > > > VBLANK Events and Asynchronous Commits
> > > > > > ======================================
> > > > > > When should the VBLANK event complete? When the pixels have been blitted
> > > > > > to the kernel's shadow buffer? When the first frame of the waveform is
> > > > > > sent to the panel? When the last frame is sent to the panel?
> > > > > > 
> > > > > > Currently, the driver is taking the first option, letting
> > > > > > drm_atomic_helper_fake_vblank() send the VBLANK event without waiting og
> > > > > > the refresh thread. This is the only way I was able to get good
> > > > > > performance with existing userspace.
> > > > > 
> > > > > I've been having the same kind of discussions in private lately, so I'm
> > > > > interested by the answer as well :)
> > > > > 
> > > > > It would be worth looking into the SPI/I2C panels for this, since it's
> > > > > basically the same case.
> > > > 
> > > > So it's maybe a bit misnamed and maybe kerneldocs aren't super clear (pls
> > > > help improve them), but there's two modes:
> > > > 
> > > > - drivers which have vblank, which might be somewhat variable (VRR) or
> > > >   become simulated (self-refresh panels), but otherwise is a more-or-less
> > > >   regular clock. For this case the atomic commit event must match the
> > > >   vblank events exactly (frame count and timestamp)
> > > 
> > > Part of my interrogation there is do we have any kind of expectation
> > > on whether or not, when we commit, the next vblank is going to be the
> > > one matching that commit or we're allowed to defer it by an arbitrary
> > > number of frames (provided that the frame count and timestamps are
> > > correct) ?
> > 
> > In general yes, but there's no guarantee. The only guarante we give for
> > drivers with vblank counters is that if you receive a vblank event (flip
> > complete or vblank event) for frame #n, then an immediate flip/atomic
> > ioctl call will display earliest for frame #n+1.
> > 
> > Also usually you should be able to hit #n+1, but even today with fun stuff
> > like self refresh panels getting out of self refresh mode might take a bit
> > more than a few frames, and so you might end up being late. But otoh if
> > you just do a page flip loop then on average (after the crtc is fully
> > resumed) you should be able to update at vrefresh rate exactly.
> 
> I had more the next item in mind there: if we were to write something in
> the kernel that would transparently behave like a full-blown KMS driver,
> but would pipe the commits through a KMS writeback driver before sending
> them to our SPI panel, we would always be at best two vblanks late.
> 
> So this would mean that userspace would do a page flip, get a first
> vblank, but the actual vblank for that commit would be the next one (at
> best), consistently.
> 
> > > > - drivers which don't have vblank at all, mostly these are i2c/spi panels
> > > >   or virtual hw and stuff like that. In this case the event simply happens
> > > >   when the driver is done with refresh/upload, and the frame count should
> > > >   be zero (since it's meaningless).
> > > > 
> > > > Unfortuantely the helper to dtrt has fake_vblank in it's name, maybe
> > > > should be renamed to no_vblank or so (the various flags that control it
> > > > are a bit better named).
> > > > 
> > > > Again the docs should explain it all, but maybe we should clarify them or
> > > > perhaps rename that helper to be more meaningful.
> > > > 
> > > > > > Blitting/Blending in Software
> > > > > > =============================
> > > > > > There are multiple layers to this topic (pun slightly intended):
> > > > > >  1) Today's userspace does not expect a grayscale framebuffer.
> > > > > >     Currently, the driver advertises XRGB8888 and converts to Y4
> > > > > >     in software. This seems to match other drivers (e.g. repaper).
> > > > > >
> > > > > >  2) Ignoring what userspace "wants", the closest existing format is
> > > > > >     DRM_FORMAT_R8. Geert sent a series[4] adding DRM_FORMAT_R1 through
> > > > > >     DRM_FORMAT_R4 (patch 9), which I believe are the "correct" formats
> > > > > >     to use.
> > > > > > 
> > > > > >  3) The RK356x SoCs have an "RGA" hardware block that can do the
> > > > > >     RGB-to-grayscale conversion, and also RGB-to-dithered-monochrome
> > > > > >     which is needed for animation/video. Currently this is exposed with
> > > > > >     a V4L2 platform driver. Can this be inserted into the pipeline in a
> > > > > >     way that is transparent to userspace? Or must some userspace library
> > > > > >     be responsible for setting up the RGA => EBC pipeline?
> > > > > 
> > > > > I'm very interested in this answer as well :)
> > > > > 
> > > > > I think the current consensus is that it's up to userspace to set this
> > > > > up though.
> > > > 
> > > > Yeah I think v4l mem2mem device is the answer for these, and then
> > > > userspace gets to set it all up.
> > > 
> > > I think the question wasn't really about where that driver should be,
> > > but more about who gets to set it up, and if the kernel could have
> > > some component to expose the formats supported by the converter, but
> > > whenever a commit is being done pipe that to the v4l2 device before
> > > doing a page flip.
> > > 
> > > We have a similar use-case for the RaspberryPi where the hardware
> > > codec will produce a framebuffer format that isn't standard. That
> > > format is understood by the display pipeline, and it can do
> > > writeback.
> > > 
> > > However, some people are using a separate display (like a SPI display
> > > supported by tinydrm) and we would still like to be able to output the
> > > decoded frames there.
> > > 
> > > Is there some way we could plumb things to "route" that buffer through
> > > the writeback engine to perform a format conversion before sending it
> > > over to the SPI display automatically?
> > 
> > Currently not transparently. Or at least no one has done that, and I'm not
> > sure that's really a great idea. With big gpus all that stuff is done with
> > separate command submission to the render side of things, and you can
> > fully pipeline all that with in/out-fences.
> > 
> > Doing that in the kms driver side in the kernel feels very wrong to me :-/
> 
> So I guess what you're saying is that there's a close to 0% chance of it
> being accepted if we were to come up with such an architecture?

Yup.

I think the only exception is if you have a multi-region memory manager
using ttm (or hand-rolled, but please don't), where we first have to move
the buffer into the right region before it can be scanned out. And that's
generally done with a copy engine, for performance reasons.

But that copy engine is really just a very dumb (but fast!) memcpy, and
doesn't do any format conversion or stride/orientation changes like a
full-blown blitter engine (or mem2mem in v4l speak) can do.

So if it's really just memory management then I think it's fine, but
anything beyond that is a no imo.

Now for an overall full-featured stack we clearly need that, and it would
be great if there's some common userspace libraries for hosting such code.
But thus far all attempts have fallen short :-/ Which I guess is another
indicator that we really shouldn't try to solve this problem in a generic
fashion, and hence really shouldn't try to solve it with magic behind the
generic kms interface in the kernel.

For even more context I do think my old "why is 2d so hard" blogpost rant
still applies:

https://blog.ffwll.ch/2018/08/no-2d-in-drm.html

The "why no 2d api for the more limited problem of handling framebuffers"
is really just a small, but not any less complex, subset of that bigger
conundrum.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2022-06-08 15:34 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-13 22:19 [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 01/16] drm: Add a helper library for EPD drivers Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding Samuel Holland
2022-04-14  8:15   ` Andreas Kemnade
2022-04-15  3:00     ` Samuel Holland
2022-04-15 11:45       ` Andreas Kemnade
2022-04-13 22:19 ` [RFC PATCH 03/16] drm/rockchip: Add EBC platform driver skeleton Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 04/16] drm/rockchip: ebc: Add DRM " Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 05/16] drm/rockchip: ebc: Add CRTC mode setting Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 06/16] drm/rockchip: ebc: Add CRTC refresh thread Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 07/16] drm/rockchip: ebc: Add CRTC buffer management Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 08/16] drm/rockchip: ebc: Add LUT loading Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 09/16] drm/rockchip: ebc: Implement global refreshes Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 10/16] drm/rockchip: ebc: Implement partial refreshes Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 11/16] drm/rockchip: ebc: Enable diff mode for " Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 12/16] drm/rockchip: ebc: Add support for direct mode Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 13/16] drm/rockchip: ebc: Add a panel reflection option Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 14/16] drm/panel-simple: Add eInk ED103TC2 Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 15/16] arm64: dts: rockchip: rk356x: Add EBC node Samuel Holland
2022-04-13 22:19 ` [RFC PATCH 16/16] [DO NOT MERGE] arm64: dts: rockchip: pinenote: Enable EBC display pipeline Samuel Holland
2022-04-14  8:50 ` [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver Maxime Ripard
2022-05-25 17:18   ` Daniel Vetter
2022-05-31  8:58     ` Maxime Ripard
2022-06-01 12:35       ` Daniel Vetter
2022-06-08 14:48         ` Maxime Ripard
2022-06-08 15:34           ` Daniel Vetter
2022-04-21  6:43 ` Andreas Kemnade
2022-04-21  7:10   ` Nicolas Frattaroli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).