[PATCH 1/2] drm/bridge: lt9611: Consolidate detection logic

[PATCH 1/2] drm/bridge: lt9611: Consolidate detection logic

[John Stultz]

This patch simply consolidates the duplicated detection
functionality in the driver.

Cc: Yongqin Liu <yongqin.liu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Peter Collingbourne <pcc@google.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Robert Foss <robert.foss@linaro.org>
Cc: kernel-team@android.com
Reviewed-by: Robert Foss <robert.foss@linaro.org>
Signed-off-by: John Stultz <jstultz@google.com>
---
 drivers/gpu/drm/bridge/lontium-lt9611.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
index 63df2e8a8abc..bf66af668f61 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
@@ -578,10 +578,8 @@ static struct lt9611_mode *lt9611_find_mode(const struct drm_display_mode *mode)
 }
 
 /* connector funcs */
-static enum drm_connector_status
-lt9611_connector_detect(struct drm_connector *connector, bool force)
+static enum drm_connector_status __lt9611_detect(struct lt9611 *lt9611)
 {
-	struct lt9611 *lt9611 = connector_to_lt9611(connector);
 	unsigned int reg_val = 0;
 	int connected = 0;
 
@@ -594,6 +592,12 @@ lt9611_connector_detect(struct drm_connector *connector, bool force)
 	return lt9611->status;
 }
 
+static enum drm_connector_status
+lt9611_connector_detect(struct drm_connector *connector, bool force)
+{
+	return __lt9611_detect(connector_to_lt9611(connector));
+}
+
 static int lt9611_read_edid(struct lt9611 *lt9611)
 {
 	unsigned int temp;
@@ -887,17 +891,7 @@ static void lt9611_bridge_mode_set(struct drm_bridge *bridge,
 
 static enum drm_connector_status lt9611_bridge_detect(struct drm_bridge *bridge)
 {
-	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
-	unsigned int reg_val = 0;
-	int connected;
-
-	regmap_read(lt9611->regmap, 0x825e, &reg_val);
-	connected  = reg_val & BIT(0);
-
-	lt9611->status = connected ?  connector_status_connected :
-				connector_status_disconnected;
-
-	return lt9611->status;
+	return __lt9611_detect(bridge_to_lt9611(bridge));
 }
 
 static struct edid *lt9611_bridge_get_edid(struct drm_bridge *bridge,
-- 
2.36.0.512.ge40c2bad7a-goog

[PATCH 2/2] drm/bridge: lt9611: Use both bits for HDMI sensing

[John Stultz]

In commit 19cf41b64e3b ("lontium-lt9611: check a different
register bit for HDMI sensing"), the bit flag used to detect
HDMI cable connect was switched from BIT(2) to BIT(0) to improve
compatibility with some monitors that didn't seem to set BIT(2).

However, with that change, I've seen occasional issues where the
detection failed, because BIT(2) was set, but not BIT(0).

Unfortunately, as I understand it, the bits and their function
was never clearly documented. So lets instead check both
(BIT(2) | BIT(0)) when checking the register.

Cc: Yongqin Liu <yongqin.liu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Peter Collingbourne <pcc@google.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Robert Foss <robert.foss@linaro.org>
Cc: kernel-team@android.com
Fixes: 19cf41b64e3b ("lontium-lt9611: check a different register bit for HDMI sensing")
Signed-off-by: John Stultz <jstultz@google.com>
---
 drivers/gpu/drm/bridge/lontium-lt9611.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
index bf66af668f61..a11604a56e08 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
@@ -584,7 +584,7 @@ static enum drm_connector_status __lt9611_detect(struct lt9611 *lt9611)
 	int connected = 0;
 
 	regmap_read(lt9611->regmap, 0x825e, &reg_val);
-	connected  = (reg_val & BIT(0));
+	connected  = (reg_val & (BIT(2) | BIT(0)));
 
 	lt9611->status = connected ?  connector_status_connected :
 				connector_status_disconnected;
-- 
2.36.0.512.ge40c2bad7a-goog

Re: [PATCH 2/2] drm/bridge: lt9611: Use both bits for HDMI sensing

[Robert Foss]

On Wed, 11 May 2022 at 03:26, John Stultz <jstultz@google.com> wrote:
>
> In commit 19cf41b64e3b ("lontium-lt9611: check a different
> register bit for HDMI sensing"), the bit flag used to detect
> HDMI cable connect was switched from BIT(2) to BIT(0) to improve
> compatibility with some monitors that didn't seem to set BIT(2).
>
> However, with that change, I've seen occasional issues where the
> detection failed, because BIT(2) was set, but not BIT(0).

I was waiting for a review or testing from a previous lt9611
committer, but seeing as how functionality is being re-introduced in
this patch I think a code review is all that's needed.

>
> Unfortunately, as I understand it, the bits and their function
> was never clearly documented. So lets instead check both
> (BIT(2) | BIT(0)) when checking the register.
>
> Cc: Yongqin Liu <yongqin.liu@linaro.org>
> Cc: Amit Pundir <amit.pundir@linaro.org>
> Cc: Peter Collingbourne <pcc@google.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Robert Foss <robert.foss@linaro.org>
> Cc: kernel-team@android.com
> Fixes: 19cf41b64e3b ("lontium-lt9611: check a different register bit for HDMI sensing")
> Signed-off-by: John Stultz <jstultz@google.com>
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
> index bf66af668f61..a11604a56e08 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
> @@ -584,7 +584,7 @@ static enum drm_connector_status __lt9611_detect(struct lt9611 *lt9611)
>         int connected = 0;
>
>         regmap_read(lt9611->regmap, 0x825e, &reg_val);
> -       connected  = (reg_val & BIT(0));
> +       connected  = (reg_val & (BIT(2) | BIT(0)));
>
>         lt9611->status = connected ?  connector_status_connected :
>                                 connector_status_disconnected;
> --
> 2.36.0.512.ge40c2bad7a-goog
>

I reviewed this series and it looks good.

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

Applied to drm-misc-next.