2017-08-11 03:20:00 +08:00
|
|
|
===============================================================================
|
|
|
|
TODOs
|
|
|
|
===============================================================================
|
|
|
|
|
|
|
|
1. Base this on drm-next - WIP
|
|
|
|
|
|
|
|
|
|
|
|
2. Cleanup commit history
|
|
|
|
|
|
|
|
|
|
|
|
3. WIP - Drop page flip helper and use DRM's version
|
|
|
|
|
|
|
|
|
|
|
|
4. DONE - Flatten all DC objects
|
|
|
|
* dc_stream/core_stream/stream should just be dc_stream
|
|
|
|
* Same for other DC objects
|
|
|
|
|
|
|
|
"Is there any major reason to keep all those abstractions?
|
|
|
|
|
|
|
|
Could you collapse everything into struct dc_stream?
|
|
|
|
|
|
|
|
I haven't looked recently but I didn't get the impression there was a
|
|
|
|
lot of design around what was public/protected, more whatever needed
|
|
|
|
to be used by someone else was in public."
|
|
|
|
~ Dave Airlie
|
|
|
|
|
|
|
|
|
|
|
|
5. DONE - Rename DC objects to align more with DRM
|
|
|
|
* dc_surface -> dc_plane_state
|
|
|
|
* dc_stream -> dc_stream_state
|
|
|
|
|
|
|
|
|
|
|
|
6. DONE - Per-plane and per-stream validation
|
|
|
|
|
|
|
|
|
|
|
|
7. WIP - Per-plane and per-stream commit
|
|
|
|
|
|
|
|
|
|
|
|
8. WIP - Split pipe_ctx into plane and stream resource structs
|
|
|
|
|
|
|
|
|
|
|
|
9. Attach plane and stream reources to state object instead of validate_context
|
|
|
|
|
|
|
|
|
|
|
|
10. Remove dc_edid_caps and drm_helpers_parse_edid_caps
|
|
|
|
* Use drm_display_info instead
|
|
|
|
* Remove DC's edid quirks and rely on DRM's quirks (add quirks if needed)
|
|
|
|
|
|
|
|
"Making sure you use the sink-specific helper libraries and kernel
|
|
|
|
subsystems, since there's really no good reason to have 2nd
|
|
|
|
implementation of those in the kernel. Looks likes that's done for mst
|
|
|
|
and edid parsing. There's still a bit a midlayer feeling to the edid
|
|
|
|
parsing side (e.g. dc_edid_caps and dm_helpers_parse_edid_caps, I
|
|
|
|
think it'd be much better if you convert that over to reading stuff
|
|
|
|
from drm_display_info and if needed, push stuff into the core). Also,
|
|
|
|
I can't come up with a good reason why DC needs all this (except to
|
|
|
|
reimplement half of our edid quirk table, which really isn't a good
|
|
|
|
idea). Might be good if you put this onto the list of things to fix
|
|
|
|
long-term, but imo not a blocker. Definitely make sure new stuff
|
|
|
|
doesn't slip in (i.e. if you start adding edid quirks to DC instead of
|
|
|
|
the drm core, refactoring to use the core edid stuff was pointless)."
|
|
|
|
~ Daniel Vetter
|
|
|
|
|
|
|
|
|
2017-09-28 03:36:11 +08:00
|
|
|
11. Remove dc/i2caux. This folder can be somewhat misleading. It's basically an
|
|
|
|
overy complicated HW programming function for sendind and receiving i2c/aux
|
|
|
|
commands. We can greatly simplify that and move it into dc/dceXYZ like other
|
|
|
|
HW blocks.
|
2017-08-11 03:20:00 +08:00
|
|
|
|
|
|
|
12. drm_modeset_lock in MST should no longer be needed in recent kernels
|
|
|
|
* Adopt appropriate locking scheme
|
drm/amd: DC pull request review
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: Daniel 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: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
2017-09-27 18:15:50 +08:00
|
|
|
|
|
|
|
13. get_modes and best_encoder callbacks look a bit funny. Can probably rip out
|
|
|
|
a few indirections, and consider removing entirely and using the
|
|
|
|
drm_atomic_helper_best_encoder default behaviour.
|
|
|
|
|
|
|
|
14. core/dc_debug.c, consider switching to the atomic state debug helpers and
|
|
|
|
moving all your driver state printing into the various atomic_print_state
|
|
|
|
callbacks. There's also plans to expose this stuff in a standard way across all
|
|
|
|
drivers, to make debugging userspace compositors easier across different hw.
|
|
|
|
|
2017-09-28 03:36:11 +08:00
|
|
|
15. Move DP/HDMI dual mode adaptors to drm_dp_dual_mode_helper.c. See
|
|
|
|
dal_ddc_service_i2c_query_dp_dual_mode_adaptor.
|
drm/amd: DC pull request review
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: Daniel 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: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
2017-09-27 18:15:50 +08:00
|
|
|
|
|
|
|
16. Move to core SCDC helpers (I think those are new since initial DC review).
|
|
|
|
|
|
|
|
17. There's still a pretty massive layer cake around dp aux and DPCD handling,
|
|
|
|
with like 3 levels of abstraction and using your own structures instead of the
|
|
|
|
stuff in drm_dp_helper.h. drm_dp_helper.h isn't really great and already has 2
|
|
|
|
incompatible styles, just means more reasons not to add a third (or well third
|
|
|
|
one gets to do the cleanup refactor).
|
|
|
|
|
|
|
|
18. There's a pile of sink handling code, both for DP and HDMI where I didn't
|
|
|
|
immediately recognize the standard. I think long term it'd be best for the drm
|
|
|
|
subsystem if we try to move as much of that into helpers/core as possible, and
|
|
|
|
share it with drivers. But that's a very long term goal, and by far not just an
|
|
|
|
issue with DC - other drivers, especially around DP sink handling, are equally
|
|
|
|
guilty.
|
|
|
|
|
2018-06-19 21:58:24 +08:00
|
|
|
19. DONE - The DC logger is still a rather sore thing, but I know that the
|
|
|
|
DRM_DEBUG stuff just isn't up to the challenges either. We need to figure out
|
|
|
|
something that integrates better with DRM and linux debug printing, while not
|
|
|
|
being useless with filtering output. dynamic debug printing might be an option.
|
2017-09-28 03:36:11 +08:00
|
|
|
|
|
|
|
20. Use kernel i2c device to program HDMI retimer. Some boards have an HDMI
|
|
|
|
retimer that we need to program to pass PHY compliance. Currently that's
|
|
|
|
bypassing the i2c device and goes directly to HW. This should be changed.
|
2017-11-23 23:37:29 +08:00
|
|
|
|
|
|
|
21. Remove vector.c from dc/basics. It's used in DDC code which can probably
|
|
|
|
be simplified enough to no longer need a vector implementation.
|