Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control

From: Kishon Vijay Abraham I <kishon@ti.com>
To: Rahul Sharma <r.sh.open@gmail.com>
Cc: sw0312.kim@samsung.com, Inki Dae <inki.dae@samsung.com>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Rahul Sharma <rahul.sharma@samsung.com>,
	linux-samsung-soc@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org,
	DRI mailing list <dri-devel@lists.freedesktop.org>,
	Kukjin Kim <kgene.kim@samsung.com>,
	Sean Paul <seanpaul@chromium.org>,
	sunil joshi <joshi@samsung.com>,
	Stephen Warren <swarren@wwwdotorg.org>,
	grant.likely@linaro.org
Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control
Date: Tue, 30 Jul 2013 10:37:38 +0530	[thread overview]
Message-ID: <51F74A1A.7000608@ti.com> (raw)
In-Reply-To: <CAPdUM4O7cbBEEc4VdmcyryqFxpek6SsEcG-g-KyxwVypdwJnaw@mail.gmail.com>

Hi,

On Tuesday 30 July 2013 09:12 AM, Rahul Sharma wrote:
> 
> 
> On Tue, Jun 18, 2013 at 5:07 PM, Kishon Vijay Abraham I <kishon@ti.com
> <mailto:kishon@ti.com>> wrote:
> 
>     Hi,
> 
>     On Tuesday 18 June 2013 03:33 PM, Rahul Sharma wrote:
>     > Thanks all,
>     >
>     > On Fri, Jun 14, 2013 at 11:39 AM, 김승우 <sw0312.kim@samsung.com
>     <mailto:sw0312.kim@samsung.com>> wrote:
>     >> Hello Kishon,
>     >>
>     >> On 2013년 06월 13일 21:54, Kishon Vijay Abraham I wrote:
>     >>> Hi,
>     >>>
>     >>> On Thursday 13 June 2013 04:51 PM, Inki Dae wrote:
>     >>>>
>     >>>>
>     >>>>> -----Original Message-----
>     >>>>> From: Sylwester Nawrocki [mailto:s.nawrocki@samsung.com
>     <mailto:s.nawrocki@samsung.com>]
>     >>>>> Sent: Thursday, June 13, 2013 5:56 PM
>     >>>>> To: Rahul Sharma
>     >>>>> Cc: Rahul Sharma; Inki Dae; linux-samsung-soc@vger.kernel.org
>     <mailto:linux-samsung-soc@vger.kernel.org>;
>     >>>>> devicetree-
>     >>>>> discuss@lists.ozlabs.org <mailto:discuss@lists.ozlabs.org>; DRI
>     mailing list; Kukjin Kim; Seung-Woo Kim;
>     >>>>> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren;
>     >>>>> grant.likely@linaro.org <mailto:grant.likely@linaro.org>
>     >>>>> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with
>     >>>>> pmu reg control
>     >>>>>
>     >>>>> Hi,
>     >>>>>
>     >>>>> On 06/13/2013 06:26 AM, Rahul Sharma wrote:
>     >>>>>> Mr. Dae,
>     >>>>>>
>     >>>>>> Thanks for your valuable inputs.
>     >>>>>>
>     >>>>>> I posted it as RFC because, I also have received comments to register
>     >>>>>> hdmiphy as a clock controller. As we always configure it for specific
>     >>>>>> frequency, hdmi-phy looks similar to a PLL. But it really doesn't
>     >>>>>> belong to that class. Secondly prior to exynos5420, it was a i2c
>     >>>>>> device. I am not sure we can register a I2C device as a clock
>     >>>>>> controller. I wanted to discuss and explore this option here.
>     >>>>>
>     >>>>> Have you considered using the generic PHY framework for those HDMI
>     >>>>> PHY devices [1] ? I guess we could add a dedicated group of ops for
>     >>>>> video PHYs, similarly as is is done with struct v4l2_subdev_ops. For
>     >>>>> configuring things like the carrier/pixel clock frequency or anything
>     >>>>> what's common across the video PHYs.
>     >>>>>
>     >>>>> Perhaps you could have a look and see if this framework would be
>     >>>>> useful for HDMI and possibly point out anything what might be missing ?
>     >>>>>
>     >>>>> I'm not sure it it really solves the issues specific to the Exynos
>     >>>>> HDMI but at least with a generic PHY driver the PHY module would be
>     >>>>> separate from the PHY controller, as often same HDMI DPHY can be used
>     >>>>> with various types of a HDMI controller. So this would allow to not
>     >>>>> duplicate the HDMI PHY drivers in the long-term perspective.
>     >>>>
>     >>>> Yeah, at least, it seems that we could use PHY module to control PMU
>     >>>> register, HDMI_PHY_CONTROL. However, PHY module provides only init/on/off
>     >>>> callbacks. As you may know, HDMIPHY needs i2c interfaces to control
>     >>>> HDMIPHY
>     >>>> clock. So with PHY module, HDMIPHY driver could enable PMU more
>     >>>> generically,
>     >>>> but also has to use existing i2c stuff to control HDMIPHY clock. I had a
>     >>>> quick review to Generic PHY Framework[v6] but I didn't see that the PHY
>     >>>> module could generically support more features such as i2c stuff.
>     >>>
>     >>> I don't think PHY framework needs to provide i2c interfaces to program
>     >>> certain configurations. Instead in one of the callbacks (init/on/off)
>     >>> PHY driver can program whatever it wants using any of the interfaces it
>     >>> needs. IMO PHY framework should work independent of the interfaces.
>     >>
>     >> In exnoys hdmi case, i2c interface is not the exact issue. In exynos
>     >> hdmi, hdmiphy should send i2c configuration about video clock
>     >> information as the video mode information including resolution, bit per
>     >> pixel, refresh rate passed from drm subsystem. So init/on/off callbacks
>     >> of phy framework are not enough for exynos hdmiphy and it should have a
>     >> callback to set video mode.
>     >>
>     >> Do you have plan to add driver specific extend callback pointers to phy
>     >> framework?
>     >>
>     >> Currently, hdmi directly calls phy operations, but Rahul's another patch
>     >> set, mentioned by Inki, divides hdmi and hdmiphy and hdmi and hdmiphy is
>     >> connected with exynos hdmi own sub driver callback operations.
>     >>
>     >> IMHO, if phy framework can support extend callback feature, then this
>     >> own sub driver callbacks can be replaced with phy framework at long term
>     >> view.
>     >
>     > Extended callbacks are always welcome. I can also use phy device
>     > private data to pass on private ops like get_pixelclk and set_pixelclk.
> 
>     I would recommend creating a wrapper to the existing PHY framework
>     for HDMI PHY. That way, we can have other HDMI phys added
>     easily. We need to figure out all the ops that might be needed by the
>     HDMI PHY to be added to the wrapper.
>     IMO extended callbacks can lead to abuse of the system and should be
>     used only when absolutely necessary.
> 
>     Thanks
>     Kishon
> 
> 
> Thanks Kishon,
> 
> I have started working on this wrapper layer which is customized for video phys.
> As if now, adding set_dv_timing, get_dv_timing as the only additional callbacks.
> I will post the RFC patches.

Idea of creating wrapper layer for different types of controller is shot down
in the community [1] :-s

[1] -> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/181710.html

Thanks
Kishon