2022-06-15 05:11:05

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 0/6] drm/sun4i: HDMI PHY cleanup/refactoring

This series prepares the sun8i HDMI PHY driver for supporting the new
custom PHY in the Allwinner D1 SoC. No functional change intended here.

This series was tested on D1, H3, and H6.

Changes in v2:
- Move error handling inside variant checks in probe function

Samuel Holland (6):
drm/sun4i: sun8i-hdmi-phy: Use of_device_get_match_data
drm/sun4i: sun8i-hdmi-phy: Use devm_platform_ioremap_resource
drm/sun4i: sun8i-hdmi-phy: Used device-managed clocks/resets
drm/sun4i: sun8i-hdmi-phy: Support multiple custom PHY ops
drm/sun4i: sun8i-hdmi-phy: Separate A83T and H3 PHY ops
drm/sun4i: sun8i-hdmi-phy: Group PHY ops functions by generation

drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 9 +-
drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 242 ++++++++++---------------
2 files changed, 97 insertions(+), 154 deletions(-)

--
2.35.1


2022-06-15 05:11:26

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 5/6] drm/sun4i: sun8i-hdmi-phy: Separate A83T and H3 PHY ops

Since the driver already needs to support multiple sets of ops, we can
drop the mid-layer used by the A83T and H3 PHYs. They share only a small
amount of code; factor this out as sun8i_hdmi_phy_set_polarity.

For clarity, this commit keeps the existing function order.

Signed-off-by: Samuel Holland <[email protected]>
---

(no changes since v1)

drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 5 --
drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 89 +++++++++++++-------------
2 files changed, 46 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
index f0b1aaad27d9..f082b8ecfe2c 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
@@ -156,11 +156,6 @@ struct sun8i_hdmi_phy_variant {
const struct dw_hdmi_phy_config *phy_cfg;
const struct dw_hdmi_phy_ops *phy_ops;
void (*phy_init)(struct sun8i_hdmi_phy *phy);
- void (*phy_disable)(struct dw_hdmi *hdmi,
- struct sun8i_hdmi_phy *phy);
- int (*phy_config)(struct dw_hdmi *hdmi,
- struct sun8i_hdmi_phy *phy,
- unsigned int clk_rate);
};

struct sun8i_hdmi_phy {
diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
index e6d25bbe3d2f..f94c1ddddbad 100644
--- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
+++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
@@ -123,10 +123,18 @@ static const struct dw_hdmi_phy_config sun50i_h6_phy_config[] = {
{ ~0UL, 0x0000, 0x0000, 0x0000}
};

-static int sun8i_hdmi_phy_config_a83t(struct dw_hdmi *hdmi,
- struct sun8i_hdmi_phy *phy,
- unsigned int clk_rate)
+static void sun8i_hdmi_phy_set_polarity(struct sun8i_hdmi_phy *phy,
+ const struct drm_display_mode *mode);
+
+static int sun8i_a83t_hdmi_phy_config(struct dw_hdmi *hdmi, void *data,
+ const struct drm_display_info *display,
+ const struct drm_display_mode *mode)
{
+ unsigned int clk_rate = mode->crtc_clock * 1000;
+ struct sun8i_hdmi_phy *phy = data;
+
+ sun8i_hdmi_phy_set_polarity(phy, mode);
+
regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_REXT_CTRL_REG,
SUN8I_HDMI_PHY_REXT_CTRL_REXT_EN,
SUN8I_HDMI_PHY_REXT_CTRL_REXT_EN);
@@ -185,10 +193,12 @@ static int sun8i_hdmi_phy_config_a83t(struct dw_hdmi *hdmi,
return 0;
}

-static int sun8i_hdmi_phy_config_h3(struct dw_hdmi *hdmi,
- struct sun8i_hdmi_phy *phy,
- unsigned int clk_rate)
+static int sun8i_h3_hdmi_phy_config(struct dw_hdmi *hdmi, void *data,
+ const struct drm_display_info *display,
+ const struct drm_display_mode *mode)
{
+ unsigned int clk_rate = mode->crtc_clock * 1000;
+ struct sun8i_hdmi_phy *phy = data;
u32 pll_cfg1_init;
u32 pll_cfg2_init;
u32 ana_cfg1_end;
@@ -197,6 +207,11 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi *hdmi,
u32 b_offset = 0;
u32 val;

+ if (phy->variant->has_phy_clk)
+ clk_set_rate(phy->clk_phy, clk_rate);
+
+ sun8i_hdmi_phy_set_polarity(phy, mode);
+
/* bandwidth / frequency independent settings */

pll_cfg1_init = SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN |
@@ -333,11 +348,9 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi *hdmi,
return 0;
}

-static int sun8i_hdmi_phy_config(struct dw_hdmi *hdmi, void *data,
- const struct drm_display_info *display,
- const struct drm_display_mode *mode)
+static void sun8i_hdmi_phy_set_polarity(struct sun8i_hdmi_phy *phy,
+ const struct drm_display_mode *mode)
{
- struct sun8i_hdmi_phy *phy = (struct sun8i_hdmi_phy *)data;
u32 val = 0;

if (mode->flags & DRM_MODE_FLAG_NHSYNC)
@@ -348,16 +361,12 @@ static int sun8i_hdmi_phy_config(struct dw_hdmi *hdmi, void *data,

regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_DBG_CTRL_REG,
SUN8I_HDMI_PHY_DBG_CTRL_POL_MASK, val);
-
- if (phy->variant->has_phy_clk)
- clk_set_rate(phy->clk_phy, mode->crtc_clock * 1000);
-
- return phy->variant->phy_config(hdmi, phy, mode->crtc_clock * 1000);
};

-static void sun8i_hdmi_phy_disable_a83t(struct dw_hdmi *hdmi,
- struct sun8i_hdmi_phy *phy)
+static void sun8i_a83t_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
{
+ struct sun8i_hdmi_phy *phy = data;
+
dw_hdmi_phy_gen2_txpwron(hdmi, 0);
dw_hdmi_phy_gen2_pddq(hdmi, 1);

@@ -365,9 +374,10 @@ static void sun8i_hdmi_phy_disable_a83t(struct dw_hdmi *hdmi,
SUN8I_HDMI_PHY_REXT_CTRL_REXT_EN, 0);
}

-static void sun8i_hdmi_phy_disable_h3(struct dw_hdmi *hdmi,
- struct sun8i_hdmi_phy *phy)
+static void sun8i_h3_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
{
+ struct sun8i_hdmi_phy *phy = data;
+
regmap_write(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG,
SUN8I_HDMI_PHY_ANA_CFG1_LDOEN |
SUN8I_HDMI_PHY_ANA_CFG1_ENVBS |
@@ -375,19 +385,20 @@ static void sun8i_hdmi_phy_disable_h3(struct dw_hdmi *hdmi,
regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, 0);
}

-static void sun8i_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
-{
- struct sun8i_hdmi_phy *phy = (struct sun8i_hdmi_phy *)data;
-
- phy->variant->phy_disable(hdmi, phy);
-}
+static const struct dw_hdmi_phy_ops sun8i_a83t_hdmi_phy_ops = {
+ .init = sun8i_a83t_hdmi_phy_config,
+ .disable = sun8i_a83t_hdmi_phy_disable,
+ .read_hpd = dw_hdmi_phy_read_hpd,
+ .update_hpd = dw_hdmi_phy_update_hpd,
+ .setup_hpd = dw_hdmi_phy_setup_hpd,
+};

-static const struct dw_hdmi_phy_ops sun8i_hdmi_phy_ops = {
- .init = &sun8i_hdmi_phy_config,
- .disable = &sun8i_hdmi_phy_disable,
- .read_hpd = &dw_hdmi_phy_read_hpd,
- .update_hpd = &dw_hdmi_phy_update_hpd,
- .setup_hpd = &dw_hdmi_phy_setup_hpd,
+static const struct dw_hdmi_phy_ops sun8i_h3_hdmi_phy_ops = {
+ .init = sun8i_h3_hdmi_phy_config,
+ .disable = sun8i_h3_hdmi_phy_disable,
+ .read_hpd = dw_hdmi_phy_read_hpd,
+ .update_hpd = dw_hdmi_phy_update_hpd,
+ .setup_hpd = dw_hdmi_phy_setup_hpd,
};

static void sun8i_hdmi_phy_unlock(struct sun8i_hdmi_phy *phy)
@@ -587,35 +598,27 @@ static const struct regmap_config sun8i_hdmi_phy_regmap_config = {
};

static const struct sun8i_hdmi_phy_variant sun8i_a83t_hdmi_phy = {
- .phy_ops = &sun8i_hdmi_phy_ops,
+ .phy_ops = &sun8i_a83t_hdmi_phy_ops,
.phy_init = &sun8i_hdmi_phy_init_a83t,
- .phy_disable = &sun8i_hdmi_phy_disable_a83t,
- .phy_config = &sun8i_hdmi_phy_config_a83t,
};

static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {
.has_phy_clk = true,
- .phy_ops = &sun8i_hdmi_phy_ops,
+ .phy_ops = &sun8i_h3_hdmi_phy_ops,
.phy_init = &sun8i_hdmi_phy_init_h3,
- .phy_disable = &sun8i_hdmi_phy_disable_h3,
- .phy_config = &sun8i_hdmi_phy_config_h3,
};

static const struct sun8i_hdmi_phy_variant sun8i_r40_hdmi_phy = {
.has_phy_clk = true,
.has_second_pll = true,
- .phy_ops = &sun8i_hdmi_phy_ops,
+ .phy_ops = &sun8i_h3_hdmi_phy_ops,
.phy_init = &sun8i_hdmi_phy_init_h3,
- .phy_disable = &sun8i_hdmi_phy_disable_h3,
- .phy_config = &sun8i_hdmi_phy_config_h3,
};

static const struct sun8i_hdmi_phy_variant sun50i_a64_hdmi_phy = {
.has_phy_clk = true,
- .phy_ops = &sun8i_hdmi_phy_ops,
+ .phy_ops = &sun8i_h3_hdmi_phy_ops,
.phy_init = &sun8i_hdmi_phy_init_h3,
- .phy_disable = &sun8i_hdmi_phy_disable_h3,
- .phy_config = &sun8i_hdmi_phy_config_h3,
};

static const struct sun8i_hdmi_phy_variant sun50i_h6_hdmi_phy = {
--
2.35.1

2022-06-15 05:13:30

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 6/6] drm/sun4i: sun8i-hdmi-phy: Group PHY ops functions by generation

Now that the PHY ops are separated, sort them topologically, with the
common sun8i_hdmi_phy_set_polarity helper at the top. No function
contents are changed in this commit.

Signed-off-by: Samuel Holland <[email protected]>
---

(no changes since v1)

drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 67 ++++++++++++--------------
1 file changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
index f94c1ddddbad..ca53b5e9fffc 100644
--- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
+++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
@@ -124,7 +124,19 @@ static const struct dw_hdmi_phy_config sun50i_h6_phy_config[] = {
};

static void sun8i_hdmi_phy_set_polarity(struct sun8i_hdmi_phy *phy,
- const struct drm_display_mode *mode);
+ const struct drm_display_mode *mode)
+{
+ u32 val = 0;
+
+ if (mode->flags & DRM_MODE_FLAG_NHSYNC)
+ val |= SUN8I_HDMI_PHY_DBG_CTRL_POL_NHSYNC;
+
+ if (mode->flags & DRM_MODE_FLAG_NVSYNC)
+ val |= SUN8I_HDMI_PHY_DBG_CTRL_POL_NVSYNC;
+
+ regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_DBG_CTRL_REG,
+ SUN8I_HDMI_PHY_DBG_CTRL_POL_MASK, val);
+};

static int sun8i_a83t_hdmi_phy_config(struct dw_hdmi *hdmi, void *data,
const struct drm_display_info *display,
@@ -193,6 +205,25 @@ static int sun8i_a83t_hdmi_phy_config(struct dw_hdmi *hdmi, void *data,
return 0;
}

+static void sun8i_a83t_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
+{
+ struct sun8i_hdmi_phy *phy = data;
+
+ dw_hdmi_phy_gen2_txpwron(hdmi, 0);
+ dw_hdmi_phy_gen2_pddq(hdmi, 1);
+
+ regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_REXT_CTRL_REG,
+ SUN8I_HDMI_PHY_REXT_CTRL_REXT_EN, 0);
+}
+
+static const struct dw_hdmi_phy_ops sun8i_a83t_hdmi_phy_ops = {
+ .init = sun8i_a83t_hdmi_phy_config,
+ .disable = sun8i_a83t_hdmi_phy_disable,
+ .read_hpd = dw_hdmi_phy_read_hpd,
+ .update_hpd = dw_hdmi_phy_update_hpd,
+ .setup_hpd = dw_hdmi_phy_setup_hpd,
+};
+
static int sun8i_h3_hdmi_phy_config(struct dw_hdmi *hdmi, void *data,
const struct drm_display_info *display,
const struct drm_display_mode *mode)
@@ -348,32 +379,6 @@ static int sun8i_h3_hdmi_phy_config(struct dw_hdmi *hdmi, void *data,
return 0;
}

-static void sun8i_hdmi_phy_set_polarity(struct sun8i_hdmi_phy *phy,
- const struct drm_display_mode *mode)
-{
- u32 val = 0;
-
- if (mode->flags & DRM_MODE_FLAG_NHSYNC)
- val |= SUN8I_HDMI_PHY_DBG_CTRL_POL_NHSYNC;
-
- if (mode->flags & DRM_MODE_FLAG_NVSYNC)
- val |= SUN8I_HDMI_PHY_DBG_CTRL_POL_NVSYNC;
-
- regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_DBG_CTRL_REG,
- SUN8I_HDMI_PHY_DBG_CTRL_POL_MASK, val);
-};
-
-static void sun8i_a83t_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
-{
- struct sun8i_hdmi_phy *phy = data;
-
- dw_hdmi_phy_gen2_txpwron(hdmi, 0);
- dw_hdmi_phy_gen2_pddq(hdmi, 1);
-
- regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_REXT_CTRL_REG,
- SUN8I_HDMI_PHY_REXT_CTRL_REXT_EN, 0);
-}
-
static void sun8i_h3_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
{
struct sun8i_hdmi_phy *phy = data;
@@ -385,14 +390,6 @@ static void sun8i_h3_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, 0);
}

-static const struct dw_hdmi_phy_ops sun8i_a83t_hdmi_phy_ops = {
- .init = sun8i_a83t_hdmi_phy_config,
- .disable = sun8i_a83t_hdmi_phy_disable,
- .read_hpd = dw_hdmi_phy_read_hpd,
- .update_hpd = dw_hdmi_phy_update_hpd,
- .setup_hpd = dw_hdmi_phy_setup_hpd,
-};
-
static const struct dw_hdmi_phy_ops sun8i_h3_hdmi_phy_ops = {
.init = sun8i_h3_hdmi_phy_config,
.disable = sun8i_h3_hdmi_phy_disable,
--
2.35.1

2022-06-15 05:13:50

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 4/6] drm/sun4i: sun8i-hdmi-phy: Support multiple custom PHY ops

The D1 SoC comes with a new custom HDMI PHY, which does not share any
registers with the existing custom PHY. So it needs a new set of ops.
Instead of providing a flag in the variant structure, provide the ops
themselves.

Signed-off-by: Samuel Holland <[email protected]>
---

(no changes since v1)

drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 2 +-
drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 12 ++++++------
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
index 0adbfac6eb31..f0b1aaad27d9 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
@@ -151,10 +151,10 @@ struct sun8i_hdmi_phy;
struct sun8i_hdmi_phy_variant {
bool has_phy_clk;
bool has_second_pll;
- unsigned int is_custom_phy : 1;
const struct dw_hdmi_curr_ctrl *cur_ctr;
const struct dw_hdmi_mpll_config *mpll_cfg;
const struct dw_hdmi_phy_config *phy_cfg;
+ const struct dw_hdmi_phy_ops *phy_ops;
void (*phy_init)(struct sun8i_hdmi_phy *phy);
void (*phy_disable)(struct dw_hdmi *hdmi,
struct sun8i_hdmi_phy *phy);
diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
index 9086ce547fad..e6d25bbe3d2f 100644
--- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
+++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
@@ -567,8 +567,8 @@ void sun8i_hdmi_phy_set_ops(struct sun8i_hdmi_phy *phy,
{
const struct sun8i_hdmi_phy_variant *variant = phy->variant;

- if (variant->is_custom_phy) {
- plat_data->phy_ops = &sun8i_hdmi_phy_ops;
+ if (variant->phy_ops) {
+ plat_data->phy_ops = variant->phy_ops;
plat_data->phy_name = "sun8i_dw_hdmi_phy";
plat_data->phy_data = phy;
} else {
@@ -587,7 +587,7 @@ static const struct regmap_config sun8i_hdmi_phy_regmap_config = {
};

static const struct sun8i_hdmi_phy_variant sun8i_a83t_hdmi_phy = {
- .is_custom_phy = true,
+ .phy_ops = &sun8i_hdmi_phy_ops,
.phy_init = &sun8i_hdmi_phy_init_a83t,
.phy_disable = &sun8i_hdmi_phy_disable_a83t,
.phy_config = &sun8i_hdmi_phy_config_a83t,
@@ -595,7 +595,7 @@ static const struct sun8i_hdmi_phy_variant sun8i_a83t_hdmi_phy = {

static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {
.has_phy_clk = true,
- .is_custom_phy = true,
+ .phy_ops = &sun8i_hdmi_phy_ops,
.phy_init = &sun8i_hdmi_phy_init_h3,
.phy_disable = &sun8i_hdmi_phy_disable_h3,
.phy_config = &sun8i_hdmi_phy_config_h3,
@@ -604,7 +604,7 @@ static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {
static const struct sun8i_hdmi_phy_variant sun8i_r40_hdmi_phy = {
.has_phy_clk = true,
.has_second_pll = true,
- .is_custom_phy = true,
+ .phy_ops = &sun8i_hdmi_phy_ops,
.phy_init = &sun8i_hdmi_phy_init_h3,
.phy_disable = &sun8i_hdmi_phy_disable_h3,
.phy_config = &sun8i_hdmi_phy_config_h3,
@@ -612,7 +612,7 @@ static const struct sun8i_hdmi_phy_variant sun8i_r40_hdmi_phy = {

static const struct sun8i_hdmi_phy_variant sun50i_a64_hdmi_phy = {
.has_phy_clk = true,
- .is_custom_phy = true,
+ .phy_ops = &sun8i_hdmi_phy_ops,
.phy_init = &sun8i_hdmi_phy_init_h3,
.phy_disable = &sun8i_hdmi_phy_disable_h3,
.phy_config = &sun8i_hdmi_phy_config_h3,
--
2.35.1

2022-06-16 08:01:23

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] drm/sun4i: HDMI PHY cleanup/refactoring

On Tue, 14 Jun 2022 23:55:37 -0500, Samuel Holland wrote:
> This series prepares the sun8i HDMI PHY driver for supporting the new
> custom PHY in the Allwinner D1 SoC. No functional change intended here.
>
> This series was tested on D1, H3, and H6.
>
> Changes in v2:
> - Move error handling inside variant checks in probe function
>
> [...]

Applied to drm/drm-misc (drm-misc-next).

Thanks!
Maxime