Thanks Seung Woo, Mr. Dae, On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae <inki.dae@xxxxxxxxxxx> wrote: > 2013/3/7 김승우 <sw0312.kim@xxxxxxxxxxx>: >> >> >> On 2013년 03월 04일 23:05, Rahul Sharma wrote: >>> Thanks Sean, >>> >>> On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul <seanpaul@xxxxxxxxxx> wrote: >>>> On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma <rahul.sharma@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx/msg34733.html >>>>> http://www.mail-archive.com/dri-devel@xxxxxxxxxxxxxxxxxxxxx/msg34861.html >>>>> http://www.mail-archive.com/dri-devel@xxxxxxxxxxxxxxxxxxxxx/msg34862.html >>>>> >>>>> Signed-off-by: Rahul Sharma <rahul.sharma@xxxxxxxxxxx> >>>>> --- >>>>> 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 R&D Center >> -- >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Regards, Rahul Sharma, email to: rahul.sharma@xxxxxxxxxxx Samsung India. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
