On Thu, Mar 7, 2013 at 2:05 PM, Inki Dae <inki.dae@xxxxxxxxxxx> wrote: > > >> -----Original Message----- >> From: 김승우 [mailto:sw0312.kim@xxxxxxxxxxx] >> Sent: Thursday, March 07, 2013 4:04 PM >> To: Rahul Sharma >> Cc: Inki Dae; linux-samsung-soc@xxxxxxxxxxxxxxx; Sean Paul; sunil joshi; >> dri-devel@xxxxxxxxxxxxxxxxxxxxx; Rahul Sharma; sw0312.kim@xxxxxxxxxxx >> 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.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. >> >> 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 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 >> > >> > >> > >> >> -- >> Seung-Woo Kim >> Samsung Software R&D Center >> -- > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Regards, Rahul Sharma, email to: rahul.sharma@xxxxxxxxxxx Samsung India. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |