Hi Shirish,
On 11.03.2014 12:26, Shirish S wrote:
This patch implements the power on/off sequence
(and its dependant functions) of HDMI exynos5250
as provided by the hardware team.
Signed-off-by: Shirish S <s.shirish@xxxxxxxxxxx>
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 137 +++++++++++++++++++++++++++++-----
drivers/gpu/drm/exynos/regs-hdmi.h | 15 ++++
2 files changed, 133 insertions(+), 19 deletions(-)
mode change 100644 => 100755 drivers/gpu/drm/exynos/exynos_hdmi.c
mode change 100644 => 100755 drivers/gpu/drm/exynos/regs-hdmi.h
Please do not change file modes.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
old mode 100644
new mode 100755
index 12fdf55..ee000f7
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -180,6 +180,7 @@ struct hdmi_context {
void __iomem *regs;
int irq;
+ void __iomem *phy_pow_ctrl_reg;
struct i2c_client *ddc_port;
struct i2c_client *hdmiphy_port;
@@ -387,6 +388,33 @@ static inline void hdmi_reg_writemask(struct hdmi_context *hdata,
writel(value, hdata->regs + reg_id);
}
+
+static inline void hdmi_phy_pow_ctrl_reg_writemask(struct hdmi_context *hdata,
+ u32 value, u32 mask)
+{
+ u32 old = readl(hdata->phy_pow_ctrl_reg);
+ value = (value & mask) | (old & ~mask);
+ writel(value, hdata->phy_pow_ctrl_reg);
+}
Do you really need a separate function for just two accesses?
+
+static int hdmiphy_reg_writeb(struct hdmi_context *hdata,
+ u32 reg_offset, u8 value)
+{
+ if (hdata->hdmiphy_port) {
I'd say this function should be called at all if hdmiphy_port is NULL.
Anyway...
+ u8 buffer[2];
+ int ret;
+
+ buffer[0] = reg_offset;
+ buffer[1] = value;
+
+ ret = i2c_master_send(hdata->hdmiphy_port, buffer, 2);
+ if (ret == 2)
+ return 0;
+ return ret;
+ } else
CodingStyle: If one if branch needs braces, then the other one should
have them as well.
+ return 0;
If this is still needed, the code could be simplified by rewriting this as:
if (!hdata->hdmiphy_port)
return 0;
/* ... */
+}
+
static void hdmi_v13_regs_dump(struct hdmi_context *hdata, char *prefix)
{
#define DUMPREG(reg_id) \
@@ -1386,19 +1414,14 @@ static void hdmi_mode_apply(struct hdmi_context *hdata)
static void hdmiphy_conf_reset(struct hdmi_context *hdata)
{
- u8 buffer[2];
u32 reg;
clk_disable_unprepare(hdata->res.sclk_hdmi);
clk_set_parent(hdata->res.mout_hdmi, hdata->res.sclk_pixel);
clk_prepare_enable(hdata->res.sclk_hdmi);
- /* operation mode */
- buffer[0] = 0x1f;
- buffer[1] = 0x00;
-
- if (hdata->hdmiphy_port)
- i2c_master_send(hdata->hdmiphy_port, buffer, 2);
+ hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
+ HDMI_PHY_ENABLE_MODE_SET);
Hmm, you should be able to use i2c_smbus_write_byte_data().
if (hdata->type == HDMI_TYPE13)
reg = HDMI_V13_PHY_RSTOUT;
@@ -1414,16 +1437,42 @@ static void hdmiphy_conf_reset(struct hdmi_context *hdata)
static void hdmiphy_poweron(struct hdmi_context *hdata)
{
- if (hdata->type == HDMI_TYPE14)
- hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, 0,
- HDMI_PHY_POWER_OFF_EN);
+ if (hdata->type == HDMI_TYPE13)
Shouldn't the check be HDMI_TYPE != HDMI_TYPE14 to also cover other
types than 13 and 14?
+ return;
+
+ DRM_DEBUG_KMS("\n");
+
+ /* For PHY Mode Setting */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
+ HDMI_PHY_ENABLE_MODE_SET);
+ /* Phy Power On */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_POWER,
+ HDMI_PHY_POWER_ON);
+ /* For PHY Mode Setting */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
+ HDMI_PHY_DISABLE_MODE_SET);
i2c_smbus_write_byte_data() should work for above 3 calls as well.
+ /* PHY SW Reset */
+ hdmiphy_conf_reset(hdata);
}
static void hdmiphy_poweroff(struct hdmi_context *hdata)
{
- if (hdata->type == HDMI_TYPE14)
- hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, ~0,
- HDMI_PHY_POWER_OFF_EN);
+ if (hdata->type == HDMI_TYPE13)
+ return;
Same comment about type check as above.
+
+ DRM_DEBUG_KMS("\n");
+
+ /* PHY SW Reset */
+ hdmiphy_conf_reset(hdata);
+ /* For PHY Mode Setting */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
+ HDMI_PHY_ENABLE_MODE_SET);
+ /* PHY Power Off */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_POWER,
+ HDMI_PHY_POWER_OFF);
+ /* For PHY Mode Setting */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
+ HDMI_PHY_DISABLE_MODE_SET);
For above 3 i2c_smbus_write_byte_data() could be used too.
}
static void hdmiphy_conf_apply(struct hdmi_context *hdata)
@@ -1476,6 +1525,14 @@ static void hdmiphy_conf_apply(struct hdmi_context *hdata)
DRM_ERROR("failed to read hdmiphy config\n");
return;
}
+ usleep_range(10000, 12000);
Why?
+
+ ret = hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
+ HDMI_PHY_DISABLE_MODE_SET);
+ if (ret < 0) {
+ DRM_ERROR("failed to enable hdmiphy\n");
+ return;
+ }
i2c_smbus_write_byte_data()
for (i = 0; i < ret; i++)
DRM_DEBUG_KMS("hdmiphy[0x%02x] write[0x%02x] - "
@@ -1767,10 +1824,10 @@ static void hdmi_poweron(struct exynos_drm_display *display)
if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
DRM_DEBUG_KMS("failed to enable regulator bulk\n");
- clk_prepare_enable(res->hdmiphy);
- clk_prepare_enable(res->hdmi);
- clk_prepare_enable(res->sclk_hdmi);
Are you sure about this? Where the clocks are handled after these lines
are removed?
+ hdmi_phy_pow_ctrl_reg_writemask(hdata, PMU_HDMI_PHY_ENABLE,
+ PMU_HDMI_PHY_CONTROL_MASK);
+ /* HDMI PHY Enable and Power-On */
hdmiphy_poweron(hdata);
hdmi_commit(display);
}
@@ -1790,11 +1847,16 @@ static void hdmi_poweroff(struct exynos_drm_display *display)
* its reset state seems to meet the condition.
*/
hdmiphy_conf_reset(hdata);
+
+ /* HDMI System Disable */
+ hdmi_reg_writemask(hdata, HDMI_CON_0, 0, HDMI_EN);
+
hdmiphy_poweroff(hdata);
- clk_disable_unprepare(res->sclk_hdmi);
- clk_disable_unprepare(res->hdmi);
- clk_disable_unprepare(res->hdmiphy);
Ditto.
+ /* HDMI PHY Disable */
+ hdmi_phy_pow_ctrl_reg_writemask(hdata, PMU_HDMI_PHY_DISABLE,
+ PMU_HDMI_PHY_CONTROL_MASK);
+
regulator_bulk_disable(res->regul_count, res->regul_bulk);
pm_runtime_put_sync(hdata->dev);
@@ -1947,6 +2009,36 @@ err_data:
return NULL;
}
+static int drm_hdmi_dt_parse_phy_pow_control(struct hdmi_context *hdata)
+{
+ struct device_node *phy_pow_ctrl_node;
+ u32 buf[2];
+ int ret = 0;
+
+ phy_pow_ctrl_node = of_find_node_by_name(NULL, "phy-power-control");
+ if (!phy_pow_ctrl_node)
+ return 0;
Where is this node documented?
+
+ /* reg property holds two informations: addr of pmu register, size */
+ if (of_property_read_u32_array(phy_pow_ctrl_node, "reg",
+ (u32 *)&buf, 2)) {
+ DRM_ERROR("faild to get phy power control reg\n");
typo: s/faild/failed/
+ ret = -EINVAL;
+ goto fail;
+ }
+
This is not the correct way of parsing reg property. Please see
of_iomap() or of_address_to_resource()+devm_ioremap_resource().
+ hdata->phy_pow_ctrl_reg = devm_ioremap(hdata->dev, buf[0], buf[1]);
+ if (!hdata->phy_pow_ctrl_reg) {
+ DRM_ERROR("failed to ioremap phy pmu reg\n");
+ ret = -ENOMEM;
+ goto fail;
+ }
+
+fail:
+ of_node_put(phy_pow_ctrl_node);
+ return ret;
+}
Anyway, the whole PMU mapping above seems to be wrong. Since PMU is
already defined as a syscon, a reference to the PMU node should passed
through a DT property and the syscon interface should be used to obtain
a regmap that gives you access to PMU registers.
See the following commit for an example of use:
4f1f653a68d6 watchdog: s3c2410_wdt: use syscon regmap [...]
+
static struct of_device_id hdmi_match_types[] = {
{
.compatible = "samsung,exynos5-hdmi",
@@ -2010,6 +2102,13 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data)
return ret;
}
+ /* map hdmiphy power control reg */
+ ret = drm_hdmi_dt_parse_phy_pow_control(hdata);
+ if (ret) {
+ DRM_ERROR("failed to map phy power control registers\n");
+ return ret;
+ }
+
/* DDC i2c driver */
ddc_node = of_parse_phandle(dev->of_node, "ddc", 0);
if (!ddc_node) {
diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h b/drivers/gpu/drm/exynos/regs-hdmi.h
old mode 100644
new mode 100755
index ef1b3eb..e686fe9
--- a/drivers/gpu/drm/exynos/regs-hdmi.h
+++ b/drivers/gpu/drm/exynos/regs-hdmi.h
@@ -578,4 +578,19 @@
#define HDMI_TG_VACT_ST4_H HDMI_TG_BASE(0x0074)
#define HDMI_TG_3D HDMI_TG_BASE(0x00F0)
+/* HDMI PHY Registers Offsets*/
+
+#define HDMIPHY_POWER (0x74 >> 2)
+#define HDMIPHY_MODE_SET_DONE (0x7C >> 2)
CodingStyle: Lowercase is preferred for hexadecimal numbers.
+
+/* HDMI PHY Values */
+#define HDMI_PHY_POWER_ON 0x80
+#define HDMI_PHY_POWER_OFF 0xFF
Ditto.
Best regards,
Tomasz
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
