On Mon, Sep 02, 2019 at 11:06:33AM +0200, Linus Walleij wrote: > The Sony ACX424AKP is a command/videomode DSI panel for > mobile devices. It is used on the ST-Ericsson HREF520 > reference design. We support video mode by default, but > it is possible to switch the panel into command mode > by using the bool property "dsi-command-mode". > > Signed-off-by: Linus Walleij <[email protected]> > --- > drivers/gpu/drm/panel/Kconfig | 7 + > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-sony-acx424akp.c | 530 +++++++++++++++++++ > 3 files changed, 538 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-sony-acx424akp.c > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index d9d931aa6e26..435388a874e2 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -275,6 +275,13 @@ config DRM_PANEL_SITRONIX_ST7789V > Say Y here if you want to enable support for the Sitronix > ST7789V controller for 240x320 LCD panels > > +config DRM_PANEL_SONY_ACX424AKP > + tristate "Sony ACX424AKP DSI command mode panel" > + depends on OF > + depends on DRM_MIPI_DSI > + depends on BACKLIGHT_CLASS_DEVICE > + select VIDEOMODE_HELPERS > + > config DRM_PANEL_TPO_TPG110 > tristate "TPO TPG 800x400 panel" > depends on OF && SPI && GPIOLIB > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile > index fb0cb3aaa9e6..9ed4d853267e 100644 > --- a/drivers/gpu/drm/panel/Makefile > +++ b/drivers/gpu/drm/panel/Makefile > @@ -28,5 +28,6 @@ obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += > panel-sharp-lq101r1sx01.o > obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o > obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += panel-sitronix-st7701.o > obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o > +obj-$(CONFIG_DRM_PANEL_SONY_ACX424AKP) += panel-sony-acx424akp.o > obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o > obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o > diff --git a/drivers/gpu/drm/panel/panel-sony-acx424akp.c > b/drivers/gpu/drm/panel/panel-sony-acx424akp.c > new file mode 100644 > index 000000000000..807560d2a8ec > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-sony-acx424akp.c > @@ -0,0 +1,530 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * MIPI-DSI Sony ACX424AKP panel driver. This is a 480x864 > + * AMOLED panel with a command-only DSI interface. > + * > + * Copyright (C) Linaro Ltd. 2019 > + * Author: Linus Walleij > + * Based on code and know-how from Marcus Lorentzon > + * Copyright (C) ST-Ericsson SA 2010 > + */ > + > +#include <drm/drm_modes.h> > +#include <drm/drm_mipi_dsi.h> > +#include <drm/drm_panel.h> > +#include <drm/drm_print.h> > +#include <video/mipi_display.h> > + > +#include <linux/gpio/consumer.h> > +#include <linux/regulator/consumer.h> > +#include <linux/delay.h> > +#include <linux/of.h> > +#include <linux/module.h> > +#include <linux/backlight.h> > + > +/* FIXME: move to <video/mipi_display.h> ? */ > +#define MIPI_DSI_DCS_SET_MDDI 0xAE > + > +#define DISPLAY_SONY_ACX424AKP_ID1 0x1b81 > +#define DISPLAY_SONY_ACX424AKP_ID2 0x1a81 > +#define DISPLAY_SONY_ACX424AKP_ID3 0x0080 > + > +struct acx424akp { > + struct device *dev; > + struct drm_panel panel; > + struct backlight_device *bl; > + struct regulator *supply; > + struct gpio_desc *reset_gpio; > + u16 id; > + bool video_mode; > +}; > + > +static const struct drm_display_mode sony_acx424akp_vid_mode = { > + .clock = 330000, > + .hdisplay = 480, > + .hsync_start = 480 + 15, > + .hsync_end = 480 + 15 + 0, > + .htotal = 480 + 15 + 0 + 15, > + .vdisplay = 864, > + .vsync_start = 864 + 14, > + .vsync_end = 864 + 14 + 1, > + .vtotal = 864 + 14 + 1 + 11, > + .vrefresh = 60, > + .width_mm = 48, > + .height_mm = 84, > + .flags = DRM_MODE_FLAG_PVSYNC, > +}; > + > +/* > + * The timings are not very helpful as the display is used in > + * command mode. > + */ > +static const struct drm_display_mode sony_acx424akp_cmd_mode = { > + /* HS clock, (htotal*vtotal*vrefresh)/1000 */ > + .clock = 420160, > + .hdisplay = 480, > + .hsync_start = 480 + 154, > + .hsync_end = 480 + 154 + 16, > + .htotal = 480 + 154 + 16 + 32, > + .vdisplay = 864, > + .vsync_start = 864 + 1, > + .vsync_end = 864 + 1 + 1, > + .vtotal = 864 + 1 + 1 + 1, > + /* > + * This depends on the clocking HS vs LP rate, this value > + * is calculated as: > + * vrefresh = (clock * 1000) / (htotal*vtotal) > + */ > + .vrefresh = 816,
That's a bit odd. My understanding is that command mode can be done in
continuous mode or in non-continuous mode. In continuous mode you would
typically achieve a similar refresh rate as in video mode. For non-
continuous mode you basically have a variable refresh rate.
For continuous mode you probably want 60 Hz here and for VRR there's
probably no sensible value to set this to. In the latter case, I don't
think it makes sense to set anything arbitrary like you have above.
Perhaps better to just set it to 0 as a way of signalling that this is
actually dependent on when the display hardware sends a new frame?
Have you actually run this is command mode? If so, what's the actual
refresh rate? Do you do on-demand updates or do you run in continuous
mode?
> + .width_mm = 48,
> + .height_mm = 84,
> +};
> +
> +static inline struct acx424akp *panel_to_acx424akp(struct drm_panel *panel)
> +{
> + return container_of(panel, struct acx424akp, panel);
> +}
> +
> +#define FOSC 20 /* 20Mhz */
> +#define SCALE_FACTOR_NS_DIV_MHZ 1000
> +
> +static int acx424akp_set_brightness(struct backlight_device *bl)
> +{
> + struct acx424akp *acx = bl_get_data(bl);
> + struct mipi_dsi_device *dsi = to_mipi_dsi_device(acx->dev);
> + int period_ns = 1023;
> + int duty_ns = bl->props.brightness;
> + u8 pwm_ratio;
> + u8 pwm_div;
> + u8 par;
> + int ret;
> +
> + /* Calculate the PWM duty cycle in n/256's */
> + pwm_ratio = max(((duty_ns * 256) / period_ns) - 1, 1);
> + pwm_div = max(1,
> + ((FOSC * period_ns) / 256) /
> + SCALE_FACTOR_NS_DIV_MHZ);
> +
> + /* Set up PWM dutycycle ONE byte (differs from the standard) */
> + DRM_DEV_DEBUG(acx->dev, "calculated duty cycle %02x\n", pwm_ratio);
> + ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
> + &pwm_ratio, 1);
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev,
> + "failed to set display PWM ratio (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + /*
> + * Sequence to write PWMDIV:
> + * address data
> + * 0xF3 0xAA CMD2 Unlock
> + * 0x00 0x01 Enter CMD2 page 0
> + * 0X7D 0x01 No reload MTP of CMD2 P1
> + * 0x22 PWMDIV
> + * 0x7F 0xAA CMD2 page 1 lock
> + */
> + par = 0xaa;
> + ret = mipi_dsi_dcs_write(dsi, 0xf3, &par, 1);
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev,
> + "failed to unlock CMD 2 (%d)\n",
> + ret);
> + return ret;
> + }
> + par = 0x01;
> + ret = mipi_dsi_dcs_write(dsi, 0x00, &par, 1);
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev,
> + "failed to enter page 1 (%d)\n",
> + ret);
> + return ret;
> + }
> + par = 0x01;
> + ret = mipi_dsi_dcs_write(dsi, 0x7d, &par, 1);
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev,
> + "failed to disable MTP reload (%d)\n",
> + ret);
> + return ret;
> + }
> + ret = mipi_dsi_dcs_write(dsi, 0x22, &pwm_div, 1);
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev,
> + "failed to set PWM divisor (%d)\n",
> + ret);
> + return ret;
> + }
> + par = 0xaa;
> + ret = mipi_dsi_dcs_write(dsi, 0x7f, &par, 1);
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev,
> + "failed to lock CMD 2 (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + /* Enable backlight */
> + par = 0x24;
> + ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
> + &par, 1);
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev,
> + "failed to enable display backlight (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct backlight_ops acx424akp_bl_ops = {
> + .update_status = acx424akp_set_brightness,
> +};
> +
> +static int acx424akp_read_id(struct acx424akp *acx)
> +{
> + struct mipi_dsi_device *dsi = to_mipi_dsi_device(acx->dev);
> + u8 id1, id2, id3;
> + u16 val;
> + size_t len;
> + int ret;
> +
> + len = 1;
> +
> + ret = mipi_dsi_dcs_read(dsi, 0xda, &id1, len);
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev, "could not read device ID byte 1\n");
> + return ret;
> + }
> + ret = mipi_dsi_dcs_read(dsi, 0xdb, &id2, len);
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev, "could not read device ID byte 2\n");
> + return ret;
> + }
> + ret = mipi_dsi_dcs_read(dsi, 0xdc, &id3, len);
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev, "could not read device ID byte 3\n");
> + return ret;
> + }
> +
> + val = (id3 << 8) | id2;
Don't you want to OR in id1 here as well? Seems a bit odd to read it but
then not use it.
> +
> + switch (val) {
> + case DISPLAY_SONY_ACX424AKP_ID1:
> + case DISPLAY_SONY_ACX424AKP_ID2:
> + case DISPLAY_SONY_ACX424AKP_ID3:
> + DRM_DEV_INFO(acx->dev, "panel ID: %04x\n", val);
> + break;
> + default:
> + DRM_DEV_ERROR(acx->dev, "unknown panel ID: %04x\n", val);
> + break;
> + };
> + acx->id = val;
> +
> + return 0;
> +}
> +
> +static int acx424akp_power_on(struct acx424akp *acx)
> +{
> + int ret;
> +
> + ret = regulator_enable(acx->supply);
> + if (ret) {
> + DRM_DEV_ERROR(acx->dev, "failed to enable supply (%d)\n", ret);
> + return ret;
> + }
> +
> + /* Assert RESET */
> + gpiod_set_value_cansleep(acx->reset_gpio, 1);
> + udelay(20);
> + /* De-assert RESET */
> + gpiod_set_value_cansleep(acx->reset_gpio, 0);
> + msleep(11);
> +
> + return 0;
> +}
> +
> +static int acx424akp_prepare(struct drm_panel *panel)
> +{
> + struct acx424akp *acx = panel_to_acx424akp(panel);
> + struct mipi_dsi_device *dsi = to_mipi_dsi_device(acx->dev);
> + const u8 mddi = 3;
> + int ret;
> +
> + ret = acx424akp_power_on(acx);
> + if (ret)
> + return ret;
> +
> + /* Enabe tearing mode: send TE (tearing effect) at VBLANK */
> + ret = mipi_dsi_dcs_set_tear_on(dsi,
> + MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> + if (ret) {
> + DRM_DEV_ERROR(acx->dev, "failed to enable vblank TE (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + /* Set MDDI - doesn't seem to work? */
> + ret = mipi_dsi_dcs_write(dsi, MIPI_DSI_DCS_SET_MDDI,
> + &mddi, sizeof(mddi));
> + if (ret < 0) {
> + DRM_DEV_ERROR(acx->dev, "failed to set MDDI (%d)\n", ret);
> + return ret;
> + }
> +
> + acx->bl->props.power = FB_BLANK_NORMAL;
> +
> + return 0;
> +}
> +
> +static void acx424akp_power_off(struct acx424akp *acx)
> +{
> + /* Assert RESET */
> + gpiod_set_value_cansleep(acx->reset_gpio, 1);
> + msleep(11);
> +
> + regulator_disable(acx->supply);
> +}
> +
> +static int acx424akp_unprepare(struct drm_panel *panel)
> +{
> + struct acx424akp *acx = panel_to_acx424akp(panel);
> +
> + acx424akp_power_off(acx);
> + acx->bl->props.power = FB_BLANK_POWERDOWN;
> +
> + return 0;
> +}
> +
> +static int acx424akp_enable(struct drm_panel *panel)
> +{
> + struct acx424akp *acx = panel_to_acx424akp(panel);
> + struct mipi_dsi_device *dsi = to_mipi_dsi_device(acx->dev);
> + int ret;
> +
> + /* Exit sleep mode */
> + ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> + if (ret) {
> + DRM_DEV_ERROR(acx->dev, "failed to exit sleep mode (%d)\n",
> + ret);
> + return ret;
> + }
> + msleep(140);
> +
> + ret = mipi_dsi_dcs_set_display_on(dsi);
> + if (ret) {
> + DRM_DEV_ERROR(acx->dev, "failed to turn display on (%d)\n",
> + ret);
> + return ret;
> + }
> + if (acx->video_mode) {
> + /* In video mode turn peripheral on */
> + ret = mipi_dsi_turn_on_peripheral(dsi);
> + if (ret) {
> + dev_err(acx->dev, "failed to turn on peripheral\n");
> + return ret;
> + }
> + }
> +
> + acx->bl->props.power = FB_BLANK_UNBLANK;
> +
> + return 0;
> +}
> +
> +static int acx424akp_disable(struct drm_panel *panel)
> +{
> + struct acx424akp *acx = panel_to_acx424akp(panel);
> + struct mipi_dsi_device *dsi = to_mipi_dsi_device(acx->dev);
> + u8 par;
> + int ret;
> +
> + /* Disable backlight */
> + par = 0x00;
> + ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
> + &par, 1);
> + if (ret) {
> + DRM_DEV_ERROR(acx->dev,
> + "failed to disable display backlight (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + ret = mipi_dsi_dcs_set_display_off(dsi);
> + if (ret) {
> + DRM_DEV_ERROR(acx->dev, "failed to turn display off (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + /* Enter sleep mode */
> + ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> + if (ret) {
> + DRM_DEV_ERROR(acx->dev, "failed to enter sleep mode (%d)\n",
> + ret);
> + return ret;
> + }
> + msleep(85);
> +
> + acx->bl->props.power = FB_BLANK_NORMAL;
> +
> + return 0;
> +}
> +
> +static int acx424akp_get_modes(struct drm_panel *panel)
> +{
> + struct acx424akp *acx = panel_to_acx424akp(panel);
> + struct drm_connector *connector = panel->connector;
> + struct drm_display_mode *mode;
> +
> + if (acx->video_mode)
> + mode = drm_mode_duplicate(panel->drm,
> + &sony_acx424akp_vid_mode);
> + else
> + mode = drm_mode_duplicate(panel->drm,
> + &sony_acx424akp_cmd_mode);
> + if (!mode) {
> + DRM_ERROR("bad mode or failed to add mode\n");
> + return -EINVAL;
> + }
> + drm_mode_set_name(mode);
> + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +
> + connector->display_info.width_mm = mode->width_mm;
> + connector->display_info.height_mm = mode->height_mm;
> +
> + drm_mode_probed_add(connector, mode);
> +
> + return 1; /* Number of modes */
> +}
> +
> +static const struct drm_panel_funcs acx424akp_drm_funcs = {
> + .disable = acx424akp_disable,
> + .unprepare = acx424akp_unprepare,
> + .prepare = acx424akp_prepare,
> + .enable = acx424akp_enable,
> + .get_modes = acx424akp_get_modes,
> +};
> +
> +static int acx424akp_probe(struct mipi_dsi_device *dsi)
> +{
> + struct device *dev = &dsi->dev;
> + struct acx424akp *acx;
> + int ret;
> + int i;
unsigned int?
> +
> + acx = devm_kzalloc(dev, sizeof(struct acx424akp), GFP_KERNEL);
> + if (!acx)
> + return -ENOMEM;
> + acx->video_mode = !of_property_read_bool(dev->of_node,
> + "dsi-command-mode");
> +
> + mipi_dsi_set_drvdata(dsi, acx);
> + acx->dev = dev;
> +
> + dsi->lanes = 2;
> + dsi->format = MIPI_DSI_FMT_RGB888;
> + dsi->hs_rate = 420160000;
> + dsi->lp_rate = 19200000;
> +
> + if (acx->video_mode)
> + dsi->mode_flags =
> + MIPI_DSI_MODE_VIDEO |
> + MIPI_DSI_MODE_VIDEO_BURST |
> + MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> + else
> + dsi->mode_flags =
> + MIPI_DSI_CLOCK_NON_CONTINUOUS |
> + MIPI_DSI_MODE_EOT_PACKET;
> +
> +
Gratuituous blank line.
> + acx->supply = devm_regulator_get(dev, "vddi");
> + if (IS_ERR(acx->supply))
> + return PTR_ERR(acx->supply);
> +
> + /* This asserts RESET by default */
> + acx->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(acx->reset_gpio)) {
> + ret = PTR_ERR(acx->reset_gpio);
> + if (ret != -EPROBE_DEFER)
> + DRM_DEV_ERROR(dev, "failed to request GPIO (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + drm_panel_init(&acx->panel);
> + acx->panel.dev = dev;
> + acx->panel.funcs = &acx424akp_drm_funcs;
> +
> + acx->bl = devm_backlight_device_register(dev, "acx424akp", dev, acx,
> + &acx424akp_bl_ops, NULL);
> + if (IS_ERR(acx->bl)) {
> + DRM_DEV_ERROR(dev, "failed to register backlight device\n");
> + return PTR_ERR(acx->bl);
> + }
> + acx->bl->props.max_brightness = 1023;
> + acx->bl->props.brightness = 512;
> + acx->bl->props.power = FB_BLANK_POWERDOWN;
> +
> + ret = drm_panel_add(&acx->panel);
> + if (ret < 0)
> + return ret;
> +
> + ret = acx424akp_power_on(acx);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "failed to power up display\n");
> + return ret;
> + }
> +
> + /* Read device ID */
> + i = 0;
> + do {
> + ret = acx424akp_read_id(acx);
> + if (ret)
> + continue;
> + } while (ret && i++ < 5);
Seems rather redundant to have both an "if (ret) continue;" and the ret
check in the while's condition. A more idiomatic way to write this would
be:
for (i = 0; i < 5; i++) {
ret = acx424akp_read_id(acx);
if (!ret)
break;
}
Thierry
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
