• Daniel Vetter's avatar
    drm/amd: DC pull request review · 82b400a6
    Daniel Vetter authored
    Ok, here's one more attempt at scrolling through 130k diff.
    
    Overall verdict from me is that DC is big project, and like any big
    project it's never done. So at least for me the goal isn't to make
    things perfect, becaue if that's the hoop to jump through we wouldn't
    have any gpu drivers at all. More important is whether merging a new
    driver base will benefit the overall subsystem, and here this
    primarily means whether the DC team understands how upstream works and
    is designed, and whether the code is largely aligned with upstream
    (especially the atomic modeset) architecture.
    
    Looking back over the last two years I think that's the case now, so
    Acked-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
    
    for merging this pull.
    
    While scrolling through the pull I spotted a bunch more things that
    should be refactored, but most of these will be a real pain with DC
    is out of tree, and much easier in tree since in many of these areas
    the in-tree helpers aren't up to snuff yet for what DC needs. That
    kind of work is best done when there's one tree with everything
    integrated.
    
    That's also why I think we should merge DC into drm-next directly, so
    we can get started on the integration polish right away. That has a
    bit higher risk of Linus having a spazz, so here's my recommendation
    for merging:
    
    - There's a few additions to drm_dp_helper.h sprinkled all over the
      pull. I think those should be put into a patch of it's own, and
      merged first. No need to rebase DC, git merge will dtrt and not end
      up with duplicates.
    
    - dm_alloc/realloc/free is something Dave Airlie noticed, and I agree
      it's an easy red flag that might upset Linus. cocci can fix this
      easy, so no real problem I think to patch up in one big patch (I
      thought we've had a "remove malloc wrappers" todo item in the very
      first review, apparently there was more than one such wrapper).
    
    - The history is huge, but AMD folks want to keep it if possible, and
      I see the value in that. Would be good to get an ack from Linus for
      that (but shouldn't be an issue, not the first time we've merged the
      full history of out-of-tree work).
    
    Short&longer term TODO items are still tracked, might be a good idea
    to integrate those the overall drm todo in our gpu documentation, for
    more visibility.
    
    So in a way this is kinda like staging, except not with the horribly
    broken process of having an entirely separate tree for staging drivers
    which just makes refactoring needlessly painful (which defeats the
    point of staging really). So staging-within-the-subsystem. We've had
    that before, with early nouveau.
    
    And yes some of the files are utterly horrible to read and not
    anything close to kernel coding style standards. But that's the point,
    they're essentially gospel from hw engineers that happens to be
    parseable by gcc.
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
    Reviewed-by: default avatarHarry Wentland <harry.wentland@amd.com>
    Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
    82b400a6
TODO 4.67 KB