[PATCH 8/8] Documentation/gpu: Introduce a simple contribution list for display code

Rodrigo Siqueira Rodrigo.Siqueira at amd.com
Fri Oct 20 22:05:21 UTC 2023 

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 at amd.com>
Cc: Alex Deucher <alexander.deucher at amd.com>
Cc: Harry Wentland <Harry.Wentland at amd.com>
Cc: Hamza Mahfooz <hamza.mahfooz at amd.com>
Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
---
 .../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

More information about the amd-gfx mailing list