[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On Tue, Mar 12, 2013 at 5:37 PM, Inki Dae wrote: > 2013/3/12 Rahul Sharma : >> On Mon, Mar 4, 2013 at 7:35 PM, Rahul Sharma wrote: >>> Thanks Sean, >>> >>> On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma >>> samsung.com> wrote: > Right now hdmiphy operations and configs are kept inside hdmi driver. > hdmiphy related > code is tightly coupled with hdmi ip driver. Physicaly they are different > devices and s/Physicaly/Physically/ > should be instantiated independently. > > In terms of versions/mapping/configurations Hdmi and hdmiphy are > independent of each > other. It is preferred to isolate them and maintained independently. > > This implementations is tested for: > 1) Resolutions supported by exynos4 and 5 hdmi. > 2) Runtime PM and S2R scenarions for exynos5. > I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. >>> >>> I agree to it. splitting phy to a new driver will complicate the power >>> related >>> scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of >>> config values, mapping (i2c/platform bus) etc. Handling this diversity >>> inside hdmi driver is complicating it with unrelated changes. >>> >>> I have tested this RFC for Runtime PM / S2R. But if we see any major >>> roadblock >>> we should re-factor this by explicitly calling power related callbacks >>> of mixer, phy, >>> hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf >>> device. AFAIR something like this is already in place in chrome-kernel. >>> I've made some comments below. > This patch is dependent on > http://www.mail-archive.com/dri-devel at > lists.freedesktop.org/msg34733.html > http://www.mail-archive.com/dri-devel at > lists.freedesktop.org/msg34861.html > http://www.mail-archive.com/dri-devel at > lists.freedesktop.org/msg34862.html > > Signed-off-by: Rahul Sharma > --- > It is based on exynos-drm-next-todo branch at > git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git > > drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + > drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + > drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- > drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + > drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- > drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - > drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 > ++- > drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 > 8 files changed, 738 insertions(+), 368 deletions(-) > create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index 3da5c2d..03acb6b 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -334,6 +334,11 @@ static int __init exynos_drm_init(void) > ret = platform_driver_register(_driver); > if (ret < 0) > goto out_hdmi; > + > + ret = exynos_hdmiphy_driver_register(); Not sure why you need to obfuscate the type of driver here. All the other ones are directly registered as platform drivers, I don't see the harm in just doing the i2c_add_driver directly here. >>> >>> AIMB, Phy is likely to get mapped as a platform device in later soc's. >>> I want to make driver registration (inside exynos_drm_drv.c) independent >>> of this changes. We can handle this inside exynos_hdmiphy_driver_register(). >>> > + if (ret < 0) > + goto out_hdmiphy; > + > ret = platform_driver_register(_driver); > if (ret < 0) > goto out_mixer; I think this ordering is how you plan on enforcing the suspend/resume order for hdmi/mixer/hdmiphy. In that case, however, I don't think it makes sense to suspend/resume hdmiphy in between mixer and hdmi. Ideally, hdmiphy should go down first and come up last. >>> >>> I just wanted to keep, hdmi and hdmiphy things together. But what you said >>> above makes sense. I will do that change. >>> > @@ -436,6 +441,8 @@ out_common_hdmi_dev: > out_common_hdmi: > platform_driver_unregister(_driver); > out_mixer: > + exynos_hdmiphy_driver_unregister(); > +out_hdmiphy: > platform_driver_unregister(_driver); > out_hdmi: > #endif > @@ -479,6 +486,7 @@ static void __exit exynos_drm_exit(void) >
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
2013/3/12 Rahul Sharma : > On Mon, Mar 4, 2013 at 7:35 PM, Rahul Sharma wrote: >> Thanks Sean, >> >> On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul wrote: >>> On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma >>> wrote: Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related code is tightly coupled with hdmi ip driver. Physicaly they are different devices and >>> >>> s/Physicaly/Physically/ >>> should be instantiated independently. In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each other. It is preferred to isolate them and maintained independently. This implementations is tested for: 1) Resolutions supported by exynos4 and 5 hdmi. 2) Runtime PM and S2R scenarions for exynos5. >>> >>> I don't like the idea of spawning off yet another driver in here. It >>> adds more globals, more suspend/resume ordering issues, and more >>> implicit dependencies. I understand, however, that this is the Chosen >>> Way for the exynos driver, so I will save my rant. >>> >> >> I agree to it. splitting phy to a new driver will complicate the power >> related >> scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of >> config values, mapping (i2c/platform bus) etc. Handling this diversity >> inside hdmi driver is complicating it with unrelated changes. >> >> I have tested this RFC for Runtime PM / S2R. But if we see any major >> roadblock >> we should re-factor this by explicitly calling power related callbacks >> of mixer, phy, >> hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf >> device. AFAIR something like this is already in place in chrome-kernel. >> >>> I've made some comments below. >>> This patch is dependent on http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34733.html http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34861.html http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34862.html Signed-off-by: Rahul Sharma --- It is based on exynos-drm-next-todo branch at git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++- drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 8 files changed, 738 insertions(+), 368 deletions(-) create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 3da5c2d..03acb6b 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -334,6 +334,11 @@ static int __init exynos_drm_init(void) ret = platform_driver_register(_driver); if (ret < 0) goto out_hdmi; + + ret = exynos_hdmiphy_driver_register(); >>> >>> Not sure why you need to obfuscate the type of driver here. All the >>> other ones are directly registered as platform drivers, I don't see >>> the harm in just doing the i2c_add_driver directly here. >>> >> >> AIMB, Phy is likely to get mapped as a platform device in later soc's. >> I want to make driver registration (inside exynos_drm_drv.c) independent >> of this changes. We can handle this inside exynos_hdmiphy_driver_register(). >> + if (ret < 0) + goto out_hdmiphy; + ret = platform_driver_register(_driver); if (ret < 0) goto out_mixer; >>> >>> >>> I think this ordering is how you plan on enforcing the suspend/resume >>> order for hdmi/mixer/hdmiphy. In that case, however, I don't think it >>> makes sense to suspend/resume hdmiphy in between mixer and hdmi. >>> Ideally, hdmiphy should go down first and come up last. >>> >> >> I just wanted to keep, hdmi and hdmiphy things together. But what you said >> above makes sense. I will do that change. >> >>> @@ -436,6 +441,8 @@ out_common_hdmi_dev: out_common_hdmi: platform_driver_unregister(_driver); out_mixer: + exynos_hdmiphy_driver_unregister(); +out_hdmiphy: platform_driver_unregister(_driver); out_hdmi: #endif @@ -479,6 +486,7 @@ static void __exit exynos_drm_exit(void) exynos_platform_device_hdmi_unregister(); platform_driver_unregister(_drm_common_hdmi_driver); platform_driver_unregister(_driver); + exynos_hdmiphy_driver_unregister();
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On Mon, Mar 4, 2013 at 7:35 PM, Rahul Sharma wrote: > Thanks Sean, > > On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul wrote: >> On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma >> wrote: >>> Right now hdmiphy operations and configs are kept inside hdmi driver. >>> hdmiphy related >>> code is tightly coupled with hdmi ip driver. Physicaly they are different >>> devices and >> >> s/Physicaly/Physically/ >> >>> should be instantiated independently. >>> >>> In terms of versions/mapping/configurations Hdmi and hdmiphy are >>> independent of each >>> other. It is preferred to isolate them and maintained independently. >>> >>> This implementations is tested for: >>> 1) Resolutions supported by exynos4 and 5 hdmi. >>> 2) Runtime PM and S2R scenarions for exynos5. >>> >> >> I don't like the idea of spawning off yet another driver in here. It >> adds more globals, more suspend/resume ordering issues, and more >> implicit dependencies. I understand, however, that this is the Chosen >> Way for the exynos driver, so I will save my rant. >> > > I agree to it. splitting phy to a new driver will complicate the power related > scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of > config values, mapping (i2c/platform bus) etc. Handling this diversity > inside hdmi driver is complicating it with unrelated changes. > > I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock > we should re-factor this by explicitly calling power related callbacks > of mixer, phy, > hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf > device. AFAIR something like this is already in place in chrome-kernel. > >> I've made some comments below. >> >>> This patch is dependent on >>> http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34733.html >>> http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34861.html >>> http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34862.html >>> >>> Signed-off-by: Rahul Sharma >>> --- >>> It is based on exynos-drm-next-todo branch at >>> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >>> >>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + >>> drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + >>> drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- >>> drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + >>> drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- >>> drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - >>> drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 >>> ++- >>> drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 >>> 8 files changed, 738 insertions(+), 368 deletions(-) >>> create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> b/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> index 3da5c2d..03acb6b 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> @@ -334,6 +334,11 @@ static int __init exynos_drm_init(void) >>> ret = platform_driver_register(_driver); >>> if (ret < 0) >>> goto out_hdmi; >>> + >>> + ret = exynos_hdmiphy_driver_register(); >> >> Not sure why you need to obfuscate the type of driver here. All the >> other ones are directly registered as platform drivers, I don't see >> the harm in just doing the i2c_add_driver directly here. >> > > AIMB, Phy is likely to get mapped as a platform device in later soc's. > I want to make driver registration (inside exynos_drm_drv.c) independent > of this changes. We can handle this inside exynos_hdmiphy_driver_register(). > >>> + if (ret < 0) >>> + goto out_hdmiphy; >>> + >>> ret = platform_driver_register(_driver); >>> if (ret < 0) >>> goto out_mixer; >> >> >> I think this ordering is how you plan on enforcing the suspend/resume >> order for hdmi/mixer/hdmiphy. In that case, however, I don't think it >> makes sense to suspend/resume hdmiphy in between mixer and hdmi. >> Ideally, hdmiphy should go down first and come up last. >> > > I just wanted to keep, hdmi and hdmiphy things together. But what you said > above makes sense. I will do that change. > >> >>> @@ -436,6 +441,8 @@ out_common_hdmi_dev: >>> out_common_hdmi: >>> platform_driver_unregister(_driver); >>> out_mixer: >>> + exynos_hdmiphy_driver_unregister(); >>> +out_hdmiphy: >>> platform_driver_unregister(_driver); >>> out_hdmi: >>> #endif >>> @@ -479,6 +486,7 @@ static void __exit exynos_drm_exit(void) >>> exynos_platform_device_hdmi_unregister(); >>> platform_driver_unregister(_drm_common_hdmi_driver); >>> platform_driver_unregister(_driver); >>> + exynos_hdmiphy_driver_unregister(); >>> platform_driver_unregister(_driver); >>> #endif >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h >>> b/drivers/gpu/drm/exynos/exynos_drm_drv.h >>>
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On Thu, Mar 7, 2013 at 2:05 PM, Inki Dae wrote: > > >> -Original Message- >> From: ??? [mailto:sw0312.kim at samsung.com] >> Sent: Thursday, March 07, 2013 4:04 PM >> To: Rahul Sharma >> Cc: Inki Dae; linux-samsung-soc at vger.kernel.org; Sean Paul; sunil joshi; >> dri-devel at lists.freedesktop.org; Rahul Sharma; sw0312.kim at samsung.com >> Subject: Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to >> hdmiphy driver >> >> >> >> On 2013? 03? 07? 15:45, Rahul Sharma wrote: >> > Thanks Seung Woo, Mr. Dae, >> > >> > On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae wrote: >> >> 2013/3/7 ??? : >> >>> >> >>> >> >>> On 2013? 03? 04? 23:05, Rahul Sharma wrote: >> >>>> Thanks Sean, >> >>>> >> >>>> On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul >> wrote: >> >>>>> On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma >> wrote: >> >>>>>> Right now hdmiphy operations and configs are kept inside hdmi >> driver. hdmiphy related >> >>>>>> code is tightly coupled with hdmi ip driver. Physicaly they are >> different devices and >> >>>>> >> >>>>> s/Physicaly/Physically/ >> >>>>> >> >>>>>> should be instantiated independently. >> >>>>>> >> >>>>>> In terms of versions/mapping/configurations Hdmi and hdmiphy are >> independent of each >> >>>>>> other. It is preferred to isolate them and maintained > independently. >> >>>>>> >> >>>>>> This implementations is tested for: >> >>>>>> 1) Resolutions supported by exynos4 and 5 hdmi. >> >>>>>> 2) Runtime PM and S2R scenarions for exynos5. >> >>>>>> >> >>>>> >> >>>>> I don't like the idea of spawning off yet another driver in here. It >> >>>>> adds more globals, more suspend/resume ordering issues, and more >> >>>>> implicit dependencies. I understand, however, that this is the >> Chosen >> >>>>> Way for the exynos driver, so I will save my rant. >> >>>>> >> >>>> >> >>>> I agree to it. splitting phy to a new driver will complicate the >> power related >> >>>> scenarios. But in upcoming SoC,s, Phy is changing considerably in >> terms of >> >>>> config values, mapping (i2c/platform bus) etc. Handling this >> diversity >> >>>> inside hdmi driver is complicating it with unrelated changes. >> >>> >> >>> Basically, I agree with the idea to split hdmiphy from hdmi. And it >> >>> seems that already existing hdmiphy i2c device is just reused and >> >>> hdmiphy_power_on is reorganized to hdmiphy dpms operation: even >> calling >> >>> flow of power operations is reordered. >> >>> >> >>> But I'm not sure exynos_hdmiphy_driver_register() really need to be >> >>> called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough >> to >> >>> call exynos_hdmiphy_driver_register() from hdmi_probe() because >> hdmiphy >> >>> is only used from hdmi. >> >>> >> >> >> >> I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. >> >> >> > >> > I agree to the Seung Woo's point that hdmi-phy used to be solely >> accessed by >> > hdmi driver. But in this RFC, hdmi-phy is not called by hdmi driver >> > anymore. Phy >> > ops will be called from drm-common-hdmi platform driver along with mixer >> and >> > hdmi ops. >> >> Considering this, exynos_drm_hdmi_probe() is more proper position. >> >> > >> > The rational behind my implementation is that I am projecting hdmi-phy >> as >> > a device which is peer to hdmi ip and mixer. These 3 devices together >> makes >> > DRM HDMI subsystem. >> > >> > Even physically hdmi-phy doesn't seems to be a part of hdmi ip though >> > configurations are listed under hdmi ip user manual. It looks like a >> > isolated module accessed by i2c. >> > >> > Though I don't find anything wrong with Seung Woo suggestion but above >> > placement of hdmi-phy (parallel to hdmi and mixer) makes more sense >> > to me. >> > &
Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On Thu, Mar 7, 2013 at 2:05 PM, Inki Dae inki@samsung.com wrote: -Original Message- From: 김승우 [mailto:sw0312@samsung.com] Sent: Thursday, March 07, 2013 4:04 PM To: Rahul Sharma Cc: Inki Dae; linux-samsung-...@vger.kernel.org; Sean Paul; sunil joshi; dri-devel@lists.freedesktop.org; Rahul Sharma; sw0312@samsung.com Subject: Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver On 2013년 03월 07일 15:45, Rahul Sharma wrote: Thanks Seung Woo, Mr. Dae, On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae inki@samsung.com wrote: 2013/3/7 김승우 sw0312@samsung.com: On 2013년 03월 04일 23:05, Rahul Sharma wrote: Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com wrote: Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related code is tightly coupled with hdmi ip driver. Physicaly they are different devices and s/Physicaly/Physically/ should be instantiated independently. In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each other. It is preferred to isolate them and maintained independently. This implementations is tested for: 1) Resolutions supported by exynos4 and 5 hdmi. 2) Runtime PM and S2R scenarions for exynos5. I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. Basically, I agree with the idea to split hdmiphy from hdmi. And it seems that already existing hdmiphy i2c device is just reused and hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling flow of power operations is reordered. But I'm not sure exynos_hdmiphy_driver_register() really need to be called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy is only used from hdmi. I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. I agree to the Seung Woo's point that hdmi-phy used to be solely accessed by hdmi driver. But in this RFC, hdmi-phy is not called by hdmi driver anymore. Phy ops will be called from drm-common-hdmi platform driver along with mixer and hdmi ops. Considering this, exynos_drm_hdmi_probe() is more proper position. The rational behind my implementation is that I am projecting hdmi-phy as a device which is peer to hdmi ip and mixer. These 3 devices together makes DRM HDMI subsystem. Even physically hdmi-phy doesn't seems to be a part of hdmi ip though configurations are listed under hdmi ip user manual. It looks like a isolated module accessed by i2c. Though I don't find anything wrong with Seung Woo suggestion but above placement of hdmi-phy (parallel to hdmi and mixer) makes more sense to me. Please have a another look at it and let me know your opinion. Another things which bothers me is registering mixer, hdmi driver inside exynos_drm_init(). If we strictly follow the hierarchy inside drm, exynos_drm_init() should register drm-common-hdmi only. drm-common-hdmi should register mixer and hdmi (or hdmi-phy as well). Yes, it makes sense. All real hw blocks for hdmi including mixer, hdmi, and hdmiphy shoulde be registered in exynos_drm_hdmi (drm-common-hdmi for exynos). Ideally, right. But the reason I designed and implemented hdmi subsystem framework like this, is that there was one issue that platform_device_register() can't be called at probe(). On other words, when one platform device driver is being probed, anyone can't be probed. Anyway, existing way needs to be improved. So let's find better way and improve the hdmi subsystem framework this time. :) Mr. Dae, if we move exynos_drm_drv.o towards last in the makefile, we can conveniently move all driver(or plf device for drm-comon-hdmi) registrations from exynos_drm_init to module_init of the respective drivers. I verified above changes for hdmi and S2R scenarios. regards, Rahul Sharma Thanks, Inki Dae Thanks and Regards, - Seung-Woo Kim regards, Rahul Sharma. Thanks, Inki Dae Thanks and Regards, - Seung-Woo Kim I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm- hdmi
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
> -Original Message- > From: ??? [mailto:sw0312.kim at samsung.com] > Sent: Thursday, March 07, 2013 4:04 PM > To: Rahul Sharma > Cc: Inki Dae; linux-samsung-soc at vger.kernel.org; Sean Paul; sunil joshi; > dri-devel at lists.freedesktop.org; Rahul Sharma; sw0312.kim at samsung.com > Subject: Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to > hdmiphy driver > > > > On 2013? 03? 07? 15:45, Rahul Sharma wrote: > > Thanks Seung Woo, Mr. Dae, > > > > On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae wrote: > >> 2013/3/7 ??? : > >>> > >>> > >>> On 2013? 03? 04? 23:05, Rahul Sharma wrote: > >>>> Thanks Sean, > >>>> > >>>> On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul > wrote: > >>>>> On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma > wrote: > >>>>>> Right now hdmiphy operations and configs are kept inside hdmi > driver. hdmiphy related > >>>>>> code is tightly coupled with hdmi ip driver. Physicaly they are > different devices and > >>>>> > >>>>> s/Physicaly/Physically/ > >>>>> > >>>>>> should be instantiated independently. > >>>>>> > >>>>>> In terms of versions/mapping/configurations Hdmi and hdmiphy are > independent of each > >>>>>> other. It is preferred to isolate them and maintained independently. > >>>>>> > >>>>>> This implementations is tested for: > >>>>>> 1) Resolutions supported by exynos4 and 5 hdmi. > >>>>>> 2) Runtime PM and S2R scenarions for exynos5. > >>>>>> > >>>>> > >>>>> I don't like the idea of spawning off yet another driver in here. It > >>>>> adds more globals, more suspend/resume ordering issues, and more > >>>>> implicit dependencies. I understand, however, that this is the > Chosen > >>>>> Way for the exynos driver, so I will save my rant. > >>>>> > >>>> > >>>> I agree to it. splitting phy to a new driver will complicate the > power related > >>>> scenarios. But in upcoming SoC,s, Phy is changing considerably in > terms of > >>>> config values, mapping (i2c/platform bus) etc. Handling this > diversity > >>>> inside hdmi driver is complicating it with unrelated changes. > >>> > >>> Basically, I agree with the idea to split hdmiphy from hdmi. And it > >>> seems that already existing hdmiphy i2c device is just reused and > >>> hdmiphy_power_on is reorganized to hdmiphy dpms operation: even > calling > >>> flow of power operations is reordered. > >>> > >>> But I'm not sure exynos_hdmiphy_driver_register() really need to be > >>> called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough > to > >>> call exynos_hdmiphy_driver_register() from hdmi_probe() because > hdmiphy > >>> is only used from hdmi. > >>> > >> > >> I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. > >> > > > > I agree to the Seung Woo's point that hdmi-phy used to be solely > accessed by > > hdmi driver. But in this RFC, hdmi-phy is not called by hdmi driver > > anymore. Phy > > ops will be called from drm-common-hdmi platform driver along with mixer > and > > hdmi ops. > > Considering this, exynos_drm_hdmi_probe() is more proper position. > > > > > The rational behind my implementation is that I am projecting hdmi-phy > as > > a device which is peer to hdmi ip and mixer. These 3 devices together > makes > > DRM HDMI subsystem. > > > > Even physically hdmi-phy doesn't seems to be a part of hdmi ip though > > configurations are listed under hdmi ip user manual. It looks like a > > isolated module accessed by i2c. > > > > Though I don't find anything wrong with Seung Woo suggestion but above > > placement of hdmi-phy (parallel to hdmi and mixer) makes more sense > > to me. > > > > Please have a another look at it and let me know your opinion. > > > > Another things which bothers me is registering mixer, hdmi driver inside > > exynos_drm_init(). If we strictly follow the hierarchy inside drm, > > exynos_drm_init() > > should register drm-common-hdmi only. drm-common-hdmi should register > > mixer and hdmi (or hdmi-phy as well). > > Yes, it makes sense. All real hw blocks for hdmi
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On 2013? 03? 07? 15:45, Rahul Sharma wrote: > Thanks Seung Woo, Mr. Dae, > > On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae wrote: >> 2013/3/7 ??? : >>> >>> >>> On 2013? 03? 04? 23:05, Rahul Sharma wrote: Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul wrote: > On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma samsung.com> wrote: >> Right now hdmiphy operations and configs are kept inside hdmi driver. >> hdmiphy related >> code is tightly coupled with hdmi ip driver. Physicaly they are >> different devices and > > s/Physicaly/Physically/ > >> should be instantiated independently. >> >> In terms of versions/mapping/configurations Hdmi and hdmiphy are >> independent of each >> other. It is preferred to isolate them and maintained independently. >> >> This implementations is tested for: >> 1) Resolutions supported by exynos4 and 5 hdmi. >> 2) Runtime PM and S2R scenarions for exynos5. >> > > I don't like the idea of spawning off yet another driver in here. It > adds more globals, more suspend/resume ordering issues, and more > implicit dependencies. I understand, however, that this is the Chosen > Way for the exynos driver, so I will save my rant. > I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. >>> >>> Basically, I agree with the idea to split hdmiphy from hdmi. And it >>> seems that already existing hdmiphy i2c device is just reused and >>> hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling >>> flow of power operations is reordered. >>> >>> But I'm not sure exynos_hdmiphy_driver_register() really need to be >>> called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to >>> call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy >>> is only used from hdmi. >>> >> >> I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. >> > > I agree to the Seung Woo's point that hdmi-phy used to be solely accessed by > hdmi driver. But in this RFC, hdmi-phy is not called by hdmi driver > anymore. Phy > ops will be called from drm-common-hdmi platform driver along with mixer and > hdmi ops. Considering this, exynos_drm_hdmi_probe() is more proper position. > > The rational behind my implementation is that I am projecting hdmi-phy as > a device which is peer to hdmi ip and mixer. These 3 devices together makes > DRM HDMI subsystem. > > Even physically hdmi-phy doesn't seems to be a part of hdmi ip though > configurations are listed under hdmi ip user manual. It looks like a > isolated module accessed by i2c. > > Though I don't find anything wrong with Seung Woo suggestion but above > placement of hdmi-phy (parallel to hdmi and mixer) makes more sense > to me. > > Please have a another look at it and let me know your opinion. > > Another things which bothers me is registering mixer, hdmi driver inside > exynos_drm_init(). If we strictly follow the hierarchy inside drm, > exynos_drm_init() > should register drm-common-hdmi only. drm-common-hdmi should register > mixer and hdmi (or hdmi-phy as well). Yes, it makes sense. All real hw blocks for hdmi including mixer, hdmi, and hdmiphy shoulde be registered in exynos_drm_hdmi (drm-common-hdmi for exynos). Thanks and Regards, - Seung-Woo Kim > > regards, > Rahul Sharma. > >> Thanks, >> Inki Dae >> >>> Thanks and Regards, >>> - Seung-Woo Kim >>> I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf device. AFAIR something like this is already in place in chrome-kernel. > I've made some comments below. > >> This patch is dependent on >> http://www.mail-archive.com/dri-devel at >> lists.freedesktop.org/msg34733.html >> http://www.mail-archive.com/dri-devel at >> lists.freedesktop.org/msg34861.html >> http://www.mail-archive.com/dri-devel at >> lists.freedesktop.org/msg34862.html >> >> Signed-off-by: Rahul Sharma >> --- >> It is based on exynos-drm-next-todo branch at >> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >> >> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + >> drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + >> drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- >> drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + >> drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- >> drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - >>
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On Thu, Mar 7, 2013 at 12:37 PM, St?phane Marchesin wrote: > On Wed, Mar 6, 2013 at 8:43 PM, Inki Dae wrote: >> 2013/3/7 ??? : >>> >>> >>> On 2013? 03? 04? 23:05, Rahul Sharma wrote: Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul wrote: > On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma samsung.com> wrote: >> Right now hdmiphy operations and configs are kept inside hdmi driver. >> hdmiphy related >> code is tightly coupled with hdmi ip driver. Physicaly they are >> different devices and > > s/Physicaly/Physically/ > >> should be instantiated independently. >> >> In terms of versions/mapping/configurations Hdmi and hdmiphy are >> independent of each >> other. It is preferred to isolate them and maintained independently. >> >> This implementations is tested for: >> 1) Resolutions supported by exynos4 and 5 hdmi. >> 2) Runtime PM and S2R scenarions for exynos5. >> > > I don't like the idea of spawning off yet another driver in here. It > adds more globals, more suspend/resume ordering issues, and more > implicit dependencies. I understand, however, that this is the Chosen > Way for the exynos driver, so I will save my rant. > I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. >>> >>> Basically, I agree with the idea to split hdmiphy from hdmi. And it >>> seems that already existing hdmiphy i2c device is just reused and >>> hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling >>> flow of power operations is reordered. >>> >>> But I'm not sure exynos_hdmiphy_driver_register() really need to be >>> called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to >>> call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy >>> is only used from hdmi. >>> >> >> I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. >> > > But this is probably going to break dpms and/or suspend/resume > functionality. Has that been tested? > > St?phane > Hi St?phane This has been tested for dpms and suspend/resume scenarios for exynos5. I have yet to try with hdmi, mixer, phy driver registration moved to common-drm-hdmi. regards, Rahul Sharma. >> Thanks, >> Inki Dae >> >>> Thanks and Regards, >>> - Seung-Woo Kim >>> I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf device. AFAIR something like this is already in place in chrome-kernel. > I've made some comments below. > >> This patch is dependent on >> http://www.mail-archive.com/dri-devel at >> lists.freedesktop.org/msg34733.html >> http://www.mail-archive.com/dri-devel at >> lists.freedesktop.org/msg34861.html >> http://www.mail-archive.com/dri-devel at >> lists.freedesktop.org/msg34862.html >> >> Signed-off-by: Rahul Sharma >> --- >> It is based on exynos-drm-next-todo branch at >> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >> >> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + >> drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + >> drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- >> drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + >> drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- >> drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - >> drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 >> ++- >> drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 >> 8 files changed, 738 insertions(+), 368 deletions(-) >> create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h >> >>> >>> >>> >>> -- >>> Seung-Woo Kim >>> Samsung Software R Center >>> -- >>> >>> ___ >>> dri-devel mailing list >>> dri-devel at lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Regards, Rahul Sharma, email to: rahul.sharma at samsung.com Samsung India.
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
2013/3/7 ??? : > > > On 2013? 03? 04? 23:05, Rahul Sharma wrote: >> Thanks Sean, >> >> On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul wrote: >>> On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma >>> wrote: Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related code is tightly coupled with hdmi ip driver. Physicaly they are different devices and >>> >>> s/Physicaly/Physically/ >>> should be instantiated independently. In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each other. It is preferred to isolate them and maintained independently. This implementations is tested for: 1) Resolutions supported by exynos4 and 5 hdmi. 2) Runtime PM and S2R scenarions for exynos5. >>> >>> I don't like the idea of spawning off yet another driver in here. It >>> adds more globals, more suspend/resume ordering issues, and more >>> implicit dependencies. I understand, however, that this is the Chosen >>> Way for the exynos driver, so I will save my rant. >>> >> >> I agree to it. splitting phy to a new driver will complicate the power >> related >> scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of >> config values, mapping (i2c/platform bus) etc. Handling this diversity >> inside hdmi driver is complicating it with unrelated changes. > > Basically, I agree with the idea to split hdmiphy from hdmi. And it > seems that already existing hdmiphy i2c device is just reused and > hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling > flow of power operations is reordered. > > But I'm not sure exynos_hdmiphy_driver_register() really need to be > called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to > call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy > is only used from hdmi. > I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. Thanks, Inki Dae > Thanks and Regards, > - Seung-Woo Kim > >> >> I have tested this RFC for Runtime PM / S2R. But if we see any major >> roadblock >> we should re-factor this by explicitly calling power related callbacks >> of mixer, phy, >> hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf >> device. AFAIR something like this is already in place in chrome-kernel. >> >>> I've made some comments below. >>> This patch is dependent on http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34733.html http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34861.html http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34862.html Signed-off-by: Rahul Sharma --- It is based on exynos-drm-next-todo branch at git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++- drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 8 files changed, 738 insertions(+), 368 deletions(-) create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h > > > > -- > Seung-Woo Kim > Samsung Software R Center > -- > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
Thanks Seung Woo, Mr. Dae, On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae wrote: > 2013/3/7 ??? : >> >> >> On 2013? 03? 04? 23:05, Rahul Sharma wrote: >>> Thanks Sean, >>> >>> On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma >>> samsung.com> wrote: > Right now hdmiphy operations and configs are kept inside hdmi driver. > hdmiphy related > code is tightly coupled with hdmi ip driver. Physicaly they are different > devices and s/Physicaly/Physically/ > should be instantiated independently. > > In terms of versions/mapping/configurations Hdmi and hdmiphy are > independent of each > other. It is preferred to isolate them and maintained independently. > > This implementations is tested for: > 1) Resolutions supported by exynos4 and 5 hdmi. > 2) Runtime PM and S2R scenarions for exynos5. > I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. >>> >>> I agree to it. splitting phy to a new driver will complicate the power >>> related >>> scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of >>> config values, mapping (i2c/platform bus) etc. Handling this diversity >>> inside hdmi driver is complicating it with unrelated changes. >> >> Basically, I agree with the idea to split hdmiphy from hdmi. And it >> seems that already existing hdmiphy i2c device is just reused and >> hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling >> flow of power operations is reordered. >> >> But I'm not sure exynos_hdmiphy_driver_register() really need to be >> called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to >> call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy >> is only used from hdmi. >> > > I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. > I agree to the Seung Woo's point that hdmi-phy used to be solely accessed by hdmi driver. But in this RFC, hdmi-phy is not called by hdmi driver anymore. Phy ops will be called from drm-common-hdmi platform driver along with mixer and hdmi ops. The rational behind my implementation is that I am projecting hdmi-phy as a device which is peer to hdmi ip and mixer. These 3 devices together makes DRM HDMI subsystem. Even physically hdmi-phy doesn't seems to be a part of hdmi ip though configurations are listed under hdmi ip user manual. It looks like a isolated module accessed by i2c. Though I don't find anything wrong with Seung Woo suggestion but above placement of hdmi-phy (parallel to hdmi and mixer) makes more sense to me. Please have a another look at it and let me know your opinion. Another things which bothers me is registering mixer, hdmi driver inside exynos_drm_init(). If we strictly follow the hierarchy inside drm, exynos_drm_init() should register drm-common-hdmi only. drm-common-hdmi should register mixer and hdmi (or hdmi-phy as well). regards, Rahul Sharma. > Thanks, > Inki Dae > >> Thanks and Regards, >> - Seung-Woo Kim >> >>> >>> I have tested this RFC for Runtime PM / S2R. But if we see any major >>> roadblock >>> we should re-factor this by explicitly calling power related callbacks >>> of mixer, phy, >>> hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf >>> device. AFAIR something like this is already in place in chrome-kernel. >>> I've made some comments below. > This patch is dependent on > http://www.mail-archive.com/dri-devel at > lists.freedesktop.org/msg34733.html > http://www.mail-archive.com/dri-devel at > lists.freedesktop.org/msg34861.html > http://www.mail-archive.com/dri-devel at > lists.freedesktop.org/msg34862.html > > Signed-off-by: Rahul Sharma > --- > It is based on exynos-drm-next-todo branch at > git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git > > drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + > drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + > drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- > drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + > drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- > drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - > drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 > ++- > drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 > 8 files changed, 738 insertions(+), 368 deletions(-) > create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h > >> >> >> >> -- >> Seung-Woo Kim >> Samsung Software R Center >> -- >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >>
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On 2013? 03? 04? 23:05, Rahul Sharma wrote: > Thanks Sean, > > On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul wrote: >> On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma >> wrote: >>> Right now hdmiphy operations and configs are kept inside hdmi driver. >>> hdmiphy related >>> code is tightly coupled with hdmi ip driver. Physicaly they are different >>> devices and >> >> s/Physicaly/Physically/ >> >>> should be instantiated independently. >>> >>> In terms of versions/mapping/configurations Hdmi and hdmiphy are >>> independent of each >>> other. It is preferred to isolate them and maintained independently. >>> >>> This implementations is tested for: >>> 1) Resolutions supported by exynos4 and 5 hdmi. >>> 2) Runtime PM and S2R scenarions for exynos5. >>> >> >> I don't like the idea of spawning off yet another driver in here. It >> adds more globals, more suspend/resume ordering issues, and more >> implicit dependencies. I understand, however, that this is the Chosen >> Way for the exynos driver, so I will save my rant. >> > > I agree to it. splitting phy to a new driver will complicate the power related > scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of > config values, mapping (i2c/platform bus) etc. Handling this diversity > inside hdmi driver is complicating it with unrelated changes. Basically, I agree with the idea to split hdmiphy from hdmi. And it seems that already existing hdmiphy i2c device is just reused and hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling flow of power operations is reordered. But I'm not sure exynos_hdmiphy_driver_register() really need to be called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy is only used from hdmi. Thanks and Regards, - Seung-Woo Kim > > I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock > we should re-factor this by explicitly calling power related callbacks > of mixer, phy, > hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf > device. AFAIR something like this is already in place in chrome-kernel. > >> I've made some comments below. >> >>> This patch is dependent on >>> http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34733.html >>> http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34861.html >>> http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34862.html >>> >>> Signed-off-by: Rahul Sharma >>> --- >>> It is based on exynos-drm-next-todo branch at >>> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >>> >>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + >>> drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + >>> drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- >>> drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + >>> drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- >>> drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - >>> drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 >>> ++- >>> drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 >>> 8 files changed, 738 insertions(+), 368 deletions(-) >>> create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h >>> -- Seung-Woo Kim Samsung Software R Center --
RE: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
-Original Message- From: 김승우 [mailto:sw0312@samsung.com] Sent: Thursday, March 07, 2013 4:04 PM To: Rahul Sharma Cc: Inki Dae; linux-samsung-...@vger.kernel.org; Sean Paul; sunil joshi; dri-devel@lists.freedesktop.org; Rahul Sharma; sw0312@samsung.com Subject: Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver On 2013년 03월 07일 15:45, Rahul Sharma wrote: Thanks Seung Woo, Mr. Dae, On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae inki@samsung.com wrote: 2013/3/7 김승우 sw0312@samsung.com: On 2013년 03월 04일 23:05, Rahul Sharma wrote: Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com wrote: Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related code is tightly coupled with hdmi ip driver. Physicaly they are different devices and s/Physicaly/Physically/ should be instantiated independently. In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each other. It is preferred to isolate them and maintained independently. This implementations is tested for: 1) Resolutions supported by exynos4 and 5 hdmi. 2) Runtime PM and S2R scenarions for exynos5. I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. Basically, I agree with the idea to split hdmiphy from hdmi. And it seems that already existing hdmiphy i2c device is just reused and hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling flow of power operations is reordered. But I'm not sure exynos_hdmiphy_driver_register() really need to be called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy is only used from hdmi. I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. I agree to the Seung Woo's point that hdmi-phy used to be solely accessed by hdmi driver. But in this RFC, hdmi-phy is not called by hdmi driver anymore. Phy ops will be called from drm-common-hdmi platform driver along with mixer and hdmi ops. Considering this, exynos_drm_hdmi_probe() is more proper position. The rational behind my implementation is that I am projecting hdmi-phy as a device which is peer to hdmi ip and mixer. These 3 devices together makes DRM HDMI subsystem. Even physically hdmi-phy doesn't seems to be a part of hdmi ip though configurations are listed under hdmi ip user manual. It looks like a isolated module accessed by i2c. Though I don't find anything wrong with Seung Woo suggestion but above placement of hdmi-phy (parallel to hdmi and mixer) makes more sense to me. Please have a another look at it and let me know your opinion. Another things which bothers me is registering mixer, hdmi driver inside exynos_drm_init(). If we strictly follow the hierarchy inside drm, exynos_drm_init() should register drm-common-hdmi only. drm-common-hdmi should register mixer and hdmi (or hdmi-phy as well). Yes, it makes sense. All real hw blocks for hdmi including mixer, hdmi, and hdmiphy shoulde be registered in exynos_drm_hdmi (drm-common-hdmi for exynos). Ideally, right. But the reason I designed and implemented hdmi subsystem framework like this, is that there was one issue that platform_device_register() can't be called at probe(). On other words, when one platform device driver is being probed, anyone can't be probed. Anyway, existing way needs to be improved. So let's find better way and improve the hdmi subsystem framework this time. :) Thanks, Inki Dae Thanks and Regards, - Seung-Woo Kim regards, Rahul Sharma. Thanks, Inki Dae Thanks and Regards, - Seung-Woo Kim I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm- hdmi plf device. AFAIR something like this is already in place in chrome- kernel. I've made some comments below. This patch is dependent on http://www.mail-archive.com/dri- de...@lists.freedesktop.org/msg34733.html http://www.mail-archive.com/dri- de...@lists.freedesktop.org/msg34861.html http://www.mail-archive.com/dri- de
Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On Thu, Mar 7, 2013 at 12:37 PM, Stéphane Marchesin stephane.marche...@gmail.com wrote: On Wed, Mar 6, 2013 at 8:43 PM, Inki Dae inki@samsung.com wrote: 2013/3/7 김승우 sw0312@samsung.com: On 2013년 03월 04일 23:05, Rahul Sharma wrote: Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com wrote: Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related code is tightly coupled with hdmi ip driver. Physicaly they are different devices and s/Physicaly/Physically/ should be instantiated independently. In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each other. It is preferred to isolate them and maintained independently. This implementations is tested for: 1) Resolutions supported by exynos4 and 5 hdmi. 2) Runtime PM and S2R scenarions for exynos5. I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. Basically, I agree with the idea to split hdmiphy from hdmi. And it seems that already existing hdmiphy i2c device is just reused and hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling flow of power operations is reordered. But I'm not sure exynos_hdmiphy_driver_register() really need to be called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy is only used from hdmi. I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. But this is probably going to break dpms and/or suspend/resume functionality. Has that been tested? Stéphane Hi Stéphane This has been tested for dpms and suspend/resume scenarios for exynos5. I have yet to try with hdmi, mixer, phy driver registration moved to common-drm-hdmi. regards, Rahul Sharma. Thanks, Inki Dae Thanks and Regards, - Seung-Woo Kim I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf device. AFAIR something like this is already in place in chrome-kernel. I've made some comments below. This patch is dependent on http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html Signed-off-by: Rahul Sharma rahul.sha...@samsung.com --- It is based on exynos-drm-next-todo branch at git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++- drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 8 files changed, 738 insertions(+), 368 deletions(-) create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h snip -- Seung-Woo Kim Samsung Software RD Center -- ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Regards, Rahul Sharma, email to: rahul.sha...@samsung.com Samsung India. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On Wed, Mar 6, 2013 at 8:43 PM, Inki Dae wrote: > 2013/3/7 ??? : >> >> >> On 2013? 03? 04? 23:05, Rahul Sharma wrote: >>> Thanks Sean, >>> >>> On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma >>> samsung.com> wrote: > Right now hdmiphy operations and configs are kept inside hdmi driver. > hdmiphy related > code is tightly coupled with hdmi ip driver. Physicaly they are different > devices and s/Physicaly/Physically/ > should be instantiated independently. > > In terms of versions/mapping/configurations Hdmi and hdmiphy are > independent of each > other. It is preferred to isolate them and maintained independently. > > This implementations is tested for: > 1) Resolutions supported by exynos4 and 5 hdmi. > 2) Runtime PM and S2R scenarions for exynos5. > I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. >>> >>> I agree to it. splitting phy to a new driver will complicate the power >>> related >>> scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of >>> config values, mapping (i2c/platform bus) etc. Handling this diversity >>> inside hdmi driver is complicating it with unrelated changes. >> >> Basically, I agree with the idea to split hdmiphy from hdmi. And it >> seems that already existing hdmiphy i2c device is just reused and >> hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling >> flow of power operations is reordered. >> >> But I'm not sure exynos_hdmiphy_driver_register() really need to be >> called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to >> call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy >> is only used from hdmi. >> > > I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. > But this is probably going to break dpms and/or suspend/resume functionality. Has that been tested? St?phane > Thanks, > Inki Dae > >> Thanks and Regards, >> - Seung-Woo Kim >> >>> >>> I have tested this RFC for Runtime PM / S2R. But if we see any major >>> roadblock >>> we should re-factor this by explicitly calling power related callbacks >>> of mixer, phy, >>> hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf >>> device. AFAIR something like this is already in place in chrome-kernel. >>> I've made some comments below. > This patch is dependent on > http://www.mail-archive.com/dri-devel at > lists.freedesktop.org/msg34733.html > http://www.mail-archive.com/dri-devel at > lists.freedesktop.org/msg34861.html > http://www.mail-archive.com/dri-devel at > lists.freedesktop.org/msg34862.html > > Signed-off-by: Rahul Sharma > --- > It is based on exynos-drm-next-todo branch at > git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git > > drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + > drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + > drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- > drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + > drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- > drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - > drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 > ++- > drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 > 8 files changed, 738 insertions(+), 368 deletions(-) > create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h > >> >> >> >> -- >> Seung-Woo Kim >> Samsung Software R Center >> -- >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On 2013년 03월 04일 23:05, Rahul Sharma wrote: Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com wrote: Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related code is tightly coupled with hdmi ip driver. Physicaly they are different devices and s/Physicaly/Physically/ should be instantiated independently. In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each other. It is preferred to isolate them and maintained independently. This implementations is tested for: 1) Resolutions supported by exynos4 and 5 hdmi. 2) Runtime PM and S2R scenarions for exynos5. I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. Basically, I agree with the idea to split hdmiphy from hdmi. And it seems that already existing hdmiphy i2c device is just reused and hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling flow of power operations is reordered. But I'm not sure exynos_hdmiphy_driver_register() really need to be called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy is only used from hdmi. Thanks and Regards, - Seung-Woo Kim I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf device. AFAIR something like this is already in place in chrome-kernel. I've made some comments below. This patch is dependent on http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html Signed-off-by: Rahul Sharma rahul.sha...@samsung.com --- It is based on exynos-drm-next-todo branch at git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++- drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 8 files changed, 738 insertions(+), 368 deletions(-) create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h snip -- Seung-Woo Kim Samsung Software RD Center -- ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
2013/3/7 김승우 sw0312@samsung.com: On 2013년 03월 04일 23:05, Rahul Sharma wrote: Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com wrote: Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related code is tightly coupled with hdmi ip driver. Physicaly they are different devices and s/Physicaly/Physically/ should be instantiated independently. In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each other. It is preferred to isolate them and maintained independently. This implementations is tested for: 1) Resolutions supported by exynos4 and 5 hdmi. 2) Runtime PM and S2R scenarions for exynos5. I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. Basically, I agree with the idea to split hdmiphy from hdmi. And it seems that already existing hdmiphy i2c device is just reused and hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling flow of power operations is reordered. But I'm not sure exynos_hdmiphy_driver_register() really need to be called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy is only used from hdmi. I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. Thanks, Inki Dae Thanks and Regards, - Seung-Woo Kim I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf device. AFAIR something like this is already in place in chrome-kernel. I've made some comments below. This patch is dependent on http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html Signed-off-by: Rahul Sharma rahul.sha...@samsung.com --- It is based on exynos-drm-next-todo branch at git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++- drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 8 files changed, 738 insertions(+), 368 deletions(-) create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h snip -- Seung-Woo Kim Samsung Software RD Center -- ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
Thanks Seung Woo, Mr. Dae, On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae inki@samsung.com wrote: 2013/3/7 김승우 sw0312@samsung.com: On 2013년 03월 04일 23:05, Rahul Sharma wrote: Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com wrote: Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related code is tightly coupled with hdmi ip driver. Physicaly they are different devices and s/Physicaly/Physically/ should be instantiated independently. In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each other. It is preferred to isolate them and maintained independently. This implementations is tested for: 1) Resolutions supported by exynos4 and 5 hdmi. 2) Runtime PM and S2R scenarions for exynos5. I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. Basically, I agree with the idea to split hdmiphy from hdmi. And it seems that already existing hdmiphy i2c device is just reused and hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling flow of power operations is reordered. But I'm not sure exynos_hdmiphy_driver_register() really need to be called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy is only used from hdmi. I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. I agree to the Seung Woo's point that hdmi-phy used to be solely accessed by hdmi driver. But in this RFC, hdmi-phy is not called by hdmi driver anymore. Phy ops will be called from drm-common-hdmi platform driver along with mixer and hdmi ops. The rational behind my implementation is that I am projecting hdmi-phy as a device which is peer to hdmi ip and mixer. These 3 devices together makes DRM HDMI subsystem. Even physically hdmi-phy doesn't seems to be a part of hdmi ip though configurations are listed under hdmi ip user manual. It looks like a isolated module accessed by i2c. Though I don't find anything wrong with Seung Woo suggestion but above placement of hdmi-phy (parallel to hdmi and mixer) makes more sense to me. Please have a another look at it and let me know your opinion. Another things which bothers me is registering mixer, hdmi driver inside exynos_drm_init(). If we strictly follow the hierarchy inside drm, exynos_drm_init() should register drm-common-hdmi only. drm-common-hdmi should register mixer and hdmi (or hdmi-phy as well). regards, Rahul Sharma. Thanks, Inki Dae Thanks and Regards, - Seung-Woo Kim I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf device. AFAIR something like this is already in place in chrome-kernel. I've made some comments below. This patch is dependent on http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html Signed-off-by: Rahul Sharma rahul.sha...@samsung.com --- It is based on exynos-drm-next-todo branch at git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++- drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 8 files changed, 738 insertions(+), 368 deletions(-) create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h snip -- Seung-Woo Kim Samsung Software RD Center -- ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Regards, Rahul Sharma, email to: rahul.sha...@samsung.com Samsung India. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On 2013년 03월 07일 15:45, Rahul Sharma wrote: Thanks Seung Woo, Mr. Dae, On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae inki@samsung.com wrote: 2013/3/7 김승우 sw0312@samsung.com: On 2013년 03월 04일 23:05, Rahul Sharma wrote: Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com wrote: Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related code is tightly coupled with hdmi ip driver. Physicaly they are different devices and s/Physicaly/Physically/ should be instantiated independently. In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each other. It is preferred to isolate them and maintained independently. This implementations is tested for: 1) Resolutions supported by exynos4 and 5 hdmi. 2) Runtime PM and S2R scenarions for exynos5. I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. Basically, I agree with the idea to split hdmiphy from hdmi. And it seems that already existing hdmiphy i2c device is just reused and hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling flow of power operations is reordered. But I'm not sure exynos_hdmiphy_driver_register() really need to be called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy is only used from hdmi. I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. I agree to the Seung Woo's point that hdmi-phy used to be solely accessed by hdmi driver. But in this RFC, hdmi-phy is not called by hdmi driver anymore. Phy ops will be called from drm-common-hdmi platform driver along with mixer and hdmi ops. Considering this, exynos_drm_hdmi_probe() is more proper position. The rational behind my implementation is that I am projecting hdmi-phy as a device which is peer to hdmi ip and mixer. These 3 devices together makes DRM HDMI subsystem. Even physically hdmi-phy doesn't seems to be a part of hdmi ip though configurations are listed under hdmi ip user manual. It looks like a isolated module accessed by i2c. Though I don't find anything wrong with Seung Woo suggestion but above placement of hdmi-phy (parallel to hdmi and mixer) makes more sense to me. Please have a another look at it and let me know your opinion. Another things which bothers me is registering mixer, hdmi driver inside exynos_drm_init(). If we strictly follow the hierarchy inside drm, exynos_drm_init() should register drm-common-hdmi only. drm-common-hdmi should register mixer and hdmi (or hdmi-phy as well). Yes, it makes sense. All real hw blocks for hdmi including mixer, hdmi, and hdmiphy shoulde be registered in exynos_drm_hdmi (drm-common-hdmi for exynos). Thanks and Regards, - Seung-Woo Kim regards, Rahul Sharma. Thanks, Inki Dae Thanks and Regards, - Seung-Woo Kim I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf device. AFAIR something like this is already in place in chrome-kernel. I've made some comments below. This patch is dependent on http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html Signed-off-by: Rahul Sharma rahul.sha...@samsung.com --- It is based on exynos-drm-next-todo branch at git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++- drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 8 files changed, 738 insertions(+), 368 deletions(-) create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h snip -- Seung-Woo Kim Samsung Software RD Center -- ___ dri-devel mailing
Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On Wed, Mar 6, 2013 at 8:43 PM, Inki Dae inki@samsung.com wrote: 2013/3/7 김승우 sw0312@samsung.com: On 2013년 03월 04일 23:05, Rahul Sharma wrote: Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote: On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com wrote: Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related code is tightly coupled with hdmi ip driver. Physicaly they are different devices and s/Physicaly/Physically/ should be instantiated independently. In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each other. It is preferred to isolate them and maintained independently. This implementations is tested for: 1) Resolutions supported by exynos4 and 5 hdmi. 2) Runtime PM and S2R scenarions for exynos5. I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. Basically, I agree with the idea to split hdmiphy from hdmi. And it seems that already existing hdmiphy i2c device is just reused and hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling flow of power operations is reordered. But I'm not sure exynos_hdmiphy_driver_register() really need to be called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy is only used from hdmi. I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem. But this is probably going to break dpms and/or suspend/resume functionality. Has that been tested? Stéphane Thanks, Inki Dae Thanks and Regards, - Seung-Woo Kim I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf device. AFAIR something like this is already in place in chrome-kernel. I've made some comments below. This patch is dependent on http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html Signed-off-by: Rahul Sharma rahul.sha...@samsung.com --- It is based on exynos-drm-next-todo branch at git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++- drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 8 files changed, 738 insertions(+), 368 deletions(-) create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h snip -- Seung-Woo Kim Samsung Software RD Center -- ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
Thanks Sean, On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul wrote: > On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma > wrote: >> Right now hdmiphy operations and configs are kept inside hdmi driver. >> hdmiphy related >> code is tightly coupled with hdmi ip driver. Physicaly they are different >> devices and > > s/Physicaly/Physically/ > >> should be instantiated independently. >> >> In terms of versions/mapping/configurations Hdmi and hdmiphy are independent >> of each >> other. It is preferred to isolate them and maintained independently. >> >> This implementations is tested for: >> 1) Resolutions supported by exynos4 and 5 hdmi. >> 2) Runtime PM and S2R scenarions for exynos5. >> > > I don't like the idea of spawning off yet another driver in here. It > adds more globals, more suspend/resume ordering issues, and more > implicit dependencies. I understand, however, that this is the Chosen > Way for the exynos driver, so I will save my rant. > I agree to it. splitting phy to a new driver will complicate the power related scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of config values, mapping (i2c/platform bus) etc. Handling this diversity inside hdmi driver is complicating it with unrelated changes. I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock we should re-factor this by explicitly calling power related callbacks of mixer, phy, hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf device. AFAIR something like this is already in place in chrome-kernel. > I've made some comments below. > >> This patch is dependent on >> http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34733.html >> http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34861.html >> http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34862.html >> >> Signed-off-by: Rahul Sharma >> --- >> It is based on exynos-drm-next-todo branch at >> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >> >> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + >> drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + >> drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- >> drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + >> drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- >> drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - >> drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 >> ++- >> drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 >> 8 files changed, 738 insertions(+), 368 deletions(-) >> create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> index 3da5c2d..03acb6b 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> @@ -334,6 +334,11 @@ static int __init exynos_drm_init(void) >> ret = platform_driver_register(_driver); >> if (ret < 0) >> goto out_hdmi; >> + >> + ret = exynos_hdmiphy_driver_register(); > > Not sure why you need to obfuscate the type of driver here. All the > other ones are directly registered as platform drivers, I don't see > the harm in just doing the i2c_add_driver directly here. > AIMB, Phy is likely to get mapped as a platform device in later soc's. I want to make driver registration (inside exynos_drm_drv.c) independent of this changes. We can handle this inside exynos_hdmiphy_driver_register(). >> + if (ret < 0) >> + goto out_hdmiphy; >> + >> ret = platform_driver_register(_driver); >> if (ret < 0) >> goto out_mixer; > > > I think this ordering is how you plan on enforcing the suspend/resume > order for hdmi/mixer/hdmiphy. In that case, however, I don't think it > makes sense to suspend/resume hdmiphy in between mixer and hdmi. > Ideally, hdmiphy should go down first and come up last. > I just wanted to keep, hdmi and hdmiphy things together. But what you said above makes sense. I will do that change. > >> @@ -436,6 +441,8 @@ out_common_hdmi_dev: >> out_common_hdmi: >> platform_driver_unregister(_driver); >> out_mixer: >> + exynos_hdmiphy_driver_unregister(); >> +out_hdmiphy: >> platform_driver_unregister(_driver); >> out_hdmi: >> #endif >> @@ -479,6 +486,7 @@ static void __exit exynos_drm_exit(void) >> exynos_platform_device_hdmi_unregister(); >> platform_driver_unregister(_drm_common_hdmi_driver); >> platform_driver_unregister(_driver); >> + exynos_hdmiphy_driver_unregister(); >> platform_driver_unregister(_driver); >> #endif >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h >> b/drivers/gpu/drm/exynos/exynos_drm_drv.h >> index 4606fac..17c53e3 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h >> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h >> @@ -325,6 +325,12 @@ void exynos_drm_subdrv_close(struct
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma wrote: > Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy > related > code is tightly coupled with hdmi ip driver. Physicaly they are different > devices and s/Physicaly/Physically/ > should be instantiated independently. > > In terms of versions/mapping/configurations Hdmi and hdmiphy are independent > of each > other. It is preferred to isolate them and maintained independently. > > This implementations is tested for: > 1) Resolutions supported by exynos4 and 5 hdmi. > 2) Runtime PM and S2R scenarions for exynos5. > I don't like the idea of spawning off yet another driver in here. It adds more globals, more suspend/resume ordering issues, and more implicit dependencies. I understand, however, that this is the Chosen Way for the exynos driver, so I will save my rant. I've made some comments below. > This patch is dependent on > http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34733.html > http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34861.html > http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34862.html > > Signed-off-by: Rahul Sharma > --- > It is based on exynos-drm-next-todo branch at > git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git > > drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + > drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + > drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- > drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + > drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++-- > drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - > drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 > ++- > drivers/gpu/drm/exynos/regs-hdmiphy.h| 61 > 8 files changed, 738 insertions(+), 368 deletions(-) > create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index 3da5c2d..03acb6b 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -334,6 +334,11 @@ static int __init exynos_drm_init(void) > ret = platform_driver_register(_driver); > if (ret < 0) > goto out_hdmi; > + > + ret = exynos_hdmiphy_driver_register(); Not sure why you need to obfuscate the type of driver here. All the other ones are directly registered as platform drivers, I don't see the harm in just doing the i2c_add_driver directly here. > + if (ret < 0) > + goto out_hdmiphy; > + > ret = platform_driver_register(_driver); > if (ret < 0) > goto out_mixer; I think this ordering is how you plan on enforcing the suspend/resume order for hdmi/mixer/hdmiphy. In that case, however, I don't think it makes sense to suspend/resume hdmiphy in between mixer and hdmi. Ideally, hdmiphy should go down first and come up last. > @@ -436,6 +441,8 @@ out_common_hdmi_dev: > out_common_hdmi: > platform_driver_unregister(_driver); > out_mixer: > + exynos_hdmiphy_driver_unregister(); > +out_hdmiphy: > platform_driver_unregister(_driver); > out_hdmi: > #endif > @@ -479,6 +486,7 @@ static void __exit exynos_drm_exit(void) > exynos_platform_device_hdmi_unregister(); > platform_driver_unregister(_drm_common_hdmi_driver); > platform_driver_unregister(_driver); > + exynos_hdmiphy_driver_unregister(); > platform_driver_unregister(_driver); > #endif > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h > b/drivers/gpu/drm/exynos/exynos_drm_drv.h > index 4606fac..17c53e3 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h > @@ -325,6 +325,12 @@ void exynos_drm_subdrv_close(struct drm_device *dev, > struct drm_file *file); > extern int exynos_platform_device_hdmi_register(void); > > /* > + * these functions registers/unregisters exynos drm hdmiphy driver. > + */ > +extern int exynos_hdmiphy_driver_register(void); > +extern void exynos_hdmiphy_driver_unregister(void); > + > +/* > * this function unregisters exynos drm hdmi platform device if it exists. > */ > void exynos_platform_device_hdmi_unregister(void); > diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c > b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c > index 5dc956b..45ef331 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c > @@ -32,19 +32,22 @@ > /* platform device pointer for common drm hdmi device. */ > static struct platform_device *exynos_drm_hdmi_pdev; > > -/* Common hdmi subdrv needs to access the hdmi and mixer though context. > -* These should be initialied by the repective drivers */ > +/* Common hdmi subdrv needs to access the hdmi, hdmiphy and mixer though > +* context. These should be initialied by the repective drivers */ > static struct
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy
related
code is tightly coupled with hdmi ip driver. Physicaly they are different
devices and
should be instantiated independently.
In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of
each
other. It is preferred to isolate them and maintained independently.
This implementations is tested for:
1) Resolutions supported by exynos4 and 5 hdmi.
2) Runtime PM and S2R scenarions for exynos5.
This patch is dependent on
http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34733.html
http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34861.html
http://www.mail-archive.com/dri-devel at lists.freedesktop.org/msg34862.html
Signed-off-by: Rahul Sharma
---
It is based on exynos-drm-next-todo branch at
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 +
drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 +
drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++-
drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 +
drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++--
drivers/gpu/drm/exynos/exynos_hdmi.h | 1 -
drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++-
drivers/gpu/drm/exynos/regs-hdmiphy.h| 61
8 files changed, 738 insertions(+), 368 deletions(-)
create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 3da5c2d..03acb6b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -334,6 +334,11 @@ static int __init exynos_drm_init(void)
ret = platform_driver_register(_driver);
if (ret < 0)
goto out_hdmi;
+
+ ret = exynos_hdmiphy_driver_register();
+ if (ret < 0)
+ goto out_hdmiphy;
+
ret = platform_driver_register(_driver);
if (ret < 0)
goto out_mixer;
@@ -436,6 +441,8 @@ out_common_hdmi_dev:
out_common_hdmi:
platform_driver_unregister(_driver);
out_mixer:
+ exynos_hdmiphy_driver_unregister();
+out_hdmiphy:
platform_driver_unregister(_driver);
out_hdmi:
#endif
@@ -479,6 +486,7 @@ static void __exit exynos_drm_exit(void)
exynos_platform_device_hdmi_unregister();
platform_driver_unregister(_drm_common_hdmi_driver);
platform_driver_unregister(_driver);
+ exynos_hdmiphy_driver_unregister();
platform_driver_unregister(_driver);
#endif
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 4606fac..17c53e3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -325,6 +325,12 @@ void exynos_drm_subdrv_close(struct drm_device *dev,
struct drm_file *file);
extern int exynos_platform_device_hdmi_register(void);
/*
+ * these functions registers/unregisters exynos drm hdmiphy driver.
+ */
+extern int exynos_hdmiphy_driver_register(void);
+extern void exynos_hdmiphy_driver_unregister(void);
+
+/*
* this function unregisters exynos drm hdmi platform device if it exists.
*/
void exynos_platform_device_hdmi_unregister(void);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
index 5dc956b..45ef331 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
@@ -32,19 +32,22 @@
/* platform device pointer for common drm hdmi device. */
static struct platform_device *exynos_drm_hdmi_pdev;
-/* Common hdmi subdrv needs to access the hdmi and mixer though context.
-* These should be initialied by the repective drivers */
+/* Common hdmi subdrv needs to access the hdmi, hdmiphy and mixer though
+* context. These should be initialied by the repective drivers */
static struct exynos_drm_hdmi_context *hdmi_ctx;
+static struct exynos_drm_hdmi_context *hdmiphy_ctx;
static struct exynos_drm_hdmi_context *mixer_ctx;
/* these callback points shoud be set by specific drivers. */
static struct exynos_hdmi_ops *hdmi_ops;
+static struct exynos_hdmiphy_ops *hdmiphy_ops;
static struct exynos_mixer_ops *mixer_ops;
struct drm_hdmi_context {
struct exynos_drm_subdrvsubdrv;
struct exynos_drm_hdmi_context *hdmi_ctx;
struct exynos_drm_hdmi_context *mixer_ctx;
+ struct exynos_drm_hdmi_context *hdmiphy_ctx;
boolenabled[MIXER_WIN_NR];
};
@@ -74,6 +77,12 @@ void exynos_hdmi_drv_attach(struct exynos_drm_hdmi_context
*ctx)
hdmi_ctx = ctx;
}
+void exynos_hdmiphy_drv_attach(struct exynos_drm_hdmi_context *ctx)
+{
+ if (ctx)
+ hdmiphy_ctx = ctx;
+}
+
void exynos_mixer_drv_attach(struct exynos_drm_hdmi_context *ctx)
{
if (ctx)
@@ -88,6 +97,14 @@ void exynos_hdmi_ops_register(struct exynos_hdmi_ops *ops)
[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy
related
code is tightly coupled with hdmi ip driver. Physicaly they are different
devices and
should be instantiated independently.
In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of
each
other. It is preferred to isolate them and maintained independently.
This implementations is tested for:
1) Resolutions supported by exynos4 and 5 hdmi.
2) Runtime PM and S2R scenarions for exynos5.
This patch is dependent on
http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html
http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html
http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html
Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
---
It is based on exynos-drm-next-todo branch at
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 +
drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 +
drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++-
drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 +
drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++--
drivers/gpu/drm/exynos/exynos_hdmi.h | 1 -
drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++-
drivers/gpu/drm/exynos/regs-hdmiphy.h| 61
8 files changed, 738 insertions(+), 368 deletions(-)
create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 3da5c2d..03acb6b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -334,6 +334,11 @@ static int __init exynos_drm_init(void)
ret = platform_driver_register(hdmi_driver);
if (ret 0)
goto out_hdmi;
+
+ ret = exynos_hdmiphy_driver_register();
+ if (ret 0)
+ goto out_hdmiphy;
+
ret = platform_driver_register(mixer_driver);
if (ret 0)
goto out_mixer;
@@ -436,6 +441,8 @@ out_common_hdmi_dev:
out_common_hdmi:
platform_driver_unregister(mixer_driver);
out_mixer:
+ exynos_hdmiphy_driver_unregister();
+out_hdmiphy:
platform_driver_unregister(hdmi_driver);
out_hdmi:
#endif
@@ -479,6 +486,7 @@ static void __exit exynos_drm_exit(void)
exynos_platform_device_hdmi_unregister();
platform_driver_unregister(exynos_drm_common_hdmi_driver);
platform_driver_unregister(mixer_driver);
+ exynos_hdmiphy_driver_unregister();
platform_driver_unregister(hdmi_driver);
#endif
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 4606fac..17c53e3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -325,6 +325,12 @@ void exynos_drm_subdrv_close(struct drm_device *dev,
struct drm_file *file);
extern int exynos_platform_device_hdmi_register(void);
/*
+ * these functions registers/unregisters exynos drm hdmiphy driver.
+ */
+extern int exynos_hdmiphy_driver_register(void);
+extern void exynos_hdmiphy_driver_unregister(void);
+
+/*
* this function unregisters exynos drm hdmi platform device if it exists.
*/
void exynos_platform_device_hdmi_unregister(void);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
index 5dc956b..45ef331 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
@@ -32,19 +32,22 @@
/* platform device pointer for common drm hdmi device. */
static struct platform_device *exynos_drm_hdmi_pdev;
-/* Common hdmi subdrv needs to access the hdmi and mixer though context.
-* These should be initialied by the repective drivers */
+/* Common hdmi subdrv needs to access the hdmi, hdmiphy and mixer though
+* context. These should be initialied by the repective drivers */
static struct exynos_drm_hdmi_context *hdmi_ctx;
+static struct exynos_drm_hdmi_context *hdmiphy_ctx;
static struct exynos_drm_hdmi_context *mixer_ctx;
/* these callback points shoud be set by specific drivers. */
static struct exynos_hdmi_ops *hdmi_ops;
+static struct exynos_hdmiphy_ops *hdmiphy_ops;
static struct exynos_mixer_ops *mixer_ops;
struct drm_hdmi_context {
struct exynos_drm_subdrvsubdrv;
struct exynos_drm_hdmi_context *hdmi_ctx;
struct exynos_drm_hdmi_context *mixer_ctx;
+ struct exynos_drm_hdmi_context *hdmiphy_ctx;
boolenabled[MIXER_WIN_NR];
};
@@ -74,6 +77,12 @@ void exynos_hdmi_drv_attach(struct exynos_drm_hdmi_context
*ctx)
hdmi_ctx = ctx;
}
+void exynos_hdmiphy_drv_attach(struct exynos_drm_hdmi_context *ctx)
+{
+ if (ctx)
+ hdmiphy_ctx = ctx;
+}
+
void exynos_mixer_drv_attach(struct exynos_drm_hdmi_context *ctx)
{
if (ctx)
@@ -88,6 +97,14 @@ void
