diff mbox series

[7/9] drm/meson: dw-hdmi: use matched data

Message ID 20240730125023.710237-8-jbrunet@baylibre.com (mailing list archive)
State New, archived
Headers show
Series drm/meson: dw-hdmi: clean-up | expand

Commit Message

Jerome Brunet July 30, 2024, 12:50 p.m. UTC 
Using several string comparisons with if/else if/else clauses
is fairly inefficient and does not scale well.

Use matched data to tweak the driver depending on the matched
SoC instead.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 209 +++++++++++++++++---------
 1 file changed, 139 insertions(+), 70 deletions(-)

Comments

Martin Blumenstingl Aug. 6, 2024, 9:03 p.m. UTC | #1 
Hi Jerome,

On Tue, Jul 30, 2024 at 2:50 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
[...]
> +       }, {
> +               .limit = 297000,
> +               .regs = gxbb_3g_regs,
> +               .reg_num = ARRAY_SIZE(gxbb_3g_regs)
Just as a side-note: this looked odd when reading for the first time
as I thought that it's a typo (and it should be gxbb_2g97_regs - but
that name is not used).

[...]
> +static const struct meson_dw_hdmi_speed gxl_speeds[] = {
> +       {
> +               .limit = 371250,
> +               .regs = gxl_3g7_regs,
> +               .reg_num = ARRAY_SIZE(gxl_3g7_regs)
> +       }, {
> +               .limit = 297000,
> +               .regs = gxl_3g_regs,
> +               .reg_num = ARRAY_SIZE(gxl_3g_regs)
> +       }, {
> +               .limit = 148500,
> +               .regs = gxl_def_regs,
> +               .reg_num = ARRAY_SIZE(gxl_def_regs)
this is not consistent with what we have above or below so it either
needs to be updated or a comment.
I think this should be called gxl_1g48_regs

> +       }, {
> +               .regs = gxl_270m_regs,
> +               .reg_num = ARRAY_SIZE(gxl_270m_regs)
and this should be called gxl_def_regs



Best regards,
Martin
Jerome Brunet Aug. 7, 2024, 9:12 a.m. UTC | #2 
On Tue 06 Aug 2024 at 23:03, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Jerome,
>
> On Tue, Jul 30, 2024 at 2:50 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> [...]
>> +       }, {
>> +               .limit = 297000,
>> +               .regs = gxbb_3g_regs,
>> +               .reg_num = ARRAY_SIZE(gxbb_3g_regs)
> Just as a side-note: this looked odd when reading for the first time
> as I thought that it's a typo (and it should be gxbb_2g97_regs - but
> that name is not used).

I know it looks odd but there is a (perhaps silly) reason for it.

The names are derived from PHY modes used in the Amlogic vendor tree.
Those are magic pokes and we don't really know anything about it. The
vendor tree often update and mainline does not always follow. I know
that we are not 100% aligned right now. No one knows what branch is the
reference to follow anyway.

Using the same names is way to leave breadcrumbs that may help people
linking this to vendor code later on (if necessary)

I think the modes are named after the (rounded) bandwidth they provide,
not necessarily the limit we are using to pick it ... except for def,
which might just mean 'default' I guess.

https://github.com/khadas/linux/blob/khadas-vims-5.4.y/drivers/amlogic/media/vout/hdmitx/hdmi_tx_20/hw/hw_g12a.c#L589

I focused on keeping mainline as it was for the value poked, retaining
as much information as possible to find our way back.

Your proposed naming convention is fine by me as well.

>
> [...]
>> +static const struct meson_dw_hdmi_speed gxl_speeds[] = {
>> +       {
>> +               .limit = 371250,
>> +               .regs = gxl_3g7_regs,
>> +               .reg_num = ARRAY_SIZE(gxl_3g7_regs)
>> +       }, {
>> +               .limit = 297000,
>> +               .regs = gxl_3g_regs,
>> +               .reg_num = ARRAY_SIZE(gxl_3g_regs)
>> +       }, {
>> +               .limit = 148500,
>> +               .regs = gxl_def_regs,
>> +               .reg_num = ARRAY_SIZE(gxl_def_regs)
> this is not consistent with what we have above or below so it either
> needs to be updated or a comment.
> I think this should be called gxl_1g48_regs
>
>> +       }, {
>> +               .regs = gxl_270m_regs,
>> +               .reg_num = ARRAY_SIZE(gxl_270m_regs)
> and this should be called gxl_def_regs

def in not the last one, it is another name used by AML

It so happens that on mainline with we have only put the SD/270M for
gxl. In the AML tree, it does exist for G12 too. Maybe it will be needed someday.

>
>
>
> Best regards,
> Martin
Neil Armstrong Aug. 19, 2024, 4:25 p.m. UTC | #3 
On 30/07/2024 14:50, Jerome Brunet wrote:
> Using several string comparisons with if/else if/else clauses
> is fairly inefficient and does not scale well.

Inefficient in which way ? speed ? code size ?

It doesn't scale, but AFAIK Amlogic stopped using the Synopsys DWC controller after the G12B SoCs....

> 
> Use matched data to tweak the driver depending on the matched
> SoC instead.

This leads to a very overcomplicated code I'll need some time to review and understand

> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>   drivers/gpu/drm/meson/meson_dw_hdmi.c | 209 +++++++++++++++++---------
>   1 file changed, 139 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 7c39e5c99043..ef059c5ef520 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -125,8 +125,17 @@
>   #define HHI_HDMI_PHY_CNTL4	0x3b0 /* 0xec */
>   #define HHI_HDMI_PHY_CNTL5	0x3b4 /* 0xed */
>   
> +struct meson_dw_hdmi_speed {
> +	const struct reg_sequence *regs;
> +	unsigned int reg_num;
> +	unsigned int limit;
> +};
> +
>   struct meson_dw_hdmi_data {
>   	int (*reg_init)(struct device *dev);
> +	const struct meson_dw_hdmi_speed *speeds;
> +	unsigned int speed_num;
> +	bool use_drm_infoframe;
>   	u32 cntl0_init;
>   	u32 cntl1_init;
>   };
> @@ -185,78 +194,33 @@ struct meson_dw_hdmi {
>   	struct regmap *top;
>   };
>   
> -static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi,
> -					const char *compat)
> -{
> -	return of_device_is_compatible(dw_hdmi->dev->of_node, compat);
> -}
> -
> -/* Bridge */
> -
>   /* Setup PHY bandwidth modes */
> -static void meson_hdmi_phy_setup_mode(struct meson_dw_hdmi *dw_hdmi,
> +static int meson_hdmi_phy_setup_mode(struct meson_dw_hdmi *dw_hdmi,
>   				      const struct drm_display_mode *mode,
>   				      bool mode_is_420)
>   {
>   	struct meson_drm *priv = dw_hdmi->priv;
>   	unsigned int pixel_clock = mode->clock;
> +	int i;
>   
>   	/* For 420, pixel clock is half unlike venc clock */
> -	if (mode_is_420) pixel_clock /= 2;
> -
> -	if (dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
> -	    dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxm-dw-hdmi")) {
> -		if (pixel_clock >= 371250) {
> -			/* 5.94Gbps, 3.7125Gbps */
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x333d3282);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2136315b);
> -		} else if (pixel_clock >= 297000) {
> -			/* 2.97Gbps */
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33303382);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2036315b);
> -		} else if (pixel_clock >= 148500) {
> -			/* 1.485Gbps */
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33303362);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2016315b);
> -		} else {
> -			/* 742.5Mbps, and below */
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33604142);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x0016315b);
> -		}
> -	} else if (dw_hdmi_is_compatible(dw_hdmi,
> -					 "amlogic,meson-gxbb-dw-hdmi")) {
> -		if (pixel_clock >= 371250) {
> -			/* 5.94Gbps, 3.7125Gbps */
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33353245);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2100115b);
> -		} else if (pixel_clock >= 297000) {
> -			/* 2.97Gbps */
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33634283);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0xb000115b);
> -		} else {
> -			/* 1.485Gbps, and below */
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33632122);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2000115b);
> -		}
> -	} else if (dw_hdmi_is_compatible(dw_hdmi,
> -					 "amlogic,meson-g12a-dw-hdmi")) {
> -		if (pixel_clock >= 371250) {
> -			/* 5.94Gbps, 3.7125Gbps */
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x37eb65c4);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2ab0ff3b);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL5, 0x0000080b);
> -		} else if (pixel_clock >= 297000) {
> -			/* 2.97Gbps */
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33eb6262);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2ab0ff3b);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL5, 0x00000003);
> -		} else {
> -			/* 1.485Gbps, and below */
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33eb4242);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2ab0ff3b);
> -			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL5, 0x00000003);
> -		}
> +	if (mode_is_420)
> +		pixel_clock /= 2;
> +
> +	for (i = 0; i < dw_hdmi->data->speed_num; i++) {
> +		if (pixel_clock >= dw_hdmi->data->speeds[i].limit)
> +			break;
>   	}
> +
> +	/* No match found - Last entry should have a 0 limit */
> +	if (WARN_ON(i == dw_hdmi->data->speed_num))
> +		return -EINVAL;
> +
> +	regmap_multi_reg_write(priv->hhi,
> +			       dw_hdmi->data->speeds[i].regs,
> +			       dw_hdmi->data->speeds[i].reg_num);
> +
> +	return 0;
>   }
>   
>   static inline void meson_dw_hdmi_phy_reset(struct meson_dw_hdmi *dw_hdmi)
> @@ -543,22 +507,133 @@ static int meson_dw_init_regmap_g12(struct device *dev)
>   	return 0;
>   }
>   
> +static const struct reg_sequence gxbb_3g7_regs[] = {
> +	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33353245 },
> +	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2100115b },
> +};
> +
> +static const struct reg_sequence gxbb_3g_regs[] = {
> +	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33634283 },
> +	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0xb000115b },
> +};
> +
> +static const struct reg_sequence gxbb_def_regs[] = {
> +	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33632122 },
> +	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2000115b },
> +};
> +
> +static const struct meson_dw_hdmi_speed gxbb_speeds[] = {
> +	{
> +		.limit = 371250,
> +		.regs = gxbb_3g7_regs,
> +		.reg_num = ARRAY_SIZE(gxbb_3g7_regs)
> +	}, {
> +		.limit = 297000,
> +		.regs = gxbb_3g_regs,
> +		.reg_num = ARRAY_SIZE(gxbb_3g_regs)
> +	}, {
> +		.regs = gxbb_def_regs,
> +		.reg_num = ARRAY_SIZE(gxbb_def_regs)
> +	}
> +};
> +
>   static const struct meson_dw_hdmi_data meson_dw_hdmi_gxbb_data = {
>   	.reg_init = meson_dw_init_regmap_gx,
>   	.cntl0_init = 0x0,
>   	.cntl1_init = PHY_CNTL1_INIT | PHY_INVERT,
> +	.use_drm_infoframe = false,
> +	.speeds = gxbb_speeds,
> +	.speed_num = ARRAY_SIZE(gxbb_speeds),
> +};
> +
> +static const struct reg_sequence gxl_3g7_regs[] = {
> +	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x333d3282 },
> +	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2136315b },
> +};
> +
> +static const struct reg_sequence gxl_3g_regs[] = {
> +	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33303382 },
> +	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2036315b },
> +};
> +
> +static const struct reg_sequence gxl_def_regs[] = {
> +	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33303362 },
> +	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2016315b },
> +};
> +
> +static const struct reg_sequence gxl_270m_regs[] = {
> +	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33604142 },
> +	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x0016315b },
> +};
> +
> +static const struct meson_dw_hdmi_speed gxl_speeds[] = {
> +	{
> +		.limit = 371250,
> +		.regs = gxl_3g7_regs,
> +		.reg_num = ARRAY_SIZE(gxl_3g7_regs)
> +	}, {
> +		.limit = 297000,
> +		.regs = gxl_3g_regs,
> +		.reg_num = ARRAY_SIZE(gxl_3g_regs)
> +	}, {
> +		.limit = 148500,
> +		.regs = gxl_def_regs,
> +		.reg_num = ARRAY_SIZE(gxl_def_regs)
> +	}, {
> +		.regs = gxl_270m_regs,
> +		.reg_num = ARRAY_SIZE(gxl_270m_regs)
> +	}
>   };
>   
>   static const struct meson_dw_hdmi_data meson_dw_hdmi_gxl_data = {
>   	.reg_init = meson_dw_init_regmap_gx,
>   	.cntl0_init = 0x0,
>   	.cntl1_init = PHY_CNTL1_INIT,
> +	.use_drm_infoframe = true,
> +	.speeds = gxl_speeds,
> +	.speed_num = ARRAY_SIZE(gxl_speeds),
> +};
> +
> +static const struct reg_sequence g12a_3g7_regs[] = {
> +	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x37eb65c4 },
> +	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2ab0ff3b },
> +	{ .reg = HHI_HDMI_PHY_CNTL5, .def = 0x0000080b },
> +};
> +
> +static const struct reg_sequence g12a_3g_regs[] = {
> +	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33eb6262 },
> +	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2ab0ff3b },
> +	{ .reg = HHI_HDMI_PHY_CNTL5, .def = 0x00000003 },
> +};
> +
> +static const struct reg_sequence g12a_def_regs[] = {
> +	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33eb4242 },
> +	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2ab0ff3b },
> +	{ .reg = HHI_HDMI_PHY_CNTL5, .def = 0x00000003 },
> +};
> +
> +static const struct meson_dw_hdmi_speed g12a_speeds[] = {
> +	{
> +		.limit = 371250,
> +		.regs = g12a_3g7_regs,
> +		.reg_num = ARRAY_SIZE(g12a_3g7_regs)
> +	}, {
> +		.limit = 297000,
> +		.regs = g12a_3g_regs,
> +		.reg_num = ARRAY_SIZE(g12a_3g_regs)
> +	}, {
> +		.regs = g12a_def_regs,
> +		.reg_num = ARRAY_SIZE(g12a_def_regs)
> +	}
>   };
>   
>   static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = {
>   	.reg_init = meson_dw_init_regmap_g12,
>   	.cntl0_init = 0x000b4242, /* Bandgap */
>   	.cntl1_init = PHY_CNTL1_INIT,
> +	.use_drm_infoframe = true,
> +	.speeds = g12a_speeds,
> +	.speed_num = ARRAY_SIZE(g12a_speeds),
>   };
>   
>   static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
> @@ -590,7 +665,6 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>   	platform_set_drvdata(pdev, meson_dw_hdmi);
>   
>   	meson_dw_hdmi->priv = priv;
> -	meson_dw_hdmi->dev = dev;

Unrelated change

>   	meson_dw_hdmi->data = match;
>   	dw_plat_data = &meson_dw_hdmi->dw_plat_data;
>   
> @@ -650,7 +724,6 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>   	meson_dw_hdmi_init(meson_dw_hdmi);
>   
>   	/* Bridge / Connector */
> -

Unrelated change

>   	dw_plat_data->priv_data = meson_dw_hdmi;
>   	dw_plat_data->phy_ops = &meson_dw_hdmi_phy_ops;
>   	dw_plat_data->phy_name = "meson_dw_hdmi_phy";
> @@ -659,11 +732,7 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>   	dw_plat_data->ycbcr_420_allowed = true;
>   	dw_plat_data->disable_cec = true;
>   	dw_plat_data->output_port = 1;
> -
> -	if (dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
> -	    dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxm-dw-hdmi") ||
> -	    dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-g12a-dw-hdmi"))
> -		dw_plat_data->use_drm_infoframe = true;
> +	dw_plat_data->use_drm_infoframe = meson_dw_hdmi->data->use_drm_infoframe;

Move this to a separate patch

>   
>   	meson_dw_hdmi->hdmi = dw_hdmi_probe(pdev, &meson_dw_hdmi->dw_plat_data);
>   	if (IS_ERR(meson_dw_hdmi->hdmi))
diff mbox series

Patch

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 7c39e5c99043..ef059c5ef520 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -125,8 +125,17 @@ 
 #define HHI_HDMI_PHY_CNTL4	0x3b0 /* 0xec */
 #define HHI_HDMI_PHY_CNTL5	0x3b4 /* 0xed */
 
+struct meson_dw_hdmi_speed {
+	const struct reg_sequence *regs;
+	unsigned int reg_num;
+	unsigned int limit;
+};
+
 struct meson_dw_hdmi_data {
 	int (*reg_init)(struct device *dev);
+	const struct meson_dw_hdmi_speed *speeds;
+	unsigned int speed_num;
+	bool use_drm_infoframe;
 	u32 cntl0_init;
 	u32 cntl1_init;
 };
@@ -185,78 +194,33 @@  struct meson_dw_hdmi {
 	struct regmap *top;
 };
 
-static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi,
-					const char *compat)
-{
-	return of_device_is_compatible(dw_hdmi->dev->of_node, compat);
-}
-
-/* Bridge */
-
 /* Setup PHY bandwidth modes */
-static void meson_hdmi_phy_setup_mode(struct meson_dw_hdmi *dw_hdmi,
+static int meson_hdmi_phy_setup_mode(struct meson_dw_hdmi *dw_hdmi,
 				      const struct drm_display_mode *mode,
 				      bool mode_is_420)
 {
 	struct meson_drm *priv = dw_hdmi->priv;
 	unsigned int pixel_clock = mode->clock;
+	int i;
 
 	/* For 420, pixel clock is half unlike venc clock */
-	if (mode_is_420) pixel_clock /= 2;
-
-	if (dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
-	    dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxm-dw-hdmi")) {
-		if (pixel_clock >= 371250) {
-			/* 5.94Gbps, 3.7125Gbps */
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x333d3282);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2136315b);
-		} else if (pixel_clock >= 297000) {
-			/* 2.97Gbps */
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33303382);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2036315b);
-		} else if (pixel_clock >= 148500) {
-			/* 1.485Gbps */
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33303362);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2016315b);
-		} else {
-			/* 742.5Mbps, and below */
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33604142);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x0016315b);
-		}
-	} else if (dw_hdmi_is_compatible(dw_hdmi,
-					 "amlogic,meson-gxbb-dw-hdmi")) {
-		if (pixel_clock >= 371250) {
-			/* 5.94Gbps, 3.7125Gbps */
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33353245);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2100115b);
-		} else if (pixel_clock >= 297000) {
-			/* 2.97Gbps */
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33634283);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0xb000115b);
-		} else {
-			/* 1.485Gbps, and below */
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33632122);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2000115b);
-		}
-	} else if (dw_hdmi_is_compatible(dw_hdmi,
-					 "amlogic,meson-g12a-dw-hdmi")) {
-		if (pixel_clock >= 371250) {
-			/* 5.94Gbps, 3.7125Gbps */
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x37eb65c4);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2ab0ff3b);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL5, 0x0000080b);
-		} else if (pixel_clock >= 297000) {
-			/* 2.97Gbps */
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33eb6262);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2ab0ff3b);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL5, 0x00000003);
-		} else {
-			/* 1.485Gbps, and below */
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0x33eb4242);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL3, 0x2ab0ff3b);
-			regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL5, 0x00000003);
-		}
+	if (mode_is_420)
+		pixel_clock /= 2;
+
+	for (i = 0; i < dw_hdmi->data->speed_num; i++) {
+		if (pixel_clock >= dw_hdmi->data->speeds[i].limit)
+			break;
 	}
+
+	/* No match found - Last entry should have a 0 limit */
+	if (WARN_ON(i == dw_hdmi->data->speed_num))
+		return -EINVAL;
+
+	regmap_multi_reg_write(priv->hhi,
+			       dw_hdmi->data->speeds[i].regs,
+			       dw_hdmi->data->speeds[i].reg_num);
+
+	return 0;
 }
 
 static inline void meson_dw_hdmi_phy_reset(struct meson_dw_hdmi *dw_hdmi)
@@ -543,22 +507,133 @@  static int meson_dw_init_regmap_g12(struct device *dev)
 	return 0;
 }
 
+static const struct reg_sequence gxbb_3g7_regs[] = {
+	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33353245 },
+	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2100115b },
+};
+
+static const struct reg_sequence gxbb_3g_regs[] = {
+	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33634283 },
+	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0xb000115b },
+};
+
+static const struct reg_sequence gxbb_def_regs[] = {
+	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33632122 },
+	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2000115b },
+};
+
+static const struct meson_dw_hdmi_speed gxbb_speeds[] = {
+	{
+		.limit = 371250,
+		.regs = gxbb_3g7_regs,
+		.reg_num = ARRAY_SIZE(gxbb_3g7_regs)
+	}, {
+		.limit = 297000,
+		.regs = gxbb_3g_regs,
+		.reg_num = ARRAY_SIZE(gxbb_3g_regs)
+	}, {
+		.regs = gxbb_def_regs,
+		.reg_num = ARRAY_SIZE(gxbb_def_regs)
+	}
+};
+
 static const struct meson_dw_hdmi_data meson_dw_hdmi_gxbb_data = {
 	.reg_init = meson_dw_init_regmap_gx,
 	.cntl0_init = 0x0,
 	.cntl1_init = PHY_CNTL1_INIT | PHY_INVERT,
+	.use_drm_infoframe = false,
+	.speeds = gxbb_speeds,
+	.speed_num = ARRAY_SIZE(gxbb_speeds),
+};
+
+static const struct reg_sequence gxl_3g7_regs[] = {
+	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x333d3282 },
+	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2136315b },
+};
+
+static const struct reg_sequence gxl_3g_regs[] = {
+	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33303382 },
+	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2036315b },
+};
+
+static const struct reg_sequence gxl_def_regs[] = {
+	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33303362 },
+	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2016315b },
+};
+
+static const struct reg_sequence gxl_270m_regs[] = {
+	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33604142 },
+	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x0016315b },
+};
+
+static const struct meson_dw_hdmi_speed gxl_speeds[] = {
+	{
+		.limit = 371250,
+		.regs = gxl_3g7_regs,
+		.reg_num = ARRAY_SIZE(gxl_3g7_regs)
+	}, {
+		.limit = 297000,
+		.regs = gxl_3g_regs,
+		.reg_num = ARRAY_SIZE(gxl_3g_regs)
+	}, {
+		.limit = 148500,
+		.regs = gxl_def_regs,
+		.reg_num = ARRAY_SIZE(gxl_def_regs)
+	}, {
+		.regs = gxl_270m_regs,
+		.reg_num = ARRAY_SIZE(gxl_270m_regs)
+	}
 };
 
 static const struct meson_dw_hdmi_data meson_dw_hdmi_gxl_data = {
 	.reg_init = meson_dw_init_regmap_gx,
 	.cntl0_init = 0x0,
 	.cntl1_init = PHY_CNTL1_INIT,
+	.use_drm_infoframe = true,
+	.speeds = gxl_speeds,
+	.speed_num = ARRAY_SIZE(gxl_speeds),
+};
+
+static const struct reg_sequence g12a_3g7_regs[] = {
+	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x37eb65c4 },
+	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2ab0ff3b },
+	{ .reg = HHI_HDMI_PHY_CNTL5, .def = 0x0000080b },
+};
+
+static const struct reg_sequence g12a_3g_regs[] = {
+	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33eb6262 },
+	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2ab0ff3b },
+	{ .reg = HHI_HDMI_PHY_CNTL5, .def = 0x00000003 },
+};
+
+static const struct reg_sequence g12a_def_regs[] = {
+	{ .reg = HHI_HDMI_PHY_CNTL0, .def = 0x33eb4242 },
+	{ .reg = HHI_HDMI_PHY_CNTL3, .def = 0x2ab0ff3b },
+	{ .reg = HHI_HDMI_PHY_CNTL5, .def = 0x00000003 },
+};
+
+static const struct meson_dw_hdmi_speed g12a_speeds[] = {
+	{
+		.limit = 371250,
+		.regs = g12a_3g7_regs,
+		.reg_num = ARRAY_SIZE(g12a_3g7_regs)
+	}, {
+		.limit = 297000,
+		.regs = g12a_3g_regs,
+		.reg_num = ARRAY_SIZE(g12a_3g_regs)
+	}, {
+		.regs = g12a_def_regs,
+		.reg_num = ARRAY_SIZE(g12a_def_regs)
+	}
 };
 
 static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = {
 	.reg_init = meson_dw_init_regmap_g12,
 	.cntl0_init = 0x000b4242, /* Bandgap */
 	.cntl1_init = PHY_CNTL1_INIT,
+	.use_drm_infoframe = true,
+	.speeds = g12a_speeds,
+	.speed_num = ARRAY_SIZE(g12a_speeds),
 };
 
 static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
@@ -590,7 +665,6 @@  static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 	platform_set_drvdata(pdev, meson_dw_hdmi);
 
 	meson_dw_hdmi->priv = priv;
-	meson_dw_hdmi->dev = dev;
 	meson_dw_hdmi->data = match;
 	dw_plat_data = &meson_dw_hdmi->dw_plat_data;
 
@@ -650,7 +724,6 @@  static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 	meson_dw_hdmi_init(meson_dw_hdmi);
 
 	/* Bridge / Connector */
-
 	dw_plat_data->priv_data = meson_dw_hdmi;
 	dw_plat_data->phy_ops = &meson_dw_hdmi_phy_ops;
 	dw_plat_data->phy_name = "meson_dw_hdmi_phy";
@@ -659,11 +732,7 @@  static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 	dw_plat_data->ycbcr_420_allowed = true;
 	dw_plat_data->disable_cec = true;
 	dw_plat_data->output_port = 1;
-
-	if (dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
-	    dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxm-dw-hdmi") ||
-	    dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-g12a-dw-hdmi"))
-		dw_plat_data->use_drm_infoframe = true;
+	dw_plat_data->use_drm_infoframe = meson_dw_hdmi->data->use_drm_infoframe;
 
 	meson_dw_hdmi->hdmi = dw_hdmi_probe(pdev, &meson_dw_hdmi->dw_plat_data);
 	if (IS_ERR(meson_dw_hdmi->hdmi))