Date:	Wed, 24 Sep 2014 18:30:33 +0800
From:	jeff chen <jeff.chen@...k-chips.com>
To:	Rob Clark <robdclark@...il.com>
CC:	Mark Rutland <mark.rutland@....com>,
	Heiko Stübner <heiko@...ech.de>,
	linux-doc@...r.kernel.org, Kever Yang <kever.yang@...k-chips.com>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Jianqun Xu <xjq@...k-chips.com>,
	Chris Zhong <zyw@...k-chips.com>, linux-api@...r.kernel.org,
	David Airlie <airlied@...il.com>,
	Boris BREZILLON <boris.brezillon@...e-electrons.com>,
	simon xue <xxm@...k-chips.com>,
	linux-rockchip@...ts.infradead.org, kfx@...k-chips.com,
	Grant Likely <grant.likely@...aro.org>,
	王晓腾 <wxt@...k-chips.com>,
	Tao Huang <huangtao@...k-chips.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	dbehr@...omium.org, yxj@...k-chips.com,
	Eddie Cai <cf@...k-chips.com>,
	Rob Herring <robh+dt@...nel.org>,
	John Stultz <john.stultz@...aro.org>,
	Daniel Kurtz <djkurtz@...omium.org>,
	Stéphane Marchesin <marcheu@...omium.org>,
	Mark yao <mark.yao@...k-chips.com>,
	Rom Lemarchand <romlem@...gle.com>, xw@...k-chips.com,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Randy Dunlap <rdunlap@...radead.org>,
	Douglas Anderson <dianders@...omium.org>,
	Kumar Gala <galak@...eaurora.org>,
	Olof Johansson <olof@...om.net>
Subject: Re: [PATCH v4 5/5] drm/rockchip: Add support for Rockchip Soc EDP


On 2014/9/24 7:35, Rob Clark wrote:
> On Tue, Sep 23, 2014 at 9:56 AM, Rob Clark <robdclark@...il.com> wrote:
>> On Tue, Sep 23, 2014 at 4:47 AM, cym <cym@...k-chips.com> 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@...k-chips.com> wrote:
>>>>> This adds support for Rockchip soc edp found on rk3288
>>>>>
>>>>> Signed-off-by: Mark Yao <mark.yao@...k-chips.com>
>>>>> Signed-off-by: Jeff Chen <jeff.chen@...k-chips.com>
>>>>> ---
>>>>> 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@...k-chips.com>
>>>>> +*      Jeff chen <jeff.chen@...k-chips.com>
>>>>> +*
>>>>> +* 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
thanks,I will modify the core part of the driver.

BR,
-Jeff
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>


--
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