Commit ba162ae7 authored by Rodrigo Siqueira's avatar Rodrigo Siqueira Committed by Alex Deucher

Documentation/gpu: Introduce a simple contribution list for display code

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@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Harry Wentland <Harry.Wentland@amd.com>
Cc: Hamza Mahfooz <hamza.mahfooz@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
Reviewed-by: default avatarMario Limonciello <mario.limonciello@amd.com>
Acked-by: default avatarChristian König <christian.koenig@amd.com>
Acked-by: default avatarHamza Mahfooz <hamza.mahfooz@amd.com>
Signed-off-by: default avatarRodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent 21dd98b0
.. _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; keep in mind that
this is a static page, and it is always a good idea to try to reach developers
in the amdgfx or some of the maintainers. Finally, 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
Fix compilation issues when using um architecture
-------------------------------------------------
Linux has a User-mode Linux (UML) feature, and the kernel can be compiled to
the **um** architecture. Compiling for **um** can bring multiple advantages
from the test perspective. We currently have some compilation issues in this
area that we need to fix.
Level: Intermediate
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 try to make sense of a
bug. Ftrace provides 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 functions has a proper prefix, it becomes easy to create a good
filter. Additionally, prefixes can improve stack trace readability.
The DC code does not follow some prefix rules, which makes the Ftrace filter
more complicated and reduces the readability of the stack trace. 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. However, we
strongly advise not to send huge patches changing these prefixes; otherwise, it
will be hard to review and test, which can generate second thoughts from
maintainers. Try small steps; in case of double, you can ask before you put in
effort. We recommend first looking at folders like dceXYZ, dcnXYZ, basics,
bios, core, clk_mgr, hwss, resource, and irq.
Level: Starter
Reduce code duplication
-----------------------
AMD has an extensive portfolio with various dGPUs and APUs that amdgpu
supports. To maintain the new hardware release cadence, DCE/DCN was designed in
a modular design, making the bring-up for new hardware fast. Over the years,
amdgpu accumulated some technical debt in the code duplication area. For this
task, it would be a good idea to find a tool that can discover code duplication
(including patterns) and use it as guidance to reduce duplications.
Level: Intermediate
Make atomic_commit_[check|tail] more readable
---------------------------------------------
The functions responsible for atomic commit and tail are intricate and
extensive. In particular `amdgpu_dm_atomic_commit_tail` is a long function and
could benefit from being split into smaller helpers. Improvements in this area
are more than welcome, but keep in mind that changes in this area will affect
all ASICs, meaning that refactoring requires a comprehensive verification; in
other words, this effort can take some time for validation.
Level: Advanced
Documentation
=============
Expand kernel-doc
-----------------
Many DC functions do not have a proper kernel-doc; understanding a function and
adding documentation is a great way to learn more about the amdgpu driver and
also leave an outstanding contribution to the entire community.
Level: Starter
Beyond AMDGPU
=============
AMDGPU provides features that are not yet enabled in the userspace. This
section highlights some of the coolest display features, which could be enabled
with the userspace developer helper.
Enable underlay
---------------
AMD display has this feature called underlay (which you can read more about at
'Documentation/GPU/amdgpu/display/mpo-overview.rst') which is intended to
save power when playing a video. The basic idea is to put a video in the
underlay plane at the bottom and the desktop in the plane above it with a hole
in the video area. This feature is enabled in ChromeOS, and from our data
measurement, it can save power.
Level: Unknown
Adaptive Backlight Modulation (ABM)
-----------------------------------
ABM is a feature that adjusts the display panel's backlight level and pixel
values depending on the displayed image. This power-saving feature can be very
useful when the system starts to run off battery; since this will impact the
display output fidelity, it would be good if this option was something that
users could turn on or off.
Level: Unknown
HDR & Color management & VRR
----------------------------
HDR, Color Management, and VRR are huge topics and it's hard to put these into
concise ToDos. If you are interested in this topic, we recommend checking some
blog posts from the community developers to better understand some of the
specific challenges and people working on the subject. If anyone wants to work
on some particular part, we can try to help with some basic guidance. Finally,
keep in mind that we already have some kernel-doc in place for those areas.
Level: Unknown
......@@ -92,4 +92,5 @@ table of content:
dcn-blocks.rst
mpo-overview.rst
dc-debug.rst
display-contributing.rst
dc-glossary.rst
===============================================================================
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.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment