[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fec9efa1-8e8a-48a5-8d8b-8c6346a06dad@quicinc.com>
Date: Tue, 11 Jun 2024 11:16:21 -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 02/14] drm/msm/hdmi: simplify extp clock handling
On 5/22/2024 3:50 AM, Dmitry Baryshkov wrote:
> With the extp being the only "power" clock left, remove the surrounding
> loops and handle the extp clock directly.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Reviewed-by: Jessica Zhang <quic_jesszhan@...cinc.com>
> ---
> drivers/gpu/drm/msm/hdmi/hdmi.c | 24 ++++--------------------
> drivers/gpu/drm/msm/hdmi/hdmi.h | 6 +-----
> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 33 +++++++++++++--------------------
> 3 files changed, 18 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index 108c86925780..681265e29aa0 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -235,13 +235,11 @@ static const struct hdmi_platform_config hdmi_tx_8960_config = {
> };
>
> static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
> -static const char *pwr_clk_names_8x74[] = {"extp"};
> static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core", "alt_iface"};
> static unsigned long hpd_clk_freq_8x74[] = {0, 19200000, 0, 0};
>
> static const struct hdmi_platform_config hdmi_tx_8974_config = {
> HDMI_CFG(pwr_reg, 8x74),
> - HDMI_CFG(pwr_clk, 8x74),
> HDMI_CFG(hpd_clk, 8x74),
> .hpd_freq = hpd_clk_freq_8x74,
> };
> @@ -485,24 +483,10 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
> hdmi->hpd_clks[i] = clk;
> }
>
> - hdmi->pwr_clks = devm_kcalloc(&pdev->dev,
> - config->pwr_clk_cnt,
> - sizeof(hdmi->pwr_clks[0]),
> - GFP_KERNEL);
> - if (!hdmi->pwr_clks)
> - return -ENOMEM;
> -
> - for (i = 0; i < config->pwr_clk_cnt; i++) {
> - struct clk *clk;
> -
> - clk = msm_clk_get(pdev, config->pwr_clk_names[i]);
> - if (IS_ERR(clk))
> - return dev_err_probe(dev, PTR_ERR(clk),
> - "failed to get pwr clk: %s\n",
> - config->pwr_clk_names[i]);
> -
> - hdmi->pwr_clks[i] = clk;
> - }
> + hdmi->extp_clk = devm_clk_get_optional(&pdev->dev, "extp");
> + if (IS_ERR(hdmi->extp_clk))
> + return dev_err_probe(dev, PTR_ERR(hdmi->extp_clk),
> + "failed to get extp clock\n");
>
> hdmi->hpd_gpiod = devm_gpiod_get_optional(&pdev->dev, "hpd", GPIOD_IN);
> /* This will catch e.g. -EPROBE_DEFER */
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
> index 4586baf36415..abdbe4779cf9 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.h
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
> @@ -51,7 +51,7 @@ struct hdmi {
> struct regulator_bulk_data *hpd_regs;
> struct regulator_bulk_data *pwr_regs;
> struct clk **hpd_clks;
> - struct clk **pwr_clks;
> + struct clk *extp_clk;
>
> struct gpio_desc *hpd_gpiod;
>
> @@ -98,10 +98,6 @@ struct hdmi_platform_config {
> const char **hpd_clk_names;
> const long unsigned *hpd_freq;
> int hpd_clk_cnt;
> -
> - /* clks that need to be on for screen pwr (ie pixel clk): */
> - const char **pwr_clk_names;
> - int pwr_clk_cnt;
> };
>
> struct hdmi_bridge {
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> index 4a5b5112227f..9eb4d06bdc0e 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> @@ -17,7 +17,7 @@ static void msm_hdmi_power_on(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;
> - int i, ret;
> + int ret;
>
> pm_runtime_get_sync(&hdmi->pdev->dev);
>
> @@ -25,21 +25,15 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)
> if (ret)
> DRM_DEV_ERROR(dev->dev, "failed to enable pwr regulator: %d\n", ret);
>
> - if (config->pwr_clk_cnt > 0) {
> + if (hdmi->extp_clk) {
> DBG("pixclock: %lu", hdmi->pixclock);
> - ret = clk_set_rate(hdmi->pwr_clks[0], hdmi->pixclock);
> - if (ret) {
> - DRM_DEV_ERROR(dev->dev, "failed to set pixel clk: %s (%d)\n",
> - config->pwr_clk_names[0], ret);
> - }
> - }
> + ret = clk_set_rate(hdmi->extp_clk, hdmi->pixclock);
> + if (ret)
> + DRM_DEV_ERROR(dev->dev, "failed to set extp clk rate: %d\n", ret);
>
> - for (i = 0; i < config->pwr_clk_cnt; i++) {
> - ret = clk_prepare_enable(hdmi->pwr_clks[i]);
> - if (ret) {
> - DRM_DEV_ERROR(dev->dev, "failed to enable pwr clk: %s (%d)\n",
> - config->pwr_clk_names[i], ret);
> - }
> + ret = clk_prepare_enable(hdmi->extp_clk);
> + if (ret)
> + DRM_DEV_ERROR(dev->dev, "failed to enable extp clk: %d\n", ret);
> }
> }
>
> @@ -49,15 +43,15 @@ static void power_off(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;
> - int i, ret;
> + int ret;
>
> /* TODO do we need to wait for final vblank somewhere before
> * cutting the clocks?
> */
> mdelay(16 + 4);
>
> - for (i = 0; i < config->pwr_clk_cnt; i++)
> - clk_disable_unprepare(hdmi->pwr_clks[i]);
> + if (hdmi->extp_clk)
> + clk_disable_unprepare(hdmi->extp_clk);
>
> ret = regulator_bulk_disable(config->pwr_reg_cnt, hdmi->pwr_regs);
> if (ret)
> @@ -271,7 +265,6 @@ static enum drm_mode_status msm_hdmi_bridge_mode_valid(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 msm_drm_private *priv = bridge->dev->dev_private;
> struct msm_kms *kms = priv->kms;
> long actual, requested;
> @@ -285,8 +278,8 @@ static enum drm_mode_status msm_hdmi_bridge_mode_valid(struct drm_bridge *bridge
> if (kms->funcs->round_pixclk)
> actual = kms->funcs->round_pixclk(kms,
> requested, hdmi_bridge->hdmi->encoder);
> - else if (config->pwr_clk_cnt > 0)
> - actual = clk_round_rate(hdmi->pwr_clks[0], requested);
> + else if (hdmi->extp_clk)
> + actual = clk_round_rate(hdmi->extp_clk, requested);
> else
> actual = requested;
>
>
> --
> 2.39.2
>
Powered by blists - more mailing lists