This series aims to keep colorop state consistent across atomic
transactions by ensuring it accurately reflects committed hardware
state and remains part of the atomic update whenever its associated
plane is involved.
It contains two changes:
- Preserves the bypass value in duplicated colorop state.
_drm_atomic_helper_colorop_duplicate_state() unconditionally reset
bypass to true, which means the duplicated state no longer reflects the
committed hardware state. Since bypass directly controls whether the
colorop is active in hardware, this can lead to an unintended disable
during subsequent commits.
This could potentially be a problem also for colorops where bypass value
is immutably false.
Conceptually, I consider 'bypass' to behave similar to 'visible' in plane
state - it represents current HW state and should therefore be preserved
across duplication.
- Add affected colorops with affected plane
Colorops are unique in the DRM model. While they are DRM objects with their
own states, they are logically attached to a plane and exposed through
a plane property. In some sense, they share the same hierarchy as CRTC and
planes while following a different 'ownership' model.
Given that enabling a CRTC pulls in all its affected planes into the atomic
state, it follows that when a plane is added, its associated colorops are
also included. Otherwise, during modesets or internal commits, colorop state
may be missing from the transaction, resulting in inconsistent or incomplete
state updates.
That said, I do have a concern about potentially inflating the atomic
state by automatically pulling in colorops from the core. It is not
entirely clear to me whether inclusion of affected colorops should be
handled in core, or left to individual drivers.
My understanding of the atomic framework is still evolving, so
I would appreciate feedback from those more familiar with the intended
design direction.
==
Chaitanya
P.S/Background/TL;DR:
I discovered inconsistency with the colorop state while analysing CRC mismatches
in kms_color_pipeline test cases[1]. Visual inspection reveals that while CRC is
being collected degamma block has been reset. This was traced back to the
internal
commit that the driver does to disable PSR2 and selective fetch for CRC
collection.
crtc_crc_open
-> intel_crtc_set_crc_source
-> intel_crtc_crc_setup_workarounds
-> drm_atomic_commit
During this flow colorop states are never added to the atomic state which in
turn
makes intel_plane_color_copy_uapi_to_hw_state() disable the colorops.
If we add the colorops, to the atomic state, the problem still persisted because
while duplicating the colorop state, 'bypass' was getting reset to true.
The two changes made in this series fixes the issue.
[1]
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18001/shard-mtlp-6/igt@[email protected]
Cc: Simon Ser <[email protected]>
Cc: Alex Hung <[email protected]>
Cc: Harry Wentland <[email protected]>
Cc: Daniel Stone <[email protected]>
Cc: Melissa Wen <[email protected]>
Cc: Sebastian Wick <[email protected]>
Cc: Alex Hung <[email protected]>
Cc: Uma Shankar <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Louis Chauvet <[email protected]>
Cc: <[email protected]> #v6.19+
Chaitanya Kumar Borah (2):
drm/colorop: Preserve bypass value in duplicate_state()
drm/atomic: Add affected colorops with affected planes
drivers/gpu/drm/drm_atomic.c | 5 +++++
drivers/gpu/drm/drm_colorop.c | 2 --
2 files changed, 5 insertions(+), 2 deletions(-)
--
2.25.1
