| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2c1ffbc6-0d67-4787-9fbe-03255b377c76@linaro.org>
Date: Fri, 29 Nov 2024 14:54:36 +0100
From: neil.armstrong@...aro.org
To: Xiangxu Yin <quic_xiangxuy@...cinc.com>, Rob Clark <robdclark@...il.com>,
Abhinav Kumar <quic_abhinavk@...cinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Kuogee Hsieh <quic_khsieh@...cinc.com>,
Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
<brgl@...ev.pl>, quic_lliu6@...cinc.com, quic_fangez@...cinc.com
Cc: linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org,
linux-gpio@...r.kernel.org
Subject: Re: [PATCH 8/8] drm/msm/dp: Support external GPIO HPD with 3rd
pinctrl chip
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@...cinc.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);
> }
>
>
Powered by blists - more mailing lists