From 428da2bdb05d76c48d0bd8fbfa2e4c102685be08 Mon Sep 17 00:00:00 2001 From: Nicholas Kazlauskas Date: Mon, 14 Jan 2019 16:04:10 -0500 Subject: drm/amd/display: Enable vblank interrupt during CRC capture [Why] In order to read CRC events when CRC capture is enabled the vblank interrput handler needs to be running for the CRTC. The handler is enabled while there is an active vblank reference. When running IGT tests there will often be no active vblank reference but the test expects to read a CRC value. This is valid usage (and works on i915 since they have a CRC interrupt handler) so the reference to the vblank should be grabbed while capture is active. This issue was found running: igt@kms_plane_multiple@atomic-pipe-b-tiling-none The pipe-b is the only one in the initial commit and was not previously active so no vblank reference is grabbed. The vblank interrupt is not enabled and the test times out. [How] Keep a reference to the vblank as long as CRC capture is enabled. If userspace never explicitly disables it then the reference is also dropped when removing the CRTC from the context (stream = NULL). Signed-off-by: Nicholas Kazlauskas Reviewed-by: Harry Wentland Reviewed-by: Sun peng Li Acked-by: Leo Li Signed-off-by: Alex Deucher --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 42 +++++++++++----------- 1 file changed, 21 insertions(+), 21 deletions(-) (limited to 'drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c') diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c index f088ac585978..26b651148c67 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c @@ -66,6 +66,7 @@ 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 dc_stream_state *stream_state = crtc_state->stream; + bool enable; enum amdgpu_dm_pipe_crc_source source = dm_parse_crc_source(src_name); @@ -80,28 +81,27 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) return -EINVAL; } + enable = (source == AMDGPU_DM_PIPE_CRC_SOURCE_AUTO); + + if (!dc_stream_configure_crc(stream_state->ctx->dc, stream_state, + enable, enable)) + return -EINVAL; + /* When enabling CRC, we should also disable dithering. */ - if (source == AMDGPU_DM_PIPE_CRC_SOURCE_AUTO) { - if (dc_stream_configure_crc(stream_state->ctx->dc, - stream_state, - true, true)) { - crtc_state->crc_enabled = true; - dc_stream_set_dither_option(stream_state, - DITHER_OPTION_TRUN8); - } - else - return -EINVAL; - } else { - if (dc_stream_configure_crc(stream_state->ctx->dc, - stream_state, - false, false)) { - crtc_state->crc_enabled = false; - dc_stream_set_dither_option(stream_state, - DITHER_OPTION_DEFAULT); - } - else - return -EINVAL; - } + dc_stream_set_dither_option(stream_state, + enable ? DITHER_OPTION_TRUN8 + : DITHER_OPTION_DEFAULT); + + /* + * Reading the CRC requires the vblank interrupt handler to be + * enabled. Keep a reference until CRC capture stops. + */ + if (!crtc_state->crc_enabled && enable) + drm_crtc_vblank_get(crtc); + else if (crtc_state->crc_enabled && !enable) + drm_crtc_vblank_put(crtc); + + crtc_state->crc_enabled = enable; /* Reset crc_skipped on dm state */ crtc_state->crc_skip_count = 0; -- cgit v1.2.3-70-g09d2 From 43a6a02eb3558d1f3a0618f9bf02328662fb06e3 Mon Sep 17 00:00:00 2001 From: Nicholas Kazlauskas Date: Tue, 15 Jan 2019 10:33:58 -0500 Subject: drm/amd/display: Re-enable CRC capture following modeset [Why] During any modeset the CRTC stream is removed and a new stream is added. This new stream doesn't carry over CRC capture state if it was previously set. [How] Re-program the stream for CRC capture. The existing DRM callback can be re-used here for the most part - the only modification needed is additional locking now that it's called from within commit tail. Signed-off-by: Nicholas Kazlauskas Reviewed-by: Harry Wentland Reviewed-by: Sun peng Li Acked-by: Leo Li Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 ++++++++++--- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 8 +++++++- 2 files changed, 17 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c') diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index b0bdf5ceed0e..a247cb19077c 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4923,10 +4923,13 @@ static int amdgpu_dm_atomic_commit(struct drm_device *dev, if (drm_atomic_crtc_needs_modeset(new_crtc_state) && dm_old_crtc_state->stream) { /* - * CRC capture was enabled but not disabled. - * Release the vblank reference. + * If the stream is removed and CRC capture was + * enabled on the CRTC the extra vblank reference + * needs to be dropped since CRC capture will be + * disabled. */ - if (dm_new_crtc_state->crc_enabled) { + if (!dm_new_crtc_state->stream + && dm_new_crtc_state->crc_enabled) { drm_crtc_vblank_put(crtc); dm_new_crtc_state->crc_enabled = false; } @@ -5164,6 +5167,10 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) continue; manage_dm_interrupts(adev, acrtc, true); + + /* The stream has changed so CRC capture needs to re-enabled. */ + if (dm_new_crtc_state->crc_enabled) + amdgpu_dm_crtc_set_crc_source(crtc, "auto"); } /* update planes when needed per crtc*/ diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c index 26b651148c67..a10e3a50d9ef 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c @@ -64,6 +64,7 @@ amdgpu_dm_crtc_verify_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) { + struct amdgpu_device *adev = crtc->dev->dev_private; struct dm_crtc_state *crtc_state = to_dm_crtc_state(crtc->state); struct dc_stream_state *stream_state = crtc_state->stream; bool enable; @@ -83,15 +84,20 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) enable = (source == AMDGPU_DM_PIPE_CRC_SOURCE_AUTO); + mutex_lock(&adev->dm.dc_lock); if (!dc_stream_configure_crc(stream_state->ctx->dc, stream_state, - enable, enable)) + enable, enable)) { + mutex_unlock(&adev->dm.dc_lock); return -EINVAL; + } /* When enabling CRC, we should also disable dithering. */ dc_stream_set_dither_option(stream_state, enable ? DITHER_OPTION_TRUN8 : DITHER_OPTION_DEFAULT); + mutex_unlock(&adev->dm.dc_lock); + /* * Reading the CRC requires the vblank interrupt handler to be * enabled. Keep a reference until CRC capture stops. -- cgit v1.2.3-70-g09d2