Hi Jan,
A couple of drive by comments:
Den 30.07.2019 15.48, skrev Jan Sebastian Götte:
> These ePaper displays are made in b/w, b/w/red and b/w/yellow variations
> by Good Display (www.e-paper-display.com) and sold on breakout boards by
> waveshare (waveshare.com).
>
> This driver was tested against a 2.7" b/w/r display (IL91874 driver) as
> well as 4.2" b/w/r display (IL0389 driver) but can probably be
> configured for other displays using only a single COG driver die (i.e.
> any model except for their largest 12.5" model).
>
> Through tinydrm this driver presents a standard XRGB8888 or RGB565
> framebuffer device to userspace and internally converts RGB colors to
> the epaper's colors. RGB565 support is provided for compatibility with
> anything expecting a small-size TFT LCD whose drivers often support
> RGB565.
>
> For userspace, the most notable difference between these epaper displays
> and more traditional TFT LCDs besides their limited color space is their
> enormously long refresh times-about 5s for a b/w display and >15s for
> b/w/r and b/w/y.
>
> This driver uses the display's "4-line SPI" interface (~CS, SCK, MOSI,
> BUSY) and does not support the bi-directional "3-line SPI" interface
> since target support for bidirectional SPI is going to be patchy. This
> means it can't read the display drivers factory-programmed ROM to
> automatically recognize the display's model, resolution or color
> configuration. This information must be provided from the device tree,
> either with a compatible="" section containing the display's model or
> through manual configuration of its various parameters. See the device
> tree doc file for details.
>
> We name this driver "gdepaper" instead of naming it after the driver
> IC's part number since Good Display seems to be using a large number of
> mutually compatible distinct variations of the same driver IC, named
> using several different numbering schemes. Our plan is to add this
> generic driver, and to provide support for individual chips' quirks as
> they are discovered.
>
> This driver uses four ioctls:
> * Force full display refresh, which may be needed since some epaper
> displays need to be refreshed periodically (every few minutes to
> hours) or the image degrades. This ioctl allows a user program to
> trigger this hardware refresh from the epapers internal frame buffer
> without need for a full redraw.
> * Get/set refresh params, which controls the display's waveform LUTs and
> voltage levels. This is necessary in case a user wants to tune paint/
> refresh performance to their particular content and temperature. In
> particular for acceptable-quality partial refresh on BWR/BWY panels
> this is essential.
> * Enable partial mode, which enables partial update and refresh on fb
> updates. By default this is disabled since careful LUT tuning is
> required for this to not look terrible. Also not all content is suited
> to this mode of operation.
>
> Tested-by: Jan Sebastian Götte <linux@xxxxxxxxx>
No need for this, it's assumed that you have tested your own driver ;-)
> Signed-off-by: Jan Sebastian Götte <linux@xxxxxxxxx>
> ---
> drivers/gpu/drm/tinydrm/gdepaper.c | 1262 +++++++++++++++++++++
> drivers/gpu/drm/tinydrm/gdepaper_models.h | 157 +++
> 2 files changed, 1419 insertions(+)
> create mode 100644 drivers/gpu/drm/tinydrm/gdepaper.c
> create mode 100644 drivers/gpu/drm/tinydrm/gdepaper_models.h
>
> diff --git a/drivers/gpu/drm/tinydrm/gdepaper.c b/drivers/gpu/drm/tinydrm/gdepaper.c
<snip>
> +static int gdepaper_spi_transfer_cstoggle(struct gdepaper *epap, u8 *data,
> + size_t len)
> +{
> + int i, ret = 0;
> +
> + for (i = 0; i < len; i++) {
> + ret = tinydrm_spi_transfer(epap->spi, epap->spi_speed_hz,
> + NULL, 8, &data[i], 1);
tinydrm_spi_transfer() is gone now, but you can use spi_write() here. It
will run the transfer at ->max_speed_hz. The reason for
tinydrm_spi_transfer() to have a speed argument, is that some display
controllers can run pixel transfers at a higher speed than the commands,
hence the need to override and run commands at a lower speed.
> + if (ret)
> + return ret;
> + udelay(1); /* FIXME necessary? */
> + }
> +
> + return ret;
> +}
<snip>
> +static void gdepaper_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
> +{
> + struct gdepaper *epap = drm_to_gdepaper(fb->dev);
> + struct device *dev = epap->drm.dev;
> + unsigned int w, h;
> + struct drm_rect rect_aligned;
> + int idx, ret = 0;
> +
> + dev_dbg(dev, "fbdirty\n");
> +
> + if (!epap->partial_update_en) {
> + *rect = (struct drm_rect) {
> + .x1 = 0,
> + .x2 = fb->width,
> + .y1 = 0,
> + .y2 = fb->height,
> + };
> + }
> + w = rect->x2 - rect->x1;
> + h = rect->y2 - rect->y1;
> +
> + if (!epap->enabled) {
> + dev_dbg(dev, "panel is disabled, returning\n");
> + return;
> + }
> +
> + if (!drm_dev_enter(fb->dev, &idx)) {
> + dev_dbg(dev, "can't acquire drm dev lock\n");
> + return;
> + }
> +
> + dev_dbg(dev, "Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id,
> + DRM_RECT_ARG(rect));
> +
> + ret = gdepaper_power_on(epap);
> + if (ret)
> + goto err_out;
> +
> + if (w == fb->width && h == fb->height) { /* full refresh */
> + /* black */
> + ret = gdepaper_txbuf_pack((u8 *)epap->tx_buf, fb, rect,
> + GDEP_CH_BLACK);
> +
> + dev_dbg(dev, "Sending %d byte full framebuf\n", ret);
> + ret = gdepaper_command(epap, GDEP_CMD_DATA_START_TX_COL1,
> + (u8 *)epap->tx_buf, ret);
> + if (ret)
> + goto err_out;
> +
> + /* red/yellow */
> + ret = gdepaper_txbuf_pack((u8 *)epap->tx_buf, fb, rect,
> + GDEP_CH_RED_YELLOW);
> +
> + ret = gdepaper_command(epap, GDEP_CMD_DATA_START_TX_COL2,
> + (u8 *)epap->tx_buf, ret);
> + if (ret)
> + goto err_out;
> +
> + ret = gdepaper_command(epap, GDEP_CMD_DATA_STOP, NULL, 0);
> + if (ret)
> + goto err_out;
> +
> + ret = gdepaper_command(epap, GDEP_CMD_DISP_RF, NULL, 0);
> + if (ret)
> + goto err_out;
> +
> + } else {
> + rect_aligned.x1 = rect->x1 & (~7U);
This can also be written as: round_down(rect->x1, 8);
> + rect_aligned.y1 = rect->y1;
> + rect_aligned.y2 = rect->y2;
> + rect_aligned.x2 = rect_aligned.x1 +
> + ((rect->x2 - rect_aligned.x1 + 7) & (~7U));
Are you rounding up here?
If so: rect_aligned.x2 = round_up(rect->x2, 8);
> +
> + /* black */
> + ret = gdepaper_txbuf_pack((u8 *)epap->tx_buf, fb, &rect_aligned,
> + GDEP_CH_BLACK);
> + dev_dbg(dev, "Sending %d byte partial framebuf\n", ret);
> + if (ret < 0)
> + goto err_out;
> + ret = gdepaper_partial_cmd(epap, &rect_aligned,
> + GDEP_CMD_PD_START_TX_COL1, (u8 *)epap->tx_buf, ret);
> + if (ret)
> + goto err_out;
> +
> + /* red/yellow */
> + ret = gdepaper_txbuf_pack((u8 *)epap->tx_buf, fb, &rect_aligned,
> + GDEP_CH_RED_YELLOW);
> + if (ret < 0)
> + goto err_out;
> + ret = gdepaper_partial_cmd(epap, &rect_aligned,
> + GDEP_CMD_PD_START_TX_COL2, (u8 *)epap->tx_buf,
> + ret);
> + if (ret)
> + goto err_out;
> + ret = gdepaper_command(epap, GDEP_CMD_DATA_STOP, NULL, 0);
> + if (ret)
> + goto err_out;
> + ret = gdepaper_partial_cmd(epap, rect, GDEP_CMD_PART_DISP_RF,
> + NULL, 0);
> + if (ret)
> + goto err_out;
> + }
> +
> + if (gdepaper_wait_busy(epap)) {
> + dev_err(dev, "Timeout on partial refresh cmd\n");
> + goto err_out;
> + }
> +
> + ret = gdepaper_power_off(epap);
> + if (ret)
> + goto err_out;
> +
> + drm_dev_exit(idx);
> + return;
> +
> +err_out:
> + /* Try to power off anyway */
> + gdepaper_power_off(epap);
> +
> + dev_err(fb->dev->dev, "Failed to update display %d\n", ret);
> + drm_dev_exit(idx);
> +}
<snip>
> +static int gdepaper_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct device_node *np = dev->of_node;
> + const struct of_device_id *of_id;
> + struct drm_device *drm;
> + struct drm_display_mode *mode;
> + struct gdepaper *epap;
> + const struct gdepaper_type_descriptor *type_desc;
> + int ret;
> + size_t bufsize;
> +
> + of_id = of_match_node(gdepaper_of_match, np);
> + if (WARN_ON(of_id == NULL)) {
> + dev_warn(dev, "dt node didn't match, aborting probe\n");
> + return -EINVAL;
> + }
> + type_desc = of_id->data;
> +
> + dev_dbg(dev, "Probing gdepaper module\n");
> + epap = kzalloc(sizeof(*epap), GFP_KERNEL);
> + if (!epap)
> + return -ENOMEM;
> +
> + epap->enabled = false;
> + mutex_init(&epap->cmdlock);
> + epap->tx_buf = NULL;
> + epap->spi = spi;
> +
> + drm = &epap->drm;
> + ret = devm_drm_dev_init(dev, drm, &gdepaper_driver);
> + if (ret) {
> + dev_warn(dev, "failed to init drm dev\n");
> + goto err_free;
> + }
> +
> + drm_mode_config_init(drm);
> +
> + epap->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(epap->reset)) {
> + dev_err(dev, "Failed to get reset GPIO\n");
> + ret = PTR_ERR(epap->reset);
> + goto err_free;
> + }
> +
> + epap->busy = devm_gpiod_get(dev, "busy", GPIOD_IN);
> + if (IS_ERR(epap->busy)) {
> + dev_err(dev, "Failed to get busy GPIO\n");
> + ret = PTR_ERR(epap->busy);
> + goto err_free;
> + }
> +
> + epap->dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
> + if (IS_ERR(epap->dc)) {
> + dev_err(dev, "Failed to get dc GPIO\n");
> + ret = PTR_ERR(epap->dc);
> + goto err_free;
> + }
> +
> + epap->spi_speed_hz = 2000000;
> + epap->pll_div = 1;
> + epap->framerate_mHz = 81850;
> + epap->rfp.vg_lv = GDEP_PWR_VGHL_16V;
> + epap->rfp.vcom_sel = 0;
> + epap->rfp.vdh_bw_mv = 11000; /* drive high level, b/w pixel */
> + epap->rfp.vdh_col_mv = 4200; /* drive high level, red/yellow pixel */
> + epap->rfp.vdl_mv = -11000; /* drive low level */
> + epap->rfp.border_data_sel = 2; /* "vbd" */
> + epap->rfp.data_polarity = 0; /* "ddx" */
> + epap->rfp.vcom_dc_mv = -1000;
> + epap->rfp.vcom_data_ivl_hsync = 10; /* hsync periods */
> + epap->rfp.use_otp_luts_flag = 1;
> + epap->ss_param[0] = 0x07;
> + epap->ss_param[1] = 0x07;
> + epap->ss_param[2] = 0x17;
> + epap->controller_res = GDEP_CTRL_RES_320X300;
> +
> + ret = gdepaper_of_read_luts(epap, np, dev);
> + if (ret) {
> + dev_warn(dev, "can't read LUTs from dt\n");
> + goto err_free;
> + }
> +
> + of_property_read_u32(np, "controller-resolution",
> + &epap->controller_res);
> + of_property_read_u32(np, "spi-speed-hz", &epap->spi_speed_hz);
> + epap->partial_update_en = of_property_read_bool(np, "partial-update");
> + ret = of_property_read_u32(np, "colors", &epap->display_colors);
> + if (ret == -EINVAL) {
> + if (type_desc) {
> + epap->display_colors = type_desc->colors;
> +
> + } else {
> + dev_err(dev, "colors must be set in dt\n");
> + ret = -EINVAL;
> + goto err_free;
> + }
> + } else if (ret) {
> + dev_err(dev, "Invalid dt colors property\n");
> + goto err_free;
> + }
> + if (epap->display_colors < 0 ||
> + epap->display_colors >= GDEPAPER_COL_END) {
> + dev_err(dev, "invalid colors value\n");
> + ret = -EINVAL;
> + goto err_free;
> + }
> + epap->mirror_x = of_property_read_bool(np, "mirror-x");
> + epap->mirror_y = of_property_read_bool(np, "mirror-y");
> + of_property_read_u32(np, "pll-div", &epap->pll_div);
> + of_property_read_u32(np, "fps-millihertz", &epap->framerate_mHz);
> + of_property_read_u32(np, "vghl-level", &epap->rfp.vg_lv);
> + epap->vds_en = !of_property_read_bool(np, "vds-external");
> + epap->vdg_en = !of_property_read_bool(np, "vdg-external");
> + of_property_read_u32(np, "vcom", &epap->rfp.vcom_sel);
> + of_property_read_u32(np, "vdh-bw-millivolts", &epap->rfp.vdh_bw_mv);
> + of_property_read_u32(np, "vdh-color-millivolts", &epap->rfp.vdh_col_mv);
> + of_property_read_u32(np, "vdl-millivolts", &epap->rfp.vdl_mv);
> + of_property_read_u32(np, "border-data", &epap->rfp.border_data_sel);
> + of_property_read_u32(np, "data-polarity", &epap->rfp.data_polarity);
> + ret = of_property_read_u8_array(np, "boost-soft-start",
> + (u8 *)&epap->ss_param, sizeof(epap->ss_param));
> + if (ret && ret != -EINVAL)
> + dev_err(dev, "invalid boost-soft-start value, ignoring\n");
> + of_property_read_u32(np, "vcom-data-interval-periods",
> + &epap->rfp.vcom_data_ivl_hsync);
Why do you need these DT properties when you define compatibles for all
the panels, can't you include these settings in the type descriptor?
> +
> + /* Accept both positive and negative notation */
> + if (epap->rfp.vdl_mv < 0)
> + epap->rfp.vdl_mv = -epap->rfp.vdl_mv;
> + if (epap->rfp.vcom_dc_mv < 0)
> + epap->rfp.vcom_dc_mv = -epap->rfp.vcom_dc_mv;
> +
> + /* (from mipi-dbi.c:)
> + * Even though it's not the SPI device that does DMA (the master does),
> + * the dma mask is necessary for the dma_alloc_wc() in
> + * drm_gem_cma_create(). The dma_addr returned will be a physical
> + * address which might be different from the bus address, but this is
> + * not a problem since the address will not be used.
> + * The virtual address is used in the transfer and the SPI core
> + * re-maps it on the SPI master device using the DMA streaming API
> + * (spi_map_buf()).
> + */
> + if (!dev->coherent_dma_mask) {
> + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> + if (ret) {
> + dev_warn(dev, "Failed to set dma mask %d\n", ret);
> + goto err_free;
> + }
> + }
> +
> + mode = gdepaper_of_read_mode(type_desc, np, dev);
> + if (IS_ERR(mode)) {
> + dev_warn(dev, "Failed to read mode: %ld\n", PTR_ERR(mode));
> + ret = PTR_ERR(mode);
> + goto err_free;
> + }
> +
> + /* 8 pixels per byte, bit-packed */
> + bufsize = (mode->vdisplay * mode->hdisplay + 7)/8;
DIV_ROUND_UP(mode->vdisplay * mode->hdisplay, 8)
> + epap->tx_buf = devm_kmalloc(drm->dev, bufsize, GFP_KERNEL);
> + if (!epap->tx_buf) {
> + ret = -ENOMEM;
> + goto err_free;
> + }
> +
> + /* TODO rotation support? */
> + ret = tinydrm_display_pipe_init(drm, &epap->pipe, &gdepaper_pipe_funcs,
> + DRM_MODE_CONNECTOR_VIRTUAL,
> + gdepaper_formats,
> + ARRAY_SIZE(gdepaper_formats), mode, 0);
tinydrm_display_pipe_init() is gone now, here's how I replaced it in the
other e-ink driver:
drm/tinydrm/repaper: Don't use tinydrm_display_pipe_init()
https://cgit.freedesktop.org/drm/drm-misc/commit?id=1321db837549a0ff9dc2c95ff76c46770f7f02a1
Noralf.
> + if (ret) {
> + dev_warn(dev, "Failed to initialize display pipe: %d\n", ret);
> + goto err_free;
> + }
> +
> + drm->mode_config.funcs = &gdepaper_dbi_mode_config_funcs;
> + drm->mode_config.preferred_depth = 32;
> + drm_plane_enable_fb_damage_clips(&epap->pipe.plane);
> + drm_mode_config_reset(drm);
> +
> + ret = drm_dev_register(drm, 0);
> + if (ret) {
> + dev_warn(dev, "Failed to register drm device: %d\n", ret);
> + goto err_free;
> + }
> +
> + spi_set_drvdata(spi, drm);
> + drm_fbdev_generic_setup(drm, 0);
> +
> + dev_dbg(dev, "Probed gdepaper module\n");
> + return 0;
> +err_free:
> + kfree(epap);
> + return ret;
> +}
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
