diff mbox

[v2,15/28] drm: omapdrm: hdmi: Store PHY features in HDMI transmitter drivers

Message ID 20170508113303.27521-16-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show

Commit Message

Laurent Pinchart May 8, 2017, 11:32 a.m. UTC 
The HDMI PHY features are dependent on the HDMI transmitter model. The
PHY driver contains features for all supported transmitters, and selects
the appropriate features based on the OMAP SoC version.

It makes more sense to store the features in the HDMI transmitter
drivers and pass them to the HDMI PHY initialization function, as the
HDMI transmitter drivers are transmitter-specific and don't need to
check the OMAP SoC version.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/dss/hdmi.h     |  3 +-
 drivers/gpu/drm/omapdrm/dss/hdmi4.c    |  8 ++++-
 drivers/gpu/drm/omapdrm/dss/hdmi5.c    |  8 ++++-
 drivers/gpu/drm/omapdrm/dss/hdmi_phy.c | 55 ++--------------------------------
 4 files changed, 19 insertions(+), 55 deletions(-)

Comments

Tomi Valkeinen May 9, 2017, 11:27 a.m. UTC | #1 
On 08/05/17 14:32, Laurent Pinchart wrote:
> The HDMI PHY features are dependent on the HDMI transmitter model. The
> PHY driver contains features for all supported transmitters, and selects
> the appropriate features based on the OMAP SoC version.
> 
> It makes more sense to store the features in the HDMI transmitter
> drivers and pass them to the HDMI PHY initialization function, as the
> HDMI transmitter drivers are transmitter-specific and don't need to
> check the OMAP SoC version.

I don't think this is correct. PHY and HDMI are separate components, you
could swap the PHY. In theory, at least. While the PHY .c is not a
separate driver, I still like that it contains information about the PHY
hardware versions which it supports, like a real driver would.

The same applies to the next patch, the PLL is not part of HDMI.

In the third patch you pass 4 or 5 to the WP code as a version. So maybe
rather create an HDMI_VERSION (or something) enum, which can be passed
to phy, pll and wp?

 Tomi
diff mbox

Patch

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi.h b/drivers/gpu/drm/omapdrm/dss/hdmi.h
index 851e5e6f6820..2b4312f8e97d 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi.h
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi.h
@@ -323,7 +323,8 @@  void hdmi_pll_uninit(struct hdmi_pll_data *hpll);
 int hdmi_phy_configure(struct hdmi_phy_data *phy, unsigned long hfbitclk,
 	unsigned long lfbitclk);
 void hdmi_phy_dump(struct hdmi_phy_data *phy, struct seq_file *s);
-int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy);
+int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy,
+		  const struct hdmi_phy_features *features);
 int hdmi_phy_parse_lanes(struct hdmi_phy_data *phy, const u32 *lanes);
 
 /* HDMI common funcs */
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index ff4a0c04a22c..1feb6deab108 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -683,6 +683,12 @@  static int hdmi_audio_register(struct device *dev)
 }
 
 /* HDMI HW IP initialisation */
+static const struct hdmi_phy_features hdmi4_phy_feats = {
+	.bist_ctrl	=	false,
+	.ldo_voltage	=	true,
+	.max_phy	=	185675000,
+};
+
 static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -707,7 +713,7 @@  static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 	if (r)
 		return r;
 
-	r = hdmi_phy_init(pdev, &hdmi.phy);
+	r = hdmi_phy_init(pdev, &hdmi.phy, &hdmi4_phy_feats);
 	if (r)
 		goto err;
 
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
index c037b61f7920..529653180bfc 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
@@ -715,6 +715,12 @@  static int hdmi_audio_register(struct device *dev)
 }
 
 /* HDMI HW IP initialisation */
+static const struct hdmi_phy_features hdmi5_phy_feats = {
+	.bist_ctrl	=	true,
+	.ldo_voltage	=	false,
+	.max_phy	=	186000000,
+};
+
 static int hdmi5_bind(struct device *dev, struct device *master, void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -739,7 +745,7 @@  static int hdmi5_bind(struct device *dev, struct device *master, void *data)
 	if (r)
 		return r;
 
-	r = hdmi_phy_init(pdev, &hdmi.phy);
+	r = hdmi_phy_init(pdev, &hdmi.phy, &hdmi5_phy_feats);
 	if (r)
 		goto err;
 
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_phy.c b/drivers/gpu/drm/omapdrm/dss/hdmi_phy.c
index bff1ea11ed2f..87ed0bc5b146 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi_phy.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi_phy.c
@@ -12,7 +12,6 @@ 
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
-#include <linux/slab.h>
 #include <linux/seq_file.h>
 
 #include "omapdss.h"
@@ -170,60 +169,12 @@  int hdmi_phy_configure(struct hdmi_phy_data *phy, unsigned long hfbitclk,
 	return 0;
 }
 
-static const struct hdmi_phy_features omap44xx_phy_feats = {
-	.bist_ctrl	=	false,
-	.ldo_voltage	=	true,
-	.max_phy	=	185675000,
-};
-
-static const struct hdmi_phy_features omap54xx_phy_feats = {
-	.bist_ctrl	=	true,
-	.ldo_voltage	=	false,
-	.max_phy	=	186000000,
-};
-
-static int hdmi_phy_init_features(struct platform_device *pdev,
-				  struct hdmi_phy_data *phy)
-{
-	struct hdmi_phy_features *dst;
-	const struct hdmi_phy_features *src;
-
-	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
-	if (!dst) {
-		dev_err(&pdev->dev, "Failed to allocate HDMI PHY Features\n");
-		return -ENOMEM;
-	}
-
-	switch (omapdss_get_version()) {
-	case OMAPDSS_VER_OMAP4430_ES1:
-	case OMAPDSS_VER_OMAP4430_ES2:
-	case OMAPDSS_VER_OMAP4:
-		src = &omap44xx_phy_feats;
-		break;
-
-	case OMAPDSS_VER_OMAP5:
-	case OMAPDSS_VER_DRA7xx:
-		src = &omap54xx_phy_feats;
-		break;
-
-	default:
-		return -ENODEV;
-	}
-
-	memcpy(dst, src, sizeof(*dst));
-	phy->features = dst;
-
-	return 0;
-}
-
-int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy)
+int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy,
+		  const struct hdmi_phy_features *features)
 {
-	int r;
 	struct resource *res;
 
-	r = hdmi_phy_init_features(pdev, phy);
-	if (r)
-		return r;
+	phy->features = features;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
 	phy->base = devm_ioremap_resource(&pdev->dev, res);