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

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

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.

BR,
-R
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html