This file is not using any symbols from <linux/gpio.h> so just drop this include.
Cc: Rob Clark robdclark@gmail.com Cc: Sean Paul sean@poorly.run Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Reviewed-by: Brian Masney masneyb@onstation.org Signed-off-by: Linus Walleij linus.walleij@linaro.org --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c index ecef4f5b9f26..9262ed2dc8c3 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c @@ -5,8 +5,6 @@ * Author: Vinay Simha vinaysimha@inforcecomputing.com */
-#include <linux/gpio.h> - #include "mdp4_kms.h"
struct mdp4_lvds_connector {
This DSI driver uses the new descriptor API so these old GPIO API includes are surplus.
Cc: Rob Clark robdclark@gmail.com Cc: Sean Paul sean@poorly.run Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Reviewed-by: Brian Masney masneyb@onstation.org Signed-off-by: Linus Walleij linus.walleij@linaro.org --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index aa35d18ab43c..4b6f62138390 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -6,11 +6,9 @@ #include <linux/clk.h> #include <linux/delay.h> #include <linux/err.h> -#include <linux/gpio.h> #include <linux/gpio/consumer.h> #include <linux/interrupt.h> #include <linux/of_device.h> -#include <linux/of_gpio.h> #include <linux/of_irq.h> #include <linux/pinctrl/consumer.h> #include <linux/of_graph.h>
The DPU has some kind of idea that it wants to be able to bring up power using GPIO lines. The struct dss_gpio is however completely unused and should this be done, it should be done using the GPIO descriptor framework rather than this API which relies on the global GPIO numberspace. Delete this code before anyone hurt themselves.
The inclusion of <linux/gpio.h> was abused to get some OF and IRQ headers implicitly included into the DPU utilities, make these includes explicit and push them down into the actual implementation.
Cc: Rob Clark robdclark@gmail.com Cc: Sean Paul sean@poorly.run Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Reviewed-by: Brian Masney masneyb@onstation.org Signed-off-by: Linus Walleij linus.walleij@linaro.org --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h | 9 --------- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 4 ++++ 3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c index 71b6987bff1e..27fbeb504362 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c @@ -7,6 +7,7 @@ #include <linux/clk/clk-conf.h> #include <linux/err.h> #include <linux/delay.h> +#include <linux/of.h>
#include <drm/drm_print.h>
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h index 09083e9f06bb..e6b5c772fa3b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h @@ -5,7 +5,6 @@ #ifndef __DPU_IO_UTIL_H__ #define __DPU_IO_UTIL_H__
-#include <linux/gpio.h> #include <linux/platform_device.h> #include <linux/types.h>
@@ -14,12 +13,6 @@ #define DEV_WARN(fmt, args...) pr_warn(fmt, ##args) #define DEV_ERR(fmt, args...) pr_err(fmt, ##args)
-struct dss_gpio { - unsigned int gpio; - unsigned int value; - char gpio_name[32]; -}; - enum dss_clk_type { DSS_CLK_AHB, /* no set rate. rate controlled through rpm */ DSS_CLK_PCLK, @@ -34,8 +27,6 @@ struct dss_clk { };
struct dss_module_power { - unsigned int num_gpio; - struct dss_gpio *gpio_config; unsigned int num_clk; struct dss_clk *clk_config; }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c index 986915bbbc02..c977baddfffd 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c @@ -3,6 +3,10 @@ * Copyright (c) 2018, The Linux Foundation */
+#include <linux/irq.h> +#include <linux/irqchip.h> +#include <linux/irqdesc.h> +#include <linux/irqchip/chained_irq.h> #include "dpu_kms.h" #include <linux/interconnect.h>
This switches the MSM HDMI code to use GPIO descriptors. Normally we would fetch the GPIOs from the device with the flags GPIOD_IN or GPIOD_OUT_[LOW|HIGH] to set up the lines immediately, but since the code seems eager to actively drive the lines high/low when turning HDMI on and off, we just fetch the GPIOs as-is and keep the code explicitly driving them.
The old code would try legacy bindings (GPIOs without any "-gpios" suffix) but this has been moved to the gpiolib as a quirk by the previous patch.
Cc: Rob Clark robdclark@gmail.com Cc: Sean Paul sean@poorly.run Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Reviewed-by: Brian Masney masneyb@onstation.org Signed-off-by: Linus Walleij linus.walleij@linaro.org --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/hdmi/hdmi.c | 66 +++++++++++------------ drivers/gpu/drm/msm/hdmi/hdmi.h | 4 +- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 42 +++++---------- 3 files changed, 45 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 0e4217be3f00..355afb936401 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -425,38 +425,6 @@ static const struct { { "qcom,hdmi-tx-mux-lpm", true, 1, "HDMI_MUX_LPM" }, };
-static int msm_hdmi_get_gpio(struct device_node *of_node, const char *name) -{ - int gpio; - - /* try with the gpio names as in the table (downstream bindings) */ - gpio = of_get_named_gpio(of_node, name, 0); - if (gpio < 0) { - char name2[32]; - - /* try with the gpio names as in the upstream bindings */ - snprintf(name2, sizeof(name2), "%s-gpios", name); - gpio = of_get_named_gpio(of_node, name2, 0); - if (gpio < 0) { - char name3[32]; - - /* - * try again after stripping out the "qcom,hdmi-tx" - * prefix. This is mainly to match "hpd-gpios" used - * in the upstream bindings - */ - if (sscanf(name2, "qcom,hdmi-tx-%s", name3)) - gpio = of_get_named_gpio(of_node, name3, 0); - } - - if (gpio < 0) { - DBG("failed to get gpio: %s (%d)", name, gpio); - gpio = -1; - } - } - return gpio; -} - /* * HDMI audio codec callbacks */ @@ -582,11 +550,39 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data) hdmi_cfg->qfprom_mmio_name = "qfprom_physical";
for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { - hdmi_cfg->gpios[i].num = msm_hdmi_get_gpio(of_node, - msm_hdmi_gpio_pdata[i].name); + const char *name = msm_hdmi_gpio_pdata[i].name; + struct gpio_desc *gpiod; + + /* + * We are fetching the GPIO lines "as is" since the connector + * code is enabling and disabling the lines. Until that point + * the power-on default value will be kept. + */ + gpiod = devm_gpiod_get_optional(dev, name, GPIOD_ASIS); + /* This will catch e.g. -PROBE_DEFER */ + if (IS_ERR(gpiod)) + return PTR_ERR(gpiod); + if (!gpiod) { + /* Try a second time, stripping down the name */ + char name3[32]; + + /* + * Try again after stripping out the "qcom,hdmi-tx" + * prefix. This is mainly to match "hpd-gpios" used + * in the upstream bindings. + */ + if (sscanf(name, "qcom,hdmi-tx-%s", name3)) + gpiod = devm_gpiod_get_optional(dev, name3, GPIOD_ASIS); + if (IS_ERR(gpiod)) + return PTR_ERR(gpiod); + if (!gpiod) + DBG("failed to get gpio: %s", name); + } + hdmi_cfg->gpios[i].gpiod = gpiod; + if (gpiod) + gpiod_set_consumer_name(gpiod, msm_hdmi_gpio_pdata[i].label); hdmi_cfg->gpios[i].output = msm_hdmi_gpio_pdata[i].output; hdmi_cfg->gpios[i].value = msm_hdmi_gpio_pdata[i].value; - hdmi_cfg->gpios[i].label = msm_hdmi_gpio_pdata[i].label; }
dev->platform_data = hdmi_cfg; diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h index 982865866a29..bdac452b00fb 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.h +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h @@ -11,6 +11,7 @@ #include <linux/clk.h> #include <linux/platform_device.h> #include <linux/regulator/consumer.h> +#include <linux/gpio/consumer.h> #include <linux/hdmi.h>
#include "msm_drv.h" @@ -22,10 +23,9 @@ struct hdmi_phy; struct hdmi_platform_config;
struct hdmi_gpio_data { - int num; + struct gpio_desc *gpiod; bool output; int value; - const char *label; };
struct hdmi_audio { diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c index 07b4cb877d82..d0575d4f747d 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c @@ -4,7 +4,7 @@ * Author: Rob Clark robdclark@gmail.com */
-#include <linux/gpio.h> +#include <linux/gpio/consumer.h> #include <linux/pinctrl/consumer.h>
#include "msm_kms.h" @@ -68,30 +68,21 @@ static void msm_hdmi_phy_reset(struct hdmi *hdmi)
static int gpio_config(struct hdmi *hdmi, bool on) { - struct device *dev = &hdmi->pdev->dev; const struct hdmi_platform_config *config = hdmi->config; - int ret, i; + int i;
if (on) { for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { struct hdmi_gpio_data gpio = config->gpios[i];
- if (gpio.num != -1) { - ret = gpio_request(gpio.num, gpio.label); - if (ret) { - DRM_DEV_ERROR(dev, - "'%s'(%d) gpio_request failed: %d\n", - gpio.label, gpio.num, ret); - goto err; - } - + if (gpio.gpiod) { if (gpio.output) { - gpio_direction_output(gpio.num, - gpio.value); + gpiod_direction_output(gpio.gpiod, + gpio.value); } else { - gpio_direction_input(gpio.num); - gpio_set_value_cansleep(gpio.num, - gpio.value); + gpiod_direction_input(gpio.gpiod); + gpiod_set_value_cansleep(gpio.gpiod, + gpio.value); } } } @@ -101,29 +92,20 @@ static int gpio_config(struct hdmi *hdmi, bool on) for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { struct hdmi_gpio_data gpio = config->gpios[i];
- if (gpio.num == -1) + if (!gpio.gpiod) continue;
if (gpio.output) { int value = gpio.value ? 0 : 1;
- gpio_set_value_cansleep(gpio.num, value); + gpiod_set_value_cansleep(gpio.gpiod, value); } - - gpio_free(gpio.num); };
DBG("gpio off"); }
return 0; -err: - while (i--) { - if (config->gpios[i].num != -1) - gpio_free(config->gpios[i].num); - } - - return ret; }
static void enable_hpd_clocks(struct hdmi *hdmi, bool enable) @@ -311,7 +293,7 @@ static enum drm_connector_status detect_gpio(struct hdmi *hdmi) const struct hdmi_platform_config *config = hdmi->config; struct hdmi_gpio_data hpd_gpio = config->gpios[HPD_GPIO_INDEX];
- return gpio_get_value(hpd_gpio.num) ? + return gpiod_get_value(hpd_gpio.gpiod) ? connector_status_connected : connector_status_disconnected; } @@ -330,7 +312,7 @@ static enum drm_connector_status hdmi_connector_detect( * some platforms may not have hpd gpio. Rely only on the status * provided by REG_HDMI_HPD_INT_STATUS in this case. */ - if (hpd_gpio.num == -1) + if (!hpd_gpio.gpiod) return detect_reg(hdmi);
do {
There is elaborate code in the HDMI connector handling to leave the connector in the state it was at power-on and only touch the GPIOs when the connector .enable() and .disable() callbacks are called.
I don't think this is what we normally want, initialize the connector as OFF (possibly saving power?) using the appropriate GPIO descriptor flags. It will still be switched on/off in the enable()/disable() connector callback as before, but we can drop some strange surplus code.
Cc: Rob Clark robdclark@gmail.com Cc: Sean Paul sean@poorly.run Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Reviewed-by: Brian Masney masneyb@onstation.org Signed-off-by: Linus Walleij linus.walleij@linaro.org --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/hdmi/hdmi.c | 19 ++++++++++++----- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 25 ++++++----------------- 2 files changed, 20 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 355afb936401..5739eec65659 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -552,13 +552,22 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data) for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { const char *name = msm_hdmi_gpio_pdata[i].name; struct gpio_desc *gpiod; + enum gpiod_flags flags;
/* - * We are fetching the GPIO lines "as is" since the connector - * code is enabling and disabling the lines. Until that point - * the power-on default value will be kept. + * Notice the inverse set up here: we initialize the connector + * to OFF state. */ - gpiod = devm_gpiod_get_optional(dev, name, GPIOD_ASIS); + if (msm_hdmi_gpio_pdata[i].output) { + if (msm_hdmi_gpio_pdata[i].value) + flags = GPIOD_OUT_LOW; + else + flags = GPIOD_OUT_HIGH; + } else { + flags = GPIOD_IN; + } + + gpiod = devm_gpiod_get_optional(dev, name, flags); /* This will catch e.g. -PROBE_DEFER */ if (IS_ERR(gpiod)) return PTR_ERR(gpiod); @@ -572,7 +581,7 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data) * in the upstream bindings. */ if (sscanf(name, "qcom,hdmi-tx-%s", name3)) - gpiod = devm_gpiod_get_optional(dev, name3, GPIOD_ASIS); + gpiod = devm_gpiod_get_optional(dev, name3, flags); if (IS_ERR(gpiod)) return PTR_ERR(gpiod); if (!gpiod) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c index d0575d4f747d..f006682935e9 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c @@ -75,16 +75,9 @@ static int gpio_config(struct hdmi *hdmi, bool on) for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { struct hdmi_gpio_data gpio = config->gpios[i];
- if (gpio.gpiod) { - if (gpio.output) { - gpiod_direction_output(gpio.gpiod, - gpio.value); - } else { - gpiod_direction_input(gpio.gpiod); - gpiod_set_value_cansleep(gpio.gpiod, - gpio.value); - } - } + /* The value indicates the value for turning things on */ + if (gpio.gpiod) + gpiod_set_value_cansleep(gpio.gpiod, gpio.value); }
DBG("gpio on"); @@ -92,16 +85,10 @@ static int gpio_config(struct hdmi *hdmi, bool on) for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { struct hdmi_gpio_data gpio = config->gpios[i];
- if (!gpio.gpiod) - continue; - - if (gpio.output) { - int value = gpio.value ? 0 : 1; - - gpiod_set_value_cansleep(gpio.gpiod, value); - } + /* The inverse value turns stuff off */ + if (gpio.gpiod && gpio.output) + gpiod_set_value_cansleep(gpio.gpiod, !gpio.value); }; - DBG("gpio off"); }
On Fri, Aug 23, 2019 at 12:35 AM Linus Walleij linus.walleij@linaro.org wrote:
There is elaborate code in the HDMI connector handling to leave the connector in the state it was at power-on and only touch the GPIOs when the connector .enable() and .disable() callbacks are called.
I don't think this is what we normally want, initialize the connector as OFF (possibly saving power?) using the appropriate GPIO descriptor flags. It will still be switched on/off in the enable()/disable() connector callback as before, but we can drop some strange surplus code.
I'm picking up the earlier patches in this series.. although I won't have a good way to test on devices which use the hdmi block for a few weeks (when I complete my move, and get back a bunch of boards that are in boxes right now) so I'm going to wait on the last two I think.
When we get to dealing with devices with hdmi enabled by bootloader, we may actually want to preserve the pre-probe state of the GPIOs. Although maybe not worth worrying about until we can handle DSI enabled at boot.
BR, -R
Cc: Rob Clark robdclark@gmail.com Cc: Sean Paul sean@poorly.run Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Reviewed-by: Brian Masney masneyb@onstation.org Signed-off-by: Linus Walleij linus.walleij@linaro.org
ChangeLog v1->v2:
- Rebased on v5.3-rc1
- Collected review tag
drivers/gpu/drm/msm/hdmi/hdmi.c | 19 ++++++++++++----- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 25 ++++++----------------- 2 files changed, 20 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 355afb936401..5739eec65659 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -552,13 +552,22 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data) for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { const char *name = msm_hdmi_gpio_pdata[i].name; struct gpio_desc *gpiod;
enum gpiod_flags flags; /*
* We are fetching the GPIO lines "as is" since the connector* code is enabling and disabling the lines. Until that point* the power-on default value will be kept.
* Notice the inverse set up here: we initialize the connector* to OFF state. */
gpiod = devm_gpiod_get_optional(dev, name, GPIOD_ASIS);
if (msm_hdmi_gpio_pdata[i].output) {if (msm_hdmi_gpio_pdata[i].value)flags = GPIOD_OUT_LOW;elseflags = GPIOD_OUT_HIGH;} else {flags = GPIOD_IN;}gpiod = devm_gpiod_get_optional(dev, name, flags); /* This will catch e.g. -PROBE_DEFER */ if (IS_ERR(gpiod)) return PTR_ERR(gpiod);@@ -572,7 +581,7 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data) * in the upstream bindings. */ if (sscanf(name, "qcom,hdmi-tx-%s", name3))
gpiod = devm_gpiod_get_optional(dev, name3, GPIOD_ASIS);
gpiod = devm_gpiod_get_optional(dev, name3, flags); if (IS_ERR(gpiod)) return PTR_ERR(gpiod); if (!gpiod)diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c index d0575d4f747d..f006682935e9 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c @@ -75,16 +75,9 @@ static int gpio_config(struct hdmi *hdmi, bool on) for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { struct hdmi_gpio_data gpio = config->gpios[i];
if (gpio.gpiod) {if (gpio.output) {gpiod_direction_output(gpio.gpiod,gpio.value);} else {gpiod_direction_input(gpio.gpiod);gpiod_set_value_cansleep(gpio.gpiod,gpio.value);}}
/* The value indicates the value for turning things on */if (gpio.gpiod)gpiod_set_value_cansleep(gpio.gpiod, gpio.value); } DBG("gpio on");@@ -92,16 +85,10 @@ static int gpio_config(struct hdmi *hdmi, bool on) for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { struct hdmi_gpio_data gpio = config->gpios[i];
if (!gpio.gpiod)continue;if (gpio.output) {int value = gpio.value ? 0 : 1;gpiod_set_value_cansleep(gpio.gpiod, value);}
/* The inverse value turns stuff off */if (gpio.gpiod && gpio.output)gpiod_set_value_cansleep(gpio.gpiod, !gpio.value); };
DBG("gpio off"); }-- 2.21.0
On Sun, Sep 1, 2019 at 6:28 PM Rob Clark robdclark@gmail.com wrote:
I'm picking up the earlier patches in this series..
Thanks a lot :)
although I won't have a good way to test on devices which use the hdmi block for a few weeks (when I complete my move, and get back a bunch of boards that are in boxes right now) so I'm going to wait on the last two I think.
That's fine. It is just occasional drive-by coding anyway.
Yours, Linus Walleij
After untangling the MSM HDMI GPIO code we see that the code is deliberately setting the output value of the HPD (hot plug detect) line to high, even though it is being used as input which is of course the only viable use of a HPD pin.
This seems dubious: GPIO lines set up as input will have high impedance (tristate) and the typical electronic construction involves this line being used with a pull-down resistor around 10KOhm to keep it low (this is sometimes part of a levelshifter component) and then an inserted connector will pull it up to VDD and this asserts the HPD signal, as can be seen from the code reading the HPD GPIO.
Stop try driving a value to the HPD input GPIO.
Cc: Rob Clark robdclark@gmail.com Cc: Sean Paul sean@poorly.run Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Reviewed-by: Brian Masney masneyb@onstation.org Signed-off-by: Linus Walleij linus.walleij@linaro.org --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c index f006682935e9..bb1c49e3c9dd 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c @@ -76,7 +76,7 @@ static int gpio_config(struct hdmi *hdmi, bool on) struct hdmi_gpio_data gpio = config->gpios[i];
/* The value indicates the value for turning things on */ - if (gpio.gpiod) + if (gpio.gpiod && gpio.output) gpiod_set_value_cansleep(gpio.gpiod, gpio.value); }
dri-devel@lists.freedesktop.org
-
Linus Walleij -
Rob Clark