Commit 57638021 authored by Nicholas Kazlauskas's avatar Nicholas Kazlauskas Committed by Alex Deucher

drm/amd/display: Split out DC programming for CRC capture

[Why]
Calling amdgpu_dm_crtc_set_crc_source in amdgpu_dm directly has the
consequence of adding additional vblank references or starting DPRX
CRC capture more than once without calling stop first.

Vblank references for CRC capture should be managed entirely by opening
and closing the CRC file from userspace.

Stream state also shouldn't be required on the CRC so we can close the
file after the CRTC has been disabled.

[How]
Do DC programming required for configuring CRC capture separately from
setting the source. Whenever we re-enable or reset a CRC this
programming should be reapplied.

CRC vblank reference handling in amdgpu_dm can be entirely dropped after
this.

Stream state also no longer needs to be required since we can just defer
the programming to when the stream is actually enabled.
Signed-off-by: default avatarNicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Reviewed-by: default avatarDavid Francis <David.Francis@amd.com>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent df61eae4
...@@ -6018,11 +6018,9 @@ static void amdgpu_dm_enable_crtc_interrupts(struct drm_device *dev, ...@@ -6018,11 +6018,9 @@ static void amdgpu_dm_enable_crtc_interrupts(struct drm_device *dev,
/* The stream has changed so CRC capture needs to re-enabled. */ /* The stream has changed so CRC capture needs to re-enabled. */
source = dm_new_crtc_state->crc_src; source = dm_new_crtc_state->crc_src;
if (amdgpu_dm_is_valid_crc_source(source)) { if (amdgpu_dm_is_valid_crc_source(source)) {
dm_new_crtc_state->crc_src = AMDGPU_DM_PIPE_CRC_SOURCE_NONE; amdgpu_dm_crtc_configure_crc_source(
if (source == AMDGPU_DM_PIPE_CRC_SOURCE_CRTC) crtc, dm_new_crtc_state,
amdgpu_dm_crtc_set_crc_source(crtc, "crtc"); dm_new_crtc_state->crc_src);
else if (source == AMDGPU_DM_PIPE_CRC_SOURCE_DPRX)
amdgpu_dm_crtc_set_crc_source(crtc, "dprx");
} }
#endif #endif
} }
...@@ -6073,24 +6071,9 @@ static int amdgpu_dm_atomic_commit(struct drm_device *dev, ...@@ -6073,24 +6071,9 @@ static int amdgpu_dm_atomic_commit(struct drm_device *dev,
if (dm_old_crtc_state->interrupts_enabled && if (dm_old_crtc_state->interrupts_enabled &&
(!dm_new_crtc_state->interrupts_enabled || (!dm_new_crtc_state->interrupts_enabled ||
drm_atomic_crtc_needs_modeset(new_crtc_state))) { drm_atomic_crtc_needs_modeset(new_crtc_state)))
/*
* Drop the extra vblank reference added by CRC
* capture if applicable.
*/
if (amdgpu_dm_is_valid_crc_source(dm_new_crtc_state->crc_src))
drm_crtc_vblank_put(crtc);
/*
* Only keep CRC capture enabled if there's
* still a stream for the CRTC.
*/
if (!dm_new_crtc_state->stream)
dm_new_crtc_state->crc_src = AMDGPU_DM_PIPE_CRC_SOURCE_NONE;
manage_dm_interrupts(adev, acrtc, false); manage_dm_interrupts(adev, acrtc, false);
} }
}
/* /*
* Add check here for SoC's that support hardware cursor plane, to * Add check here for SoC's that support hardware cursor plane, to
* unset legacy_cursor_update * unset legacy_cursor_update
......
...@@ -97,11 +97,46 @@ amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc, const char *src_name, ...@@ -97,11 +97,46 @@ amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
return 0; return 0;
} }
int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) int amdgpu_dm_crtc_configure_crc_source(struct drm_crtc *crtc,
struct dm_crtc_state *dm_crtc_state,
enum amdgpu_dm_pipe_crc_source source)
{ {
struct amdgpu_device *adev = crtc->dev->dev_private; struct amdgpu_device *adev = crtc->dev->dev_private;
struct dc_stream_state *stream_state = dm_crtc_state->stream;
bool enable = amdgpu_dm_is_valid_crc_source(source);
int ret = 0;
/* Configuration will be deferred to stream enable. */
if (!stream_state)
return 0;
mutex_lock(&adev->dm.dc_lock);
/* Enable CRTC CRC generation if necessary. */
if (dm_is_crc_source_crtc(source)) {
if (!dc_stream_configure_crc(stream_state->ctx->dc,
stream_state, enable, enable)) {
ret = -EINVAL;
goto unlock;
}
}
/* Configure dithering */
if (!dm_need_crc_dither(source))
dc_stream_set_dither_option(stream_state, DITHER_OPTION_TRUN8);
else
dc_stream_set_dither_option(stream_state,
DITHER_OPTION_DEFAULT);
unlock:
mutex_unlock(&adev->dm.dc_lock);
return ret;
}
int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
{
struct dm_crtc_state *crtc_state = to_dm_crtc_state(crtc->state); struct dm_crtc_state *crtc_state = to_dm_crtc_state(crtc->state);
struct dc_stream_state *stream_state = crtc_state->stream;
struct drm_dp_aux *aux = NULL; struct drm_dp_aux *aux = NULL;
bool enable = false; bool enable = false;
bool enabled = false; bool enabled = false;
...@@ -115,14 +150,8 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) ...@@ -115,14 +150,8 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
return -EINVAL; return -EINVAL;
} }
if (!stream_state) {
DRM_ERROR("No stream state for CRTC%d\n", crtc->index);
return -EINVAL;
}
enable = amdgpu_dm_is_valid_crc_source(source); enable = amdgpu_dm_is_valid_crc_source(source);
mutex_lock(&adev->dm.dc_lock);
/* /*
* USER REQ SRC | CURRENT SRC | BEHAVIOR * USER REQ SRC | CURRENT SRC | BEHAVIOR
* ----------------------------- * -----------------------------
...@@ -155,7 +184,6 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) ...@@ -155,7 +184,6 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
if (!aconn) { if (!aconn) {
DRM_DEBUG_DRIVER("No amd connector matching CRTC-%d\n", crtc->index); DRM_DEBUG_DRIVER("No amd connector matching CRTC-%d\n", crtc->index);
mutex_unlock(&adev->dm.dc_lock);
return -EINVAL; return -EINVAL;
} }
...@@ -163,24 +191,12 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) ...@@ -163,24 +191,12 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
if (!aux) { if (!aux) {
DRM_DEBUG_DRIVER("No dp aux for amd connector\n"); DRM_DEBUG_DRIVER("No dp aux for amd connector\n");
mutex_unlock(&adev->dm.dc_lock);
return -EINVAL; return -EINVAL;
} }
} else if (dm_is_crc_source_crtc(source)) {
if (!dc_stream_configure_crc(stream_state->ctx->dc, stream_state,
enable, enable)) {
mutex_unlock(&adev->dm.dc_lock);
return -EINVAL;
} }
}
/* configure dithering */
if (!dm_need_crc_dither(source))
dc_stream_set_dither_option(stream_state, DITHER_OPTION_TRUN8);
else if (!dm_need_crc_dither(crtc_state->crc_src))
dc_stream_set_dither_option(stream_state, DITHER_OPTION_DEFAULT);
mutex_unlock(&adev->dm.dc_lock); if (amdgpu_dm_crtc_configure_crc_source(crtc, crtc_state, source))
return -EINVAL;
/* /*
* Reading the CRC requires the vblank interrupt handler to be * Reading the CRC requires the vblank interrupt handler to be
......
...@@ -26,6 +26,9 @@ ...@@ -26,6 +26,9 @@
#ifndef AMD_DAL_DEV_AMDGPU_DM_AMDGPU_DM_CRC_H_ #ifndef AMD_DAL_DEV_AMDGPU_DM_AMDGPU_DM_CRC_H_
#define AMD_DAL_DEV_AMDGPU_DM_AMDGPU_DM_CRC_H_ #define AMD_DAL_DEV_AMDGPU_DM_AMDGPU_DM_CRC_H_
struct drm_crtc;
struct dm_crtc_state;
enum amdgpu_dm_pipe_crc_source { enum amdgpu_dm_pipe_crc_source {
AMDGPU_DM_PIPE_CRC_SOURCE_NONE = 0, AMDGPU_DM_PIPE_CRC_SOURCE_NONE = 0,
AMDGPU_DM_PIPE_CRC_SOURCE_CRTC, AMDGPU_DM_PIPE_CRC_SOURCE_CRTC,
...@@ -44,6 +47,9 @@ static inline bool amdgpu_dm_is_valid_crc_source(enum amdgpu_dm_pipe_crc_source ...@@ -44,6 +47,9 @@ static inline bool amdgpu_dm_is_valid_crc_source(enum amdgpu_dm_pipe_crc_source
/* amdgpu_dm_crc.c */ /* amdgpu_dm_crc.c */
#ifdef CONFIG_DEBUG_FS #ifdef CONFIG_DEBUG_FS
int amdgpu_dm_crtc_configure_crc_source(struct drm_crtc *crtc,
struct dm_crtc_state *dm_crtc_state,
enum amdgpu_dm_pipe_crc_source source);
int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name); int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name);
int amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc, int amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc,
const char *src_name, const char *src_name,
......
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