On 29/09/2026 12:36, Krzysztof Kozlowski wrote:
On 29/09/2026 11:59, David Heidelberg wrote:
On 29/09/2026 11:51, Krzysztof Kozlowski wrote:
On 29/09/2026 11:41, David Heidelberg wrote:
On 29/09/2026 09:45, Krzysztof Kozlowski wrote:
On Thu, Sep 24, 2026 at 04:01:34PM +0200, David Heidelberg wrote:
The reset was introduced with wrong polarity. Correct for the future
compatibles and keep current with reverted logic.
Old DTs keep GPIO_ACTIVE_HIGH and are fixed up via
gpiod_toggle_active_low() on the deprecated compatible.
Assisted-by: LLM
Reviewed-by: Neil Armstrong <[email protected]>
Signed-off-by: David Heidelberg <[email protected]>
---
drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
b/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
index 5e1e997b83b36..99290913de69a 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
@@ -20,16 +20,17 @@
#include "panel-samsung-dsi.h"
struct s6e3ha8_desc {
const struct drm_panel_funcs *funcs;
const struct drm_display_mode *mode;
unsigned long mode_flags;
const struct regulator_bulk_data *supplies;
unsigned int num_supplies;
+ bool broken_reset_polarity;
};
struct s6e3ha8 {
struct drm_panel panel;
struct mipi_dsi_device *dsi;
const struct s6e3ha8_desc *desc;
struct drm_dsc_config dsc;
struct gpio_desc *reset_gpio;
@@ -62,22 +63,22 @@ static int s6e3ha8_unprepare(struct drm_panel *panel)
{
struct s6e3ha8 *priv = to_s6e3ha8(panel);
return regulator_bulk_disable(priv->desc->num_supplies, priv->supplies);
}
static void s6e3ha8_amb577px01_wqhd_reset(struct s6e3ha8 *priv)
{
- gpiod_set_value_cansleep(priv->reset_gpio, 1);
- usleep_range(5000, 6000);
gpiod_set_value_cansleep(priv->reset_gpio, 0);
usleep_range(5000, 6000);
gpiod_set_value_cansleep(priv->reset_gpio, 1);
usleep_range(5000, 6000);
+ gpiod_set_value_cansleep(priv->reset_gpio, 0);
This breaks all users and this usage of ABI was already released.
See the gpiod_toggle_active_low() usage later in the patch which keep the logic
for the original compatible as intended.
OK, I went way too fast, that's correct part. But splitting fix is still
just confusing. Backporting to stable is a different thing than fixing
issues.
Sure, I already droped the previous commit changing it for stable.
Btw. looking at gpiod_toggle_active_low(), would it make sense to do a series
correcting panel reset logic? I see many panels keep "reset asserted" in the
driver (but ofc not in the reality).
To my knowledge it is impossible task to do, without breaking something.
I would do the partial change (only driver, not full DT).
So we end up with drivers having the correct logic with extra 2-line DT quirk
using gpiod_toggle_active_low() for given compatible.
This could solve people implementing new panel on top of existing DDIC and allow
them to use the right polarity for new compatible.
David
Either you break users of ABI (so the DTS) or break existing users of
DTS. One could try to avoid both by using your approach here with
compatibles having fallback. But then what polarity actually would be in
such DTS node? If you know your users, like for some SoC components, you
could argue that none of then will be affected. But both the driver and
DTS here can be used externally.
Best regards,
Krzysztof
