[PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver
Rahul Sharma
r.sh.open at gmail.com
Fri Mar 8 00:32:42 PST 2013
On Thu, Mar 7, 2013 at 2:05 PM, Inki Dae <inki.dae at samsung.com> 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 <inki.dae at samsung.com> wrote:
>> >> 2013/3/7 김승우 <sw0312.kim at samsung.com>:
>> >>>
>> >>>
>> >>> On 2013년 03월 04일 23:05, Rahul Sharma wrote:
>> >>>> Thanks Sean,
>> >>>>
>> >>>> On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul <seanpaul at google.com>
>> wrote:
>> >>>>> On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma
>> <rahul.sharma at 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 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 <rahul.sharma at 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 R&D Center
>> >>> --
>> >>>
>> >>> _______________________________________________
>> >>> dri-devel mailing list
>> >>> dri-devel at lists.freedesktop.org
>> >>> 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 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.
More information about the dri-devel
mailing list