Re: [PATCH 3/3] drm/panel-mipi-dbi: drop GPIOD_FLAGS_BIT_NONEXCLUSIVE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/panel-mipi-dbi: broken shared dc GPIO in panel_mipi_dbi_spi_probe()
--

commit 90b279985dbeced362ebac928f364d9afff9972e
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

drm/panel-mipi-dbi: drop GPIOD_FLAGS_BIT_NONEXCLUSIVE

This commit removes the deprecated GPIOD_FLAGS_BIT_NONEXCLUSIVE flag when
requesting the dc GPIO in panel_mipi_dbi_spi_probe(), relying on the GPIOLIB
shared GPIO proxy instead.

> diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
> index 60cd65cae41d8..6f09b723cd7c3 100644
> --- a/drivers/gpu/drm/tiny/panel-mipi-dbi.c
> +++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
> @@ -418,8 +418,7 @@ static int panel_mipi_dbi_spi_probe(struct spi_device *spi)
>  	if (IS_ERR(dbi->reset))
>  		return dev_err_probe(dev, PTR_ERR(dbi->reset), "Failed to get GPIO 'reset'\n");
>  
> -	/* Multiple panels can share the "dc" GPIO, but only if they are on the same SPI bus! */
> -	dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW | GPIOD_FLAGS_BIT_NONEXCLUSIVE);
> +	dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);

[Severity: High]
Can gpio-shared-proxy work for a shared MIPI DBI D/C line across multiple
panels on the same SPI bus?

Unlike a shared reset or enable pin, the D/C GPIO in MIPI DBI Type C Option 3
is a bus control signal (0 for commands, 1 for parameters and pixel data)
multiplexed under spi_bus_lock(). In drivers/gpu/drm/drm_mipi_dbi.c,
mipi_dbi_typec3_command() leaves dbi->dc asserted at 1 after transferring
command parameters or pixel data:

drivers/gpu/drm/drm_mipi_dbi.c:mipi_dbi_typec3_command() {
	...
	spi_bus_lock(spi->controller);
	gpiod_set_value_cansleep(dbi->dc, 0);
	speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
	ret = mipi_dbi_spi_transfer(spi, speed_hz, 8, &cmd, 1);
	spi_bus_unlock(spi->controller);
	if (ret || !num)
		return ret;

	...
	spi_bus_lock(spi->controller);
	gpiod_set_value_cansleep(dbi->dc, 1);
	speed_hz = mipi_dbi_spi_cmd_max_speed(spi, num);
	ret = mipi_dbi_spi_transfer(spi, speed_hz, bpw, par, num);
	spi_bus_unlock(spi->controller);

	return ret;
}

When panel_mipi_dbi_spi_probe() requests dc with GPIOD_OUT_LOW,
shared_desc->def_val is 0. Once panel A sends a command with parameters and
sets dbi->dc to 1, gpio_shared_proxy_set_unlocked() sets panel A's
proxy->voted_change = true and increments shared_desc->votecnt to 1. Because
mipi_dbi_typec3_command() returns with dbi->dc still at 1, panel A retains its
vote after releasing spi_bus_lock().

When panel B on the same SPI bus then calls gpiod_set_value_cansleep(dbi->dc, 0)
in mipi_dbi_typec3_command() or mipi_dbi_typec3_command_read() to send a
command byte, gpio_shared_proxy_set_unlocked() checks whether any other proxy
still holds a vote for 1:

drivers/gpio/gpio-shared-proxy.c:gpio_shared_proxy_set_unlocked() {
	...
	/* Desired value is the default. */
	if (!proxy->voted_change) {
		if (!shared_desc->votecnt)
			ret = gpiod_set_value_cansleep(desc, shared_desc->def_val);
		goto out;
	}

	/* We previously voted for change. */
	if (shared_desc->votecnt == 1) {
		/* This is the last remaining vote for change, set value to default. */
		ret = gpiod_set_value_cansleep(desc, shared_desc->def_val);
		if (ret)
			goto out;
	}
	...
}

Because panel A's vote keeps shared_desc->votecnt >= 1, gpio-shared-proxy will
not drive the physical D/C GPIO back to 0 when panel B requests 0, leaving the
physical D/C line stuck high in data mode during panel B's command transfers
(and vice versa once panel B also sets dc to 1).

Also, the two in-tree DT users mentioned in the commit message only define a
single panel each and do not share dc-gpios, whereas DRM_PANEL_MIPI_DBI itself
does not select HAVE_SHARED_GPIOS and can be used on any platform or via DT
overlays where shared dc-gpios configurations (added in commits 3a63ef6e4b9e
and 8cc8ccbaa5d8) would now either fail probe with -EBUSY or fail to toggle
the physical D/C line.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-panel-mipi-non-exclusive-gpios-v1-0-aa862c1d71e3@oss.qualcomm.com?part=3