Re: [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@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 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. &g
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 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 > 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 >> --- >> 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&D 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
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 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 bloc
Re: [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 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 > --- > 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&D 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
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 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 >> --- >> 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 >>
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 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. 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 > --- > 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&D Center >> -- >> >> ___ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Regar
Re: [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@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 --- 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&D 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
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@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 >>> --- >>> 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&D Center -- ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
