lkml.org 
[lkml]   [2016]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v14 0/17] Add Analogix Core Display Port Driver
    On Thu, Mar 31, 2016 at 12:15:35PM +0200, Daniel Vetter wrote:
    > On Mon, Feb 15, 2016 at 07:08:05PM +0800, 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
    > >
    > > But there are still three light registers setting different between
    > > exynos and rk3288.
    > > 1. RK3288 have five special pll registers which not indicate 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).
    > >
    > > Due to Mark Yao have introduced the ATOMIC support to Rockchip drm, so it's
    > > okay to use the ATOMIC helpers functions in connector_funcs. No need to splict
    > > the connector init to platform driver anymore, and this is the biggest change
    > > since version 11.
    > >
    > > This v14 didn't have lots of new changes which seems not the correct time to
    > > upgrade the version number, but I have changed ordering of patches (adding 2
    > > more, and removing 2 out). Especially to prevent confusing people, so I updated
    > > the whole series.
    >
    > So I'm jumping into this part way late, but just noticed (well Thierry
    > pointed this out to me) that the exynos dp driver reinvents all the dp and
    > dp-aux helpers we already. That's somewhat okish for a private driver (and
    > exynos has a reputation for that kind of stuff), but imo not ok for a
    > shared driver.
    >
    > Not saying this should block merging this patch, but it really needs to be
    > addressed. All the dp aux and edid read code in the current
    > exynos_dp_core/reg.c files needs to be replaced with dp helpers and the
    > core i2c edid reading code.

    Agreed. I think a first step would be to implement and register a
    drm_dp_aux object for Analogix DP hardware. That will give you access to
    a slew of helpers that allow to read DPCD, parse link status and use the
    standard EDID reading routines that the DRM core already has. This
    should remove a lot of duplication from this driver.

    For extra incentive, doing this might even fix a bug or two.

    Thierry
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2016-03-31 12:41    [W:4.563 / U:0.456 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site