diff mbox series

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

Message ID 20201127092316.122246-2-dmitry.baryshkov@linaro.org (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/bridge/lontium-lt9611uxc: fix waiting for EDID to become available | expand

Commit Message

Dmitry Baryshkov Nov. 27, 2020, 9:23 a.m. UTC 
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>
---
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 30 +++++++++++++++++-----
 1 file changed, 24 insertions(+), 6 deletions(-)

Comments

Bjorn Andersson Jan. 15, 2021, 3:33 a.m. UTC | #1 
On Fri 27 Nov 03:23 CST 2020, Dmitry Baryshkov wrote:

> 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.
> 

I see a couple of other drivers doing the same, and the patch looks
good.

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

Regards,
Bjorn

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@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
>
diff mbox series

Patch

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);