[PATCH v2 0/3] drm/bridge/lontium-lt9611uxc: fix handling of EDID/HPD

[PATCH v2 0/3] drm/bridge/lontium-lt9611uxc: fix handling of EDID/HPD

[Dmitry Baryshkov]

These three patches provide fixes for HPD handling and EDID readout for
Lontium lt9611uxc DSI-to-HDMI bridge driver.

Dmitry Baryshkov (3):
      drm/bridge/lontium-lt9611uxc: fix waiting for EDID to become available
      drm/bridge/lontium-lt9611uxc: fix get_edid return code
      drm/bridge/lontium-lt9611uxc: move HPD notification out of IRQ handler

 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 41 +++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 9 deletions(-)

Changes since v1:
 - Split first patch into two smaller patches
 - Add Fixes tags

[PATCH v2 1/3] drm/bridge/lontium-lt9611uxc: fix waiting for EDID to become available

[Dmitry Baryshkov]

- Call wake_up() when EDID ready event is received to wake
  wait_event_interruptible_timeout()

- Increase waiting timeout, reading EDID can take longer than 100ms, so
  let's be on a safe side.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Fixes: 0cbbd5b1a012 ("drm: bridge: add support for lontium LT9611UXC bridge")
---
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index 0c98d27f84ac..a59e811f1705 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -145,8 +145,10 @@ static irqreturn_t lt9611uxc_irq_thread_handler(int irq, void *dev_id)
 
 	lt9611uxc_unlock(lt9611uxc);
 
-	if (irq_status & BIT(0))
+	if (irq_status & BIT(0)) {
 		lt9611uxc->edid_read = !!(hpd_status & BIT(0));
+		wake_up_all(&lt9611uxc->wq);
+	}
 
 	if (irq_status & BIT(1)) {
 		if (lt9611uxc->connector.dev)
@@ -465,7 +467,7 @@ static enum drm_connector_status lt9611uxc_bridge_detect(struct drm_bridge *brid
 static int lt9611uxc_wait_for_edid(struct lt9611uxc *lt9611uxc)
 {
 	return wait_event_interruptible_timeout(lt9611uxc->wq, lt9611uxc->edid_read,
-			msecs_to_jiffies(100));
+			msecs_to_jiffies(500));
 }
 
 static int lt9611uxc_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len)
-- 
2.29.2

Re: [PATCH v2 1/3] drm/bridge/lontium-lt9611uxc: fix waiting for EDID to become available

[Bjorn Andersson]

On Fri 15 Jan 05:02 CST 2021, Dmitry Baryshkov wrote:

> - Call wake_up() when EDID ready event is received to wake
>   wait_event_interruptible_timeout()
> 
> - Increase waiting timeout, reading EDID can take longer than 100ms, so
>   let's be on a safe side.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Fixes: 0cbbd5b1a012 ("drm: bridge: add support for lontium LT9611UXC bridge")
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> index 0c98d27f84ac..a59e811f1705 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> @@ -145,8 +145,10 @@ static irqreturn_t lt9611uxc_irq_thread_handler(int irq, void *dev_id)
>  
>  	lt9611uxc_unlock(lt9611uxc);
>  
> -	if (irq_status & BIT(0))
> +	if (irq_status & BIT(0)) {
>  		lt9611uxc->edid_read = !!(hpd_status & BIT(0));
> +		wake_up_all(&lt9611uxc->wq);
> +	}
>  
>  	if (irq_status & BIT(1)) {
>  		if (lt9611uxc->connector.dev)
> @@ -465,7 +467,7 @@ static enum drm_connector_status lt9611uxc_bridge_detect(struct drm_bridge *brid
>  static int lt9611uxc_wait_for_edid(struct lt9611uxc *lt9611uxc)
>  {
>  	return wait_event_interruptible_timeout(lt9611uxc->wq, lt9611uxc->edid_read,
> -			msecs_to_jiffies(100));
> +			msecs_to_jiffies(500));
>  }
>  
>  static int lt9611uxc_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len)
> -- 
> 2.29.2
> 

[PATCH v2 2/3] drm/bridge/lontium-lt9611uxc: fix get_edid return code

[Dmitry Baryshkov]

Return NULL pointer from get_edid() callback rather than ERR_PTR()
pointer, as DRM code does NULL checks rather than IS_ERR(). Also while
we are at it, return NULL if getting EDID timed out.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Fixes: 0cbbd5b1a012 ("drm: bridge: add support for lontium LT9611UXC bridge")
---
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index a59e811f1705..b708700e182d 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -505,7 +505,10 @@ static struct edid *lt9611uxc_bridge_get_edid(struct drm_bridge *bridge,
 	ret = lt9611uxc_wait_for_edid(lt9611uxc);
 	if (ret < 0) {
 		dev_err(lt9611uxc->dev, "wait for EDID failed: %d\n", ret);
-		return ERR_PTR(ret);
+		return NULL;
+	} else if (ret == 0) {
+		dev_err(lt9611uxc->dev, "wait for EDID timeout\n");
+		return NULL;
 	}
 
 	return drm_do_get_edid(connector, lt9611uxc_get_edid_block, lt9611uxc);
-- 
2.29.2

Re: [PATCH v2 2/3] drm/bridge/lontium-lt9611uxc: fix get_edid return code

[Bjorn Andersson]

On Fri 15 Jan 05:02 CST 2021, Dmitry Baryshkov wrote:

> Return NULL pointer from get_edid() callback rather than ERR_PTR()
> pointer, as DRM code does NULL checks rather than IS_ERR(). Also while
> we are at it, return NULL if getting EDID timed out.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Fixes: 0cbbd5b1a012 ("drm: bridge: add support for lontium LT9611UXC bridge")
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> index a59e811f1705..b708700e182d 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> @@ -505,7 +505,10 @@ static struct edid *lt9611uxc_bridge_get_edid(struct drm_bridge *bridge,
>  	ret = lt9611uxc_wait_for_edid(lt9611uxc);
>  	if (ret < 0) {
>  		dev_err(lt9611uxc->dev, "wait for EDID failed: %d\n", ret);
> -		return ERR_PTR(ret);
> +		return NULL;
> +	} else if (ret == 0) {
> +		dev_err(lt9611uxc->dev, "wait for EDID timeout\n");
> +		return NULL;
>  	}
>  
>  	return drm_do_get_edid(connector, lt9611uxc_get_edid_block, lt9611uxc);
> -- 
> 2.29.2
> 

[PATCH v2 3/3] drm/bridge/lontium-lt9611uxc: move HPD notification out of IRQ handler

[Dmitry Baryshkov]

drm hotplug handling code (drm_client_dev_hotplug()) can wait on mutex,
thus delaying further lt9611uxc IRQ events processing.  It was observed
occasionally during bootups, when drm_client_modeset_probe() was waiting
for EDID ready event, which was delayed because IRQ handler was stuck
trying to deliver hotplug event.
Move hotplug notifications from IRQ handler to separate work to be able
to process IRQ events without delays.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Fixes: 0cbbd5b1a012 ("drm: bridge: add support for lontium LT9611UXC bridge")
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 30 +++++++++++++++++-----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index b708700e182d..88630bc2921f 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -14,6 +14,7 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/wait.h>
+#include <linux/workqueue.h>
 
 #include <sound/hdmi-codec.h>
 
@@ -36,6 +37,7 @@ struct lt9611uxc {
 	struct mutex ocm_lock;
 
 	struct wait_queue_head wq;
+	struct work_struct work;
 
 	struct device_node *dsi0_node;
 	struct device_node *dsi1_node;
@@ -52,6 +54,7 @@ struct lt9611uxc {
 
 	bool hpd_supported;
 	bool edid_read;
+	bool hdmi_connected;
 	uint8_t fw_version;
 };
 
@@ -151,15 +154,26 @@ static irqreturn_t lt9611uxc_irq_thread_handler(int irq, void *dev_id)
 	}
 
 	if (irq_status & BIT(1)) {
-		if (lt9611uxc->connector.dev)
-			drm_kms_helper_hotplug_event(lt9611uxc->connector.dev);
-		else
-			drm_bridge_hpd_notify(&lt9611uxc->bridge, !!(hpd_status & BIT(1)));
+		lt9611uxc->hdmi_connected = !!(hpd_status & BIT(1));
+		schedule_work(&lt9611uxc->work);
 	}
 
 	return IRQ_HANDLED;
 }
 
+void lt9611uxc_hpd_work(struct work_struct *work)
+{
+	struct lt9611uxc *lt9611uxc = container_of(work, struct lt9611uxc, work);
+
+	if (lt9611uxc->connector.dev)
+		drm_kms_helper_hotplug_event(lt9611uxc->connector.dev);
+	else
+		drm_bridge_hpd_notify(&lt9611uxc->bridge,
+				      lt9611uxc->hdmi_connected ?
+				      connector_status_connected :
+				      connector_status_disconnected);
+}
+
 static void lt9611uxc_reset(struct lt9611uxc *lt9611uxc)
 {
 	gpiod_set_value_cansleep(lt9611uxc->reset_gpio, 1);
@@ -447,7 +461,7 @@ static enum drm_connector_status lt9611uxc_bridge_detect(struct drm_bridge *brid
 	struct lt9611uxc *lt9611uxc = bridge_to_lt9611uxc(bridge);
 	unsigned int reg_val = 0;
 	int ret;
-	int connected = 1;
+	bool connected = true;
 
 	if (lt9611uxc->hpd_supported) {
 		lt9611uxc_lock(lt9611uxc);
@@ -457,8 +471,9 @@ static enum drm_connector_status lt9611uxc_bridge_detect(struct drm_bridge *brid
 		if (ret)
 			dev_err(lt9611uxc->dev, "failed to read hpd status: %d\n", ret);
 		else
-			connected  = reg_val & BIT(1);
+			connected  = !!(reg_val & BIT(1));
 	}
+	lt9611uxc->hdmi_connected = connected;
 
 	return connected ?  connector_status_connected :
 				connector_status_disconnected;
@@ -931,6 +946,8 @@ static int lt9611uxc_probe(struct i2c_client *client,
 	lt9611uxc->fw_version = ret;
 
 	init_waitqueue_head(&lt9611uxc->wq);
+	INIT_WORK(&lt9611uxc->work, lt9611uxc_hpd_work);
+
 	ret = devm_request_threaded_irq(dev, client->irq, NULL,
 					lt9611uxc_irq_thread_handler,
 					IRQF_ONESHOT, "lt9611uxc", lt9611uxc);
@@ -967,6 +984,7 @@ static int lt9611uxc_remove(struct i2c_client *client)
 	struct lt9611uxc *lt9611uxc = i2c_get_clientdata(client);
 
 	disable_irq(client->irq);
+	flush_scheduled_work();
 	lt9611uxc_audio_exit(lt9611uxc);
 	drm_bridge_remove(&lt9611uxc->bridge);
 
-- 
2.29.2

Re: [PATCH v2 3/3] drm/bridge/lontium-lt9611uxc: move HPD notification out of IRQ handler

[kernel test robot]

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

Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.11-rc3 next-20210115]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/drm-bridge-lontium-lt9611uxc-fix-handling-of-EDID-HPD/20210116-184620
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a004-20210116 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project d7bc3b7ce23b664d6620cdc32370a8614523ca2f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/cfb7e9605e8955217dcee379ef7cd41ebc1d0fad
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dmitry-Baryshkov/drm-bridge-lontium-lt9611uxc-fix-handling-of-EDID-HPD/20210116-184620
        git checkout cfb7e9605e8955217dcee379ef7cd41ebc1d0fad
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/bridge/lontium-lt9611uxc.c:164:6: warning: no previous prototype for function 'lt9611uxc_hpd_work' [-Wmissing-prototypes]
   void lt9611uxc_hpd_work(struct work_struct *work)
        ^
   drivers/gpu/drm/bridge/lontium-lt9611uxc.c:164:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void lt9611uxc_hpd_work(struct work_struct *work)
   ^
   static 
   1 warning generated.


vim +/lt9611uxc_hpd_work +164 drivers/gpu/drm/bridge/lontium-lt9611uxc.c

   163	
 > 164	void lt9611uxc_hpd_work(struct work_struct *work)
   165	{
   166		struct lt9611uxc *lt9611uxc = container_of(work, struct lt9611uxc, work);
   167	
   168		if (lt9611uxc->connector.dev)
   169			drm_kms_helper_hotplug_event(lt9611uxc->connector.dev);
   170		else
   171			drm_bridge_hpd_notify(&lt9611uxc->bridge,
   172					      lt9611uxc->hdmi_connected ?
   173					      connector_status_connected :
   174					      connector_status_disconnected);
   175	}
   176	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37996 bytes --]

Re: [PATCH v2 3/3] drm/bridge/lontium-lt9611uxc: move HPD notification out of IRQ handler

[kernel test robot]

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

Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.11-rc3 next-20210115]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/drm-bridge-lontium-lt9611uxc-fix-handling-of-EDID-HPD/20210116-184620
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: nds32-randconfig-r033-20210116 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/cfb7e9605e8955217dcee379ef7cd41ebc1d0fad
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dmitry-Baryshkov/drm-bridge-lontium-lt9611uxc-fix-handling-of-EDID-HPD/20210116-184620
        git checkout cfb7e9605e8955217dcee379ef7cd41ebc1d0fad
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nds32 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/bridge/lontium-lt9611uxc.c:164:6: warning: no previous prototype for 'lt9611uxc_hpd_work' [-Wmissing-prototypes]
     164 | void lt9611uxc_hpd_work(struct work_struct *work)
         |      ^~~~~~~~~~~~~~~~~~


vim +/lt9611uxc_hpd_work +164 drivers/gpu/drm/bridge/lontium-lt9611uxc.c

   163	
 > 164	void lt9611uxc_hpd_work(struct work_struct *work)
   165	{
   166		struct lt9611uxc *lt9611uxc = container_of(work, struct lt9611uxc, work);
   167	
   168		if (lt9611uxc->connector.dev)
   169			drm_kms_helper_hotplug_event(lt9611uxc->connector.dev);
   170		else
   171			drm_bridge_hpd_notify(&lt9611uxc->bridge,
   172					      lt9611uxc->hdmi_connected ?
   173					      connector_status_connected :
   174					      connector_status_disconnected);
   175	}
   176	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36603 bytes --]