[PATCH v4] drm/panel: db7430: Add driver for Samsung DB7430

[Noralf Trønnes]

Den 10.06.2021 15.25, skrev Linus Walleij:
> This adds a new driver for the Samsung DB7430 DPI display
> controller as controlled over SPI.
> 
> Right now the only panel product we know that is using this
> display controller is the LMS397KF04 but there may be more.
> 
> This is the first regular panel driver making use of the
> MIPI DBI helper library. The DBI "device" portions can not
> be used because that code assumes the use of a single
> regulator and specific timings around the reset pulse that
> do not match the DB7430 datasheet.
> 
> Cc: Noralf Trønnes <noralf at tronnes.org>
> Cc: Paul Cercueil <paul at crapouillou.net>
> Cc: Doug Anderson <dianders at chromium.org>
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
> ---
> ChangeLog v3->v4:
> - Managed to make use of the in-kernel DBI support to
>   conjure and send 9bit DBI SPI messages.

When I moved the DBI code out of tinydrm I split it up into an interface
part and a framebuffer part so panel drivers could use the interface
part like you have done here.

> - This cuts out a bit of overhead.
> - Deeper integration with the DBI library is not done, as
>   assumes too much about the device, such as being used
>   as a stand-alone framebuffer (this device is not).

> diff --git a/drivers/gpu/drm/panel/panel-samsung-db7430.c b/drivers/gpu/drm/panel/panel-samsung-db7430.c
> new file mode 100644
> index 000000000000..c11b3da65896
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-db7430.c

> +/**
> + * struct db7430 - state container for a panel controlled by the DB7430
> + * controller
> + */
> +struct db7430 {
> +	/** @dev: the container device */
> +	struct device *dev;
> +	/** @dbi: the DBI bus abstraction handle */
> +	struct mipi_dbi dbi;
> +	/** @panel: the DRM panel instance for this device */
> +	struct drm_panel panel;
> +	/** @width: the width of this panel in mm */
> +	u32 width;
> +	/** @height: the height of this panel in mm */
> +	u32 height;
> +	/** @reset: reset GPIO line */
> +	struct gpio_desc *reset;

You can use dbi->reset.

> +	/** @regulators: VCCIO and VIO supply regulators */
> +	struct regulator_bulk_data regulators[2];
> +};

> +static int db7430_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct db7430 *db;
> +	int ret;
> +
> +	db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
> +	if (!db)
> +		return -ENOMEM;
> +	db->dev = dev;
> +
> +	/*
> +	 * VCI   is the analog voltage supply
> +	 * VCCIO is the digital I/O voltage supply
> +	 */
> +	db->regulators[0].supply = "vci";
> +	db->regulators[1].supply = "vccio";
> +	ret = devm_regulator_bulk_get(dev,
> +				      ARRAY_SIZE(db->regulators),
> +				      db->regulators);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get regulators\n");
> +
> +	db->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(db->reset)) {
> +		ret = PTR_ERR(db->reset);
> +		return dev_err_probe(dev, ret, "no RESET GPIO\n");
> +	}
> +
> +	spi->bits_per_word = 9;

Do you need to set this? The DBI helper sets it on the SPI transfer if
the SPI controller supports 9-bit or uses emulation code if not.

> +	/* Preserve e.g. SPI_3WIRE setting */
> +	spi->mode |= SPI_MODE_3;

You can use the DT properties 'spi-cpha' and 'spi-cpol' to set this.

> +	ret = spi_setup(spi);

So I don't think you need to call spi_setup() here.

This code as-is will fail on a SPI controller that doesn't support 9
bits per word, and the emulation code in the DBI helper won't be utilised.

With the comments considered:

Acked-by: Noralf Trønnes <noralf at tronnes.org>

Noralf.

> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "spi setup failed\n");
> +
> +	ret = mipi_dbi_spi_init(spi, &db->dbi, NULL);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
> +
> +	drm_panel_init(&db->panel, dev, &db7430_drm_funcs,
> +		       DRM_MODE_CONNECTOR_DPI);
> +
> +	/* FIXME: if no external backlight, use internal backlight */
> +	ret = drm_panel_of_backlight(&db->panel);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to add backlight\n");
> +
> +	spi_set_drvdata(spi, db);
> +
> +	drm_panel_add(&db->panel);
> +	dev_dbg(dev, "added panel\n");
> +
> +	return 0;
> +}