[PATCH 1/3] dt-bindings: display: panel: himax,hx83121a: add optional bl supply

[PATCH 1/3] dt-bindings: display: panel: himax,hx83121a: add optional bl supply

[Icenowy Zheng]

When the backlight is managed by the panel controller IC, an external
power rail might be powering the backlight.

Add an optional `bl-supply` property to describe such power rail, thus
allow disabling the backlight.

Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
 .../devicetree/bindings/display/panel/himax,hx83121a.yaml      | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/himax,hx83121a.yaml b/Documentation/devicetree/bindings/display/panel/himax,hx83121a.yaml
index e067a2f6d0b22..aeca3c9a599c2 100644
--- a/Documentation/devicetree/bindings/display/panel/himax,hx83121a.yaml
+++ b/Documentation/devicetree/bindings/display/panel/himax,hx83121a.yaml
@@ -40,6 +40,9 @@ properties:
   vddi-supply:
     description: power supply for IC
 
+  bl-supply:
+    description: power supply for backlight, in case it's managed via DSC
+
   backlight: true
   ports: true
 
-- 
2.52.0

[PATCH 2/3] drm/panel: himax-hx83121a: pass the panel pointer when creating BL

[Icenowy Zheng]

As backlight powering on/off support will be added, more fields of the
panel context will be accessed in the backlight update function.

Pass the whole panel struct instead of the DSI device when creating the
backlight device.

Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
 drivers/gpu/drm/panel/panel-himax-hx83121a.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-himax-hx83121a.c b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
index bed79aa06f46a..1a7e0125bced8 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx83121a.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
@@ -193,10 +193,11 @@ static const struct drm_panel_funcs himax_panel_funcs = {
 
 static int himax_bl_update_status(struct backlight_device *bl)
 {
-	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	struct himax *ctx = bl_get_data(bl);
 	u16 brightness = backlight_get_brightness(bl);
 	/* TODO: brightness to raw map table */
-	return mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
+	return mipi_dsi_dcs_set_display_brightness_large(to_primary_dsi(ctx),
+							 brightness);
 }
 
 static const struct backlight_ops himax_bl_ops = {
@@ -205,9 +206,9 @@ static const struct backlight_ops himax_bl_ops = {
 };
 
 static struct backlight_device *
-himax_create_backlight(struct mipi_dsi_device *dsi)
+himax_create_backlight(struct himax *ctx)
 {
-	struct device *dev = &dsi->dev;
+	struct device *dev = &to_primary_dsi(ctx)->dev;
 	const struct backlight_properties props = {
 		.type = BACKLIGHT_RAW,
 		.brightness = 512,
@@ -215,7 +216,7 @@ himax_create_backlight(struct mipi_dsi_device *dsi)
 		.scale = BACKLIGHT_SCALE_NON_LINEAR,
 	};
 
-	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
+	return devm_backlight_device_register(dev, dev_name(dev), dev, ctx,
 					      &himax_bl_ops, &props);
 }
 
@@ -646,7 +647,7 @@ static int himax_probe(struct mipi_dsi_device *dsi)
 	ctx->panel.prepare_prev_first = true;
 
 	if (desc->has_dcs_backlight) {
-		ctx->backlight = himax_create_backlight(to_primary_dsi(ctx));
+		ctx->backlight = himax_create_backlight(ctx);
 		if (IS_ERR(ctx->backlight))
 			return dev_err_probe(dev, PTR_ERR(ctx->backlight),
 					     "Failed to create backlight\n");
-- 
2.52.0

Re: [PATCH 2/3] drm/panel: himax-hx83121a: pass the panel pointer when creating BL

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing device link to secondary DSI host can lead to Use-After-Free if the host driver is unbound.
- [High] The backlight device is exposed to userspace before the DSI device is attached to the host, allowing premature DCS transfers.
- [High] Unprotected MIPI DSI hardware access due to missing backlight disable allows DCS commands to a powered-off panel.
--

commit 4b6dd00d8fec3ba9b10693589e0413bbbce2ec2f
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

drm/panel: himax-hx83121a: pass the panel pointer when creating BL

This commit modifies the backlight creation function to accept the entire
panel context instead of just the DSI device. This prepares the driver for
future backlight powering on/off support.

> diff --git a/drivers/gpu/drm/panel/panel-himax-hx83121a.c b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> index bed79aa06f46a..1a7e0125bced8 100644
> --- a/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> +++ b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> @@ -193,10 +193,11 @@ static const struct drm_panel_funcs himax_panel_funcs = {
>  
>  static int himax_bl_update_status(struct backlight_device *bl)
>  {
> -	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	struct himax *ctx = bl_get_data(bl);
>  	u16 brightness = backlight_get_brightness(bl);
>  	/* TODO: brightness to raw map table */
> -	return mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
> +	return mipi_dsi_dcs_set_display_brightness_large(to_primary_dsi(ctx),
> +							 brightness);

[Severity: High]
This isn't a bug introduced by this patch, but does this function leave the
MIPI DSI hardware vulnerable to unprotected access?

Because ctx->backlight is not assigned to ctx->panel.backlight during
probe, the DRM core does not automatically manage its state. In
himax_unprepare(), the panel's regulators are disabled and the reset GPIO
is asserted, but there is no corresponding backlight_disable() call.

If userspace writes to the backlight sysfs interface after the panel is
powered off, this function will unconditionally send DCS commands via
mipi_dsi_dcs_set_display_brightness_large() to the off panel. This
typically causes the MIPI DSI host to experience transfer timeouts and
potential hardware errors.

[ ... ]

> @@ -646,7 +647,7 @@ static int himax_probe(struct mipi_dsi_device *dsi)
>  	ctx->panel.prepare_prev_first = true;
>  
>  	if (desc->has_dcs_backlight) {
> -		ctx->backlight = himax_create_backlight(to_primary_dsi(ctx));
> +		ctx->backlight = himax_create_backlight(ctx);

[Severity: High]
This is a pre-existing issue, but does himax_create_backlight() expose the
device to userspace too early?

In himax_probe(), this registers the backlight device and immediately exposes
its sysfs interface. However, this happens before devm_mipi_dsi_attach() is
called later in the probe routine.

If userspace writes to the brightness attribute in this window,
himax_bl_update_status() could send DCS commands over the MIPI DSI bus
before the hardware is actually attached to the host, which could result in
transfers on uninitialized hardware.

[Severity: High]
This is also a pre-existing issue, but does himax_probe() need to establish
a device link to the secondary DSI host?

Earlier in himax_probe(), the secondary DSI host is looked up and the
device is created:

    dsi1_host = of_find_mipi_dsi_host_by_node(dsi1);
    ...
    ctx->dsi[1] = devm_mipi_dsi_device_register_full(dev, dsi1_host, &dsi_info);

Without establishing a device link (e.g., using device_link_add()) to
ensure proper driver unbind synchronization, if the secondary DSI host driver
is unbound, its mipi_dsi_host and the associated DSI device are unregistered
and freed.

Since the panel driver remains bound and retains ctx->dsi[1], a subsequent
DCS command or modeset could access this freed memory, leading to a
use-after-free.

>  		if (IS_ERR(ctx->backlight))
>  			return dev_err_probe(dev, PTR_ERR(ctx->backlight),
>  					     "Failed to create backlight\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608162622.403713-1-zhengxingda@iscas.ac.cn?part=2

[PATCH 3/3] drm/panel: himax-hx83121a: add backlight regulator support

[Icenowy Zheng]

The backlight, when managed by the panel controller, could be powered by
an external regulator, and shutting down the regulator could power off
the backlight.

Add support for such a regulator. It's powered off when the backlight is
0 (either by setting brightness to 0 or setting bl_power), and powered
on when the backlight should be operating.

Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
 drivers/gpu/drm/panel/panel-himax-hx83121a.c | 26 ++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-himax-hx83121a.c b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
index 1a7e0125bced8..e31e2fba0a787 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx83121a.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
@@ -34,7 +34,9 @@ struct himax {
 	struct drm_dsc_config dsc;
 	struct gpio_desc *reset_gpio;
 	struct regulator_bulk_data *supplies;
+	struct regulator *bl_supply;
 	struct backlight_device *backlight;
+	bool backlight_enabled;
 };
 
 struct panel_desc {
@@ -195,7 +197,27 @@ static int himax_bl_update_status(struct backlight_device *bl)
 {
 	struct himax *ctx = bl_get_data(bl);
 	u16 brightness = backlight_get_brightness(bl);
+	int ret = 0;
+
+	if (!brightness) {
+		if (ctx->backlight_enabled)
+			ret = regulator_disable(ctx->bl_supply);
+		if (ret)
+			return ret;
+
+		ctx->backlight_enabled = false;
+
+		return 0;
+	}
+
 	/* TODO: brightness to raw map table */
+	if (!ctx->backlight_enabled)
+		ret = regulator_enable(ctx->bl_supply);
+	if (ret)
+		return ret;
+
+	ctx->backlight_enabled = true;
+
 	return mipi_dsi_dcs_set_display_brightness_large(to_primary_dsi(ctx),
 							 brightness);
 }
@@ -647,6 +669,10 @@ static int himax_probe(struct mipi_dsi_device *dsi)
 	ctx->panel.prepare_prev_first = true;
 
 	if (desc->has_dcs_backlight) {
+		ctx->bl_supply = devm_regulator_get_optional(dev, "bl");
+		if (IS_ERR(ctx->bl_supply))
+			return dev_err_probe(dev, PTR_ERR(ctx->bl_supply),
+					     "Failed to get backlight supply\n");
 		ctx->backlight = himax_create_backlight(ctx);
 		if (IS_ERR(ctx->backlight))
 			return dev_err_probe(dev, PTR_ERR(ctx->backlight),
-- 
2.52.0

Re: [PATCH 3/3] drm/panel: himax-hx83121a: add backlight regulator support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Probe fails unconditionally if the optional backlight regulator is not present in the device tree, breaking backward compatibility.
- [High] Skipping the DCS brightness command when brightness is 0 prevents the backlight from turning off on devices without an external regulator.
- [Medium] The backlight supply regulator's enable count is leaked if the driver is unbound while the backlight is active.
--

commit 700de7e190150a663957123f411692a25d437849
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

drm/panel: himax-hx83121a: add backlight regulator support

This patch adds support for an external backlight regulator to the
himax-hx83121a panel driver. It turns off the regulator when brightness
is 0 and enables it when the backlight is operating.

> diff --git a/drivers/gpu/drm/panel/panel-himax-hx83121a.c b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> index 1a7e0125bced8..e31e2fba0a787 100644
> --- a/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> +++ b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
[ ... ]
> @@ -195,7 +197,27 @@ static int himax_bl_update_status(struct backlight_device *bl)
>  {
>  	struct himax *ctx = bl_get_data(bl);
>  	u16 brightness = backlight_get_brightness(bl);
> +	int ret = 0;
> +
> +	if (!brightness) {
> +		if (ctx->backlight_enabled)
> +			ret = regulator_disable(ctx->bl_supply);
> +		if (ret)
> +			return ret;
> +
> +		ctx->backlight_enabled = false;
> +
> +		return 0;
> +	}

[Severity: High]
Does this early return prevent the display from turning off the backlight 
via DCS?

For panels that do not have an external bl-supply regulator and rely entirely
on DCS commands, disabling the dummy/absent regulator is a no-op, but the
early return bypasses the mipi_dsi_dcs_set_display_brightness_large() call.
Will this leave the backlight powered on at its previous level when it should
be turned off?

> +
>  	/* TODO: brightness to raw map table */
> +	if (!ctx->backlight_enabled)
> +		ret = regulator_enable(ctx->bl_supply);

[Severity: Medium]
Could this leak the regulator enable count if the driver is unbound while the
backlight is active?

If the panel driver is unbound or the module is removed while brightness is
greater than 0, the devres infrastructure will unregister the backlight
device without triggering an update_status to 0. Since there is no explicit
disable in a remove callback or devm_add_action teardown hook, will the
subsequent regulator_put() hit a warning for a non-zero enable count?

> +	if (ret)
> +		return ret;
> +
> +	ctx->backlight_enabled = true;
> +
>  	return mipi_dsi_dcs_set_display_brightness_large(to_primary_dsi(ctx),
>  							 brightness);
>  }
[ ... ]
> @@ -647,6 +669,10 @@ static int himax_probe(struct mipi_dsi_device *dsi)
>  	ctx->panel.prepare_prev_first = true;
>  
>  	if (desc->has_dcs_backlight) {
> +		ctx->bl_supply = devm_regulator_get_optional(dev, "bl");
> +		if (IS_ERR(ctx->bl_supply))
> +			return dev_err_probe(dev, PTR_ERR(ctx->bl_supply),
> +					     "Failed to get backlight supply\n");

[Severity: High]
Will this break probe backward compatibility for existing device trees?

The devm_regulator_get_optional() function returns ERR_PTR(-ENODEV) when the
requested regulator is absent from the device tree. Because this bl-supply
property was just introduced and is optional, existing devices do not have it.
Should this code explicitly check for and handle -ENODEV (for example, by
setting the pointer to NULL) instead of failing the probe unconditionally?

>  		ctx->backlight = himax_create_backlight(ctx);
>  		if (IS_ERR(ctx->backlight))
>  			return dev_err_probe(dev, PTR_ERR(ctx->backlight),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608162622.403713-1-zhengxingda@iscas.ac.cn?part=3

Re: [PATCH 1/3] dt-bindings: display: panel: himax,hx83121a: add optional bl supply

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]