drm: bridge: adv7511: Fix ADV7535 HPD enablement

Message ID 20220109172949.168167-1-jagan@amarulasolutions.com
State New
Headers show
Series
  • drm: bridge: adv7511: Fix ADV7535 HPD enablement
Related show

Commit Message

Jagan Teki Jan. 9, 2022, 5:29 p.m. UTC
Existing HPD enablement logic is not compatible with ADV7535
bridge, thus any runtime plug-in of HDMI cable is not working
on these bridge designs.

Unlike other ADV7511 family of bridges, the ADV7535 require
HPD_OVERRIDE bit to set and reset for proper handling of HPD
functionality.

Fix it.

Fixes: 8501fe4b14a3 ("drm: bridge: adv7511: Add support for ADV7535")
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/gpu/drm/bridge/adv7511/adv7511.h     |  1 +
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 29 +++++++++++++++-----
 2 files changed, 23 insertions(+), 7 deletions(-)

Comments

Robert Foss Jan. 10, 2022, 11:07 a.m. UTC | #1
Hey Jagan,

Thanks for submitting this fix.

On Sun, 9 Jan 2022 at 18:30, Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Existing HPD enablement logic is not compatible with ADV7535
> bridge, thus any runtime plug-in of HDMI cable is not working
> on these bridge designs.
>
> Unlike other ADV7511 family of bridges, the ADV7535 require
> HPD_OVERRIDE bit to set and reset for proper handling of HPD
> functionality.
>
> Fix it.
>
> Fixes: 8501fe4b14a3 ("drm: bridge: adv7511: Add support for ADV7535")
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511.h     |  1 +
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 29 +++++++++++++++-----
>  2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index 592ecfcf00ca..6a882891d91c 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -169,6 +169,7 @@
>  #define ADV7511_PACKET_ENABLE_SPARE2           BIT(1)
>  #define ADV7511_PACKET_ENABLE_SPARE1           BIT(0)
>
> +#define ADV7535_REG_POWER2_HPD_OVERRIDE                BIT(6)
>  #define ADV7511_REG_POWER2_HPD_SRC_MASK                0xc0
>  #define ADV7511_REG_POWER2_HPD_SRC_BOTH                0x00
>  #define ADV7511_REG_POWER2_HPD_SRC_HPD         0x40
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index f8e5da148599..77118c3395bf 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -351,11 +351,17 @@ static void __adv7511_power_on(struct adv7511 *adv7511)
>          * from standby or are enabled. When the HPD goes low the adv7511 is
>          * reset and the outputs are disabled which might cause the monitor to
>          * go to standby again. To avoid this we ignore the HPD pin for the
> -        * first few seconds after enabling the output.
> +        * first few seconds after enabling the output. On the other hand
> +        * adv7535 require to enable HPD Override bit for proper HPD.
>          */
> -       regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
> -                          ADV7511_REG_POWER2_HPD_SRC_MASK,
> -                          ADV7511_REG_POWER2_HPD_SRC_NONE);
> +       if (adv7511->type == ADV7535)
> +               regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
> +                                  ADV7535_REG_POWER2_HPD_OVERRIDE,
> +                                  ADV7535_REG_POWER2_HPD_OVERRIDE);
> +       else
> +               regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
> +                                  ADV7511_REG_POWER2_HPD_SRC_MASK,
> +                                  ADV7511_REG_POWER2_HPD_SRC_NONE);
>  }
>
>  static void adv7511_power_on(struct adv7511 *adv7511)
> @@ -375,6 +381,10 @@ static void adv7511_power_on(struct adv7511 *adv7511)
>  static void __adv7511_power_off(struct adv7511 *adv7511)
>  {
>         /* TODO: setup additional power down modes */
> +       if (adv7511->type == ADV7535)
> +               regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
> +                                  ADV7535_REG_POWER2_HPD_OVERRIDE, 0);
> +
>         regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
>                            ADV7511_POWER_POWER_DOWN,
>                            ADV7511_POWER_POWER_DOWN);
> @@ -672,9 +682,14 @@ adv7511_detect(struct adv7511 *adv7511, struct drm_connector *connector)
>                         status = connector_status_disconnected;
>         } else {
>                 /* Renable HPD sensing */
> -               regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
> -                                  ADV7511_REG_POWER2_HPD_SRC_MASK,
> -                                  ADV7511_REG_POWER2_HPD_SRC_BOTH);
> +               if (adv7511->type == ADV7535)
> +                       regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
> +                                          ADV7535_REG_POWER2_HPD_OVERRIDE,
> +                                          ADV7535_REG_POWER2_HPD_OVERRIDE);
> +               else
> +                       regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
> +                                          ADV7511_REG_POWER2_HPD_SRC_MASK,
> +                                          ADV7511_REG_POWER2_HPD_SRC_BOTH);
>         }
>
>         adv7511->status = status;
> --
> 2.25.1
>

Reviewed-by: Robert Foss  <robert.foss@linaro.org>

Applied to drm-misc-next

Patch

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 592ecfcf00ca..6a882891d91c 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -169,6 +169,7 @@ 
 #define ADV7511_PACKET_ENABLE_SPARE2		BIT(1)
 #define ADV7511_PACKET_ENABLE_SPARE1		BIT(0)
 
+#define ADV7535_REG_POWER2_HPD_OVERRIDE		BIT(6)
 #define ADV7511_REG_POWER2_HPD_SRC_MASK		0xc0
 #define ADV7511_REG_POWER2_HPD_SRC_BOTH		0x00
 #define ADV7511_REG_POWER2_HPD_SRC_HPD		0x40
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index f8e5da148599..77118c3395bf 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -351,11 +351,17 @@  static void __adv7511_power_on(struct adv7511 *adv7511)
 	 * from standby or are enabled. When the HPD goes low the adv7511 is
 	 * reset and the outputs are disabled which might cause the monitor to
 	 * go to standby again. To avoid this we ignore the HPD pin for the
-	 * first few seconds after enabling the output.
+	 * first few seconds after enabling the output. On the other hand
+	 * adv7535 require to enable HPD Override bit for proper HPD.
 	 */
-	regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
-			   ADV7511_REG_POWER2_HPD_SRC_MASK,
-			   ADV7511_REG_POWER2_HPD_SRC_NONE);
+	if (adv7511->type == ADV7535)
+		regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
+				   ADV7535_REG_POWER2_HPD_OVERRIDE,
+				   ADV7535_REG_POWER2_HPD_OVERRIDE);
+	else
+		regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
+				   ADV7511_REG_POWER2_HPD_SRC_MASK,
+				   ADV7511_REG_POWER2_HPD_SRC_NONE);
 }
 
 static void adv7511_power_on(struct adv7511 *adv7511)
@@ -375,6 +381,10 @@  static void adv7511_power_on(struct adv7511 *adv7511)
 static void __adv7511_power_off(struct adv7511 *adv7511)
 {
 	/* TODO: setup additional power down modes */
+	if (adv7511->type == ADV7535)
+		regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
+				   ADV7535_REG_POWER2_HPD_OVERRIDE, 0);
+
 	regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
 			   ADV7511_POWER_POWER_DOWN,
 			   ADV7511_POWER_POWER_DOWN);
@@ -672,9 +682,14 @@  adv7511_detect(struct adv7511 *adv7511, struct drm_connector *connector)
 			status = connector_status_disconnected;
 	} else {
 		/* Renable HPD sensing */
-		regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
-				   ADV7511_REG_POWER2_HPD_SRC_MASK,
-				   ADV7511_REG_POWER2_HPD_SRC_BOTH);
+		if (adv7511->type == ADV7535)
+			regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
+					   ADV7535_REG_POWER2_HPD_OVERRIDE,
+					   ADV7535_REG_POWER2_HPD_OVERRIDE);
+		else
+			regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2,
+					   ADV7511_REG_POWER2_HPD_SRC_MASK,
+					   ADV7511_REG_POWER2_HPD_SRC_BOTH);
 	}
 
 	adv7511->status = status;