All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: dri-devel@lists.freedesktop.org,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Sean Paul <sean@poorly.run>
Cc: linux-arm-kernel@lists.infradead.org,
	Linus Walleij <linus.walleij@linaro.org>,
	Rob Clark <robdclark@gmail.com>,
	linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org
Subject: [PATCH 6/7] RFT: drm/msm/hdmi: Bring up HDMI connector OFF
Date: Sat, 29 Jun 2019 14:59:32 +0200	[thread overview]
Message-ID: <20190629125933.679-6-linus.walleij@linaro.org> (raw)
In-Reply-To: <20190629125933.679-1-linus.walleij@linaro.org>

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
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 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 c8e8268c76e2..7d87f8821d2f 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -563,13 +563,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);
@@ -583,7 +592,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 c4e9f6d7960f..89c64cc85027 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -86,16 +86,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");
@@ -103,16 +96,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.20.1


WARNING: multiple messages have this Message-ID (diff)
From: Linus Walleij <linus.walleij@linaro.org>
To: dri-devel@lists.freedesktop.org,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Sean Paul <sean@poorly.run>
Cc: linux-arm-msm@vger.kernel.org,
	Linus Walleij <linus.walleij@linaro.org>,
	freedreno@lists.freedesktop.org, Rob Clark <robdclark@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH 6/7] RFT: drm/msm/hdmi: Bring up HDMI connector OFF
Date: Sat, 29 Jun 2019 14:59:32 +0200	[thread overview]
Message-ID: <20190629125933.679-6-linus.walleij@linaro.org> (raw)
In-Reply-To: <20190629125933.679-1-linus.walleij@linaro.org>

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
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 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 c8e8268c76e2..7d87f8821d2f 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -563,13 +563,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);
@@ -583,7 +592,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 c4e9f6d7960f..89c64cc85027 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -86,16 +86,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");
@@ -103,16 +96,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.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	Maarten Lankhorst
	<maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	Maxime Ripard
	<maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>,
	Sean Paul <sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linus Walleij
	<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: [PATCH 6/7] RFT: drm/msm/hdmi: Bring up HDMI connector OFF
Date: Sat, 29 Jun 2019 14:59:32 +0200	[thread overview]
Message-ID: <20190629125933.679-6-linus.walleij@linaro.org> (raw)
In-Reply-To: <20190629125933.679-1-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

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
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 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 c8e8268c76e2..7d87f8821d2f 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -563,13 +563,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);
@@ -583,7 +592,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 c4e9f6d7960f..89c64cc85027 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -86,16 +86,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");
@@ -103,16 +96,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.20.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

  parent reply	other threads:[~2019-06-29 13:02 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-29 12:59 [PATCH 1/7] drm/msm/mdp4: Drop unused GPIO include Linus Walleij
2019-06-29 12:59 ` Linus Walleij
2019-06-29 12:59 ` Linus Walleij
2019-06-29 12:59 ` [PATCH 2/7] drm/msm/dsi: Drop unused GPIO includes Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-06-29 12:59 ` [PATCH 3/7] drm/msm/dpu: Drop unused GPIO code Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-06-29 12:59 ` [PATCH 4/7] gpio: of: Support some legacy Qualcomm HDMI bindings Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-06-30 13:26   ` Rob Clark
2019-06-30 13:26     ` Rob Clark
2019-06-30 13:26     ` Rob Clark
2019-06-30 15:18     ` Linus Walleij
2019-06-30 15:18       ` Linus Walleij
2019-06-30 15:18       ` Linus Walleij
2019-06-30 15:25       ` Rob Clark
2019-06-30 15:25         ` Rob Clark
2019-06-30 15:25         ` Rob Clark
2019-06-29 12:59 ` [PATCH 5/7] drm/msm/hdmi: Convert to use GPIO descriptors Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-06-29 12:59 ` Linus Walleij [this message]
2019-06-29 12:59   ` [PATCH 6/7] RFT: drm/msm/hdmi: Bring up HDMI connector OFF Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-06-29 12:59 ` [PATCH 7/7] RFT: drm/msm/hdmi: Do not initialize HPD line value Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-06-29 12:59   ` Linus Walleij
2019-07-28 10:02 ` [PATCH 1/7] drm/msm/mdp4: Drop unused GPIO include Linus Walleij
2019-07-28 10:02   ` Linus Walleij
2019-07-28 10:02   ` Linus Walleij
2019-08-05  9:01   ` Linus Walleij
2019-08-05  9:01     ` Linus Walleij
2019-08-05  9:01     ` Linus Walleij
2019-08-13  0:22 ` Brian Masney
2019-08-13  0:22   ` Brian Masney
2019-08-13  0:22   ` Brian Masney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190629125933.679-6-linus.walleij@linaro.org \
    --to=linus.walleij@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=maxime.ripard@bootlin.com \
    --cc=robdclark@gmail.com \
    --cc=sean@poorly.run \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.