Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver

[김승우 <sw0312.kim@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@google.com> wrote:
>> On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma <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.

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.sharma@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
--