| Message ID | 1433190528-30026-1-git-send-email-sviau@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <sviau@codeaurora.org> > --- > 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 <sviau@codeaurora.org> >> --- >> 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
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 <sviau@codeaurora.org> > > > --- > > > 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 <sviau@codeaurora.org> >> > > --- >> > > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
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)
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 <sviau@codeaurora.org> --- 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(+)