[PATCH 1/2] drm/panel: Allow powering on panel follower after panel is enabled

[PATCH 1/2] drm/panel: Allow powering on panel follower after panel is enabled

[Pin-Yen Lin]

Some touch controllers have to be powered on after the panel's backlight
is enabled. To support these controllers, introduce after_panel_enabled
flag to the panel follower and power on the device after the panel and
its backlight are enabled.

Signed-off-by: Pin-Yen Lin <treapking@google.com>
---

 drivers/gpu/drm/drm_panel.c | 47 +++++++++++++++++++++++++++++++++----
 include/drm/drm_panel.h     |  8 +++++++
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 650de4da08537..58125f8418f37 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -133,6 +133,9 @@ void drm_panel_prepare(struct drm_panel *panel)
 	panel->prepared = true;
 
 	list_for_each_entry(follower, &panel->followers, list) {
+		if (follower->after_panel_enabled)
+			continue;
+
 		ret = follower->funcs->panel_prepared(follower);
 		if (ret < 0)
 			dev_info(panel->dev, "%ps failed: %d\n",
@@ -178,6 +181,9 @@ void drm_panel_unprepare(struct drm_panel *panel)
 	mutex_lock(&panel->follower_lock);
 
 	list_for_each_entry(follower, &panel->followers, list) {
+		if (follower->after_panel_enabled)
+			continue;
+
 		ret = follower->funcs->panel_unpreparing(follower);
 		if (ret < 0)
 			dev_info(panel->dev, "%ps failed: %d\n",
@@ -208,6 +214,7 @@ EXPORT_SYMBOL(drm_panel_unprepare);
  */
 void drm_panel_enable(struct drm_panel *panel)
 {
+	struct drm_panel_follower *follower;
 	int ret;
 
 	if (!panel)
@@ -218,10 +225,12 @@ void drm_panel_enable(struct drm_panel *panel)
 		return;
 	}
 
+	mutex_lock(&panel->follower_lock);
+
 	if (panel->funcs && panel->funcs->enable) {
 		ret = panel->funcs->enable(panel);
 		if (ret < 0)
-			return;
+			goto exit;
 	}
 	panel->enabled = true;
 
@@ -229,6 +238,18 @@ void drm_panel_enable(struct drm_panel *panel)
 	if (ret < 0)
 		DRM_DEV_INFO(panel->dev, "failed to enable backlight: %d\n",
 			     ret);
+
+	list_for_each_entry(follower, &panel->followers, list) {
+		if (!follower->after_panel_enabled)
+			continue;
+
+		ret = follower->funcs->panel_prepared(follower);
+		if (ret < 0)
+			dev_info(panel->dev, "%ps failed: %d\n",
+				 follower->funcs->panel_prepared, ret);
+	}
+exit:
+	mutex_unlock(&panel->follower_lock);
 }
 EXPORT_SYMBOL(drm_panel_enable);
 
@@ -242,6 +263,7 @@ EXPORT_SYMBOL(drm_panel_enable);
  */
 void drm_panel_disable(struct drm_panel *panel)
 {
+	struct drm_panel_follower *follower;
 	int ret;
 
 	if (!panel)
@@ -261,6 +283,18 @@ void drm_panel_disable(struct drm_panel *panel)
 		return;
 	}
 
+	mutex_lock(&panel->follower_lock);
+
+	list_for_each_entry(follower, &panel->followers, list) {
+		if (!follower->after_panel_enabled)
+			continue;
+
+		ret = follower->funcs->panel_unpreparing(follower);
+		if (ret < 0)
+			dev_info(panel->dev, "%ps failed: %d\n",
+				 follower->funcs->panel_unpreparing, ret);
+	}
+
 	ret = backlight_disable(panel->backlight);
 	if (ret < 0)
 		DRM_DEV_INFO(panel->dev, "failed to disable backlight: %d\n",
@@ -269,9 +303,12 @@ void drm_panel_disable(struct drm_panel *panel)
 	if (panel->funcs && panel->funcs->disable) {
 		ret = panel->funcs->disable(panel);
 		if (ret < 0)
-			return;
+			goto exit;
 	}
 	panel->enabled = false;
+
+exit:
+	mutex_unlock(&panel->follower_lock);
 }
 EXPORT_SYMBOL(drm_panel_disable);
 
@@ -537,7 +574,8 @@ int drm_panel_add_follower(struct device *follower_dev,
 	mutex_lock(&panel->follower_lock);
 
 	list_add_tail(&follower->list, &panel->followers);
-	if (panel->prepared) {
+	if ((panel->prepared && !follower->after_panel_enabled) ||
+	    (panel->enabled && follower->after_panel_enabled)) {
 		ret = follower->funcs->panel_prepared(follower);
 		if (ret < 0)
 			dev_info(panel->dev, "%ps failed: %d\n",
@@ -566,7 +604,8 @@ void drm_panel_remove_follower(struct drm_panel_follower *follower)
 
 	mutex_lock(&panel->follower_lock);
 
-	if (panel->prepared) {
+	if ((panel->prepared && !follower->after_panel_enabled) ||
+	    (panel->enabled && follower->after_panel_enabled)) {
 		ret = follower->funcs->panel_unpreparing(follower);
 		if (ret < 0)
 			dev_info(panel->dev, "%ps failed: %d\n",
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 843fb756a2950..aa9b6218702fd 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -183,6 +183,14 @@ struct drm_panel_follower {
 	 * The panel we're dependent on; set by drm_panel_add_follower().
 	 */
 	struct drm_panel *panel;
+
+	/**
+	 * @after_panel_enabled
+	 *
+	 * If true then this panel follower should be powered after the panel
+	 * and the backlight are enabled, instead of after panel is prepared.
+	 */
+	bool after_panel_enabled;
 };
 
 /**
-- 
2.50.1.552.g942d659e1b-goog

[PATCH 2/2] HID: Make elan touch controllers power on after panel is enabled

[Pin-Yen Lin]

Introduce a new HID quirk to indicate that this device has to be enabled
after the panel's backlight is enabled, and update the driver data for
the elan devices to enable this quirk. This cannot be a I2C HID quirk
because the kernel needs to acknowledge this before powering up the
device and read the VID/PID.

Signed-off-by: Pin-Yen Lin <treapking@google.com>

---

 drivers/hid/i2c-hid/i2c-hid-core.c    |  2 ++
 drivers/hid/i2c-hid/i2c-hid-of-elan.c | 11 ++++++++++-
 include/linux/hid.h                   |  2 ++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index d3912e3f2f13a..8dc4d5d56d399 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1183,6 +1183,8 @@ static int i2c_hid_core_register_panel_follower(struct i2c_hid *ihid)
 	int ret;
 
 	ihid->panel_follower.funcs = &i2c_hid_core_panel_follower_funcs;
+	if (ihid->hid->initial_quirks | HID_QUIRK_POWER_ON_AFTER_BACKLIGHT)
+		ihid->panel_follower.after_panel_enabled = true;
 
 	/*
 	 * If we're not in control of our own power up/power down then we can't
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 3fcff6daa0d3a..0215f217f6d86 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -8,6 +8,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio/consumer.h>
+#include <linux/hid.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -23,6 +24,7 @@ struct elan_i2c_hid_chip_data {
 	unsigned int post_power_delay_ms;
 	u16 hid_descriptor_address;
 	const char *main_supply_name;
+	bool power_after_backlight;
 };
 
 struct i2c_hid_of_elan {
@@ -97,6 +99,7 @@ static int i2c_hid_of_elan_probe(struct i2c_client *client)
 {
 	struct i2c_hid_of_elan *ihid_elan;
 	int ret;
+	u32 quirks = 0;
 
 	ihid_elan = devm_kzalloc(&client->dev, sizeof(*ihid_elan), GFP_KERNEL);
 	if (!ihid_elan)
@@ -131,8 +134,12 @@ static int i2c_hid_of_elan_probe(struct i2c_client *client)
 		}
 	}
 
+	if (ihid_elan->chip_data->power_after_backlight)
+		quirks = HID_QUIRK_POWER_ON_AFTER_BACKLIGHT;
+
 	ret = i2c_hid_core_probe(client, &ihid_elan->ops,
-				 ihid_elan->chip_data->hid_descriptor_address, 0);
+				 ihid_elan->chip_data->hid_descriptor_address,
+				 quirks);
 	if (ret)
 		goto err_deassert_reset;
 
@@ -150,6 +157,7 @@ static const struct elan_i2c_hid_chip_data elan_ekth6915_chip_data = {
 	.post_gpio_reset_on_delay_ms = 300,
 	.hid_descriptor_address = 0x0001,
 	.main_supply_name = "vcc33",
+	.power_after_backlight = true,
 };
 
 static const struct elan_i2c_hid_chip_data elan_ekth6a12nay_chip_data = {
@@ -157,6 +165,7 @@ static const struct elan_i2c_hid_chip_data elan_ekth6a12nay_chip_data = {
 	.post_gpio_reset_on_delay_ms = 300,
 	.hid_descriptor_address = 0x0001,
 	.main_supply_name = "vcc33",
+	.power_after_backlight = true,
 };
 
 static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = {
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 2cc4f1e4ea963..c32425b5d0119 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -364,6 +364,7 @@ struct hid_item {
  * | @HID_QUIRK_HAVE_SPECIAL_DRIVER:
  * | @HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE:
  * | @HID_QUIRK_IGNORE_SPECIAL_DRIVER
+ * | @HID_QUIRK_POWER_ON_AFTER_BACKLIGHT
  * | @HID_QUIRK_FULLSPEED_INTERVAL:
  * | @HID_QUIRK_NO_INIT_REPORTS:
  * | @HID_QUIRK_NO_IGNORE:
@@ -391,6 +392,7 @@ struct hid_item {
 #define HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE	BIT(20)
 #define HID_QUIRK_NOINVERT			BIT(21)
 #define HID_QUIRK_IGNORE_SPECIAL_DRIVER		BIT(22)
+#define HID_QUIRK_POWER_ON_AFTER_BACKLIGHT	BIT(23)
 #define HID_QUIRK_FULLSPEED_INTERVAL		BIT(28)
 #define HID_QUIRK_NO_INIT_REPORTS		BIT(29)
 #define HID_QUIRK_NO_IGNORE			BIT(30)
-- 
2.50.1.552.g942d659e1b-goog

Re: [PATCH 1/2] drm/panel: Allow powering on panel follower after panel is enabled

[Jani Nikula]

On Thu, 31 Jul 2025, Pin-Yen Lin <treapking@chromium.org> wrote:
> Some touch controllers have to be powered on after the panel's backlight
> is enabled. To support these controllers, introduce after_panel_enabled
> flag to the panel follower and power on the device after the panel and
> its backlight are enabled.

I think it's *very* confusing and error prone to call follower hooks at
different places depending on a flag. The hook names and documentation
say *when* they get called, and that should not change.

I think the right approach would be to add .panel_enabled and
.panel_disabling hooks to struct drm_panel_follower_funcs, and have them
called after panel (and backlight) have been enabled and before panel
(and backlight) are disabled, respectively.

In i2c-hid-core.c, you'd then have two copies of struct
drm_panel_follower_funcs, and use one or the other depending on the
quirk. You can even reuse the functions.

I think overall it'll be be more consistent, more flexible, and probably
fewer lines of code too.


BR,
Jani.

> Signed-off-by: Pin-Yen Lin <treapking@google.com>

PS. Your Signed-off-by doesn't match the patch author.

> ---
>
>  drivers/gpu/drm/drm_panel.c | 47 +++++++++++++++++++++++++++++++++----
>  include/drm/drm_panel.h     |  8 +++++++
>  2 files changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 650de4da08537..58125f8418f37 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -133,6 +133,9 @@ void drm_panel_prepare(struct drm_panel *panel)
>  	panel->prepared = true;
>  
>  	list_for_each_entry(follower, &panel->followers, list) {
> +		if (follower->after_panel_enabled)
> +			continue;
> +
>  		ret = follower->funcs->panel_prepared(follower);
>  		if (ret < 0)
>  			dev_info(panel->dev, "%ps failed: %d\n",
> @@ -178,6 +181,9 @@ void drm_panel_unprepare(struct drm_panel *panel)
>  	mutex_lock(&panel->follower_lock);
>  
>  	list_for_each_entry(follower, &panel->followers, list) {
> +		if (follower->after_panel_enabled)
> +			continue;
> +
>  		ret = follower->funcs->panel_unpreparing(follower);
>  		if (ret < 0)
>  			dev_info(panel->dev, "%ps failed: %d\n",
> @@ -208,6 +214,7 @@ EXPORT_SYMBOL(drm_panel_unprepare);
>   */
>  void drm_panel_enable(struct drm_panel *panel)
>  {
> +	struct drm_panel_follower *follower;
>  	int ret;
>  
>  	if (!panel)
> @@ -218,10 +225,12 @@ void drm_panel_enable(struct drm_panel *panel)
>  		return;
>  	}
>  
> +	mutex_lock(&panel->follower_lock);
> +
>  	if (panel->funcs && panel->funcs->enable) {
>  		ret = panel->funcs->enable(panel);
>  		if (ret < 0)
> -			return;
> +			goto exit;
>  	}
>  	panel->enabled = true;
>  
> @@ -229,6 +238,18 @@ void drm_panel_enable(struct drm_panel *panel)
>  	if (ret < 0)
>  		DRM_DEV_INFO(panel->dev, "failed to enable backlight: %d\n",
>  			     ret);
> +
> +	list_for_each_entry(follower, &panel->followers, list) {
> +		if (!follower->after_panel_enabled)
> +			continue;
> +
> +		ret = follower->funcs->panel_prepared(follower);
> +		if (ret < 0)
> +			dev_info(panel->dev, "%ps failed: %d\n",
> +				 follower->funcs->panel_prepared, ret);
> +	}
> +exit:
> +	mutex_unlock(&panel->follower_lock);
>  }
>  EXPORT_SYMBOL(drm_panel_enable);
>  
> @@ -242,6 +263,7 @@ EXPORT_SYMBOL(drm_panel_enable);
>   */
>  void drm_panel_disable(struct drm_panel *panel)
>  {
> +	struct drm_panel_follower *follower;
>  	int ret;
>  
>  	if (!panel)
> @@ -261,6 +283,18 @@ void drm_panel_disable(struct drm_panel *panel)
>  		return;
>  	}
>  
> +	mutex_lock(&panel->follower_lock);
> +
> +	list_for_each_entry(follower, &panel->followers, list) {
> +		if (!follower->after_panel_enabled)
> +			continue;
> +
> +		ret = follower->funcs->panel_unpreparing(follower);
> +		if (ret < 0)
> +			dev_info(panel->dev, "%ps failed: %d\n",
> +				 follower->funcs->panel_unpreparing, ret);
> +	}
> +
>  	ret = backlight_disable(panel->backlight);
>  	if (ret < 0)
>  		DRM_DEV_INFO(panel->dev, "failed to disable backlight: %d\n",
> @@ -269,9 +303,12 @@ void drm_panel_disable(struct drm_panel *panel)
>  	if (panel->funcs && panel->funcs->disable) {
>  		ret = panel->funcs->disable(panel);
>  		if (ret < 0)
> -			return;
> +			goto exit;
>  	}
>  	panel->enabled = false;
> +
> +exit:
> +	mutex_unlock(&panel->follower_lock);
>  }
>  EXPORT_SYMBOL(drm_panel_disable);
>  
> @@ -537,7 +574,8 @@ int drm_panel_add_follower(struct device *follower_dev,
>  	mutex_lock(&panel->follower_lock);
>  
>  	list_add_tail(&follower->list, &panel->followers);
> -	if (panel->prepared) {
> +	if ((panel->prepared && !follower->after_panel_enabled) ||
> +	    (panel->enabled && follower->after_panel_enabled)) {
>  		ret = follower->funcs->panel_prepared(follower);
>  		if (ret < 0)
>  			dev_info(panel->dev, "%ps failed: %d\n",
> @@ -566,7 +604,8 @@ void drm_panel_remove_follower(struct drm_panel_follower *follower)
>  
>  	mutex_lock(&panel->follower_lock);
>  
> -	if (panel->prepared) {
> +	if ((panel->prepared && !follower->after_panel_enabled) ||
> +	    (panel->enabled && follower->after_panel_enabled)) {
>  		ret = follower->funcs->panel_unpreparing(follower);
>  		if (ret < 0)
>  			dev_info(panel->dev, "%ps failed: %d\n",
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 843fb756a2950..aa9b6218702fd 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -183,6 +183,14 @@ struct drm_panel_follower {
>  	 * The panel we're dependent on; set by drm_panel_add_follower().
>  	 */
>  	struct drm_panel *panel;
> +
> +	/**
> +	 * @after_panel_enabled
> +	 *
> +	 * If true then this panel follower should be powered after the panel
> +	 * and the backlight are enabled, instead of after panel is prepared.
> +	 */
> +	bool after_panel_enabled;
>  };
>  
>  /**

-- 
Jani Nikula, Intel

Re: [PATCH 1/2] drm/panel: Allow powering on panel follower after panel is enabled

[Doug Anderson]

Hi,

On Thu, Jul 31, 2025 at 3:31 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Thu, 31 Jul 2025, Pin-Yen Lin <treapking@chromium.org> wrote:
> > Some touch controllers have to be powered on after the panel's backlight
> > is enabled. To support these controllers, introduce after_panel_enabled
> > flag to the panel follower and power on the device after the panel and
> > its backlight are enabled.
>
> I think it's *very* confusing and error prone to call follower hooks at
> different places depending on a flag. The hook names and documentation
> say *when* they get called, and that should not change.
>
> I think the right approach would be to add .panel_enabled and
> .panel_disabling hooks to struct drm_panel_follower_funcs, and have them
> called after panel (and backlight) have been enabled and before panel
> (and backlight) are disabled, respectively.
>
> In i2c-hid-core.c, you'd then have two copies of struct
> drm_panel_follower_funcs, and use one or the other depending on the
> quirk. You can even reuse the functions.
>
> I think overall it'll be be more consistent, more flexible, and probably
> fewer lines of code too.

Yes, exactly what Jani said. We've wanted to do this before, but I
just never got around to it. There's even a (public) bug for it in the
Google bug tracker and I've just assigned it to you. :-P

https://issuetracker.google.com/305780363

-Doug

Re: [PATCH 1/2] drm/panel: Allow powering on panel follower after panel is enabled

[Pin-yen Lin]

Hi Doug and Jani,

Thanks for the review.

On Fri, Aug 1, 2025 at 12:38 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Jul 31, 2025 at 3:31 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >
> > On Thu, 31 Jul 2025, Pin-Yen Lin <treapking@chromium.org> wrote:
> > > Some touch controllers have to be powered on after the panel's backlight
> > > is enabled. To support these controllers, introduce after_panel_enabled
> > > flag to the panel follower and power on the device after the panel and
> > > its backlight are enabled.
> >
> > I think it's *very* confusing and error prone to call follower hooks at
> > different places depending on a flag. The hook names and documentation
> > say *when* they get called, and that should not change.
> >
> > I think the right approach would be to add .panel_enabled and
> > .panel_disabling hooks to struct drm_panel_follower_funcs, and have them
> > called after panel (and backlight) have been enabled and before panel
> > (and backlight) are disabled, respectively.
> >
> > In i2c-hid-core.c, you'd then have two copies of struct
> > drm_panel_follower_funcs, and use one or the other depending on the
> > quirk. You can even reuse the functions.
> >
> > I think overall it'll be be more consistent, more flexible, and probably
> > fewer lines of code too.

I was thinking that we probably will never have a device that needs to
register both .panel_prepared() and .panel_enabled(), so I implemented
it like this. I'll update this in the next version.

I'll also fix the s-o-b line. Apparently I've messed up with my local
git setting.
>
> Yes, exactly what Jani said. We've wanted to do this before, but I
> just never got around to it. There's even a (public) bug for it in the
> Google bug tracker and I've just assigned it to you. :-P
>
> https://issuetracker.google.com/305780363

So my series is not a new idea :P
>
> -Doug


Regards,
Pin-yen