[PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios

[PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios

[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

[PATCH 2/2] drm/vc4: hdmi: Convert to gpiod

[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

Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod

[Linus Walleij]

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

Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod

[Maxime Ripard]

[-- 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 --]

Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod

[Linus Walleij]

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

Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod

[Maxime Ripard]

[-- 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 --]

Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod

[Nathan Chancellor]

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

Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod

[Maxime Ripard]

[-- 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 --]

Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod

[Nathan Chancellor]

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

Re: [PATCH 1/2] drm/vc4: hdmi: Fix error path of hpd-gpios

[Linus Walleij]

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