[PATCH v2 3/3] drm: panel: Add a driver for Generic MIPI-DSI/DPI(+SPI) panels

[Krzysztof Kozlowski]

On Wed, Feb 26, 2025 at 08:25:50PM +0900, Hironori KIKUCHI wrote:
> +
> +struct panel_firmware_header {
> +	u8 magic[15];
> +	u8 file_format_version; /* must be 1 */
> +} __packed;
> +
> +struct panel_firmware_config {
> +	u16 width_mm, height_mm;
> +	u16 rotation;
> +	u8 _reserved_1[2];
> +	u8 _reserved_2[8];
> +
> +	u16 reset_delay; /* delay after the reset command, in ms */
> +	u16 init_delay; /* delay for sending the initial command sequence, in ms */
> +	u16 sleep_delay; /* delay after the sleep command, in ms */
> +	u16 backlight_delay; /* delay for enabling the backlight, in ms */

These should be implied by compatible.

> +	u16 _reserved_3[4];
> +
> +	u16 dsi_lanes; /* unsigned int */
> +	u16 dsi_format; /* enum mipi_dsi_pixel_format */
> +	u32 dsi_mode_flags; /* unsigned long */
> +	u32 bus_flags; /* struct drm_bus_flags */
> +	u8 _reserved_4[2];
> +	u8 preferred_timing;
> +	u8 num_timings;
> +} __packed;
> +

...

> +
> +static int panel_mipi_probe(struct device *dev, int connector_type)
> +{
> +	struct panel_mipi *mipi;
> +	int err;
> +
> +	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
> +	if (!mipi)
> +		return -ENOMEM;
> +
> +	mutex_init(&mipi->lock);
> +
> +	mipi->display_id = -1;
> +
> +	/* Get `power-supply` and `io-supply` (if any) */
> +	mipi->supplies[0].supply = "power";
> +	mipi->supplies[1].supply = "io";
> +	err = devm_regulator_bulk_get(dev, ARRAY_SIZE(mipi->supplies),
> +				      mipi->supplies);
> +	if (err < 0) {
> +		return dev_err_probe(dev, err,
> +				     "%pOF: Failed to get regulators\n",
> +				     dev->of_node);

Drop pOF. Device name already tells this.

> +	}
> +
> +	/* GPIO for /RESET */
> +	mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(mipi->reset))
> +		return dev_err_probe(dev, PTR_ERR(mipi->reset),
> +				     "%pOF: Failed to get GPIO for RESET\n",

Drop pOF. Device name already tells this.

> +				     dev->of_node);
> +
> +	/* Load the firmware */
> +	mipi->firmware = panel_mipi_load_firmware(dev);
> +	if (IS_ERR(mipi->firmware))
> +		return dev_err_probe(dev, PTR_ERR(mipi->firmware),
> +				     "Failed to load firmware\n");
> +
> +	err = panel_mipi_read_firmware(dev, mipi, mipi->firmware);
> +	if (err)
> +		return err;
> +
> +	err = panel_mipi_probe_modes(dev, mipi);
> +	if (err)
> +		return err;
> +
> +	/* DRM panel setup */
> +	drm_panel_init(&mipi->panel, dev, &panel_mipi_funcs, connector_type);
> +
> +	err = panel_mipi_set_backlight(&mipi->panel, dev, mipi);
> +	if (err)
> +		return dev_err_probe(dev, err, "Failed to set backlight\n");
> +
> +	drm_panel_add(&mipi->panel);
> +
> +	dev_set_drvdata(dev, mipi);
> +
> +	panel_mipi_debugfs_init(dev, mipi);
> +
> +	return devm_add_action_or_reset(dev, panel_mipi_cleanup, mipi);
> +}
> +
> +static int panel_mipi_dsi_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct panel_mipi *mipi;
> +	int err;
> +
> +	err = panel_mipi_probe(&dsi->dev, DRM_MODE_CONNECTOR_DSI);
> +	if (err)
> +		return err;
> +
> +	mipi = dev_get_drvdata(&dsi->dev);
> +	mipi->dsi = dsi;
> +	mipi->write_command = panel_mipi_dsi_write;
> +	mipi->read_command = panel_mipi_dsi_read;
> +
> +	/* Read from the firmware */
> +	dsi->lanes = be16_to_cpu(mipi->firmware->config->dsi_lanes);

lanes are from DT, because they are depending on how panel is wired. At
least usually.

> +	dsi->format = be16_to_cpu(mipi->firmware->config->dsi_format);
> +	dsi->mode_flags = be32_to_cpu(mipi->firmware->config->dsi_mode_flags);
> +
> +	if (!dsi->lanes)
> +		return dev_err_probe(&dsi->dev, -EINVAL,
> +				     "dsi-lanes == 0 for DSI panel\n");
> +
> +	/* Adjust bus_format */
> +	switch (dsi->format) {
> +	case MIPI_DSI_FMT_RGB888:
> +		mipi->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +		break;
> +	case MIPI_DSI_FMT_RGB666:
> +		mipi->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
> +		break;
> +	case MIPI_DSI_FMT_RGB666_PACKED:
> +		mipi->bus_format = MEDIA_BUS_FMT_RGB666_1X18;
> +		break;
> +	case MIPI_DSI_FMT_RGB565:
> +		mipi->bus_format = MEDIA_BUS_FMT_RGB565_1X16;
> +		break;
> +	}

Best regards,
Krzysztof