2022-04-14 09:46:31

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver

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/[email protected]/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/[email protected]/T/
[5]: https://lore.kernel.org/lkml/[email protected]/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


2022-04-14 11:05:13

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 10/16] drm/rockchip: ebc: Implement partial refreshes

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 <[email protected]>
---

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

2022-04-14 12:54:19

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 15/16] arm64: dts: rockchip: rk356x: Add EBC node

The RK356x SoCs contain an EBC. Add its node.

Signed-off-by: Samuel Holland <[email protected]>
---

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

2022-04-14 14:11:44

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 12/16] drm/rockchip: ebc: Add support for direct mode

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 <[email protected]>
---

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

2022-04-14 14:34:41

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 16/16] [DO NOT MERGE] arm64: dts: rockchip: pinenote: Enable EBC display pipeline

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

Signed-off-by: Samuel Holland <[email protected]>
---

.../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

2022-04-14 14:56:57

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 03/16] drm/rockchip: Add EBC platform driver skeleton

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 <[email protected]>
---

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 <[email protected]>
+ */
+
+#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 <[email protected]>");
+MODULE_DESCRIPTION("Rockchip EBC driver");
+MODULE_LICENSE("GPL");
--
2.35.1

2022-04-14 14:58:58

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 13/16] drm/rockchip: ebc: Add a panel reflection option

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 <[email protected]>
---

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

2022-04-14 15:20:28

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 06/16] drm/rockchip: ebc: Add CRTC refresh thread

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 <[email protected]>
---

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

2022-04-14 15:22:21

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 08/16] drm/rockchip: ebc: Add LUT loading

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 <[email protected]>
---

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

2022-04-14 15:24:14

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 11/16] drm/rockchip: ebc: Enable diff mode for partial refreshes

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 <[email protected]>
---

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

2022-04-14 23:37:10

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 09/16] drm/rockchip: ebc: Implement global refreshes

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 <[email protected]>
---

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

2022-04-15 07:21:04

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 04/16] drm/rockchip: ebc: Add DRM driver skeleton

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 <[email protected]>
---

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

2022-04-15 16:14:52

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 05/16] drm/rockchip: ebc: Add CRTC mode setting

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 <[email protected]>
---

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

2022-04-15 18:06:47

by Samuel Holland

[permalink] [raw]
Subject: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding

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 <[email protected]>
---

.../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 <[email protected]>
+
+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

2022-04-16 01:01:44

by Maxime Ripard

[permalink] [raw]
Subject: Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver

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


Attachments:
(No filename) (14.09 kB)
signature.asc (235.00 B)
Download all attachments

2022-04-22 18:12:05

by Nicolas Frattaroli

[permalink] [raw]
Subject: Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver

On Donnerstag, 21. April 2022 08:43:38 CEST Andreas Kemnade wrote:
> On Wed, 13 Apr 2022 17:19:00 -0500
> Samuel Holland <[email protected]> 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


2022-04-22 18:16:00

by Andreas Kemnade

[permalink] [raw]
Subject: Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver

On Wed, 13 Apr 2022 17:19:00 -0500
Samuel Holland <[email protected]> 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

2022-05-26 13:11:36

by Daniel Vetter

[permalink] [raw]
Subject: Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver

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

2022-06-01 15:00:36

by Daniel Vetter

[permalink] [raw]
Subject: Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver

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

2022-06-01 15:50:04

by Maxime Ripard

[permalink] [raw]
Subject: Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver

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

2022-06-08 15:23:53

by Maxime Ripard

[permalink] [raw]
Subject: Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver

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


Attachments:
(No filename) (6.41 kB)
signature.asc (235.00 B)
Download all attachments

2022-06-08 16:04:32

by Daniel Vetter

[permalink] [raw]
Subject: Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver

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