On Tue, Sep 23, 2014 at 4:47 AM, cym <cym@xxxxxxxxxxxxxx> wrote: > > On Tuesday, September 23, 2014 03:20 AM, Rob Clark wrote: >> >> On Mon, Sep 22, 2014 at 7:02 AM, Mark yao <mark.yao@xxxxxxxxxxxxxx> wrote: >>> >>> This adds support for Rockchip soc edp found on rk3288 >>> >>> Signed-off-by: Mark Yao <mark.yao@xxxxxxxxxxxxxx> >>> Signed-off-by: Jeff Chen <jeff.chen@xxxxxxxxxxxxxx> >>> --- >>> Changes in v2: >>> - fix code sytle >>> - use some define from drm_dp_helper.h >>> - use panel-simple driver for primary display. >>> - remove unnecessary clock clk_24m_parent. >>> >>> Changes in v3: None >>> >>> Changes in v4: None >>> >>> drivers/gpu/drm/rockchip/Kconfig | 9 + >>> drivers/gpu/drm/rockchip/Makefile | 2 + >>> drivers/gpu/drm/rockchip/rockchip_edp_core.c | 853 ++++++++++++++++++ >>> drivers/gpu/drm/rockchip/rockchip_edp_core.h | 309 +++++++ >>> drivers/gpu/drm/rockchip/rockchip_edp_reg.c | 1202 >>> ++++++++++++++++++++++++++ >>> drivers/gpu/drm/rockchip/rockchip_edp_reg.h | 345 ++++++++ >>> 6 files changed, 2720 insertions(+) >>> create mode 100644 drivers/gpu/drm/rockchip/rockchip_edp_core.c >>> create mode 100644 drivers/gpu/drm/rockchip/rockchip_edp_core.h >>> create mode 100644 drivers/gpu/drm/rockchip/rockchip_edp_reg.c >>> create mode 100644 drivers/gpu/drm/rockchip/rockchip_edp_reg.h >>> >>> diff --git a/drivers/gpu/drm/rockchip/Kconfig >>> b/drivers/gpu/drm/rockchip/Kconfig >>> index 7146c80..04b1f8c 100644 >>> --- a/drivers/gpu/drm/rockchip/Kconfig >>> +++ b/drivers/gpu/drm/rockchip/Kconfig >>> @@ -17,3 +17,12 @@ config DRM_ROCKCHIP >>> management to userspace. This driver does not provides >>> 2D or 3D acceleration; acceleration is performed by other >>> IP found on the SoC. >>> + >>> +config ROCKCHIP_EDP >>> + bool "Rockchip edp support" >>> + depends on DRM_ROCKCHIP >>> + help >>> + Choose this option if you have a Rockchip eDP. >>> + Rockchip rk3288 SoC has eDP TX Controller can be used. >>> + If you have an Embedded DisplayPort Panel, say Y to enable its >>> + driver. >>> diff --git a/drivers/gpu/drm/rockchip/Makefile >>> b/drivers/gpu/drm/rockchip/Makefile >>> index 6e6d468..a0fc3a1 100644 >>> --- a/drivers/gpu/drm/rockchip/Makefile >>> +++ b/drivers/gpu/drm/rockchip/Makefile >>> @@ -7,4 +7,6 @@ ccflags-y := -Iinclude/drm -Idrivers/gpu/drm/rockchip >>> rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o >>> rockchip_drm_fbdev.o \ >>> rockchip_drm_gem.o rockchip_drm_vop.o >>> >>> +rockchipdrm-$(CONFIG_ROCKCHIP_EDP) += rockchip_edp_core.o >>> rockchip_edp_reg.o >>> + >>> obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_edp_core.c >>> b/drivers/gpu/drm/rockchip/rockchip_edp_core.c >>> new file mode 100644 >>> index 0000000..5450d1fa >>> --- /dev/null >>> +++ b/drivers/gpu/drm/rockchip/rockchip_edp_core.c >>> @@ -0,0 +1,853 @@ >>> +/* >>> +* Copyright (C) Fuzhou Rockchip Electronics Co.Ltd >>> +* Author: >>> +* Andy yan <andy.yan@xxxxxxxxxxxxxx> >>> +* Jeff chen <jeff.chen@xxxxxxxxxxxxxx> >>> +* >>> +* based on exynos_dp_core.c >>> +* >> >> hmm, did you look at all at drm_dp_helpers? The exynos code probably >> pre-dates the helpers, so might not be the best example to work off >> of.. >> >> If there is actually a valid reason not to use the dp-helpers, then >> you should mention the reasons, at least in the commit msg if not the >> code >> >> BR, >> -R > > Thanks Rob,Because RK3288 eDP controller IP design is similar to exynos.They > from same IP vendors but have some difference. > So we choosed exynos_dp as example to work off of.exynos_dp only used some > defines from drm_dp_helper.h like DPCD. > Hmm, it sounds like it perhaps should be refactored out into a drm_bridge so more of it can be shared between rockchip and exynos. Either way, it should be using the drm_dp_helpers.. That "the code I copied did it wrong" isn't a terribly good reason for new drivers to do it wrong. So NAK for the eDP part until you use the helpers. BR, -R -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
