| 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
| ||
|
Date: Wed, 19 Jun 2024 10:36:27 -0700
From: Jessica Zhang <quic_jesszhan@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Rob Clark
<robdclark@...il.com>,
Abhinav Kumar <quic_abhinavk@...cinc.com>,
Sean Paul
<sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
"David
Airlie" <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>
CC: <linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
<freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 09/14] drm/msm/hdmi: implement proper runtime PM
handling
On 5/22/2024 3:51 AM, Dmitry Baryshkov wrote:
> It is completely not obvious, but the so-called 'hpd' clocks and
> regulators are required for the HDMI host to function properly. Merge
> pwr and hpd regulators. Use regulators, clocks and pinctrl to implement
> proper runtime PM callbacks.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Reviewed-by: Jessica Zhang <quic_jesszhan@...cinc.com>
> ---
> drivers/gpu/drm/msm/hdmi/hdmi.c | 62 +++++++++++++++++++++++++---------
> drivers/gpu/drm/msm/hdmi/hdmi.h | 7 +---
> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 12 -------
> drivers/gpu/drm/msm/hdmi/hdmi_hpd.c | 42 +----------------------
> 4 files changed, 48 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index 7ec4ca3b7597..cc671baad87b 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -8,6 +8,7 @@
> #include <linux/gpio/consumer.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> +#include <linux/pinctrl/consumer.h>
> #include <linux/platform_device.h>
>
> #include <drm/drm_bridge_connector.h>
> @@ -226,11 +227,11 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
> .item ## _names = item ##_names_ ## entry, \
> .item ## _cnt = ARRAY_SIZE(item ## _names_ ## entry)
>
> -static const char *hpd_reg_names_8960[] = {"core-vdda"};
> +static const char *pwr_reg_names_8960[] = {"core-vdda"};
> static const char *hpd_clk_names_8960[] = {"core", "master_iface", "slave_iface"};
>
> static const struct hdmi_platform_config hdmi_tx_8960_config = {
> - HDMI_CFG(hpd_reg, 8960),
> + HDMI_CFG(pwr_reg, 8960),
> HDMI_CFG(hpd_clk, 8960),
> };
>
> @@ -434,20 +435,6 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
> if (hdmi->irq < 0)
> return hdmi->irq;
>
> - hdmi->hpd_regs = devm_kcalloc(&pdev->dev,
> - config->hpd_reg_cnt,
> - sizeof(hdmi->hpd_regs[0]),
> - GFP_KERNEL);
> - if (!hdmi->hpd_regs)
> - return -ENOMEM;
> -
> - for (i = 0; i < config->hpd_reg_cnt; i++)
> - hdmi->hpd_regs[i].supply = config->hpd_reg_names[i];
> -
> - ret = devm_regulator_bulk_get(&pdev->dev, config->hpd_reg_cnt, hdmi->hpd_regs);
> - if (ret)
> - return dev_err_probe(dev, ret, "failed to get hpd regulators\n");
> -
> hdmi->pwr_regs = devm_kcalloc(&pdev->dev,
> config->pwr_reg_cnt,
> sizeof(hdmi->pwr_regs[0]),
> @@ -525,6 +512,48 @@ static void msm_hdmi_dev_remove(struct platform_device *pdev)
> msm_hdmi_put_phy(hdmi);
> }
>
> +static int msm_hdmi_runtime_suspend(struct device *dev)
> +{
> + struct hdmi *hdmi = dev_get_drvdata(dev);
> + const struct hdmi_platform_config *config = hdmi->config;
> +
> + clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
> +
> + pinctrl_pm_select_sleep_state(dev);
> +
> + regulator_bulk_disable(config->pwr_reg_cnt, hdmi->pwr_regs);
> +
> + return 0;
> +}
> +
> +static int msm_hdmi_runtime_resume(struct device *dev)
> +{
> + struct hdmi *hdmi = dev_get_drvdata(dev);
> + const struct hdmi_platform_config *config = hdmi->config;
> + int ret;
> +
> + ret = regulator_bulk_enable(config->pwr_reg_cnt, hdmi->pwr_regs);
> + if (ret)
> + return ret;
> +
> + ret = pinctrl_pm_select_default_state(dev);
> + if (ret)
> + goto fail;
> +
> + ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
> + if (ret)
> + goto fail;
> +
> + return 0;
> +
> +fail:
> + pinctrl_pm_select_sleep_state(dev);
> +
> + return ret;
> +}
> +
> +DEFINE_RUNTIME_DEV_PM_OPS(msm_hdmi_pm_ops, msm_hdmi_runtime_suspend, msm_hdmi_runtime_resume, NULL);
> +
> static const struct of_device_id msm_hdmi_dt_match[] = {
> { .compatible = "qcom,hdmi-tx-8996", .data = &hdmi_tx_8974_config },
> { .compatible = "qcom,hdmi-tx-8994", .data = &hdmi_tx_8974_config },
> @@ -541,6 +570,7 @@ static struct platform_driver msm_hdmi_driver = {
> .driver = {
> .name = "hdmi_msm",
> .of_match_table = msm_hdmi_dt_match,
> + .pm = &msm_hdmi_pm_ops,
> },
> };
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
> index eeba85ffef09..ee5463eb41b6 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.h
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
> @@ -48,7 +48,6 @@ struct hdmi {
> void __iomem *qfprom_mmio;
> phys_addr_t mmio_phy_addr;
>
> - struct regulator_bulk_data *hpd_regs;
> struct regulator_bulk_data *pwr_regs;
> struct clk_bulk_data *hpd_clks;
> struct clk *extp_clk;
> @@ -86,11 +85,7 @@ struct hdmi {
>
> /* platform config data (ie. from DT, or pdata) */
> struct hdmi_platform_config {
> - /* regulators that need to be on for hpd: */
> - const char **hpd_reg_names;
> - int hpd_reg_cnt;
> -
> - /* regulators that need to be on for screen pwr: */
> + /* regulators that need to be on: */
> const char **pwr_reg_names;
> int pwr_reg_cnt;
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> index d1b35328b6e8..cddba640d292 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> @@ -16,15 +16,10 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)
> struct drm_device *dev = bridge->dev;
> struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
> struct hdmi *hdmi = hdmi_bridge->hdmi;
> - const struct hdmi_platform_config *config = hdmi->config;
> int ret;
>
> pm_runtime_resume_and_get(&hdmi->pdev->dev);
>
> - ret = regulator_bulk_enable(config->pwr_reg_cnt, hdmi->pwr_regs);
> - if (ret)
> - DRM_DEV_ERROR(dev->dev, "failed to enable pwr regulator: %d\n", ret);
> -
> if (hdmi->extp_clk) {
> DBG("pixclock: %lu", hdmi->pixclock);
> ret = clk_set_rate(hdmi->extp_clk, hdmi->pixclock);
> @@ -39,11 +34,8 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)
>
> static void power_off(struct drm_bridge *bridge)
> {
> - struct drm_device *dev = bridge->dev;
> struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
> struct hdmi *hdmi = hdmi_bridge->hdmi;
> - const struct hdmi_platform_config *config = hdmi->config;
> - int ret;
>
> /* TODO do we need to wait for final vblank somewhere before
> * cutting the clocks?
> @@ -53,10 +45,6 @@ static void power_off(struct drm_bridge *bridge)
> if (hdmi->extp_clk)
> clk_disable_unprepare(hdmi->extp_clk);
>
> - ret = regulator_bulk_disable(config->pwr_reg_cnt, hdmi->pwr_regs);
> - if (ret)
> - DRM_DEV_ERROR(dev->dev, "failed to disable pwr regulator: %d\n", ret);
> -
> pm_runtime_put(&hdmi->pdev->dev);
> }
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> index fc21ad3b01dc..32e447267e3b 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> @@ -64,36 +64,17 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
> {
> struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
> struct hdmi *hdmi = hdmi_bridge->hdmi;
> - const struct hdmi_platform_config *config = hdmi->config;
> struct device *dev = &hdmi->pdev->dev;
> uint32_t hpd_ctrl;
> int ret;
> unsigned long flags;
>
> - ret = regulator_bulk_enable(config->hpd_reg_cnt, hdmi->hpd_regs);
> - if (ret) {
> - DRM_DEV_ERROR(dev, "failed to enable hpd regulators: %d\n", ret);
> - goto fail;
> - }
> -
> - ret = pinctrl_pm_select_default_state(dev);
> - if (ret) {
> - DRM_DEV_ERROR(dev, "pinctrl state chg failed: %d\n", ret);
> - goto fail;
> - }
> -
> if (hdmi->hpd_gpiod)
> gpiod_set_value_cansleep(hdmi->hpd_gpiod, 1);
>
> ret = pm_runtime_resume_and_get(dev);
> - if (ret) {
> - DRM_DEV_ERROR(dev, "runtime resume failed: %d\n", ret);
> - goto fail;
> - }
> -
> - ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
> if (ret)
> - goto fail;
> + return ret;
>
> msm_hdmi_set_mode(hdmi, false);
> msm_hdmi_phy_reset(hdmi);
> @@ -119,32 +100,18 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
> spin_unlock_irqrestore(&hdmi->reg_lock, flags);
>
> return 0;
> -
> -fail:
> - return ret;
> }
>
> void msm_hdmi_hpd_disable(struct hdmi *hdmi)
> {
> - const struct hdmi_platform_config *config = hdmi->config;
> struct device *dev = &hdmi->pdev->dev;
> - int ret;
>
> /* Disable HPD interrupt */
> hdmi_write(hdmi, REG_HDMI_HPD_INT_CTRL, 0);
>
> msm_hdmi_set_mode(hdmi, false);
>
> - clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
> pm_runtime_put(dev);
> -
> - ret = pinctrl_pm_select_sleep_state(dev);
> - if (ret)
> - dev_warn(dev, "pinctrl state chg failed: %d\n", ret);
> -
> - ret = regulator_bulk_disable(config->hpd_reg_cnt, hdmi->hpd_regs);
> - if (ret)
> - dev_warn(dev, "failed to disable hpd regulator: %d\n", ret);
> }
>
> void msm_hdmi_hpd_irq(struct drm_bridge *bridge)
> @@ -179,7 +146,6 @@ void msm_hdmi_hpd_irq(struct drm_bridge *bridge)
>
> static enum drm_connector_status detect_reg(struct hdmi *hdmi)
> {
> - const struct hdmi_platform_config *config = hdmi->config;
> uint32_t hpd_int_status = 0;
> int ret;
>
> @@ -187,14 +153,8 @@ static enum drm_connector_status detect_reg(struct hdmi *hdmi)
> if (ret)
> goto out;
>
> - ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
> - if (ret)
> - goto out;
> -
> hpd_int_status = hdmi_read(hdmi, REG_HDMI_HPD_INT_STATUS);
>
> - clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
> -
> out:
> pm_runtime_put(&hdmi->pdev->dev);
>
>
> --
> 2.39.2
>
Powered by blists - more mailing lists