Some targets (eg: msm8994) use the pinctrl framework to configure
interface pins. This change adds support for initialization and
pinctrl active/sleep state control for the HDMI driver.
Signed-off-by: Stephane Viau <[email protected]>
---
v2:
- Add devicetree binding documentation for pinctrl property [Ivan]
- Use pinctrl framework's PINCTRL_STATE_DEFAULT/SLEEP states [Ivan]
- Fix a couple of checkpatch warnings
Documentation/devicetree/bindings/drm/msm/hdmi.txt | 6 +++++
drivers/gpu/drm/msm/hdmi/hdmi.c | 27 +++++++++++++++++++
drivers/gpu/drm/msm/hdmi/hdmi.h | 5 ++++
drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 31 ++++++++++++++++++++++
4 files changed, 69 insertions(+)
diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt
index a29a55f..01ff7c6 100644
--- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt
+++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt
@@ -20,6 +20,9 @@ Required properties:
Optional properties:
- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin
- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin
+- pinctrl-names: the pin control state names; should contain "default"
+- pinctrl-0: the default pinctrl state (active)
+- pinctrl-n: the "sleep" pinctrl state (n == 1) and others
Example:
@@ -44,5 +47,8 @@ Example:
qcom,hdmi-tx-hpd = <&msmgpio 72 GPIO_ACTIVE_HIGH>;
core-vdda-supply = <&pm8921_hdmi_mvs>;
hdmi-mux-supply = <&ext_3p3v>;
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&hpd_active &ddc_active &cec_active>;
+ pinctrl-1 = <&hpd_suspend &ddc_suspend &cec_suspend>;
};
};
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 8145362..1197086 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -354,6 +354,20 @@ static int get_gpio(struct device *dev, struct device_node *of_node, const char
}
return gpio;
}
+
+static struct pinctrl_state *get_pinctrl_state(struct device *dev,
+ struct pinctrl *pinctrl, const char *name)
+{
+ struct pinctrl_state *state = pinctrl_lookup_state(pinctrl, name);
+
+ if (IS_ERR_OR_NULL(state)) {
+ dev_err(dev, "failed to get pinctrl state \"%s\" (%ld)",
+ name, PTR_ERR(state));
+ return NULL;
+ }
+
+ return state;
+}
#endif
static int hdmi_bind(struct device *dev, struct device *master, void *data)
@@ -365,6 +379,7 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data)
#ifdef CONFIG_OF
struct device_node *of_node = dev->of_node;
const struct of_device_id *match;
+ struct pinctrl *pinctrl;
match = of_match_node(dt_match, of_node);
if (match && match->data) {
@@ -383,6 +398,18 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data)
hdmi_cfg->mux_sel_gpio = get_gpio(dev, of_node, "qcom,hdmi-tx-mux-sel");
hdmi_cfg->mux_lpm_gpio = get_gpio(dev, of_node, "qcom,hdmi-tx-mux-lpm");
+ /* not all targets have pinctrl, do not fail in case of error: */
+ pinctrl = devm_pinctrl_get(dev);
+ if (IS_ERR_OR_NULL(pinctrl)) {
+ dev_warn(dev, "cannot get pinctrl: %s\n", of_node->name);
+ } else {
+ hdmi_cfg->active = get_pinctrl_state(dev, pinctrl,
+ PINCTRL_STATE_DEFAULT);
+ hdmi_cfg->sleep = get_pinctrl_state(dev, pinctrl,
+ PINCTRL_STATE_SLEEP);
+ hdmi_cfg->pinctrl = pinctrl;
+ DBG("pinctrl initialized.");
+ }
#else
static struct hdmi_platform_config config = {};
static const char *hpd_clk_names[] = {
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index a155c4a..4742df2 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -22,6 +22,7 @@
#include <linux/clk.h>
#include <linux/platform_device.h>
#include <linux/regulator/consumer.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/hdmi.h>
#include "msm_drv.h"
@@ -95,6 +96,10 @@ struct hdmi_platform_config {
/* gpio's: */
int ddc_clk_gpio, ddc_data_gpio, hpd_gpio, mux_en_gpio, mux_sel_gpio;
int mux_lpm_gpio;
+
+ /* pinctrl: */
+ struct pinctrl *pinctrl;
+ struct pinctrl_state *active, *sleep;
};
void hdmi_set_mode(struct hdmi *hdmi, bool power_on);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
index 914bf95..cbffb8a 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -181,6 +181,23 @@ error1:
return ret;
}
+static int pinctrl_config(struct hdmi *hdmi, bool on)
+{
+ struct drm_device *dev = hdmi->dev;
+ const struct hdmi_platform_config *config = hdmi->config;
+ struct pinctrl_state *state = on ? config->active : config->sleep;
+ int ret;
+
+ ret = pinctrl_select_state(config->pinctrl, state);
+ if (ret)
+ dev_err(dev->dev, "failed to set pinctrl state to %s: %d\n",
+ on ? "active" : "sleep", ret);
+
+ DBG("pinctrl %s", on ? "on" : "off");
+
+ return ret;
+}
+
static int hpd_enable(struct hdmi_connector *hdmi_connector)
{
struct hdmi *hdmi = hdmi_connector->hdmi;
@@ -199,6 +216,14 @@ static int hpd_enable(struct hdmi_connector *hdmi_connector)
}
}
+ if (config->pinctrl) {
+ ret = pinctrl_config(hdmi, true);
+ if (ret) {
+ dev_err(dev->dev, "failed pinctrl config: %d\n", ret);
+ goto fail;
+ }
+ }
+
ret = gpio_config(hdmi, true);
if (ret) {
dev_err(dev->dev, "failed to configure GPIOs: %d\n", ret);
@@ -268,6 +293,12 @@ static void hdp_disable(struct hdmi_connector *hdmi_connector)
if (ret)
dev_warn(dev->dev, "failed to unconfigure GPIOs: %d\n", ret);
+ if (config->pinctrl) {
+ ret = pinctrl_config(hdmi, false);
+ if (ret)
+ dev_warn(dev->dev, "failed pinctrl config: %d\n", ret);
+ }
+
for (i = 0; i < config->hpd_reg_cnt; i++) {
ret = regulator_disable(hdmi->hpd_regs[i]);
if (ret)
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Stephane,
On Mon, 2015-06-01 at 16:28 -0400, Stephane Viau wrote:
> Some targets (eg: msm8994) use the pinctrl framework to configure
> interface pins. This change adds support for initialization and
> pinctrl active/sleep state control for the HDMI driver.
>
> Signed-off-by: Stephane Viau <[email protected]>
> ---
> v2:
> - Add devicetree binding documentation for pinctrl property [Ivan]
> - Use pinctrl framework's PINCTRL_STATE_DEFAULT/SLEEP states [Ivan]
>
<snip>
>
> static int hdmi_bind(struct device *dev, struct device *master, void *data)
> @@ -365,6 +379,7 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data)
> #ifdef CONFIG_OF
> struct device_node *of_node = dev->of_node;
> const struct of_device_id *match;
> + struct pinctrl *pinctrl;
>
> match = of_match_node(dt_match, of_node);
> if (match && match->data) {
> @@ -383,6 +398,18 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data)
> hdmi_cfg->mux_sel_gpio = get_gpio(dev, of_node, "qcom,hdmi-tx-mux-sel");
> hdmi_cfg->mux_lpm_gpio = get_gpio(dev, of_node, "qcom,hdmi-tx-mux-lpm");
>
> + /* not all targets have pinctrl, do not fail in case of error: */
> + pinctrl = devm_pinctrl_get(dev);
Probably I have to be more explicit. Why not using pins binding handled in driver
really_probe()? I have to admit that I am not familiar with DRM subsystem.
Regards,
Ivan
Hi Ivan,
>
> Hi Stephane,
>
> On Mon, 2015-06-01 at 16:28 -0400, Stephane Viau wrote:
>> Some targets (eg: msm8994) use the pinctrl framework to configure
>> interface pins. This change adds support for initialization and
>> pinctrl active/sleep state control for the HDMI driver.
>>
>> Signed-off-by: Stephane Viau <[email protected]>
>> ---
>> v2:
>> - Add devicetree binding documentation for pinctrl property [Ivan]
>> - Use pinctrl framework's PINCTRL_STATE_DEFAULT/SLEEP states [Ivan]
>>
>
> <snip>
>
>>
>> static int hdmi_bind(struct device *dev, struct device *master, void
>> *data)
>> @@ -365,6 +379,7 @@ static int hdmi_bind(struct device *dev, struct
>> device *master, void *data)
>> #ifdef CONFIG_OF
>> struct device_node *of_node = dev->of_node;
>> const struct of_device_id *match;
>> + struct pinctrl *pinctrl;
>>
>> match = of_match_node(dt_match, of_node);
>> if (match && match->data) {
>> @@ -383,6 +398,18 @@ static int hdmi_bind(struct device *dev, struct
>> device *master, void *data)
>> hdmi_cfg->mux_sel_gpio = get_gpio(dev, of_node,
>> "qcom,hdmi-tx-mux-sel");
>> hdmi_cfg->mux_lpm_gpio = get_gpio(dev, of_node,
>> "qcom,hdmi-tx-mux-lpm");
>>
>> + /* not all targets have pinctrl, do not fail in case of error:
>> */
>> + pinctrl = devm_pinctrl_get(dev);
>
> Probably I have to be more explicit. Why not using pins binding handled in
> driver
> really_probe()? I have to admit that I am not familiar with DRM subsystem.
This would work, indeed, for default/sleep/idle states.
I actually had in mind that we'd need to keep track of HDMI pinctrl states
because we may need to add a couple more in the near future in order to
independently enable/disable certain parts of the HDMI controller (eg:
HPD, DDC, CEC..).
Each of this HW sub-sections of the controller are driven by a different
"pin" in the downstream driver... But since this is nowhere close to being
upstream-ed yet, I'll go ahead with your idea of using the common pins
binding (v3 to follow).
Thanks,
Stephane.
>
> Regards,
> Ivan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
On Tue, 2015-06-02 at 10:12 -0500, "Stéphane Viau" wrote:
> Hi Ivan,
>
> > Hi Stephane,
> >
> > On Mon, 2015-06-01 at 16:28 -0400, Stephane Viau wrote:
> > > Some targets (eg: msm8994) use the pinctrl framework to configure
> > > interface pins. This change adds support for initialization and
> > > pinctrl active/sleep state control for the HDMI driver.
> > >
> > > Signed-off-by: Stephane Viau <[email protected]>
> > > ---
> > > v2:
> > > - Add devicetree binding documentation for pinctrl property [Ivan]
> > > - Use pinctrl framework's PINCTRL_STATE_DEFAULT/SLEEP states [Ivan]
> > >
> >
> > <snip>
> >
> > > static int hdmi_bind(struct device *dev, struct device *master, void
> > > *data)
> > > @@ -365,6 +379,7 @@ static int hdmi_bind(struct device *dev, struct
> > > device *master, void *data)
> > > #ifdef CONFIG_OF
> > > struct device_node *of_node = dev->of_node;
> > > const struct of_device_id *match;
> > > + struct pinctrl *pinctrl;
> > >
> > > match = of_match_node(dt_match, of_node);
> > > if (match && match->data) {
> > > @@ -383,6 +398,18 @@ static int hdmi_bind(struct device *dev, struct
> > > device *master, void *data)
> > > hdmi_cfg->mux_sel_gpio = get_gpio(dev, of_node,
> > > "qcom,hdmi-tx-mux-sel");
> > > hdmi_cfg->mux_lpm_gpio = get_gpio(dev, of_node,
> > > "qcom,hdmi-tx-mux-lpm");
> > >
> > > + /* not all targets have pinctrl, do not fail in case of error:
> > > */
> > > + pinctrl = devm_pinctrl_get(dev);
> >
> > Probably I have to be more explicit. Why not using pins binding handled in
> > driver
> > really_probe()? I have to admit that I am not familiar with DRM subsystem.
>
> This would work, indeed, for default/sleep/idle states.
>
> I actually had in mind that we'd need to keep track of HDMI pinctrl states
> because we may need to add a couple more in the near future in order to
> independently enable/disable certain parts of the HDMI controller (eg:
> HPD, DDC, CEC..).
>
> Each of this HW sub-sections of the controller are driven by a different
> "pin" in the downstream driver... But since this is nowhere close to being
> upstream-ed yet, I'll go ahead with your idea of using the common pins
> binding (v3 to follow).
v3? Why we need a patch if we already have required support?
Regards,
Ivan
>
> On Tue, 2015-06-02 at 10:12 -0500, "Stéphane Viau" wrote:
>> Hi Ivan,
>>
>> > Hi Stephane,
>> >
>> > On Mon, 2015-06-01 at 16:28 -0400, Stephane Viau wrote:
>> > > Some targets (eg: msm8994) use the pinctrl framework to configure
>> > > interface pins. This change adds support for initialization and
>> > > pinctrl active/sleep state control for the HDMI driver.
>> > >
>> > > Signed-off-by: Stephane Viau <[email protected]>
>> > > ---
>> > > v2:
>> > > - Add devicetree binding documentation for pinctrl property [Ivan]
>> > > - Use pinctrl framework's PINCTRL_STATE_DEFAULT/SLEEP states [Ivan]
>> > >
>> >
>> > <snip>
>> >
>> > > static int hdmi_bind(struct device *dev, struct device *master,
>> void
>> > > *data)
>> > > @@ -365,6 +379,7 @@ static int hdmi_bind(struct device *dev, struct
>> > > device *master, void *data)
>> > > #ifdef CONFIG_OF
>> > > struct device_node *of_node = dev->of_node;
>> > > const struct of_device_id *match;
>> > > + struct pinctrl *pinctrl;
>> > >
>> > > match = of_match_node(dt_match, of_node);
>> > > if (match && match->data) {
>> > > @@ -383,6 +398,18 @@ static int hdmi_bind(struct device *dev, struct
>> > > device *master, void *data)
>> > > hdmi_cfg->mux_sel_gpio = get_gpio(dev, of_node,
>> > > "qcom,hdmi-tx-mux-sel");
>> > > hdmi_cfg->mux_lpm_gpio = get_gpio(dev, of_node,
>> > > "qcom,hdmi-tx-mux-lpm");
>> > >
>> > > + /* not all targets have pinctrl, do not fail in case of
>> error:
>> > > */
>> > > + pinctrl = devm_pinctrl_get(dev);
>> >
>> > Probably I have to be more explicit. Why not using pins binding
>> handled in
>> > driver
>> > really_probe()? I have to admit that I am not familiar with DRM
>> subsystem.
>>
>> This would work, indeed, for default/sleep/idle states.
>>
>> I actually had in mind that we'd need to keep track of HDMI pinctrl
>> states
>> because we may need to add a couple more in the near future in order to
>> independently enable/disable certain parts of the HDMI controller (eg:
>> HPD, DDC, CEC..).
>>
>> Each of this HW sub-sections of the controller are driven by a different
>> "pin" in the downstream driver... But since this is nowhere close to
>> being
>> upstream-ed yet, I'll go ahead with your idea of using the common pins
>> binding (v3 to follow).
>
> v3? Why we need a patch if we already have required support?
To actually call pinctrl_pm_select_default/sleep_state() from the
drm/msm/hdmi driver (in hdp_enable/disable() functions).
Stephane.
>
> Regards,
> Ivan
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project