[RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Rahul Sharma]

Previously, hdmiphy is added as a dummy clock in clock file for
exynos SoCs. Enable/Disable to this clock, actually toggles the power
control bit in PMU, instead of controlling the clock gate.

This RFC adds the support to parse hdmiphy control node which is a child
node to hdmi, and map the pmu register to toggle the power control bit.

This is based on drm-next branch in Inki Dae's tree.

Rahul Sharma (2):
  drm/exynos: replace dummy hdmiphy clock with pmu register control
  ARM/dts: add hdmiphy power control pmu register to hdmi dt node

 arch/arm/boot/dts/exynos5250.dtsi    |    6 +++
 drivers/gpu/drm/exynos/exynos_hdmi.c |   69 ++++++++++++++++++++++++++++++----
 drivers/gpu/drm/exynos/regs-hdmi.h   |    4 ++
 3 files changed, 71 insertions(+), 8 deletions(-)

-- 
1.7.10.4

[RFC 1/2] drm/exynos: replace dummy hdmiphy clock with pmu register control

[Rahul Sharma]

Previous to CCF, hdmiphy is added as a dummy clock in clock file for
exynos SoCs. Enable/Disable to this clock, actually toggles the power
control bit in PMU, instead of controlling the clock gate.

Patch adds the support to parse hdmiphy control node which is a child
node to hdmi, and map the pmu register to toggle the power control bit.

Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c |   69 ++++++++++++++++++++++++++++++----
 drivers/gpu/drm/exynos/regs-hdmi.h   |    4 ++
 2 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 3b5e215..75a6bf3 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -34,6 +34,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/io.h>
 #include <linux/of_gpio.h>
+#include <linux/of_address.h>
 
 #include <drm/exynos_drm.h>
 
@@ -82,7 +83,6 @@ struct hdmi_resources {
 	struct clk			*sclk_hdmi;
 	struct clk			*sclk_pixel;
 	struct clk			*sclk_hdmiphy;
-	struct clk			*hdmiphy;
 	struct clk			*mout_hdmi;
 	struct regulator_bulk_data	*regul_bulk;
 	int				regul_count;
@@ -189,6 +189,7 @@ struct hdmi_context {
 	struct mutex			hdmi_mutex;
 
 	void __iomem			*regs;
+	void __iomem			*phy_pow_ctrl_reg;
 	void				*parent_ctx;
 	int				irq;
 
@@ -404,6 +405,14 @@ 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);
+}
+
 static void hdmi_v13_regs_dump(struct hdmi_context *hdata, char *prefix)
 {
 #define DUMPREG(reg_id) \
@@ -1702,7 +1711,8 @@ static void hdmi_poweron(struct hdmi_context *hdata)
 	if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
 		DRM_DEBUG_KMS("failed to enable regulator bulk\n");
 
-	clk_prepare_enable(res->hdmiphy);
+	hdmi_phy_pow_ctrl_reg_writemask(hdata, PMU_HDMI_PHY_ENABLE,
+		PMU_HDMI_PHY_CONTROL_MASK);
 	clk_prepare_enable(res->hdmi);
 	clk_prepare_enable(res->sclk_hdmi);
 
@@ -1729,7 +1739,8 @@ static void hdmi_poweroff(struct hdmi_context *hdata)
 
 	clk_disable_unprepare(res->sclk_hdmi);
 	clk_disable_unprepare(res->hdmi);
-	clk_disable_unprepare(res->hdmiphy);
+	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);
 
 	mutex_lock(&hdata->hdmi_mutex);
@@ -1828,11 +1839,6 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
 		DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n");
 		goto fail;
 	}
-	res->hdmiphy = devm_clk_get(dev, "hdmiphy");
-	if (IS_ERR(res->hdmiphy)) {
-		DRM_ERROR("failed to get clock 'hdmiphy'\n");
-		goto fail;
-	}
 	res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
 	if (IS_ERR(res->mout_hdmi)) {
 		DRM_ERROR("failed to get clock 'mout_hdmi'\n");
@@ -1905,12 +1911,52 @@ static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata
 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) {
+		DRM_ERROR("Failed to find phy power control node\n");
+		ret = -ENODEV;
+		goto fail;
+	}
+
+	/* 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");
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	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;
+}
+
 #else
 static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata
 					(struct device *dev)
 {
 	return NULL;
 }
+
+static int drm_hdmi_dt_parse_phy_pow_control(struct hdmi_context *hdata)
+{
+	return 0;
+}
+
 #endif
 
 static struct platform_device_id hdmi_driver_types[] = {
@@ -2022,6 +2068,13 @@ static int hdmi_probe(struct platform_device *pdev)
 		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 */
 	if (i2c_add_driver(&ddc_driver)) {
 		DRM_ERROR("failed to register ddc i2c driver\n");
diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h b/drivers/gpu/drm/exynos/regs-hdmi.h
index ef1b3eb..8d9ca25 100644
--- a/drivers/gpu/drm/exynos/regs-hdmi.h
+++ b/drivers/gpu/drm/exynos/regs-hdmi.h
@@ -578,4 +578,8 @@
 #define HDMI_TG_VACT_ST4_H		HDMI_TG_BASE(0x0074)
 #define HDMI_TG_3D			HDMI_TG_BASE(0x00F0)
 
+#define PMU_HDMI_PHY_CONTROL_MASK	(1 << 0)
+#define PMU_HDMI_PHY_ENABLE		(1)
+#define PMU_HDMI_PHY_DISABLE		(0)
+
 #endif /* SAMSUNG_REGS_HDMI_H */
-- 
1.7.10.4

[RFC 2/2] ARM/dts: add hdmiphy power control pmu register to hdmi dt node

[Rahul Sharma]

Add hdmiphy power control node as a child to hdmi node. This
node will be parsed by hdmi driver to map phy control pmu reg and
control the phy power.

Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 354e14a..5549236 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -608,6 +608,12 @@
 				<&clock 157>, <&clock 1024>;
 		clock-names = "hdmi", "sclk_hdmi", "sclk_pixel",
 				"sclk_hdmiphy", "mout_hdmi";
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		phy-power-control {
+			reg = <0x10040700 0x04>;
+		};
 	};
 
 	mixer {
-- 
1.7.10.4

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Inki Dae]

[-- Attachment #1.1: Type: text/plain, Size: 1849 bytes --]

Hi Rahul,

This patch is important to us. Actually, previous hdmi driver had
controlled hdmiphy HDMI_PHY_CONTROL as if that were a clock but now that
doesn't exist anymore. So we need to discuss how hdmiphy should be handled.
I konw that you had already posted hdmiphy relevant patch set, [PATCH 0/4]
drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver.

I think we can couple pmu register controlling codes with that patch set
without RFC. Could you update and post them again? like below,
[PATCH 0/4] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver +
[RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

And then let's start review :)

Thanks,
Inki Dae



2013/6/11 Rahul Sharma <rahul.sharma-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

> Previously, hdmiphy is added as a dummy clock in clock file for
> exynos SoCs. Enable/Disable to this clock, actually toggles the power
> control bit in PMU, instead of controlling the clock gate.
>
> This RFC adds the support to parse hdmiphy control node which is a child
> node to hdmi, and map the pmu register to toggle the power control bit.
>
> This is based on drm-next branch in Inki Dae's tree.
>
> Rahul Sharma (2):
>   drm/exynos: replace dummy hdmiphy clock with pmu register control
>   ARM/dts: add hdmiphy power control pmu register to hdmi dt node
>
>  arch/arm/boot/dts/exynos5250.dtsi    |    6 +++
>  drivers/gpu/drm/exynos/exynos_hdmi.c |   69
> ++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/exynos/regs-hdmi.h   |    4 ++
>  3 files changed, 71 insertions(+), 8 deletions(-)
>
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-samsung-soc" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[-- Attachment #1.2: Type: text/html, Size: 2615 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Inki Dae]

[-- Attachment #1.1: Type: text/plain, Size: 2142 bytes --]

2013/6/12 Inki Dae <inki.dae-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

> Hi Rahul,
>
> This patch is important to us. Actually, previous hdmi driver had
> controlled hdmiphy HDMI_PHY_CONTROL as if that were a clock but now that
> doesn't exist anymore. So we need to discuss how hdmiphy should be handled.
> I konw that you had already posted hdmiphy relevant patch set, [PATCH 0/4]
> drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver.
>
> I think we can couple pmu register controlling codes with that patch set
> without RFC. Could you update and post them again? like below,
> [PATCH 0/4] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
> + [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg
> control
>
> And then let's start review :)
>

And I think It would be better to move the pmu register controlling codes
into hdmiphy driver like drivers/usb/phy/phy-samsung-usb2.c driver does.


>
> Thanks,
> Inki Dae
>
>
>
> 2013/6/11 Rahul Sharma <rahul.sharma-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
>> Previously, hdmiphy is added as a dummy clock in clock file for
>> exynos SoCs. Enable/Disable to this clock, actually toggles the power
>> control bit in PMU, instead of controlling the clock gate.
>>
>> This RFC adds the support to parse hdmiphy control node which is a child
>> node to hdmi, and map the pmu register to toggle the power control bit.
>>
>> This is based on drm-next branch in Inki Dae's tree.
>>
>> Rahul Sharma (2):
>>   drm/exynos: replace dummy hdmiphy clock with pmu register control
>>   ARM/dts: add hdmiphy power control pmu register to hdmi dt node
>>
>>  arch/arm/boot/dts/exynos5250.dtsi    |    6 +++
>>  drivers/gpu/drm/exynos/exynos_hdmi.c |   69
>> ++++++++++++++++++++++++++++++----
>>  drivers/gpu/drm/exynos/regs-hdmi.h   |    4 ++
>>  3 files changed, 71 insertions(+), 8 deletions(-)
>>
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-samsung-soc" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>

[-- Attachment #1.2: Type: text/html, Size: 3438 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Rahul Sharma]

Mr. Dae,

Thanks for your valuable inputs.

I posted it as RFC because, I also have received comments to register
hdmiphy as a clock controller. As we always configure it for specific
frequency, hdmi-phy looks similar to a PLL. But it really doesn't
belong to that class. Secondly prior to exynos5420, it was a i2c
device. I am not sure we can register a I2C device as a clock
controller. I wanted to discuss and explore this option here.

As you said, in parallel, I will align these changes and along with
"drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver"
series and post them.

I hope we should be able to close on one of the above approaches for
hdmiphy.

regards,
Rahul Sharma.

On Wed, Jun 12, 2013 at 9:57 AM, Inki Dae <inki.dae@samsung.com> wrote:
>
>
>
> 2013/6/12 Inki Dae <inki.dae@samsung.com>
>>
>> Hi Rahul,
>>
>> This patch is important to us. Actually, previous hdmi driver had
>> controlled hdmiphy HDMI_PHY_CONTROL as if that were a clock but now that
>> doesn't exist anymore. So we need to discuss how hdmiphy should be handled.
>> I konw that you had already posted hdmiphy relevant patch set, [PATCH 0/4]
>> drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver.
>>
>> I think we can couple pmu register controlling codes with that patch set
>> without RFC. Could you update and post them again? like below,
>> [PATCH 0/4] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
>> + [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg
>> control
>>
>> And then let's start review :)
>
>
> And I think It would be better to move the pmu register controlling codes
> into hdmiphy driver like drivers/usb/phy/phy-samsung-usb2.c driver does.
>
>>
>>
>> Thanks,
>> Inki Dae
>>
>>
>>
>> 2013/6/11 Rahul Sharma <rahul.sharma@samsung.com>
>>>
>>> Previously, hdmiphy is added as a dummy clock in clock file for
>>> exynos SoCs. Enable/Disable to this clock, actually toggles the power
>>> control bit in PMU, instead of controlling the clock gate.
>>>
>>> This RFC adds the support to parse hdmiphy control node which is a child
>>> node to hdmi, and map the pmu register to toggle the power control bit.
>>>
>>> This is based on drm-next branch in Inki Dae's tree.
>>>
>>> Rahul Sharma (2):
>>>   drm/exynos: replace dummy hdmiphy clock with pmu register control
>>>   ARM/dts: add hdmiphy power control pmu register to hdmi dt node
>>>
>>>  arch/arm/boot/dts/exynos5250.dtsi    |    6 +++
>>>  drivers/gpu/drm/exynos/exynos_hdmi.c |   69
>>> ++++++++++++++++++++++++++++++----
>>>  drivers/gpu/drm/exynos/regs-hdmi.h   |    4 ++
>>>  3 files changed, 71 insertions(+), 8 deletions(-)
>>>
>>> --
>>> 1.7.10.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe
>>> linux-samsung-soc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Sylwester Nawrocki]

Hi,

On 06/13/2013 06:26 AM, Rahul Sharma wrote:
> Mr. Dae,
> 
> Thanks for your valuable inputs.
> 
> I posted it as RFC because, I also have received comments to register
> hdmiphy as a clock controller. As we always configure it for specific
> frequency, hdmi-phy looks similar to a PLL. But it really doesn't
> belong to that class. Secondly prior to exynos5420, it was a i2c
> device. I am not sure we can register a I2C device as a clock
> controller. I wanted to discuss and explore this option here.

Have you considered using the generic PHY framework for those HDMI 
PHY devices [1] ? I guess we could add a dedicated group of ops for 
video PHYs, similarly as is is done with struct v4l2_subdev_ops. For
configuring things like the carrier/pixel clock frequency or anything 
what's common across the video PHYs.

Perhaps you could have a look and see if this framework would be 
useful for HDMI and possibly point out anything what might be missing ?
 
I'm not sure it it really solves the issues specific to the Exynos 
HDMI but at least with a generic PHY driver the PHY module would be 
separate from the PHY controller, as often same HDMI DPHY can be used 
with various types of a HDMI controller. So this would allow to not 
duplicate the HDMI PHY drivers in the long-term perspective.

[1] https://lkml.org/lkml/2013/4/29/95

Thanks,
Sylwester

> As you said, in parallel, I will align these changes and along with
> "drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver"
> series and post them.
> 
> I hope we should be able to close on one of the above approaches for
> hdmiphy.
> 
> regards,
> Rahul Sharma.
> 
> On Wed, Jun 12, 2013 at 9:57 AM, Inki Dae <inki.dae@samsung.com> wrote:
>>
>> 2013/6/12 Inki Dae <inki.dae@samsung.com>
>>>
>>> Hi Rahul,
>>>
>>> This patch is important to us. Actually, previous hdmi driver had
>>> controlled hdmiphy HDMI_PHY_CONTROL as if that were a clock but now that
>>> doesn't exist anymore. So we need to discuss how hdmiphy should be handled.
>>> I konw that you had already posted hdmiphy relevant patch set, [PATCH 0/4]
>>> drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver.
>>>
>>> I think we can couple pmu register controlling codes with that patch set
>>> without RFC. Could you update and post them again? like below,
>>> [PATCH 0/4] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
>>> + [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg
>>> control
>>>
>>> And then let's start review :)
>>
>> And I think It would be better to move the pmu register controlling codes
>> into hdmiphy driver like drivers/usb/phy/phy-samsung-usb2.c driver does.
>>>
>>> Thanks,
>>> Inki Dae
>>>
>>> 2013/6/11 Rahul Sharma <rahul.sharma@samsung.com>
>>>>
>>>> Previously, hdmiphy is added as a dummy clock in clock file for
>>>> exynos SoCs. Enable/Disable to this clock, actually toggles the power
>>>> control bit in PMU, instead of controlling the clock gate.
>>>>
>>>> This RFC adds the support to parse hdmiphy control node which is a child
>>>> node to hdmi, and map the pmu register to toggle the power control bit.
>>>>
>>>> This is based on drm-next branch in Inki Dae's tree.
>>>>
>>>> Rahul Sharma (2):
>>>>   drm/exynos: replace dummy hdmiphy clock with pmu register control
>>>>   ARM/dts: add hdmiphy power control pmu register to hdmi dt node
>>>>
>>>>  arch/arm/boot/dts/exynos5250.dtsi    |    6 +++
>>>>  drivers/gpu/drm/exynos/exynos_hdmi.c |   69
>>>> ++++++++++++++++++++++++++++++----
>>>>  drivers/gpu/drm/exynos/regs-hdmi.h   |    4 ++
>>>>  3 files changed, 71 insertions(+), 8 deletions(-)
>>>>
>>>> --
>>>> 1.7.10.4

-- 
Sylwester Nawrocki
Samsung R&D Institute Poland
Samsung Electronics

RE: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Inki Dae]

> -----Original Message-----
> From: Sylwester Nawrocki [mailto:s.nawrocki@samsung.com]
> Sent: Thursday, June 13, 2013 5:56 PM
> To: Rahul Sharma
> Cc: Rahul Sharma; Inki Dae; linux-samsung-soc@vger.kernel.org; devicetree-
> discuss@lists.ozlabs.org; DRI mailing list; Kukjin Kim; Seung-Woo Kim;
> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren;
> grant.likely@linaro.org
> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with
> pmu reg control
> 
> Hi,
> 
> On 06/13/2013 06:26 AM, Rahul Sharma wrote:
> > Mr. Dae,
> >
> > Thanks for your valuable inputs.
> >
> > I posted it as RFC because, I also have received comments to register
> > hdmiphy as a clock controller. As we always configure it for specific
> > frequency, hdmi-phy looks similar to a PLL. But it really doesn't
> > belong to that class. Secondly prior to exynos5420, it was a i2c
> > device. I am not sure we can register a I2C device as a clock
> > controller. I wanted to discuss and explore this option here.
> 
> Have you considered using the generic PHY framework for those HDMI
> PHY devices [1] ? I guess we could add a dedicated group of ops for
> video PHYs, similarly as is is done with struct v4l2_subdev_ops. For
> configuring things like the carrier/pixel clock frequency or anything
> what's common across the video PHYs.
> 
> Perhaps you could have a look and see if this framework would be
> useful for HDMI and possibly point out anything what might be missing ?
> 
> I'm not sure it it really solves the issues specific to the Exynos
> HDMI but at least with a generic PHY driver the PHY module would be
> separate from the PHY controller, as often same HDMI DPHY can be used
> with various types of a HDMI controller. So this would allow to not
> duplicate the HDMI PHY drivers in the long-term perspective.

Yeah, at least, it seems that we could use PHY module to control PMU
register, HDMI_PHY_CONTROL. However, PHY module provides only init/on/off
callbacks. As you may know, HDMIPHY needs i2c interfaces to control HDMIPHY
clock. So with PHY module, HDMIPHY driver could enable PMU more generically,
but also has to use existing i2c stuff to control HDMIPHY clock. I had a
quick review to Generic PHY Framework[v6] but I didn't see that the PHY
module could generically support more features such as i2c stuff.

Thanks,
Inki Dae

> 
> [1] https://lkml.org/lkml/2013/4/29/95
> 
> Thanks,
> Sylwester
> 
> > As you said, in parallel, I will align these changes and along with
> > "drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver"
> > series and post them.
> >
> > I hope we should be able to close on one of the above approaches for
> > hdmiphy.
> >
> > regards,
> > Rahul Sharma.
> >
> > On Wed, Jun 12, 2013 at 9:57 AM, Inki Dae <inki.dae@samsung.com> wrote:
> >>
> >> 2013/6/12 Inki Dae <inki.dae@samsung.com>
> >>>
> >>> Hi Rahul,
> >>>
> >>> This patch is important to us. Actually, previous hdmi driver had
> >>> controlled hdmiphy HDMI_PHY_CONTROL as if that were a clock but now
> that
> >>> doesn't exist anymore. So we need to discuss how hdmiphy should be
> handled.
> >>> I konw that you had already posted hdmiphy relevant patch set, [PATCH
> 0/4]
> >>> drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver.
> >>>
> >>> I think we can couple pmu register controlling codes with that patch
> set
> >>> without RFC. Could you update and post them again? like below,
> >>> [PATCH 0/4] drm/exynos: hdmi: move hdmiphy related code to hdmiphy
> driver
> >>> + [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg
> >>> control
> >>>
> >>> And then let's start review :)
> >>
> >> And I think It would be better to move the pmu register controlling
> codes
> >> into hdmiphy driver like drivers/usb/phy/phy-samsung-usb2.c driver
does.
> >>>
> >>> Thanks,
> >>> Inki Dae
> >>>
> >>> 2013/6/11 Rahul Sharma <rahul.sharma@samsung.com>
> >>>>
> >>>> Previously, hdmiphy is added as a dummy clock in clock file for
> >>>> exynos SoCs. Enable/Disable to this clock, actually toggles the power
> >>>> control bit in PMU, instead of controlling the clock gate.
> >>>>
> >>>> This RFC adds the support to parse hdmiphy control node which is a
> child
> >>>> node to hdmi, and map the pmu register to toggle the power control
> bit.
> >>>>
> >>>> This is based on drm-next branch in Inki Dae's tree.
> >>>>
> >>>> Rahul Sharma (2):
> >>>>   drm/exynos: replace dummy hdmiphy clock with pmu register control
> >>>>   ARM/dts: add hdmiphy power control pmu register to hdmi dt node
> >>>>
> >>>>  arch/arm/boot/dts/exynos5250.dtsi    |    6 +++
> >>>>  drivers/gpu/drm/exynos/exynos_hdmi.c |   69
> >>>> ++++++++++++++++++++++++++++++----
> >>>>  drivers/gpu/drm/exynos/regs-hdmi.h   |    4 ++
> >>>>  3 files changed, 71 insertions(+), 8 deletions(-)
> >>>>
> >>>> --
> >>>> 1.7.10.4
> 
> --
> Sylwester Nawrocki
> Samsung R&D Institute Poland
> Samsung Electronics

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Kishon Vijay Abraham I]

Hi,

On Thursday 13 June 2013 04:51 PM, Inki Dae wrote:
>
>
>> -----Original Message-----
>> From: Sylwester Nawrocki [mailto:s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org]
>> Sent: Thursday, June 13, 2013 5:56 PM
>> To: Rahul Sharma
>> Cc: Rahul Sharma; Inki Dae; linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; devicetree-
>> discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; DRI mailing list; Kukjin Kim; Seung-Woo Kim;
>> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren;
>> grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
>> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with
>> pmu reg control
>>
>> Hi,
>>
>> On 06/13/2013 06:26 AM, Rahul Sharma wrote:
>>> Mr. Dae,
>>>
>>> Thanks for your valuable inputs.
>>>
>>> I posted it as RFC because, I also have received comments to register
>>> hdmiphy as a clock controller. As we always configure it for specific
>>> frequency, hdmi-phy looks similar to a PLL. But it really doesn't
>>> belong to that class. Secondly prior to exynos5420, it was a i2c
>>> device. I am not sure we can register a I2C device as a clock
>>> controller. I wanted to discuss and explore this option here.
>>
>> Have you considered using the generic PHY framework for those HDMI
>> PHY devices [1] ? I guess we could add a dedicated group of ops for
>> video PHYs, similarly as is is done with struct v4l2_subdev_ops. For
>> configuring things like the carrier/pixel clock frequency or anything
>> what's common across the video PHYs.
>>
>> Perhaps you could have a look and see if this framework would be
>> useful for HDMI and possibly point out anything what might be missing ?
>>
>> I'm not sure it it really solves the issues specific to the Exynos
>> HDMI but at least with a generic PHY driver the PHY module would be
>> separate from the PHY controller, as often same HDMI DPHY can be used
>> with various types of a HDMI controller. So this would allow to not
>> duplicate the HDMI PHY drivers in the long-term perspective.
>
> Yeah, at least, it seems that we could use PHY module to control PMU
> register, HDMI_PHY_CONTROL. However, PHY module provides only init/on/off
> callbacks. As you may know, HDMIPHY needs i2c interfaces to control HDMIPHY
> clock. So with PHY module, HDMIPHY driver could enable PMU more generically,
> but also has to use existing i2c stuff to control HDMIPHY clock. I had a
> quick review to Generic PHY Framework[v6] but I didn't see that the PHY
> module could generically support more features such as i2c stuff.

I don't think PHY framework needs to provide i2c interfaces to program 
certain configurations. Instead in one of the callbacks (init/on/off) 
PHY driver can program whatever it wants using any of the interfaces it 
needs. IMO PHY framework should work independent of the interfaces.

For example, twl4030 phy driver actually uses i2c to program its 
registers but still it uses the PHY framework [1].

[1] --> http://www.gossamer-threads.com/lists/linux/kernel/1729414

Thanks
Kishon

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[김승우]

Hello Kishon,

On 2013년 06월 13일 21:54, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 13 June 2013 04:51 PM, Inki Dae wrote:
>>
>>
>>> -----Original Message-----
>>> From: Sylwester Nawrocki [mailto:s.nawrocki@samsung.com]
>>> Sent: Thursday, June 13, 2013 5:56 PM
>>> To: Rahul Sharma
>>> Cc: Rahul Sharma; Inki Dae; linux-samsung-soc@vger.kernel.org;
>>> devicetree-
>>> discuss@lists.ozlabs.org; DRI mailing list; Kukjin Kim; Seung-Woo Kim;
>>> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren;
>>> grant.likely@linaro.org
>>> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with
>>> pmu reg control
>>>
>>> Hi,
>>>
>>> On 06/13/2013 06:26 AM, Rahul Sharma wrote:
>>>> Mr. Dae,
>>>>
>>>> Thanks for your valuable inputs.
>>>>
>>>> I posted it as RFC because, I also have received comments to register
>>>> hdmiphy as a clock controller. As we always configure it for specific
>>>> frequency, hdmi-phy looks similar to a PLL. But it really doesn't
>>>> belong to that class. Secondly prior to exynos5420, it was a i2c
>>>> device. I am not sure we can register a I2C device as a clock
>>>> controller. I wanted to discuss and explore this option here.
>>>
>>> Have you considered using the generic PHY framework for those HDMI
>>> PHY devices [1] ? I guess we could add a dedicated group of ops for
>>> video PHYs, similarly as is is done with struct v4l2_subdev_ops. For
>>> configuring things like the carrier/pixel clock frequency or anything
>>> what's common across the video PHYs.
>>>
>>> Perhaps you could have a look and see if this framework would be
>>> useful for HDMI and possibly point out anything what might be missing ?
>>>
>>> I'm not sure it it really solves the issues specific to the Exynos
>>> HDMI but at least with a generic PHY driver the PHY module would be
>>> separate from the PHY controller, as often same HDMI DPHY can be used
>>> with various types of a HDMI controller. So this would allow to not
>>> duplicate the HDMI PHY drivers in the long-term perspective.
>>
>> Yeah, at least, it seems that we could use PHY module to control PMU
>> register, HDMI_PHY_CONTROL. However, PHY module provides only init/on/off
>> callbacks. As you may know, HDMIPHY needs i2c interfaces to control
>> HDMIPHY
>> clock. So with PHY module, HDMIPHY driver could enable PMU more
>> generically,
>> but also has to use existing i2c stuff to control HDMIPHY clock. I had a
>> quick review to Generic PHY Framework[v6] but I didn't see that the PHY
>> module could generically support more features such as i2c stuff.
> 
> I don't think PHY framework needs to provide i2c interfaces to program
> certain configurations. Instead in one of the callbacks (init/on/off)
> PHY driver can program whatever it wants using any of the interfaces it
> needs. IMO PHY framework should work independent of the interfaces.

In exnoys hdmi case, i2c interface is not the exact issue. In exynos
hdmi, hdmiphy should send i2c configuration about video clock
information as the video mode information including resolution, bit per
pixel, refresh rate passed from drm subsystem. So init/on/off callbacks
of phy framework are not enough for exynos hdmiphy and it should have a
callback to set video mode.

Do you have plan to add driver specific extend callback pointers to phy
framework?

Currently, hdmi directly calls phy operations, but Rahul's another patch
set, mentioned by Inki, divides hdmi and hdmiphy and hdmi and hdmiphy is
connected with exynos hdmi own sub driver callback operations.

IMHO, if phy framework can support extend callback feature, then this
own sub driver callbacks can be replaced with phy framework at long term
view.

Thanks and Regards,
- Seung-Woo Kim

> 
> For example, twl4030 phy driver actually uses i2c to program its
> registers but still it uses the PHY framework [1].
> 
> [1] --> http://www.gossamer-threads.com/lists/linux/kernel/1729414
> 
> Thanks
> Kishon
> -- 
> To unsubscribe from this list: send the line "unsubscribe
> linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Seung-Woo Kim
Samsung Software R&D Center
--

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Rahul Sharma]

Thanks all,

On Fri, Jun 14, 2013 at 11:39 AM, 김승우 <sw0312.kim@samsung.com> wrote:
> Hello Kishon,
>
> On 2013년 06월 13일 21:54, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 13 June 2013 04:51 PM, Inki Dae wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Sylwester Nawrocki [mailto:s.nawrocki@samsung.com]
>>>> Sent: Thursday, June 13, 2013 5:56 PM
>>>> To: Rahul Sharma
>>>> Cc: Rahul Sharma; Inki Dae; linux-samsung-soc@vger.kernel.org;
>>>> devicetree-
>>>> discuss@lists.ozlabs.org; DRI mailing list; Kukjin Kim; Seung-Woo Kim;
>>>> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren;
>>>> grant.likely@linaro.org
>>>> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with
>>>> pmu reg control
>>>>
>>>> Hi,
>>>>
>>>> On 06/13/2013 06:26 AM, Rahul Sharma wrote:
>>>>> Mr. Dae,
>>>>>
>>>>> Thanks for your valuable inputs.
>>>>>
>>>>> I posted it as RFC because, I also have received comments to register
>>>>> hdmiphy as a clock controller. As we always configure it for specific
>>>>> frequency, hdmi-phy looks similar to a PLL. But it really doesn't
>>>>> belong to that class. Secondly prior to exynos5420, it was a i2c
>>>>> device. I am not sure we can register a I2C device as a clock
>>>>> controller. I wanted to discuss and explore this option here.
>>>>
>>>> Have you considered using the generic PHY framework for those HDMI
>>>> PHY devices [1] ? I guess we could add a dedicated group of ops for
>>>> video PHYs, similarly as is is done with struct v4l2_subdev_ops. For
>>>> configuring things like the carrier/pixel clock frequency or anything
>>>> what's common across the video PHYs.
>>>>
>>>> Perhaps you could have a look and see if this framework would be
>>>> useful for HDMI and possibly point out anything what might be missing ?
>>>>
>>>> I'm not sure it it really solves the issues specific to the Exynos
>>>> HDMI but at least with a generic PHY driver the PHY module would be
>>>> separate from the PHY controller, as often same HDMI DPHY can be used
>>>> with various types of a HDMI controller. So this would allow to not
>>>> duplicate the HDMI PHY drivers in the long-term perspective.
>>>
>>> Yeah, at least, it seems that we could use PHY module to control PMU
>>> register, HDMI_PHY_CONTROL. However, PHY module provides only init/on/off
>>> callbacks. As you may know, HDMIPHY needs i2c interfaces to control
>>> HDMIPHY
>>> clock. So with PHY module, HDMIPHY driver could enable PMU more
>>> generically,
>>> but also has to use existing i2c stuff to control HDMIPHY clock. I had a
>>> quick review to Generic PHY Framework[v6] but I didn't see that the PHY
>>> module could generically support more features such as i2c stuff.
>>
>> I don't think PHY framework needs to provide i2c interfaces to program
>> certain configurations. Instead in one of the callbacks (init/on/off)
>> PHY driver can program whatever it wants using any of the interfaces it
>> needs. IMO PHY framework should work independent of the interfaces.
>
> In exnoys hdmi case, i2c interface is not the exact issue. In exynos
> hdmi, hdmiphy should send i2c configuration about video clock
> information as the video mode information including resolution, bit per
> pixel, refresh rate passed from drm subsystem. So init/on/off callbacks
> of phy framework are not enough for exynos hdmiphy and it should have a
> callback to set video mode.
>
> Do you have plan to add driver specific extend callback pointers to phy
> framework?
>
> Currently, hdmi directly calls phy operations, but Rahul's another patch
> set, mentioned by Inki, divides hdmi and hdmiphy and hdmi and hdmiphy is
> connected with exynos hdmi own sub driver callback operations.
>
> IMHO, if phy framework can support extend callback feature, then this
> own sub driver callbacks can be replaced with phy framework at long term
> view.

Extended callbacks are always welcome. I can also use phy device
private data to pass on private ops like get_pixelclk and set_pixelclk.
Similar logic has been used to pass struct omap_usb to usb phy
controller. I can add these changes for migration of hdmiphy to
generic phy framwork to my hdmiphy separation patch set.

regards,
Rahul Sharma.

>
> Thanks and Regards,
> - Seung-Woo Kim
>
>>
>> For example, twl4030 phy driver actually uses i2c to program its
>> registers but still it uses the PHY framework [1].
>>
>> [1] --> http://www.gossamer-threads.com/lists/linux/kernel/1729414
>>
>> Thanks
>> Kishon
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> --
> Seung-Woo Kim
> Samsung Software R&D Center
> --
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Kishon Vijay Abraham I]

Hi,

On Tuesday 18 June 2013 03:33 PM, Rahul Sharma wrote:
> Thanks all,
> 
> On Fri, Jun 14, 2013 at 11:39 AM, 김승우 <sw0312.kim@samsung.com> wrote:
>> Hello Kishon,
>>
>> On 2013년 06월 13일 21:54, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Thursday 13 June 2013 04:51 PM, Inki Dae wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Sylwester Nawrocki [mailto:s.nawrocki@samsung.com]
>>>>> Sent: Thursday, June 13, 2013 5:56 PM
>>>>> To: Rahul Sharma
>>>>> Cc: Rahul Sharma; Inki Dae; linux-samsung-soc@vger.kernel.org;
>>>>> devicetree-
>>>>> discuss@lists.ozlabs.org; DRI mailing list; Kukjin Kim; Seung-Woo Kim;
>>>>> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren;
>>>>> grant.likely@linaro.org
>>>>> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with
>>>>> pmu reg control
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 06/13/2013 06:26 AM, Rahul Sharma wrote:
>>>>>> Mr. Dae,
>>>>>>
>>>>>> Thanks for your valuable inputs.
>>>>>>
>>>>>> I posted it as RFC because, I also have received comments to register
>>>>>> hdmiphy as a clock controller. As we always configure it for specific
>>>>>> frequency, hdmi-phy looks similar to a PLL. But it really doesn't
>>>>>> belong to that class. Secondly prior to exynos5420, it was a i2c
>>>>>> device. I am not sure we can register a I2C device as a clock
>>>>>> controller. I wanted to discuss and explore this option here.
>>>>>
>>>>> Have you considered using the generic PHY framework for those HDMI
>>>>> PHY devices [1] ? I guess we could add a dedicated group of ops for
>>>>> video PHYs, similarly as is is done with struct v4l2_subdev_ops. For
>>>>> configuring things like the carrier/pixel clock frequency or anything
>>>>> what's common across the video PHYs.
>>>>>
>>>>> Perhaps you could have a look and see if this framework would be
>>>>> useful for HDMI and possibly point out anything what might be missing ?
>>>>>
>>>>> I'm not sure it it really solves the issues specific to the Exynos
>>>>> HDMI but at least with a generic PHY driver the PHY module would be
>>>>> separate from the PHY controller, as often same HDMI DPHY can be used
>>>>> with various types of a HDMI controller. So this would allow to not
>>>>> duplicate the HDMI PHY drivers in the long-term perspective.
>>>>
>>>> Yeah, at least, it seems that we could use PHY module to control PMU
>>>> register, HDMI_PHY_CONTROL. However, PHY module provides only init/on/off
>>>> callbacks. As you may know, HDMIPHY needs i2c interfaces to control
>>>> HDMIPHY
>>>> clock. So with PHY module, HDMIPHY driver could enable PMU more
>>>> generically,
>>>> but also has to use existing i2c stuff to control HDMIPHY clock. I had a
>>>> quick review to Generic PHY Framework[v6] but I didn't see that the PHY
>>>> module could generically support more features such as i2c stuff.
>>>
>>> I don't think PHY framework needs to provide i2c interfaces to program
>>> certain configurations. Instead in one of the callbacks (init/on/off)
>>> PHY driver can program whatever it wants using any of the interfaces it
>>> needs. IMO PHY framework should work independent of the interfaces.
>>
>> In exnoys hdmi case, i2c interface is not the exact issue. In exynos
>> hdmi, hdmiphy should send i2c configuration about video clock
>> information as the video mode information including resolution, bit per
>> pixel, refresh rate passed from drm subsystem. So init/on/off callbacks
>> of phy framework are not enough for exynos hdmiphy and it should have a
>> callback to set video mode.
>>
>> Do you have plan to add driver specific extend callback pointers to phy
>> framework?
>>
>> Currently, hdmi directly calls phy operations, but Rahul's another patch
>> set, mentioned by Inki, divides hdmi and hdmiphy and hdmi and hdmiphy is
>> connected with exynos hdmi own sub driver callback operations.
>>
>> IMHO, if phy framework can support extend callback feature, then this
>> own sub driver callbacks can be replaced with phy framework at long term
>> view.
> 
> Extended callbacks are always welcome. I can also use phy device
> private data to pass on private ops like get_pixelclk and set_pixelclk.

I would recommend creating a wrapper to the existing PHY framework
for HDMI PHY. That way, we can have other HDMI phys added
easily. We need to figure out all the ops that might be needed by the
HDMI PHY to be added to the wrapper.
IMO extended callbacks can lead to abuse of the system and should be
used only when absolutely necessary.

Thanks
Kishon

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Rahul Sharma]

[-- Attachment #1.1: Type: text/plain, Size: 5165 bytes --]

On Tue, Jun 18, 2013 at 5:07 PM, Kishon Vijay Abraham I <kishon@ti.com>wrote:

> Hi,
>
> On Tuesday 18 June 2013 03:33 PM, Rahul Sharma wrote:
> > Thanks all,
> >
> > On Fri, Jun 14, 2013 at 11:39 AM, 김승우 <sw0312.kim@samsung.com> wrote:
> >> Hello Kishon,
> >>
> >> On 2013년 06월 13일 21:54, Kishon Vijay Abraham I wrote:
> >>> Hi,
> >>>
> >>> On Thursday 13 June 2013 04:51 PM, Inki Dae wrote:
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Sylwester Nawrocki [mailto:s.nawrocki@samsung.com]
> >>>>> Sent: Thursday, June 13, 2013 5:56 PM
> >>>>> To: Rahul Sharma
> >>>>> Cc: Rahul Sharma; Inki Dae; linux-samsung-soc@vger.kernel.org;
> >>>>> devicetree-
> >>>>> discuss@lists.ozlabs.org; DRI mailing list; Kukjin Kim; Seung-Woo
> Kim;
> >>>>> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren;
> >>>>> grant.likely@linaro.org
> >>>>> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock
> with
> >>>>> pmu reg control
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> On 06/13/2013 06:26 AM, Rahul Sharma wrote:
> >>>>>> Mr. Dae,
> >>>>>>
> >>>>>> Thanks for your valuable inputs.
> >>>>>>
> >>>>>> I posted it as RFC because, I also have received comments to
> register
> >>>>>> hdmiphy as a clock controller. As we always configure it for
> specific
> >>>>>> frequency, hdmi-phy looks similar to a PLL. But it really doesn't
> >>>>>> belong to that class. Secondly prior to exynos5420, it was a i2c
> >>>>>> device. I am not sure we can register a I2C device as a clock
> >>>>>> controller. I wanted to discuss and explore this option here.
> >>>>>
> >>>>> Have you considered using the generic PHY framework for those HDMI
> >>>>> PHY devices [1] ? I guess we could add a dedicated group of ops for
> >>>>> video PHYs, similarly as is is done with struct v4l2_subdev_ops. For
> >>>>> configuring things like the carrier/pixel clock frequency or anything
> >>>>> what's common across the video PHYs.
> >>>>>
> >>>>> Perhaps you could have a look and see if this framework would be
> >>>>> useful for HDMI and possibly point out anything what might be
> missing ?
> >>>>>
> >>>>> I'm not sure it it really solves the issues specific to the Exynos
> >>>>> HDMI but at least with a generic PHY driver the PHY module would be
> >>>>> separate from the PHY controller, as often same HDMI DPHY can be used
> >>>>> with various types of a HDMI controller. So this would allow to not
> >>>>> duplicate the HDMI PHY drivers in the long-term perspective.
> >>>>
> >>>> Yeah, at least, it seems that we could use PHY module to control PMU
> >>>> register, HDMI_PHY_CONTROL. However, PHY module provides only
> init/on/off
> >>>> callbacks. As you may know, HDMIPHY needs i2c interfaces to control
> >>>> HDMIPHY
> >>>> clock. So with PHY module, HDMIPHY driver could enable PMU more
> >>>> generically,
> >>>> but also has to use existing i2c stuff to control HDMIPHY clock. I
> had a
> >>>> quick review to Generic PHY Framework[v6] but I didn't see that the
> PHY
> >>>> module could generically support more features such as i2c stuff.
> >>>
> >>> I don't think PHY framework needs to provide i2c interfaces to program
> >>> certain configurations. Instead in one of the callbacks (init/on/off)
> >>> PHY driver can program whatever it wants using any of the interfaces it
> >>> needs. IMO PHY framework should work independent of the interfaces.
> >>
> >> In exnoys hdmi case, i2c interface is not the exact issue. In exynos
> >> hdmi, hdmiphy should send i2c configuration about video clock
> >> information as the video mode information including resolution, bit per
> >> pixel, refresh rate passed from drm subsystem. So init/on/off callbacks
> >> of phy framework are not enough for exynos hdmiphy and it should have a
> >> callback to set video mode.
> >>
> >> Do you have plan to add driver specific extend callback pointers to phy
> >> framework?
> >>
> >> Currently, hdmi directly calls phy operations, but Rahul's another patch
> >> set, mentioned by Inki, divides hdmi and hdmiphy and hdmi and hdmiphy is
> >> connected with exynos hdmi own sub driver callback operations.
> >>
> >> IMHO, if phy framework can support extend callback feature, then this
> >> own sub driver callbacks can be replaced with phy framework at long term
> >> view.
> >
> > Extended callbacks are always welcome. I can also use phy device
> > private data to pass on private ops like get_pixelclk and set_pixelclk.
>
> I would recommend creating a wrapper to the existing PHY framework
> for HDMI PHY. That way, we can have other HDMI phys added
> easily. We need to figure out all the ops that might be needed by the
> HDMI PHY to be added to the wrapper.
> IMO extended callbacks can lead to abuse of the system and should be
> used only when absolutely necessary.
>
> Thanks
> Kishon
>

Thanks Kishon,

I have started working on this wrapper layer which is customized for video
phys.
As if now, adding set_dv_timing, get_dv_timing as the only additional
callbacks.
I will post the RFC patches.

regards,
Rahul Sharma.

[-- Attachment #1.2: Type: text/html, Size: 7015 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Kishon Vijay Abraham I]

Hi,

On Tuesday 30 July 2013 09:12 AM, Rahul Sharma wrote:
> 
> 
> On Tue, Jun 18, 2013 at 5:07 PM, Kishon Vijay Abraham I <kishon@ti.com
> <mailto:kishon@ti.com>> wrote:
> 
>     Hi,
> 
>     On Tuesday 18 June 2013 03:33 PM, Rahul Sharma wrote:
>     > Thanks all,
>     >
>     > On Fri, Jun 14, 2013 at 11:39 AM, 김승우 <sw0312.kim@samsung.com
>     <mailto:sw0312.kim@samsung.com>> wrote:
>     >> Hello Kishon,
>     >>
>     >> On 2013년 06월 13일 21:54, Kishon Vijay Abraham I wrote:
>     >>> Hi,
>     >>>
>     >>> On Thursday 13 June 2013 04:51 PM, Inki Dae wrote:
>     >>>>
>     >>>>
>     >>>>> -----Original Message-----
>     >>>>> From: Sylwester Nawrocki [mailto:s.nawrocki@samsung.com
>     <mailto:s.nawrocki@samsung.com>]
>     >>>>> Sent: Thursday, June 13, 2013 5:56 PM
>     >>>>> To: Rahul Sharma
>     >>>>> Cc: Rahul Sharma; Inki Dae; linux-samsung-soc@vger.kernel.org
>     <mailto:linux-samsung-soc@vger.kernel.org>;
>     >>>>> devicetree-
>     >>>>> discuss@lists.ozlabs.org <mailto:discuss@lists.ozlabs.org>; DRI
>     mailing list; Kukjin Kim; Seung-Woo Kim;
>     >>>>> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren;
>     >>>>> grant.likely@linaro.org <mailto:grant.likely@linaro.org>
>     >>>>> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with
>     >>>>> pmu reg control
>     >>>>>
>     >>>>> Hi,
>     >>>>>
>     >>>>> On 06/13/2013 06:26 AM, Rahul Sharma wrote:
>     >>>>>> Mr. Dae,
>     >>>>>>
>     >>>>>> Thanks for your valuable inputs.
>     >>>>>>
>     >>>>>> I posted it as RFC because, I also have received comments to register
>     >>>>>> hdmiphy as a clock controller. As we always configure it for specific
>     >>>>>> frequency, hdmi-phy looks similar to a PLL. But it really doesn't
>     >>>>>> belong to that class. Secondly prior to exynos5420, it was a i2c
>     >>>>>> device. I am not sure we can register a I2C device as a clock
>     >>>>>> controller. I wanted to discuss and explore this option here.
>     >>>>>
>     >>>>> Have you considered using the generic PHY framework for those HDMI
>     >>>>> PHY devices [1] ? I guess we could add a dedicated group of ops for
>     >>>>> video PHYs, similarly as is is done with struct v4l2_subdev_ops. For
>     >>>>> configuring things like the carrier/pixel clock frequency or anything
>     >>>>> what's common across the video PHYs.
>     >>>>>
>     >>>>> Perhaps you could have a look and see if this framework would be
>     >>>>> useful for HDMI and possibly point out anything what might be missing ?
>     >>>>>
>     >>>>> I'm not sure it it really solves the issues specific to the Exynos
>     >>>>> HDMI but at least with a generic PHY driver the PHY module would be
>     >>>>> separate from the PHY controller, as often same HDMI DPHY can be used
>     >>>>> with various types of a HDMI controller. So this would allow to not
>     >>>>> duplicate the HDMI PHY drivers in the long-term perspective.
>     >>>>
>     >>>> Yeah, at least, it seems that we could use PHY module to control PMU
>     >>>> register, HDMI_PHY_CONTROL. However, PHY module provides only init/on/off
>     >>>> callbacks. As you may know, HDMIPHY needs i2c interfaces to control
>     >>>> HDMIPHY
>     >>>> clock. So with PHY module, HDMIPHY driver could enable PMU more
>     >>>> generically,
>     >>>> but also has to use existing i2c stuff to control HDMIPHY clock. I had a
>     >>>> quick review to Generic PHY Framework[v6] but I didn't see that the PHY
>     >>>> module could generically support more features such as i2c stuff.
>     >>>
>     >>> I don't think PHY framework needs to provide i2c interfaces to program
>     >>> certain configurations. Instead in one of the callbacks (init/on/off)
>     >>> PHY driver can program whatever it wants using any of the interfaces it
>     >>> needs. IMO PHY framework should work independent of the interfaces.
>     >>
>     >> In exnoys hdmi case, i2c interface is not the exact issue. In exynos
>     >> hdmi, hdmiphy should send i2c configuration about video clock
>     >> information as the video mode information including resolution, bit per
>     >> pixel, refresh rate passed from drm subsystem. So init/on/off callbacks
>     >> of phy framework are not enough for exynos hdmiphy and it should have a
>     >> callback to set video mode.
>     >>
>     >> Do you have plan to add driver specific extend callback pointers to phy
>     >> framework?
>     >>
>     >> Currently, hdmi directly calls phy operations, but Rahul's another patch
>     >> set, mentioned by Inki, divides hdmi and hdmiphy and hdmi and hdmiphy is
>     >> connected with exynos hdmi own sub driver callback operations.
>     >>
>     >> IMHO, if phy framework can support extend callback feature, then this
>     >> own sub driver callbacks can be replaced with phy framework at long term
>     >> view.
>     >
>     > Extended callbacks are always welcome. I can also use phy device
>     > private data to pass on private ops like get_pixelclk and set_pixelclk.
> 
>     I would recommend creating a wrapper to the existing PHY framework
>     for HDMI PHY. That way, we can have other HDMI phys added
>     easily. We need to figure out all the ops that might be needed by the
>     HDMI PHY to be added to the wrapper.
>     IMO extended callbacks can lead to abuse of the system and should be
>     used only when absolutely necessary.
> 
>     Thanks
>     Kishon
> 
> 
> Thanks Kishon,
> 
> I have started working on this wrapper layer which is customized for video phys.
> As if now, adding set_dv_timing, get_dv_timing as the only additional callbacks.
> I will post the RFC patches.

Idea of creating wrapper layer for different types of controller is shot down
in the community [1] :-s

[1] -> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/181710.html

Thanks
Kishon

Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

[Seung-Woo Kim]

Hi Rahul,

On 2013년 07월 30일 12:42, Rahul Sharma wrote:
> 
> 
> On Tue, Jun 18, 2013 at 5:07 PM, Kishon Vijay Abraham I <kishon@ti.com
> <mailto:kishon@ti.com>> wrote:
> 
>     Hi,
> 
>     On Tuesday 18 June 2013 03:33 PM, Rahul Sharma wrote:
>     > Thanks all,
>     >
>     > On Fri, Jun 14, 2013 at 11:39 AM, 김승우 <sw0312.kim@samsung.com
>     <mailto:sw0312.kim@samsung.com>> wrote:
>     >> Hello Kishon,
>     >>
>     >> On 2013년 06월 13일 21:54, Kishon Vijay Abraham I wrote:
>     >>> Hi,
>     >>>
>     >>> On Thursday 13 June 2013 04:51 PM, Inki Dae wrote:
>     >>>>
>     >>>>
>     >>>>> -----Original Message-----
>     >>>>> From: Sylwester Nawrocki [mailto:s.nawrocki@samsung.com
>     <mailto:s.nawrocki@samsung.com>]
>     >>>>> Sent: Thursday, June 13, 2013 5:56 PM
>     >>>>> To: Rahul Sharma
>     >>>>> Cc: Rahul Sharma; Inki Dae; linux-samsung-soc@vger.kernel.org
>     <mailto:linux-samsung-soc@vger.kernel.org>;
>     >>>>> devicetree-
>     >>>>> discuss@lists.ozlabs.org <mailto:discuss@lists.ozlabs.org>;
>     DRI mailing list; Kukjin Kim; Seung-Woo Kim;
>     >>>>> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren;
>     >>>>> grant.likely@linaro.org <mailto:grant.likely@linaro.org>
>     >>>>> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy
>     clock with
>     >>>>> pmu reg control
>     >>>>>
>     >>>>> Hi,
>     >>>>>
>     >>>>> On 06/13/2013 06:26 AM, Rahul Sharma wrote:
>     >>>>>> Mr. Dae,
>     >>>>>>
>     >>>>>> Thanks for your valuable inputs.
>     >>>>>>
>     >>>>>> I posted it as RFC because, I also have received comments to
>     register
>     >>>>>> hdmiphy as a clock controller. As we always configure it for
>     specific
>     >>>>>> frequency, hdmi-phy looks similar to a PLL. But it really doesn't
>     >>>>>> belong to that class. Secondly prior to exynos5420, it was a i2c
>     >>>>>> device. I am not sure we can register a I2C device as a clock
>     >>>>>> controller. I wanted to discuss and explore this option here.
>     >>>>>
>     >>>>> Have you considered using the generic PHY framework for those HDMI
>     >>>>> PHY devices [1] ? I guess we could add a dedicated group of
>     ops for
>     >>>>> video PHYs, similarly as is is done with struct
>     v4l2_subdev_ops. For
>     >>>>> configuring things like the carrier/pixel clock frequency or
>     anything
>     >>>>> what's common across the video PHYs.
>     >>>>>
>     >>>>> Perhaps you could have a look and see if this framework would be
>     >>>>> useful for HDMI and possibly point out anything what might be
>     missing ?
>     >>>>>
>     >>>>> I'm not sure it it really solves the issues specific to the Exynos
>     >>>>> HDMI but at least with a generic PHY driver the PHY module
>     would be
>     >>>>> separate from the PHY controller, as often same HDMI DPHY can
>     be used
>     >>>>> with various types of a HDMI controller. So this would allow
>     to not
>     >>>>> duplicate the HDMI PHY drivers in the long-term perspective.
>     >>>>
>     >>>> Yeah, at least, it seems that we could use PHY module to
>     control PMU
>     >>>> register, HDMI_PHY_CONTROL. However, PHY module provides only
>     init/on/off
>     >>>> callbacks. As you may know, HDMIPHY needs i2c interfaces to control
>     >>>> HDMIPHY
>     >>>> clock. So with PHY module, HDMIPHY driver could enable PMU more
>     >>>> generically,
>     >>>> but also has to use existing i2c stuff to control HDMIPHY
>     clock. I had a
>     >>>> quick review to Generic PHY Framework[v6] but I didn't see that
>     the PHY
>     >>>> module could generically support more features such as i2c stuff.
>     >>>
>     >>> I don't think PHY framework needs to provide i2c interfaces to
>     program
>     >>> certain configurations. Instead in one of the callbacks
>     (init/on/off)
>     >>> PHY driver can program whatever it wants using any of the
>     interfaces it
>     >>> needs. IMO PHY framework should work independent of the interfaces.
>     >>
>     >> In exnoys hdmi case, i2c interface is not the exact issue. In exynos
>     >> hdmi, hdmiphy should send i2c configuration about video clock
>     >> information as the video mode information including resolution,
>     bit per
>     >> pixel, refresh rate passed from drm subsystem. So init/on/off
>     callbacks
>     >> of phy framework are not enough for exynos hdmiphy and it should
>     have a
>     >> callback to set video mode.
>     >>
>     >> Do you have plan to add driver specific extend callback pointers
>     to phy
>     >> framework?
>     >>
>     >> Currently, hdmi directly calls phy operations, but Rahul's
>     another patch
>     >> set, mentioned by Inki, divides hdmi and hdmiphy and hdmi and
>     hdmiphy is
>     >> connected with exynos hdmi own sub driver callback operations.
>     >>
>     >> IMHO, if phy framework can support extend callback feature, then this
>     >> own sub driver callbacks can be replaced with phy framework at
>     long term
>     >> view.
>     >
>     > Extended callbacks are always welcome. I can also use phy device
>     > private data to pass on private ops like get_pixelclk and
>     set_pixelclk.
> 
>     I would recommend creating a wrapper to the existing PHY framework
>     for HDMI PHY. That way, we can have other HDMI phys added
>     easily. We need to figure out all the ops that might be needed by the
>     HDMI PHY to be added to the wrapper.
>     IMO extended callbacks can lead to abuse of the system and should be
>     used only when absolutely necessary.
> 
>     Thanks
>     Kishon
> 
> 
> Thanks Kishon,
> 
> I have started working on this wrapper layer which is customized for
> video phys.
> As if now, adding set_dv_timing, get_dv_timing as the only additional
> callbacks.
> I will post the RFC patches.

I think your hdmiphy pmu patch is good enough just if dt binding for pmu
is in hdmiphy binding instead of hdmi binding. So I recommended to make
pmu patch set on the top of independent hdmiphy patch set because with
independent hdmiphy patch set hdmiphy pmu code is moved to hdmiphy driver.

Is it possible that hdmi driver references pmu information from hdmiphy
binding? If that, it seems one possible solution to fix current exynos
hdmi broken.

Thanks and Regards,
- Seung-Woo Kim

> 
> regards,
> Rahul Sharma.
> 
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Seung-Woo Kim
Samsung Software R&D Center
--