drm/amd/display: Extract DPRX CRC transition helpers for KUnit testing

Extract three pure predicate functions from amdgpu_dm_crtc_set_crc_source():
- dm_need_dp_aux
- dm_crc_source_should_start_dprx
- dm_crc_source_should_stop_dprx

Refactor set_crc_source() to use these helpers, replacing the nested
if/else if structure with flat, mutually-exclusive branches driven by
the new predicates.

Add KUnit test cases covering all relevant source combinations for each
helper, including the regression case where DPRX→NONE must trigger
drm_dp_stop_crc().

Assisted-by: Copilot:Claude-Sonnet-4.6
Reviewed-by: Bhawanpreet Lakha <bhawanpreet.lakha@amd.com>
Signed-off-by: Alex Hung <alex.hung@amd.com>
Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
Tested-by: Dan Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
This commit is contained in:
Alex Hung
2026-05-28 14:01:08 -06:00
committed by Alex Deucher
parent e561531f2f
commit 29757b93d7
3 changed files with 203 additions and 17 deletions

View File

@@ -87,6 +87,65 @@ bool dm_need_crc_dither(enum amdgpu_dm_pipe_crc_source src)
}
EXPORT_IF_KUNIT(dm_need_crc_dither);
/**
* dm_need_dp_aux() - Does this source transition require the DP AUX handle?
* @source: Requested CRC source.
* @cur_crc_src: Current CRC source.
*
* Returns true when either the new source is DPRX-based (starting DPRX CRC),
* or the current source is DPRX-based and the new source is NONE (stopping it).
*
* Return: true if the DP AUX handle is needed, false otherwise.
*/
STATIC_IFN_KUNIT
bool dm_need_dp_aux(enum amdgpu_dm_pipe_crc_source source,
enum amdgpu_dm_pipe_crc_source cur_crc_src)
{
return dm_is_crc_source_dprx(source) ||
(source == AMDGPU_DM_PIPE_CRC_SOURCE_NONE && dm_is_crc_source_dprx(cur_crc_src));
}
EXPORT_IF_KUNIT(dm_need_dp_aux);
/**
* dm_crc_source_should_start_dprx() - Should drm_dp_start_crc() be called?
* @source: Requested CRC source.
* @cur_crc_src: Current CRC source.
*
* True when CRC is transitioning from off to a DPRX source
* (!enabled && enable && is_dprx(@source)).
*
* Return: true if drm_dp_start_crc() should be called, false otherwise.
*/
STATIC_IFN_KUNIT
bool dm_crc_source_should_start_dprx(enum amdgpu_dm_pipe_crc_source source,
enum amdgpu_dm_pipe_crc_source cur_crc_src)
{
return !amdgpu_dm_is_valid_crc_source(cur_crc_src) &&
amdgpu_dm_is_valid_crc_source(source) &&
dm_is_crc_source_dprx(source);
}
EXPORT_IF_KUNIT(dm_crc_source_should_start_dprx);
/**
* dm_crc_source_should_stop_dprx() - Should drm_dp_stop_crc() be called?
* @source: Requested CRC source.
* @cur_crc_src: Current CRC source.
*
* True when CRC is transitioning from a DPRX source to off
* (enabled && !enable && is_dprx(@cur_crc_src)).
*
* Return: true if drm_dp_stop_crc() should be called, false otherwise.
*/
STATIC_IFN_KUNIT
bool dm_crc_source_should_stop_dprx(enum amdgpu_dm_pipe_crc_source source,
enum amdgpu_dm_pipe_crc_source cur_crc_src)
{
return amdgpu_dm_is_valid_crc_source(cur_crc_src) &&
!amdgpu_dm_is_valid_crc_source(source) &&
dm_is_crc_source_dprx(cur_crc_src);
}
EXPORT_IF_KUNIT(dm_crc_source_should_stop_dprx);
const char *const *amdgpu_dm_crtc_get_crc_sources(struct drm_crtc *crtc,
size_t *count)
{
@@ -650,9 +709,7 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
* CRTC DITHER | XXXX | Enable CRTC CRC, set dither
* DPRX DITHER | XXXX | Enable DPRX CRC, need 'aux', set dither
*/
if (dm_is_crc_source_dprx(source) ||
(source == AMDGPU_DM_PIPE_CRC_SOURCE_NONE &&
dm_is_crc_source_dprx(cur_crc_src))) {
if (dm_need_dp_aux(source, cur_crc_src)) {
struct amdgpu_dm_connector *aconn = NULL;
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
@@ -714,23 +771,24 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
goto cleanup;
}
if (!enabled && enable) {
if (dm_is_crc_source_dprx(source)) {
if (drm_dp_start_crc(aux, crtc)) {
DRM_DEBUG_DRIVER("dp start crc failed\n");
ret = -EINVAL;
goto cleanup;
}
if (dm_crc_source_should_start_dprx(source, cur_crc_src)) {
/* !enabled && enable && is_dprx(source): CRC off → DPRX on */
if (drm_dp_start_crc(aux, crtc)) {
DRM_DEBUG_DRIVER("dp start crc failed\n");
ret = -EINVAL;
goto cleanup;
}
} else if (dm_crc_source_should_stop_dprx(source, cur_crc_src)) {
/* enabled && !enable && is_dprx(cur_crc_src): DPRX on → CRC off */
drm_crtc_vblank_put(crtc);
if (drm_dp_stop_crc(aux)) {
DRM_DEBUG_DRIVER("dp stop crc failed\n");
ret = -EINVAL;
goto cleanup;
}
} else if (enabled && !enable) {
/* Non-DPRX source (e.g. CRTC) turning off: release vblank ref */
drm_crtc_vblank_put(crtc);
if (dm_is_crc_source_dprx(cur_crc_src)) {
if (drm_dp_stop_crc(aux)) {
DRM_DEBUG_DRIVER("dp stop crc failed\n");
ret = -EINVAL;
goto cleanup;
}
}
}
spin_lock_irq(&drm_dev->event_lock);

View File

@@ -156,6 +156,12 @@ enum amdgpu_dm_pipe_crc_source dm_parse_crc_source(const char *source);
bool dm_is_crc_source_crtc(enum amdgpu_dm_pipe_crc_source src);
bool dm_is_crc_source_dprx(enum amdgpu_dm_pipe_crc_source src);
bool dm_need_crc_dither(enum amdgpu_dm_pipe_crc_source src);
bool dm_need_dp_aux(enum amdgpu_dm_pipe_crc_source source,
enum amdgpu_dm_pipe_crc_source cur_crc_src);
bool dm_crc_source_should_start_dprx(enum amdgpu_dm_pipe_crc_source source,
enum amdgpu_dm_pipe_crc_source cur_crc_src);
bool dm_crc_source_should_stop_dprx(enum amdgpu_dm_pipe_crc_source source,
enum amdgpu_dm_pipe_crc_source cur_crc_src);
#endif
#endif /* AMD_DAL_DEV_AMDGPU_DM_AMDGPU_DM_CRC_H_ */

View File

@@ -95,17 +95,139 @@ static void dm_test_is_valid_crc_source(struct kunit *test)
KUNIT_EXPECT_FALSE(test, amdgpu_dm_is_valid_crc_source(AMDGPU_DM_PIPE_CRC_SOURCE_INVALID));
}
/**
* dm_test_need_dp_aux() - Test dm_need_dp_aux().
* @test: KUnit test context.
*
* Verifies that dm_need_dp_aux() returns true when the transition starts or
* stops a DPRX CRC source (requiring the DP AUX handle), and false for
* non-DPRX transitions such as CRTC or NONENONE.
*/
static void dm_test_need_dp_aux(struct kunit *test)
{
/* Starting a DPRX source always needs AUX, regardless of current source */
KUNIT_EXPECT_TRUE(test, dm_need_dp_aux(AMDGPU_DM_PIPE_CRC_SOURCE_DPRX,
AMDGPU_DM_PIPE_CRC_SOURCE_NONE));
KUNIT_EXPECT_TRUE(test, dm_need_dp_aux(AMDGPU_DM_PIPE_CRC_SOURCE_DPRX,
AMDGPU_DM_PIPE_CRC_SOURCE_CRTC));
KUNIT_EXPECT_TRUE(test, dm_need_dp_aux(AMDGPU_DM_PIPE_CRC_SOURCE_DPRX_DITHER,
AMDGPU_DM_PIPE_CRC_SOURCE_NONE));
/* Stopping a DPRX source (NONE requested, DPRX was active) needs AUX */
KUNIT_EXPECT_TRUE(test, dm_need_dp_aux(AMDGPU_DM_PIPE_CRC_SOURCE_NONE,
AMDGPU_DM_PIPE_CRC_SOURCE_DPRX));
KUNIT_EXPECT_TRUE(test, dm_need_dp_aux(AMDGPU_DM_PIPE_CRC_SOURCE_NONE,
AMDGPU_DM_PIPE_CRC_SOURCE_DPRX_DITHER));
/* CRTC transitions do not need AUX */
KUNIT_EXPECT_FALSE(test, dm_need_dp_aux(AMDGPU_DM_PIPE_CRC_SOURCE_CRTC,
AMDGPU_DM_PIPE_CRC_SOURCE_NONE));
KUNIT_EXPECT_FALSE(test, dm_need_dp_aux(AMDGPU_DM_PIPE_CRC_SOURCE_NONE,
AMDGPU_DM_PIPE_CRC_SOURCE_CRTC));
KUNIT_EXPECT_FALSE(test, dm_need_dp_aux(AMDGPU_DM_PIPE_CRC_SOURCE_NONE,
AMDGPU_DM_PIPE_CRC_SOURCE_NONE));
}
/**
* dm_test_crc_source_should_start_dprx() - Test dm_crc_source_should_start_dprx().
* @test: KUnit test context.
*
* Verifies that dm_crc_source_should_start_dprx() returns true only when CRC
* is transitioning from off (!enabled) to a DPRX source (enable &&
* is_dprx(source)), and false for all other combinations including
* already-enabled or non-DPRX targets.
*/
static void dm_test_crc_source_should_start_dprx(struct kunit *test)
{
/* CRC off → DPRX: should start */
KUNIT_EXPECT_TRUE(test,
dm_crc_source_should_start_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_DPRX,
AMDGPU_DM_PIPE_CRC_SOURCE_NONE));
KUNIT_EXPECT_TRUE(test,
dm_crc_source_should_start_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_DPRX_DITHER,
AMDGPU_DM_PIPE_CRC_SOURCE_NONE));
/* CRC already on (any source) → DPRX: should NOT start (already enabled) */
KUNIT_EXPECT_FALSE(test,
dm_crc_source_should_start_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_DPRX,
AMDGPU_DM_PIPE_CRC_SOURCE_CRTC));
KUNIT_EXPECT_FALSE(test,
dm_crc_source_should_start_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_DPRX,
AMDGPU_DM_PIPE_CRC_SOURCE_DPRX));
/* CRC off → CRTC: not a DPRX start */
KUNIT_EXPECT_FALSE(test,
dm_crc_source_should_start_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_CRTC,
AMDGPU_DM_PIPE_CRC_SOURCE_NONE));
/* Disabling: should not start */
KUNIT_EXPECT_FALSE(test,
dm_crc_source_should_start_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_NONE,
AMDGPU_DM_PIPE_CRC_SOURCE_DPRX));
}
/**
* dm_test_crc_source_should_stop_dprx() - Test dm_crc_source_should_stop_dprx().
* @test: KUnit test context.
*
* Verifies that dm_crc_source_should_stop_dprx() returns true only when CRC
* is transitioning from a DPRX source (enabled && is_dprx(cur_crc_src)) to
* off (!enable), and false for non-DPRX disables, DPRX starts, and no-op
* transitions.
*/
static void dm_test_crc_source_should_stop_dprx(struct kunit *test)
{
/* DPRX → off: should stop */
KUNIT_EXPECT_TRUE(test,
dm_crc_source_should_stop_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_NONE,
AMDGPU_DM_PIPE_CRC_SOURCE_DPRX));
KUNIT_EXPECT_TRUE(test,
dm_crc_source_should_stop_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_NONE,
AMDGPU_DM_PIPE_CRC_SOURCE_DPRX_DITHER));
/* CRTC → off: not a DPRX stop */
KUNIT_EXPECT_FALSE(test,
dm_crc_source_should_stop_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_NONE,
AMDGPU_DM_PIPE_CRC_SOURCE_CRTC));
/* off → DPRX: not a stop */
KUNIT_EXPECT_FALSE(test,
dm_crc_source_should_stop_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_DPRX,
AMDGPU_DM_PIPE_CRC_SOURCE_NONE));
/* DPRX → DPRX: no transition, not a stop */
KUNIT_EXPECT_FALSE(test,
dm_crc_source_should_stop_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_DPRX,
AMDGPU_DM_PIPE_CRC_SOURCE_DPRX));
/* off → off: not a stop */
KUNIT_EXPECT_FALSE(test,
dm_crc_source_should_stop_dprx(AMDGPU_DM_PIPE_CRC_SOURCE_NONE,
AMDGPU_DM_PIPE_CRC_SOURCE_NONE));
}
static struct kunit_case dm_crc_test_cases[] = {
/* dm_parse_crc_source() */
KUNIT_CASE(dm_test_parse_crc_source_none),
KUNIT_CASE(dm_test_parse_crc_source_crtc),
KUNIT_CASE(dm_test_parse_crc_source_dprx),
KUNIT_CASE(dm_test_parse_crc_source_crtc_dither),
KUNIT_CASE(dm_test_parse_crc_source_dprx_dither),
KUNIT_CASE(dm_test_parse_crc_source_invalid),
/* dm_is_crc_source_crtc() */
KUNIT_CASE(dm_test_is_crc_source_crtc),
/* dm_is_crc_source_dprx() */
KUNIT_CASE(dm_test_is_crc_source_dprx),
/* dm_need_crc_dither() */
KUNIT_CASE(dm_test_need_crc_dither),
/* amdgpu_dm_is_valid_crc_source() */
KUNIT_CASE(dm_test_is_valid_crc_source),
/* dm_need_dp_aux() */
KUNIT_CASE(dm_test_need_dp_aux),
/* dm_crc_source_should_start_dprx() */
KUNIT_CASE(dm_test_crc_source_should_start_dprx),
/* dm_crc_source_should_stop_dprx() */
KUNIT_CASE(dm_test_crc_source_should_stop_dprx),
{}
};