This commit adds a contribution list for display under the kernel documentation with some first suggestions. It also drops an old TODO list from the display folder. Cc: Mario Limonciello <mario.limonciello@xxxxxxx> Cc: Alex Deucher <alexander.deucher@xxxxxxx> Cc: Harry Wentland <Harry.Wentland@xxxxxxx> Cc: Hamza Mahfooz <hamza.mahfooz@xxxxxxx> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx> --- .../amdgpu/display/display-contributing.rst | 88 ++++++++++++++ Documentation/gpu/amdgpu/display/index.rst | 12 +- drivers/gpu/drm/amd/display/TODO | 110 ------------------ 3 files changed, 95 insertions(+), 115 deletions(-) create mode 100644 Documentation/gpu/amdgpu/display/display-contributing.rst delete mode 100644 drivers/gpu/drm/amd/display/TODO diff --git a/Documentation/gpu/amdgpu/display/display-contributing.rst b/Documentation/gpu/amdgpu/display/display-contributing.rst new file mode 100644 index 000000000000..0247e0579fd4 --- /dev/null +++ b/Documentation/gpu/amdgpu/display/display-contributing.rst @@ -0,0 +1,88 @@ +.. _display_todos: + +============================== +AMDGPU - Display Contributions +============================== + +First of all, if you are here, you probably want to give some technical +contribution to the display code, and for that, we say thank you :) + +This page summarizes some of the issues you can help with. This page follows +the DRM way of creating a TODO list; for more information, check +'Documentation/gpu/todo.rst'. + +Gitlab issues +============= + +Users can report issues associated with AMD GPUs at: + +- https://gitlab.freedesktop.org/drm/amd + +Usually, we try to add a proper label to all new tickets to make it easy to +filter issues. If you can reproduce any problem, you could help by adding more +information or fixing the issue. + +Level: diverse + +IGT +=== + +`IGT`_ provides many integration tests that can be run on your GPU. We always +want to pass a large set of tests to increase the test coverage in our CI. If +you wish to contribute to the display code but are unsure where a good place +is, we recommend you run all IGT tests and try to fix any failure you see in +your hardware. Keep in mind that this failure can be an IGT problem or a kernel +issue; it is necessary to analyze case-by-case. + +Level: diverse + +.. _IGT: https://gitlab.freedesktop.org/drm/igt-gpu-tools + +Compilation +=========== + +Fix compilation warnings +------------------------ + +Enable the W1 or W2 warning level in the kernel compilation and try to fix the +issues on the display side. + +Level: Starter + +Code Refactor +============= + +Add prefix to DC functions to improve the debug with ftrace +----------------------------------------------------------- + +The Ftrace debug feature (check 'Documentation/trace/ftrace.rst') is a +fantastic way to check the code path when developers are trying to make sense +of a bug. Ftrace provide a filter mechanism that can be useful when the +developer has some hunch of which part of the code can cause the issue; for +this reason, if a set of function has a proper prefix, it becomes easy to +create a good filter. The DC code does not follow some prefix rules, which +makes the Ftrace filter more complicated. If you want something simple to start +contributing to the display, you can make patches for adding prefixes to DC +functions. To create those prefixes, use part of the file name as a prefix for +all functions in the target file. Check the 'amdgpu_dm_crtc.c` and +`amdgpu_dm_plane.c` for some references. + +Level: Starter + + +Try to move some of the sink handling code to DRM +------------------------------------------------- + +When amdgpu was upstream for the first time, developers discussed how AMD +display handles sink. From the conversation, developers concluded that we +should move some of those codes to the DRM helpers/core in the long term. + +Level: Advanced + +Simplify DDC +------------ + +We can probably 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. + +Level: Advanced diff --git a/Documentation/gpu/amdgpu/display/index.rst b/Documentation/gpu/amdgpu/display/index.rst index 9d53a42c5339..25445a50121e 100644 --- a/Documentation/gpu/amdgpu/display/index.rst +++ b/Documentation/gpu/amdgpu/display/index.rst @@ -109,11 +109,12 @@ if possible. DC Workflow for a new feature ----------------------------- -When we enable a new feature in the DC, the entire development workflow happens -on the amd-gfx mailing list. For example, when we enabled the PSR or the Replay -feature, all the development happened on amd-gfx. When enabling a new feature, -we just use promotion for extra validation in the latest patches by asking the -quality team to test the current promotion together with the new patches. +When an AMD developer enables a new feature in the DC, the entire development +workflow happens on the amd-gfx mailing list. For example, when we enabled the +PSR or the Replay feature, all the development happened on amd-gfx. When +enabling a new feature, we just use promotion for extra validation in the +latest patches by asking the quality team to test the current promotion +together with the new patches. -------------- DC Information @@ -137,4 +138,5 @@ table of content: dcn-blocks.rst mpo-overview.rst dc-debug.rst + display-contributing.rst dc-glossary.rst diff --git a/drivers/gpu/drm/amd/display/TODO b/drivers/gpu/drm/amd/display/TODO deleted file mode 100644 index a8a6c106e8c7..000000000000 --- a/drivers/gpu/drm/amd/display/TODO +++ /dev/null @@ -1,110 +0,0 @@ -=============================================================================== -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 - - -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. - -12. drm_modeset_lock in MST should no longer be needed in recent kernels - * Adopt appropriate locking scheme - -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. - -15. Move DP/HDMI dual mode adaptors to drm_dp_dual_mode_helper.c. See -dal_ddc_service_i2c_query_dp_dual_mode_adaptor. - -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. - -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. - -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. - -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. -- 2.42.0
