Message-ID: <55D5727C.8000109@rock-chips.com>
Date:	Thu, 20 Aug 2015 01:23:56 -0500
From:	Yakir Yang <ykk@...k-chips.com>
To:	Jingoo Han <jingoohan1@...il.com>,
	Archit Taneja <architt@...eaurora.org>
CC:	Heiko Stuebner <heiko@...ech.de>,
	Thierry Reding <treding@...dia.com>,
	Fabio Estevam <fabio.estevam@...escale.com>,
	Inki Dae <inki.dae@...sung.com>,
	"joe@...ches.com" <joe@...ches.com>,
	Russell King <rmk+kernel@....linux.org.uk>,
	"seanpaul@...gle.com" <seanpaul@...gle.com>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	Andrzej Hajda <a.hajda@...sung.com>,
	Gustavo Padovan <gustavo.padovan@...labora.co.uk>,
	"linux-samsung-soc@...r.kernel.org" 
	<linux-samsung-soc@...r.kernel.org>,
	Vincent Palatin <vpalatin@...omium.org>,
	"linux-rockchip@...ts.infradead.org" 
	<linux-rockchip@...ts.infradead.org>,
	Kishon Vijay Abraham I <kishon@...com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	"dianders@...gle.com" <dianders@...gle.com>,
	Rob Herring <robh+dt@...nel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Kyungmin Park <kyungmin.park@...sung.com>,
	"djkurtz@...gle.com" <djkurtz@...gle.com>,
	Kumar Gala <galak@...eaurora.org>,
	"ajaynumb@...il.com" <ajaynumb@...il.com>,
	Andy Yan <andy.yan@...k-chips.com>
Subject: Re: [PATCH v3 0/14] Add Analogix Core Display Port Driver

Hi Jingoo & Archit,


On 08/20/2015 12:54 AM, Jingoo Han wrote:
> On 2015. 8. 20., at PM 1:29, Archit Taneja <architt@...eaurora.org> wrote:
>> Hi,
>>
>>> On 08/19/2015 08:18 PM, Yakir Yang wrote:
>>>
>>> Hi all,
>>>     The Samsung Exynos eDP controller and Rockchip RK3288 eDP controller
>>> share the same IP, so a lot of parts can be re-used. I split the common
>>> code into bridge directory, then rk3288 and exynos only need to keep
>>> some platform code. Cause I can't find the exact IP name of exynos dp
>>> controller, so I decide to name dp core driver with "analogix" which I
>>> find in rk3288 eDP TRM ;)
>>>
>>> Beyond that, there are three light registers setting differents bewteen
>>> exynos and rk3288.
>>> 1. RK3288 have five special pll resigters which not indicata in exynos
>>>     dp controller.
>>> 2. The address of DP_PHY_PD(dp phy power manager register) are different
>>>     between rk3288 and exynos.
>>> 3. Rk3288 and exynos have different setting with AUX_HW_RETRY_CTL(dp debug
>>>     register).
>>>
>>> I have verified this series on two kinds of rockchip platform board, one
>>> is rk3288 sdk board which connect with a 2K display port monitor, the other
>>> is google jerry chromebook which connect with a eDP screen "cnm,n116bgeea2",
>>> both of them works rightlly.
>>>
>>> I haven't verified the dp function on samsung platform, cause I haven't got
>>> exynos boards. I can only ensure that there are no build error on samsung
>>> platform, wish some samsung guys help to test. ;)
>>>
>>> Thanks,
>>> - Yakir
>>>
>>> Changes in v3:
>>> - Take Thierry Reding suggest, move exynos's video_timing code
>>>    to analogix_dp-exynos platform driver, add get_modes method
>>>    to struct analogix_dp_plat_data.
>>> - Take Heiko suggest, rename some "samsung*" dts propery to "analogix*".
>>> - Take Thierry Reding suggest, dynamic parse video timing info from
>>>    struct drm_display_mode and struct drm_display_info.
>>> - Take Thierry Reding suggest, link_rate and lane_count shouldn't config to
>>>    the DT property value directly, but we can take those as hardware limite.
>>>    For example, RK3288 only support 4 physical lanes of 2.7/1.62 Gbps/lane,
>>>    so DT property would like "link-rate = 0x0a" "lane-count = 4".
>>> - Take Heiko suggest, add devicetree binding documents.
>>> - Take Thierry Reding suggest, remove sync pol & colorimetry properies
>>>    from the new analogix dp driver devicetree binding.
>>> - Update the exist exynos dtsi file with the latest DP DT properies.
>>> - Take Thierry Reding and Heiko suggest, leave "sclk_edp_24m" to rockchip
>>>    dp phy driver which name to "24m", and leave "sclk_edp" to analogix dp
>>>    core driver which name to "dp", and leave "pclk_edp" to rockchip dp platform
>>>    driver which name to "pclk".
>>> - Take Heiko suggest, add devicetree binding document.
>>> - Take Heiko suggest, remove "rockchip,panel" DT property, take use of remote
>>>    point to get panel node.
>>> - Add the new function point analogix_dp_platdata.get_modes init.
>>> - Take Heiko suggest, add rockchip dp phy driver,
>>>    collect the phy clocks and power control.
>>> - Add "analogix,need-force-hpd" to indicate whether driver need foce
>>>    hpd when hpd detect failed.
>>> - move dp hpd detect to connector detect function.
>>> - Add edid modes parse support
>>>
>>> Changes in v2:
>>> - Take Joe Preches advise, improved commit message more readable, and
>>>    avoid using some uncommon style like bellow:
>>>    -  retval = exynos_dp_read_bytes_from_i2c(...
>>>                 ...)
>>>    +  retval =
>>>    +  exynos_dp_read_bytes_from_i2c(......);
>>> - Take Jingoo Han suggest, just remove my name from author list.
>>> - Take Jingoo Han suggest, remove new copyright
>>> - Fix compiled failed dut to analogix_dp_device misspell
>>> - Take Heiko suggest, get panel node with remote-endpoint method,
>>>    and create devicetree binding for driver.
>>> - Remove the clock enable/disbale with "sclk_edp" & "sclk_edp_24m",
>>>    leave those clock to rockchip dp phy driver.
>>> - Add GNU license v2 declared and samsung copyright
>>> - Fix compile failed dut to phy_pd_addr variable misspell error
>>>
>>> Yakir Yang (14):
>>>    drm: exynos/dp: fix code style
>>>    drm: exynos/dp: convert to drm bridge mode
>>>    drm: bridge: analogix_dp: split exynos dp driver to bridge dir
>>>    drm: bridge/analogix_dp: dynamic parse sync_pol & interlace &
>>>      colorimetry
>>>    drm: bridge/analogix_dp: fix link_rate & lane_count bug
>>>    Documentation: drm/bridge: add document for analogix_dp
>>>    drm: rockchip/dp: add rockchip platform dp driver
>>>    phy: Add driver for rockchip Display Port PHY
>>>    drm: bridge/analogix_dp: add platform device type support
>>>    drm: bridge: analogix_dp: add some rk3288 special registers setting
>>>    drm: bridge: analogix_dp: try force hpd after plug in lookup failed
>>>    drm: bridge/analogix_dp: expand the delay time for hpd detect
>>>    drm: bridge/analogix_dp: move hpd detect to connector detect function
>>>    drm: bridge/analogix_dp: add edid modes parse in get_modes method
>>>
>>>   .../devicetree/bindings/drm/bridge/analogix_dp.txt |   73 +
>>>   .../devicetree/bindings/phy/rockchip-dp-phy.txt    |   26 +
>>>   .../bindings/video/analogix_dp-rockchip.txt        |   83 ++
>>>   .../devicetree/bindings/video/exynos_dp.txt        |   51 +-
>>>   arch/arm/boot/dts/exynos5250-arndale.dts           |   10 +-
>>>   arch/arm/boot/dts/exynos5250-smdk5250.dts          |   10 +-
>>>   arch/arm/boot/dts/exynos5250-snow.dts              |   12 +-
>>>   arch/arm/boot/dts/exynos5250-spring.dts            |   12 +-
>>>   arch/arm/boot/dts/exynos5420-peach-pit.dts         |   12 +-
>>>   arch/arm/boot/dts/exynos5420-smdk5420.dts          |   10 +-
>>>   arch/arm/boot/dts/exynos5800-peach-pi.dts          |   12 +-
>>>   drivers/gpu/drm/bridge/Kconfig                     |    5 +
>>>   drivers/gpu/drm/bridge/Makefile                    |    1 +
>>>   drivers/gpu/drm/bridge/analogix_dp_core.c          | 1382 +++++++++++++++++++
>>>   drivers/gpu/drm/bridge/analogix_dp_core.h          |  286 ++++
>>>   drivers/gpu/drm/bridge/analogix_dp_reg.c           | 1294 ++++++++++++++++++
>>>   .../exynos_dp_reg.h => bridge/analogix_dp_reg.h}   |  270 ++--
>>>   drivers/gpu/drm/exynos/Kconfig                     |    5 +-
>>>   drivers/gpu/drm/exynos/Makefile                    |    2 +-
>>>   drivers/gpu/drm/exynos/analogix_dp-exynos.c        |  347 +++++
>>>   drivers/gpu/drm/exynos/exynos_dp_core.c            | 1416 --------------------
>>>   drivers/gpu/drm/exynos/exynos_dp_core.h            |  282 ----
>>>   drivers/gpu/drm/exynos/exynos_dp_reg.c             | 1263 -----------------
>>>   drivers/gpu/drm/rockchip/Kconfig |    9 +
>>>   drivers/gpu/drm/rockchip/Makefile                  |    1 +
>>>   drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    |  390 ++++++
>>>   drivers/phy/Kconfig                                |    7 +
>>>   drivers/phy/Makefile                               |    1 +
>>>   drivers/phy/phy-rockchip-dp.c                      |  185 +++
>>>   include/drm/bridge/analogix_dp.h |   40 +
>>>   30 files changed, 4325 insertions(+), 3172 deletions(-)
>>>   create mode 100644 Documentation/devicetree/bindings/drm/bridge/analogix_dp.txt
>>>   create mode 100644 Documentation/devicetree/bindings/phy/rockchip-dp-phy.txt
>>>   create mode 100644 Documentation/devicetree/bindings/video/analogix_dp-rockchip.txt
>>>   create mode 100644 drivers/gpu/drm/bridge/analogix_dp_core.c
>>>   create mode 100644 drivers/gpu/drm/bridge/analogix_dp_core.h
>>>   create mode 100644 drivers/gpu/drm/bridge/analogix_dp_reg.c
>> Minor comment: Since there are a few files required for the driver,
>> could you create a separate folder within drivers/gpu/drm/bridge?
> I agree on this opinion.
> Thank you.

Thanks for your comment ;)

Yeah, bridge/ have been add four new files for this driver, it would be 
better to
collect into a separate folder.

I'm wondering should I just name the folder with "analogix_dp/", or we 
can make
it more common (like "analogix/")? (personally I do like the last one)

Thanks,
- Yakir
> Best regards,
> Jingoo Han
>
>> Thanks,
>> Archit
>>
>> -- 
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists