linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios
@ 2021-05-24 13:18 Maxime Ripard
  2021-05-24 13:18 ` [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod Maxime Ripard
  2021-06-04 21:44 ` [PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios Linus Walleij
  0 siblings, 2 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-05-24 13:18 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: Eric Anholt, linux-kernel, Linus Wallei, Dave Stevenson,
	Phil Elwell, Tim Gover, Dom Cobley, Daniel Vetter, Hans Verkuil,
	Maxime Ripard

If the of_get_named_gpio_flags call fails in vc4_hdmi_bind, we jump to
the err_unprepare_hsm label. That label will then call
pm_runtime_disable and put_device on the DDC device.

We just retrieved the DDC device, so the latter is definitely justified.
However at that point we still haven't called pm_runtime_enable, so the
call to pm_runtime_disable is not supposed to be there.

Fixes: 10ee275cb12f ("drm/vc4: prepare for CEC support")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index c27b287d2053..ccc6c8079dc6 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2039,7 +2039,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 							     &hpd_gpio_flags);
 		if (vc4_hdmi->hpd_gpio < 0) {
 			ret = vc4_hdmi->hpd_gpio;
-			goto err_unprepare_hsm;
+			goto err_put_ddc;
 		}
 
 		vc4_hdmi->hpd_active_low = hpd_gpio_flags & OF_GPIO_ACTIVE_LOW;
@@ -2080,8 +2080,8 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 	vc4_hdmi_connector_destroy(&vc4_hdmi->connector);
 err_destroy_encoder:
 	drm_encoder_cleanup(encoder);
-err_unprepare_hsm:
 	pm_runtime_disable(dev);
+err_put_ddc:
 	put_device(&vc4_hdmi->ddc->dev);
 
 	return ret;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod
  2021-05-24 13:18 [PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios Maxime Ripard
@ 2021-05-24 13:18 ` Maxime Ripard
  2021-05-27 23:57   ` Linus Walleij
  2021-07-02  3:29   ` Nathan Chancellor
  2021-06-04 21:44 ` [PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios Linus Walleij
  1 sibling, 2 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-05-24 13:18 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: Eric Anholt, linux-kernel, Linus Wallei, Dave Stevenson,
	Phil Elwell, Tim Gover, Dom Cobley, Daniel Vetter, Hans Verkuil,
	Maxime Ripard

The new gpiod interface takes care of parsing the GPIO flags and to
return the logical value when accessing an active-low GPIO, so switching
to it simplifies a lot the driver.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 24 +++++++-----------------
 drivers/gpu/drm/vc4/vc4_hdmi.h |  3 +--
 2 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index ccc6c8079dc6..34622c59f6a7 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -159,10 +159,9 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
 	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
 	bool connected = false;
 
-	if (vc4_hdmi->hpd_gpio) {
-		if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^
-		    vc4_hdmi->hpd_active_low)
-			connected = true;
+	if (vc4_hdmi->hpd_gpio &&
+	    gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
+		connected = true;
 	} else if (drm_probe_ddc(vc4_hdmi->ddc)) {
 		connected = true;
 	} else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) {
@@ -1993,7 +1992,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 	struct vc4_hdmi *vc4_hdmi;
 	struct drm_encoder *encoder;
 	struct device_node *ddc_node;
-	u32 value;
 	int ret;
 
 	vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL);
@@ -2031,18 +2029,10 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 	/* Only use the GPIO HPD pin if present in the DT, otherwise
 	 * we'll use the HDMI core's register.
 	 */
-	if (of_find_property(dev->of_node, "hpd-gpios", &value)) {
-		enum of_gpio_flags hpd_gpio_flags;
-
-		vc4_hdmi->hpd_gpio = of_get_named_gpio_flags(dev->of_node,
-							     "hpd-gpios", 0,
-							     &hpd_gpio_flags);
-		if (vc4_hdmi->hpd_gpio < 0) {
-			ret = vc4_hdmi->hpd_gpio;
-			goto err_put_ddc;
-		}
-
-		vc4_hdmi->hpd_active_low = hpd_gpio_flags & OF_GPIO_ACTIVE_LOW;
+	vc4_hdmi->hpd_gpio = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN);
+	if (IS_ERR(vc4_hdmi->hpd_gpio)) {
+		ret = PTR_ERR(vc4_hdmi->hpd_gpio);
+		goto err_put_ddc;
 	}
 
 	vc4_hdmi->disable_wifi_frequencies =
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 060bcaefbeb5..2688a55461d6 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -146,8 +146,7 @@ struct vc4_hdmi {
 	/* VC5 Only */
 	void __iomem *rm_regs;
 
-	int hpd_gpio;
-	bool hpd_active_low;
+	struct gpio_desc *hpd_gpio;
 
 	/*
 	 * On some systems (like the RPi4), some modes are in the same
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod
  2021-05-24 13:18 ` [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod Maxime Ripard
@ 2021-05-27 23:57   ` Linus Walleij
  2021-06-04  8:01     ` Maxime Ripard
  2021-07-02  3:29   ` Nathan Chancellor
  1 sibling, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2021-05-27 23:57 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: open list:DRM PANEL DRIVERS, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Thomas Zimmermann, Eric Anholt, linux-kernel,
	Dave Stevenson, Phil Elwell, Tim Gover, Dom Cobley,
	Daniel Vetter, Hans Verkuil, Maxime Ripard

On Mon, May 24, 2021 at 3:19 PM Maxime Ripard <maxime@cerno.tech> wrote:

> The new gpiod interface takes care of parsing the GPIO flags and to
> return the logical value when accessing an active-low GPIO, so switching
> to it simplifies a lot the driver.
>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Thanks for fixing this!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod
  2021-05-27 23:57   ` Linus Walleij
@ 2021-06-04  8:01     ` Maxime Ripard
  2021-06-04 21:45       ` Linus Walleij
  0 siblings, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2021-06-04  8:01 UTC (permalink / raw)
  To: Linus Walleij
  Cc: open list:DRM PANEL DRIVERS, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Thomas Zimmermann, Eric Anholt, linux-kernel,
	Dave Stevenson, Phil Elwell, Tim Gover, Dom Cobley,
	Daniel Vetter, Hans Verkuil

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

Hi Linus

On Fri, May 28, 2021 at 01:57:56AM +0200, Linus Walleij wrote:
> On Mon, May 24, 2021 at 3:19 PM Maxime Ripard <maxime@cerno.tech> wrote:
> 
> > The new gpiod interface takes care of parsing the GPIO flags and to
> > return the logical value when accessing an active-low GPIO, so switching
> > to it simplifies a lot the driver.
> >
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> 
> Thanks for fixing this!
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Is it for both patches or just this one?

Thanks!
Maxime

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios
  2021-05-24 13:18 [PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios Maxime Ripard
  2021-05-24 13:18 ` [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod Maxime Ripard
@ 2021-06-04 21:44 ` Linus Walleij
  1 sibling, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2021-06-04 21:44 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: open list:DRM PANEL DRIVERS, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Thomas Zimmermann, Eric Anholt, linux-kernel,
	Dave Stevenson, Phil Elwell, Tim Gover, Dom Cobley,
	Daniel Vetter, Hans Verkuil, Maxime Ripard

On Mon, May 24, 2021 at 3:19 PM Maxime Ripard <maxime@cerno.tech> wrote:

> If the of_get_named_gpio_flags call fails in vc4_hdmi_bind, we jump to
> the err_unprepare_hsm label. That label will then call
> pm_runtime_disable and put_device on the DDC device.
>
> We just retrieved the DDC device, so the latter is definitely justified.
> However at that point we still haven't called pm_runtime_enable, so the
> call to pm_runtime_disable is not supposed to be there.
>
> Fixes: 10ee275cb12f ("drm/vc4: prepare for CEC support")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod
  2021-06-04  8:01     ` Maxime Ripard
@ 2021-06-04 21:45       ` Linus Walleij
  2021-06-07  7:32         ` Maxime Ripard
  0 siblings, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2021-06-04 21:45 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: open list:DRM PANEL DRIVERS, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Thomas Zimmermann, Eric Anholt, linux-kernel,
	Dave Stevenson, Phil Elwell, Tim Gover, Dom Cobley,
	Daniel Vetter, Hans Verkuil

On Fri, Jun 4, 2021 at 10:01 AM Maxime Ripard <maxime@cerno.tech> wrote:
> On Fri, May 28, 2021 at 01:57:56AM +0200, Linus Walleij wrote:
> > On Mon, May 24, 2021 at 3:19 PM Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > > The new gpiod interface takes care of parsing the GPIO flags and to
> > > return the logical value when accessing an active-low GPIO, so switching
> > > to it simplifies a lot the driver.
> > >
> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> >
> > Thanks for fixing this!
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Is it for both patches or just this one?

I went and added Reviewed-by: to 1/2 as well so you can merge the
patches. Was simple enough even though I'm not a VC4 expert.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod
  2021-06-04 21:45       ` Linus Walleij
@ 2021-06-07  7:32         ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-07  7:32 UTC (permalink / raw)
  To: Linus Walleij
  Cc: open list:DRM PANEL DRIVERS, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Thomas Zimmermann, Eric Anholt, linux-kernel,
	Dave Stevenson, Phil Elwell, Tim Gover, Dom Cobley,
	Daniel Vetter, Hans Verkuil

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

On Fri, Jun 04, 2021 at 11:45:36PM +0200, Linus Walleij wrote:
> On Fri, Jun 4, 2021 at 10:01 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > On Fri, May 28, 2021 at 01:57:56AM +0200, Linus Walleij wrote:
> > > On Mon, May 24, 2021 at 3:19 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > >
> > > > The new gpiod interface takes care of parsing the GPIO flags and to
> > > > return the logical value when accessing an active-low GPIO, so switching
> > > > to it simplifies a lot the driver.
> > > >
> > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > >
> > > Thanks for fixing this!
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> >
> > Is it for both patches or just this one?
> 
> I went and added Reviewed-by: to 1/2 as well so you can merge the
> patches. Was simple enough even though I'm not a VC4 expert.

Thanks :)

Applied both

Maxime

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod
  2021-05-24 13:18 ` [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod Maxime Ripard
  2021-05-27 23:57   ` Linus Walleij
@ 2021-07-02  3:29   ` Nathan Chancellor
  2021-07-02 13:16     ` Maxime Ripard
  1 sibling, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2021-07-02  3:29 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Eric Anholt, linux-kernel, Linus Wallei,
	Dave Stevenson, Phil Elwell, Tim Gover, Dom Cobley,
	Daniel Vetter, Hans Verkuil, Maxime Ripard

On Mon, May 24, 2021 at 03:18:52PM +0200, Maxime Ripard wrote:
> The new gpiod interface takes care of parsing the GPIO flags and to
> return the logical value when accessing an active-low GPIO, so switching
> to it simplifies a lot the driver.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 24 +++++++-----------------
>  drivers/gpu/drm/vc4/vc4_hdmi.h |  3 +--
>  2 files changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index ccc6c8079dc6..34622c59f6a7 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -159,10 +159,9 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
>  	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
>  	bool connected = false;
>  
> -	if (vc4_hdmi->hpd_gpio) {
> -		if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^
> -		    vc4_hdmi->hpd_active_low)
> -			connected = true;
> +	if (vc4_hdmi->hpd_gpio &&
> +	    gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
> +		connected = true;
>  	} else if (drm_probe_ddc(vc4_hdmi->ddc)) {
>  		connected = true;
>  	} else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) {
> @@ -1993,7 +1992,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
>  	struct vc4_hdmi *vc4_hdmi;
>  	struct drm_encoder *encoder;
>  	struct device_node *ddc_node;
> -	u32 value;
>  	int ret;
>  
>  	vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL);
> @@ -2031,18 +2029,10 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
>  	/* Only use the GPIO HPD pin if present in the DT, otherwise
>  	 * we'll use the HDMI core's register.
>  	 */
> -	if (of_find_property(dev->of_node, "hpd-gpios", &value)) {
> -		enum of_gpio_flags hpd_gpio_flags;
> -
> -		vc4_hdmi->hpd_gpio = of_get_named_gpio_flags(dev->of_node,
> -							     "hpd-gpios", 0,
> -							     &hpd_gpio_flags);
> -		if (vc4_hdmi->hpd_gpio < 0) {
> -			ret = vc4_hdmi->hpd_gpio;
> -			goto err_put_ddc;
> -		}
> -
> -		vc4_hdmi->hpd_active_low = hpd_gpio_flags & OF_GPIO_ACTIVE_LOW;
> +	vc4_hdmi->hpd_gpio = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN);
> +	if (IS_ERR(vc4_hdmi->hpd_gpio)) {
> +		ret = PTR_ERR(vc4_hdmi->hpd_gpio);
> +		goto err_put_ddc;
>  	}
>  
>  	vc4_hdmi->disable_wifi_frequencies =
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> index 060bcaefbeb5..2688a55461d6 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> @@ -146,8 +146,7 @@ struct vc4_hdmi {
>  	/* VC5 Only */
>  	void __iomem *rm_regs;
>  
> -	int hpd_gpio;
> -	bool hpd_active_low;
> +	struct gpio_desc *hpd_gpio;
>  
>  	/*
>  	 * On some systems (like the RPi4), some modes are in the same
> -- 
> 2.31.1

Hi Maxime,

This patch as commit 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod")
causes my Raspberry Pi 3 to lock up shortly after boot in combination
with commit 411efa18e4b0 ("drm/vc4: hdmi: Move the HSM clock enable to
runtime_pm"). The serial console and ssh are completely unresponsive and
I do not see any messages in dmesg with "debug ignore_loglevel". The
device is running with a 32-bit kernel (multi_v7_defconfig) with 32-bit
userspace. If there is any further information that I can provide,
please let me know.

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod
  2021-07-02  3:29   ` Nathan Chancellor
@ 2021-07-02 13:16     ` Maxime Ripard
  2021-07-03  5:01       ` Nathan Chancellor
  0 siblings, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2021-07-02 13:16 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Eric Anholt, linux-kernel, Linus Wallei,
	Dave Stevenson, Phil Elwell, Tim Gover, Dom Cobley,
	Daniel Vetter, Hans Verkuil

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

Hi Nathan,

On Thu, Jul 01, 2021 at 08:29:34PM -0700, Nathan Chancellor wrote:
> On Mon, May 24, 2021 at 03:18:52PM +0200, Maxime Ripard wrote:
> > The new gpiod interface takes care of parsing the GPIO flags and to
> > return the logical value when accessing an active-low GPIO, so switching
> > to it simplifies a lot the driver.
> > 
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> >  drivers/gpu/drm/vc4/vc4_hdmi.c | 24 +++++++-----------------
> >  drivers/gpu/drm/vc4/vc4_hdmi.h |  3 +--
> >  2 files changed, 8 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index ccc6c8079dc6..34622c59f6a7 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -159,10 +159,9 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
> >  	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
> >  	bool connected = false;
> >  
> > -	if (vc4_hdmi->hpd_gpio) {
> > -		if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^
> > -		    vc4_hdmi->hpd_active_low)
> > -			connected = true;
> > +	if (vc4_hdmi->hpd_gpio &&
> > +	    gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
> > +		connected = true;
> >  	} else if (drm_probe_ddc(vc4_hdmi->ddc)) {
> >  		connected = true;
> >  	} else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) {
> > @@ -1993,7 +1992,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
> >  	struct vc4_hdmi *vc4_hdmi;
> >  	struct drm_encoder *encoder;
> >  	struct device_node *ddc_node;
> > -	u32 value;
> >  	int ret;
> >  
> >  	vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL);
> > @@ -2031,18 +2029,10 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
> >  	/* Only use the GPIO HPD pin if present in the DT, otherwise
> >  	 * we'll use the HDMI core's register.
> >  	 */
> > -	if (of_find_property(dev->of_node, "hpd-gpios", &value)) {
> > -		enum of_gpio_flags hpd_gpio_flags;
> > -
> > -		vc4_hdmi->hpd_gpio = of_get_named_gpio_flags(dev->of_node,
> > -							     "hpd-gpios", 0,
> > -							     &hpd_gpio_flags);
> > -		if (vc4_hdmi->hpd_gpio < 0) {
> > -			ret = vc4_hdmi->hpd_gpio;
> > -			goto err_put_ddc;
> > -		}
> > -
> > -		vc4_hdmi->hpd_active_low = hpd_gpio_flags & OF_GPIO_ACTIVE_LOW;
> > +	vc4_hdmi->hpd_gpio = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN);
> > +	if (IS_ERR(vc4_hdmi->hpd_gpio)) {
> > +		ret = PTR_ERR(vc4_hdmi->hpd_gpio);
> > +		goto err_put_ddc;
> >  	}
> >  
> >  	vc4_hdmi->disable_wifi_frequencies =
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> > index 060bcaefbeb5..2688a55461d6 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> > @@ -146,8 +146,7 @@ struct vc4_hdmi {
> >  	/* VC5 Only */
> >  	void __iomem *rm_regs;
> >  
> > -	int hpd_gpio;
> > -	bool hpd_active_low;
> > +	struct gpio_desc *hpd_gpio;
> >  
> >  	/*
> >  	 * On some systems (like the RPi4), some modes are in the same
> > -- 
> > 2.31.1
> 
> This patch as commit 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod")
> causes my Raspberry Pi 3 to lock up shortly after boot in combination
> with commit 411efa18e4b0 ("drm/vc4: hdmi: Move the HSM clock enable to
> runtime_pm"). The serial console and ssh are completely unresponsive and
> I do not see any messages in dmesg with "debug ignore_loglevel". The
> device is running with a 32-bit kernel (multi_v7_defconfig) with 32-bit
> userspace. If there is any further information that I can provide,
> please let me know.

Thanks for reporting this. The same bug has been reported on wednesday
on the RPi repo here:
https://github.com/raspberrypi/linux/pull/4418

More specifically, this commit should fix it:
https://github.com/raspberrypi/linux/pull/4418/commits/6d404373c20a794da3d6a7b4f1373903183bb5d0

Even though it's based on the 5.10 kernel, it should apply without any
warning on a mainline tree. Let me know if it fixes your issue too

Maxime

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod
  2021-07-02 13:16     ` Maxime Ripard
@ 2021-07-03  5:01       ` Nathan Chancellor
  0 siblings, 0 replies; 10+ messages in thread
From: Nathan Chancellor @ 2021-07-03  5:01 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Eric Anholt, linux-kernel, Linus Wallei,
	Dave Stevenson, Phil Elwell, Tim Gover, Dom Cobley,
	Daniel Vetter, Hans Verkuil

On Fri, Jul 02, 2021 at 03:16:46PM +0200, Maxime Ripard wrote:
> Hi Nathan,
> 
> On Thu, Jul 01, 2021 at 08:29:34PM -0700, Nathan Chancellor wrote:
> > On Mon, May 24, 2021 at 03:18:52PM +0200, Maxime Ripard wrote:
> > > The new gpiod interface takes care of parsing the GPIO flags and to
> > > return the logical value when accessing an active-low GPIO, so switching
> > > to it simplifies a lot the driver.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > ---
> > >  drivers/gpu/drm/vc4/vc4_hdmi.c | 24 +++++++-----------------
> > >  drivers/gpu/drm/vc4/vc4_hdmi.h |  3 +--
> > >  2 files changed, 8 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > index ccc6c8079dc6..34622c59f6a7 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > @@ -159,10 +159,9 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
> > >  	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
> > >  	bool connected = false;
> > >  
> > > -	if (vc4_hdmi->hpd_gpio) {
> > > -		if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^
> > > -		    vc4_hdmi->hpd_active_low)
> > > -			connected = true;
> > > +	if (vc4_hdmi->hpd_gpio &&
> > > +	    gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
> > > +		connected = true;
> > >  	} else if (drm_probe_ddc(vc4_hdmi->ddc)) {
> > >  		connected = true;
> > >  	} else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) {
> > > @@ -1993,7 +1992,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
> > >  	struct vc4_hdmi *vc4_hdmi;
> > >  	struct drm_encoder *encoder;
> > >  	struct device_node *ddc_node;
> > > -	u32 value;
> > >  	int ret;
> > >  
> > >  	vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL);
> > > @@ -2031,18 +2029,10 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
> > >  	/* Only use the GPIO HPD pin if present in the DT, otherwise
> > >  	 * we'll use the HDMI core's register.
> > >  	 */
> > > -	if (of_find_property(dev->of_node, "hpd-gpios", &value)) {
> > > -		enum of_gpio_flags hpd_gpio_flags;
> > > -
> > > -		vc4_hdmi->hpd_gpio = of_get_named_gpio_flags(dev->of_node,
> > > -							     "hpd-gpios", 0,
> > > -							     &hpd_gpio_flags);
> > > -		if (vc4_hdmi->hpd_gpio < 0) {
> > > -			ret = vc4_hdmi->hpd_gpio;
> > > -			goto err_put_ddc;
> > > -		}
> > > -
> > > -		vc4_hdmi->hpd_active_low = hpd_gpio_flags & OF_GPIO_ACTIVE_LOW;
> > > +	vc4_hdmi->hpd_gpio = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN);
> > > +	if (IS_ERR(vc4_hdmi->hpd_gpio)) {
> > > +		ret = PTR_ERR(vc4_hdmi->hpd_gpio);
> > > +		goto err_put_ddc;
> > >  	}
> > >  
> > >  	vc4_hdmi->disable_wifi_frequencies =
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> > > index 060bcaefbeb5..2688a55461d6 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> > > @@ -146,8 +146,7 @@ struct vc4_hdmi {
> > >  	/* VC5 Only */
> > >  	void __iomem *rm_regs;
> > >  
> > > -	int hpd_gpio;
> > > -	bool hpd_active_low;
> > > +	struct gpio_desc *hpd_gpio;
> > >  
> > >  	/*
> > >  	 * On some systems (like the RPi4), some modes are in the same
> > > -- 
> > > 2.31.1
> > 
> > This patch as commit 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod")
> > causes my Raspberry Pi 3 to lock up shortly after boot in combination
> > with commit 411efa18e4b0 ("drm/vc4: hdmi: Move the HSM clock enable to
> > runtime_pm"). The serial console and ssh are completely unresponsive and
> > I do not see any messages in dmesg with "debug ignore_loglevel". The
> > device is running with a 32-bit kernel (multi_v7_defconfig) with 32-bit
> > userspace. If there is any further information that I can provide,
> > please let me know.
> 
> Thanks for reporting this. The same bug has been reported on wednesday
> on the RPi repo here:
> https://github.com/raspberrypi/linux/pull/4418
> 
> More specifically, this commit should fix it:
> https://github.com/raspberrypi/linux/pull/4418/commits/6d404373c20a794da3d6a7b4f1373903183bb5d0
> 
> Even though it's based on the 5.10 kernel, it should apply without any
> warning on a mainline tree. Let me know if it fixes your issue too

Thank you for the links and the quick reply. Unfortunately, I applied
this patch on top of commit d6b63b5b7d7f ("Merge tag 'sound-5.14-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound") in mainline,
which does reproduce this issue still and it did not fix the issue. In
fact, it did not even get to the raspberrypi login prompt before it
locked up, yet again without any real output in the serial console
except for maybe this message?

[    7.582480] vc4-drm soc:gpu: bound 3f400000.hvs (ops vc4_hvs_ops [vc4])

If there is anything further that I can provide or test, please let me
know!

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-07-03  5:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-24 13:18 [PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios Maxime Ripard
2021-05-24 13:18 ` [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod Maxime Ripard
2021-05-27 23:57   ` Linus Walleij
2021-06-04  8:01     ` Maxime Ripard
2021-06-04 21:45       ` Linus Walleij
2021-06-07  7:32         ` Maxime Ripard
2021-07-02  3:29   ` Nathan Chancellor
2021-07-02 13:16     ` Maxime Ripard
2021-07-03  5:01       ` Nathan Chancellor
2021-06-04 21:44 ` [PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).