On Wed, Mar 4, 2015 at 4:02 PM, Andrzej Hajda <a.hajda@xxxxxxxxxxx> wrote: > On 03/04/2015 10:54 AM, Ajay kumar wrote: >> On Wed, Mar 4, 2015 at 2:30 PM, Andrzej Hajda <a.hajda@xxxxxxxxxxx> wrote: >>> On 03/02/2015 09:40 AM, Ajay kumar wrote: >>>> Hi Andrej, >>>> >>>> On Fri, Feb 27, 2015 at 4:18 PM, Andrzej Hajda <a.hajda@xxxxxxxxxxx> wrote: >>>>> Hi Ajay, >>>>> >>>>> Thanks for the patch. >>>> Thanks for reviewing the patch. >>>> >>>>> On 02/26/2015 04:24 PM, Ajay Kumar wrote: >>>>>> Modify the exynos HDMI driver to support Exynos7 HDMI 1.4. >>>>>> * Add phy configs for Exynos7. >>>>>> * Exynos7 has a different clock structure for HDMI, >>>>>> so introduce the new clocks. >>>>>> * Add sysreg support to enable HDMI SYSREG on Exynos7. >>>>>> * Exynos7 based boards need a DCDC_EN and LS_EN pins >>>>>> for powering up HDMI. Add support for that too. >>>>>> >>>>>> Signed-off-by: Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx> >>>>>> --- >>>>>> .../devicetree/bindings/video/exynos_hdmi.txt | 21 +- >>>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 252 ++++++++++++++++---- >>>>>> drivers/gpu/drm/exynos/regs-hdmi.h | 4 + >>>>>> 3 files changed, 231 insertions(+), 46 deletions(-) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt b/Documentation/devicetree/bindings/video/exynos_hdmi.txt >>>>>> index 1fd8cf9..bb22a60 100644 >>>>>> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt >>>>>> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt >>>>>> @@ -6,6 +6,7 @@ Required properties: >>>>>> 2) "samsung,exynos4210-hdmi" >>>>>> 3) "samsung,exynos4212-hdmi" >>>>>> 4) "samsung,exynos5420-hdmi" >>>>>> + 5) "samsung,exynos7-hdmi" >>>>> Why not "samsung,exynos7420-hdmi" ? >>>>> What compatible will you use for Exynos7430 or higher chip number? >>>> I will leave this decision to Inki Dae or Kukjin. >>>> >>>>>> - reg: physical base address of the hdmi and length of memory mapped >>>>>> region. >>>>>> - interrupts: interrupt number to the cpu. >>>>>> @@ -15,21 +16,33 @@ Required properties: >>>>>> c) optional flags and pull up/down. >>>>>> - clocks: list of clock IDs from SoC clock driver. >>>>>> a) hdmi: Gate of HDMI IP bus clock. >>>>>> - b) sclk_hdmi: Gate of HDMI special clock. >>>>>> - c) sclk_pixel: Pixel special clock, one of the two possible inputs of >>>>>> + HDMI clocks necessary for non exynos7: >>>>>> + b) sclk_hdmi: Gate of HDMI special clock. >>>>>> + c) sclk_pixel: Pixel special clock, one of the two possible inputs of >>>>>> HDMI clock mux. >>>>>> - d) sclk_hdmiphy: HDMI PHY clock output, one of two possible inputs of >>>>>> + d) sclk_hdmiphy: HDMI PHY clock output, one of two possible inputs of >>>>>> HDMI clock mux. >>>>>> - e) mout_hdmi: It is required by the driver to switch between the 2 >>>>>> + e) mout_hdmi: It is required by the driver to switch between the 2 >>>>>> parents i.e. sclk_pixel and sclk_hdmiphy. If hdmiphy is stable >>>>>> after configuration, parent is set to sclk_hdmiphy else >>>>>> sclk_pixel. >>>>>> + HDMI clocks necessary for Exynos7: >>>>>> + b) pclk_hdmiphy: Gate to HDMIPHY clock. >>>>> According to specs there is also pclk_hdmi, why do you specify only this >>>>> one? >>>> Right, I have reused "hdmi" gating clock for pclk_hdmi. That is why I have >>>> left "hdmi" clock as common for exynos7 and non-exynos7. >>> IMO it would be better to use separate entry for pclk_hdmi. >> Ok. >> >>>>>> + c) hdmi_pixel: Gate clock of MUX output for I_PIXEL_CLK. >>>>>> + d) hdmi_tmds: Gate clock of MUX output for I_TMDS_CLK. >>>>> According to specs these clocks should be named i_pixel_clk and >>>>> i_tmds_clk, shouldn't they? >>>> I actually took these changes from an "internal" code(non-DRM). >>>> The original author used these names, and I just carried the same names. :) >>>> >>>>>> - clock-names: aliases as per driver requirements for above clock IDs: >>>>>> "hdmi", "sclk_hdmi", "sclk_pixel", "sclk_hdmiphy" and "mout_hdmi". >>>>>> - ddc: phandle to the hdmi ddc node >>>>>> - phy: phandle to the hdmi phy node >>>>>> - samsung,syscon-phandle: phandle for system controller node for PMU. >>>>>> >>>>>> +Only for Exynos7(when compatible = "samsung,exynos7-hdmi"): >>>>>> +- samsung,sysreg-phandle: phandle for system controller node for SYSREG block. >>>>>> + >>>>>> +Optional properties: >>>>>> +- dcdc-gpios: OF device-tree gpio specification for HDMI_DCDC_EN pin. >>>>>> +- lsen-gpios: OF device-tree gpio specification for HDMI_LS_EN pin. >>>>> What is purpose of these gpios? >>>> These 2 GPIOs need to be enabled to powerup HDMI on exynos7-espresso board. >>>> Other boards need not provide the GPIO. >>> HDMI is internal IP of SoC, so it is rather not configurable via pins. >>> Pin names suggests they are for DC-DC converter and level shifter, I guess >>> these are for HDMI connector, not the HDMI IP, am I right? >> Right, this is for HDMI connector. >> >>> Maybe better option is to use pinctrl for these gpios, or use gpio >>> regulator, hard >>> to guess without documentation. >> Why should I not use devm_gpiod_get_optional for these GPIOs? > > Because these gpios are board specific so they should not be handled by > hdmi driver. > > I still do not know what exactly are they for. > If they only drive power for pin18 of hdmi connector the best solution > is to create > gpio regulator and point to it hdmi-en-supply property in hdmi node, as > this is the role of this property. > I guess it should work for you. > If they are used for some other things(??) and cannot be handled using > existing mechanisms > you can try to handle it using pinctrl property in hdmi - it should set > gpios correctly without polluting > the driver and bindings with board specific code/properties. Ok. I will remove this change from the driver. Did you have a look at my comments for the other patch(DECON-EXT)? Ajay _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
