Re: [PATCH v4 5/5] drm/rockchip: Add support for Rockchip Soc EDP

On Tue, Sep 23, 2014 at 9:56 AM, Rob Clark [off-list ref] wrote:

On Tue, Sep 23, 2014 at 4:47 AM, cym [off-list ref] wrote:

quoted

On Tuesday, September 23, 2014 03:20 AM, Rob Clark wrote:

quoted

On Mon, Sep 22, 2014 at 7:02 AM, Mark yao [off-list ref] wrote:

quoted

This adds support for Rockchip soc edp found on rk3288

Signed-off-by: Mark Yao <redacted>
Signed-off-by: Jeff Chen <redacted>
---
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-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+*      Jeff chen <jeff.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+*
+* 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.

and btw, if it wasn't clear, go ahead and at least repost the core
part of the driver.. the first patch just needed a few small tweaks to
get my r-b even if it takes longer to sort out something sane for the
DP part..

BR,
-R