diff mbox series

[8/8] drm/msm/dp: Support external GPIO HPD with 3rd pinctrl chip

Message ID 20241129-add-displayport-support-for-qcs615-platform-v1-8-09a4338d93ef@quicinc.com (mailing list archive)
State New
Headers show
Series Add DisplayPort support for QCS615 platform | expand

Commit Message

Xiangxu Yin Nov. 29, 2024, 7:57 a.m. UTC
Add support for handling HPD (Hot Plug Detect) signals via external
GPIOs connected through pinctrl chips (e.g., Semtech SX1509Q). This
involves reinitializing the relevant GPIO and binding an interrupt
handler to process hot plug events. Since external GPIOs only support
edge interrupts (rising or falling) rather than state interrupts, the
GPIO state must be read during the first DP bridge HPD enablement. This
ensures the current connection state is determined and a hot plug event
is reported accordingly.

Signed-off-by: Xiangxu Yin <quic_xiangxuy@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 83 +++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

Comments

Dmitry Baryshkov Nov. 29, 2024, 1:45 p.m. UTC | #1
On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin <quic_xiangxuy@quicinc.com> wrote:
>
> Add support for handling HPD (Hot Plug Detect) signals via external
> GPIOs connected through pinctrl chips (e.g., Semtech SX1509Q). This
> involves reinitializing the relevant GPIO and binding an interrupt
> handler to process hot plug events. Since external GPIOs only support
> edge interrupts (rising or falling) rather than state interrupts, the
> GPIO state must be read during the first DP bridge HPD enablement. This
> ensures the current connection state is determined and a hot plug event
> is reported accordingly.

NAK, use dp-connector instead.

>
> Signed-off-by: Xiangxu Yin <quic_xiangxuy@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 83 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
Neil Armstrong Nov. 29, 2024, 1:54 p.m. UTC | #2
On 29/11/2024 08:57, Xiangxu Yin wrote:
> Add support for handling HPD (Hot Plug Detect) signals via external
> GPIOs connected through pinctrl chips (e.g., Semtech SX1509Q). This
> involves reinitializing the relevant GPIO and binding an interrupt
> handler to process hot plug events. Since external GPIOs only support
> edge interrupts (rising or falling) rather than state interrupts, the
> GPIO state must be read during the first DP bridge HPD enablement. This
> ensures the current connection state is determined and a hot plug event
> is reported accordingly.
> 
> Signed-off-by: Xiangxu Yin <quic_xiangxuy@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 83 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 83 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index eb6fb76c68e505fafbec563440e9784f51e1894b..22c288ca61b9b444a7b8d4a574c614bfef9d88be 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -13,6 +13,8 @@
>   #include <linux/delay.h>
>   #include <drm/display/drm_dp_aux_bus.h>
>   #include <drm/drm_edid.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/of_gpio.h>
>   
>   #include "msm_drv.h"
>   #include "msm_kms.h"
> @@ -78,6 +80,10 @@ struct msm_dp_display_private {
>   
>   	unsigned int id;
>   
> +	bool ext_gpio;
> +	int gpio_num;
> +	struct work_struct  gpio_work;
> +
>   	/* state variables */
>   	bool core_initialized;
>   	bool phy_initialized;
> @@ -1182,6 +1188,42 @@ static irqreturn_t msm_dp_display_irq_handler(int irq, void *dev_id)
>   	return ret;
>   }
>   
> +
> +static void msm_dp_gpio_work_handler(struct work_struct *work)
> +{
> +	struct msm_dp_display_private *dp = container_of(work,
> +			struct msm_dp_display_private, gpio_work);
> +	struct gpio_desc *desc;
> +	bool hpd;
> +
> +	if (dp->ext_gpio) {
> +		desc = gpio_to_desc(dp->gpio_num);
> +		if (!desc) {
> +			pr_err("Failed to get gpio_desc for GPIO %d\n", dp->gpio_num);
> +			return;
> +		}
> +
> +		hpd = gpiod_get_value_cansleep(desc);
> +		if (hpd)
> +			msm_dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
> +		else
> +			msm_dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
> +	}
> +}
> +
> +static irqreturn_t msm_dp_gpio_isr(int unused, void *data)
> +{
> +	struct msm_dp_display_private *dp = data;
> +
> +	if (!dp) {
> +		DRM_ERROR("NULL data\n");
> +		return IRQ_NONE;
> +	}
> +
> +	schedule_work(&dp->gpio_work);

this msm_dp_gpio_isr is already threaded, would would you also schedule a work ?

> +	return IRQ_HANDLED;
> +}
> +
>   static int msm_dp_display_request_irq(struct msm_dp_display_private *dp)
>   {
>   	int rc = 0;
> @@ -1193,6 +1235,21 @@ static int msm_dp_display_request_irq(struct msm_dp_display_private *dp)
>   		return dp->irq;
>   	}
>   
> +	if (dp->ext_gpio) {
> +		int edge, gpio_irq;
> +
> +		gpio_irq = gpio_to_irq(dp->gpio_num);

But as Dmitry reported, the system should use a dp-connected as a next bridge
instead which already supports all this much better:
drivers/gpu/drm/bridge/display-connector.c
Documentation/devicetree/bindings/display/connector/dp-connector.yaml

> +		edge = IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING;
> +
> +		rc = devm_request_threaded_irq(&pdev->dev, gpio_irq, NULL,
> +		msm_dp_gpio_isr, edge, "dp_gpio_isr", dp);
> +		if (rc < 0) {
> +			DRM_ERROR("failed to request ext-gpio IRQ%u: %d\n",
> +					gpio_irq, rc);
> +			return rc;
> +		}
> +	}
> +
>   	rc = devm_request_irq(&pdev->dev, dp->irq, msm_dp_display_irq_handler,
>   			      IRQF_TRIGGER_HIGH|IRQF_NO_AUTOEN,
>   			      "dp_display_isr", dp);
> @@ -1308,10 +1365,32 @@ static int msm_dp_display_probe(struct platform_device *pdev)
>   		return -EPROBE_DEFER;
>   	}
>   
> +	if (of_find_property(pdev->dev.of_node, "dp-hpd-gpio", NULL)) {
> +		dp->ext_gpio = true;
> +		dp->gpio_num = of_get_named_gpio(pdev->dev.of_node, "dp-hpd-gpio", 0);
> +		if (dp->gpio_num < 0) {
> +			dev_err(&pdev->dev, "Failed to get gpio:%d\n", dp->gpio_num);
> +			return dp->gpio_num;
> +		}
> +
> +		if (!gpio_is_valid(dp->gpio_num)) {
> +			DRM_ERROR("gpio(%d) invalid\n", dp->gpio_num);
> +			return -EINVAL;
> +		}
> +
> +		rc = gpio_request(dp->gpio_num, "dp-hpd-gpio");
> +		if (rc) {
> +			dev_err(&pdev->dev, "Failed to request gpio:%d\n", dp->gpio_num);
> +			return rc;
> +		}
> +		gpio_direction_input(dp->gpio_num);
> +	}
> +
>   	/* setup event q */
>   	mutex_init(&dp->event_mutex);
>   	init_waitqueue_head(&dp->event_q);
>   	spin_lock_init(&dp->event_lock);
> +	INIT_WORK(&dp->gpio_work, msm_dp_gpio_work_handler);
>   
>   	/* Store DP audio handle inside DP display */
>   	dp->msm_dp_display.msm_dp_audio = dp->audio;
> @@ -1678,6 +1757,10 @@ void msm_dp_bridge_hpd_enable(struct drm_bridge *bridge)
>   	msm_dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, true);
>   
>   	msm_dp_display->internal_hpd = true;
> +
> +	if (dp->ext_gpio)
> +		schedule_work(&dp->gpio_work);
> +
>   	mutex_unlock(&dp->event_mutex);
>   }
>   
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index eb6fb76c68e505fafbec563440e9784f51e1894b..22c288ca61b9b444a7b8d4a574c614bfef9d88be 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -13,6 +13,8 @@ 
 #include <linux/delay.h>
 #include <drm/display/drm_dp_aux_bus.h>
 #include <drm/drm_edid.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of_gpio.h>
 
 #include "msm_drv.h"
 #include "msm_kms.h"
@@ -78,6 +80,10 @@  struct msm_dp_display_private {
 
 	unsigned int id;
 
+	bool ext_gpio;
+	int gpio_num;
+	struct work_struct  gpio_work;
+
 	/* state variables */
 	bool core_initialized;
 	bool phy_initialized;
@@ -1182,6 +1188,42 @@  static irqreturn_t msm_dp_display_irq_handler(int irq, void *dev_id)
 	return ret;
 }
 
+
+static void msm_dp_gpio_work_handler(struct work_struct *work)
+{
+	struct msm_dp_display_private *dp = container_of(work,
+			struct msm_dp_display_private, gpio_work);
+	struct gpio_desc *desc;
+	bool hpd;
+
+	if (dp->ext_gpio) {
+		desc = gpio_to_desc(dp->gpio_num);
+		if (!desc) {
+			pr_err("Failed to get gpio_desc for GPIO %d\n", dp->gpio_num);
+			return;
+		}
+
+		hpd = gpiod_get_value_cansleep(desc);
+		if (hpd)
+			msm_dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+		else
+			msm_dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+	}
+}
+
+static irqreturn_t msm_dp_gpio_isr(int unused, void *data)
+{
+	struct msm_dp_display_private *dp = data;
+
+	if (!dp) {
+		DRM_ERROR("NULL data\n");
+		return IRQ_NONE;
+	}
+
+	schedule_work(&dp->gpio_work);
+	return IRQ_HANDLED;
+}
+
 static int msm_dp_display_request_irq(struct msm_dp_display_private *dp)
 {
 	int rc = 0;
@@ -1193,6 +1235,21 @@  static int msm_dp_display_request_irq(struct msm_dp_display_private *dp)
 		return dp->irq;
 	}
 
+	if (dp->ext_gpio) {
+		int edge, gpio_irq;
+
+		gpio_irq = gpio_to_irq(dp->gpio_num);
+		edge = IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING;
+
+		rc = devm_request_threaded_irq(&pdev->dev, gpio_irq, NULL,
+		msm_dp_gpio_isr, edge, "dp_gpio_isr", dp);
+		if (rc < 0) {
+			DRM_ERROR("failed to request ext-gpio IRQ%u: %d\n",
+					gpio_irq, rc);
+			return rc;
+		}
+	}
+
 	rc = devm_request_irq(&pdev->dev, dp->irq, msm_dp_display_irq_handler,
 			      IRQF_TRIGGER_HIGH|IRQF_NO_AUTOEN,
 			      "dp_display_isr", dp);
@@ -1308,10 +1365,32 @@  static int msm_dp_display_probe(struct platform_device *pdev)
 		return -EPROBE_DEFER;
 	}
 
+	if (of_find_property(pdev->dev.of_node, "dp-hpd-gpio", NULL)) {
+		dp->ext_gpio = true;
+		dp->gpio_num = of_get_named_gpio(pdev->dev.of_node, "dp-hpd-gpio", 0);
+		if (dp->gpio_num < 0) {
+			dev_err(&pdev->dev, "Failed to get gpio:%d\n", dp->gpio_num);
+			return dp->gpio_num;
+		}
+
+		if (!gpio_is_valid(dp->gpio_num)) {
+			DRM_ERROR("gpio(%d) invalid\n", dp->gpio_num);
+			return -EINVAL;
+		}
+
+		rc = gpio_request(dp->gpio_num, "dp-hpd-gpio");
+		if (rc) {
+			dev_err(&pdev->dev, "Failed to request gpio:%d\n", dp->gpio_num);
+			return rc;
+		}
+		gpio_direction_input(dp->gpio_num);
+	}
+
 	/* setup event q */
 	mutex_init(&dp->event_mutex);
 	init_waitqueue_head(&dp->event_q);
 	spin_lock_init(&dp->event_lock);
+	INIT_WORK(&dp->gpio_work, msm_dp_gpio_work_handler);
 
 	/* Store DP audio handle inside DP display */
 	dp->msm_dp_display.msm_dp_audio = dp->audio;
@@ -1678,6 +1757,10 @@  void msm_dp_bridge_hpd_enable(struct drm_bridge *bridge)
 	msm_dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, true);
 
 	msm_dp_display->internal_hpd = true;
+
+	if (dp->ext_gpio)
+		schedule_work(&dp->gpio_work);
+
 	mutex_unlock(&dp->event_mutex);
 }