The following patches will plug the VRAM leak that can be provoked in the radeon driver by changing the mode. The mechanism that causes the leak is described in the commit message associated with the first patch.
The way I have caused it (and tested the fix) was temporarily hack up debug counters in radeon_bo_pin and radeon_bo_unpin functions. Then I connected two monitor one of which had 1600x1200 for preferred mode, while the other had preferred mode of the other monitor was 1280x720. Then I repetitively unplugged and plugged back the 1600x1200 monitor and ran 'xrandr --auto' between each plug/unplug operation. Without the fix, the number of pinned buffers grew at each plug/unplug operation. With the fix, the counter remained stable throughout the test.
Note that I only effectively tested the first patch, because I don't have any GPUs that are old enough to use the legacy CRTC path in the driver. Still, based on my findings for AtomBIOS GPUs, I believe that the second patch is correct and necessary.
regards,
Ilija
When drm_helper_disable_unused_functions calls disable function of the CRTC, it also sets the crtc->fb pointer to NULL. This can later (when the mode on that CRTC is setup again from user space) cause ***_do_set_base functions to "think" that there is no old buffer and skip the unpinning code. Consequently, the buffer that has been NULL-ified in drm_helper_disable_unused_functions will never be unpinned causing a leak in VRAM.
This patch plugs the leak by unpinning the frame buffer in crtc_disable function.
Signed-off-by: Ilija Hadzic ihadzic@research.bell-labs.com --- drivers/gpu/drm/radeon/atombios_crtc.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c index bf87f6d..9eb24a3 100644 --- a/drivers/gpu/drm/radeon/atombios_crtc.c +++ b/drivers/gpu/drm/radeon/atombios_crtc.c @@ -1910,6 +1910,21 @@ static void atombios_crtc_disable(struct drm_crtc *crtc) int i;
atombios_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); + if (crtc->fb) { + int r; + struct radeon_framebuffer *radeon_fb; + struct radeon_bo *rbo; + + radeon_fb = to_radeon_framebuffer(crtc->fb); + rbo = gem_to_radeon_bo(radeon_fb->obj); + r = radeon_bo_reserve(rbo, false); + if (unlikely(r)) + DRM_ERROR("failed to reserve rbo before unpin\n"); + else { + radeon_bo_unpin(rbo); + radeon_bo_unreserve(rbo); + } + } /* disable the GRPH */ if (ASIC_IS_DCE4(rdev)) WREG32(EVERGREEN_GRPH_ENABLE + radeon_crtc->crtc_offset, 0);
To plug the VRAM memory leak (see previous patch for details) we must unpin the frame buffer when disabling the CRTC. This warrants the addition of disable function for legacy CRTC, which puts the CRTC in DPMS-OFF state and unpins the frame buffer if there is one associated with the CRTC.
Signed-off-by: Ilija Hadzic ihadzic@research.bell-labs.com --- drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c index 7cb178a..0c7b8c6 100644 --- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c +++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c @@ -1056,6 +1056,26 @@ static void radeon_crtc_commit(struct drm_crtc *crtc) } }
+static void radeon_crtc_disable(struct drm_crtc *crtc) +{ + radeon_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); + if (crtc->fb) { + int r; + struct radeon_framebuffer *radeon_fb; + struct radeon_bo *rbo; + + radeon_fb = to_radeon_framebuffer(crtc->fb); + rbo = gem_to_radeon_bo(radeon_fb->obj); + r = radeon_bo_reserve(rbo, false); + if (unlikely(r)) + DRM_ERROR("failed to reserve rbo before unpin\n"); + else { + radeon_bo_unpin(rbo); + radeon_bo_unreserve(rbo); + } + } +} + static const struct drm_crtc_helper_funcs legacy_helper_funcs = { .dpms = radeon_crtc_dpms, .mode_fixup = radeon_crtc_mode_fixup, @@ -1065,6 +1085,7 @@ static const struct drm_crtc_helper_funcs legacy_helper_funcs = { .prepare = radeon_crtc_prepare, .commit = radeon_crtc_commit, .load_lut = radeon_crtc_load_lut, + .disable = radeon_crtc_disable };
Applied. thanks!
Alex
On Sat, Nov 2, 2013 at 11:00 PM, Ilija Hadzic ilijahadzic@gmail.com wrote:
The following patches will plug the VRAM leak that can be provoked in the radeon driver by changing the mode. The mechanism that causes the leak is described in the commit message associated with the first patch.
The way I have caused it (and tested the fix) was temporarily hack up debug counters in radeon_bo_pin and radeon_bo_unpin functions. Then I connected two monitor one of which had 1600x1200 for preferred mode, while the other had preferred mode of the other monitor was 1280x720. Then I repetitively unplugged and plugged back the 1600x1200 monitor and ran 'xrandr --auto' between each plug/unplug operation. Without the fix, the number of pinned buffers grew at each plug/unplug operation. With the fix, the counter remained stable throughout the test.
Note that I only effectively tested the first patch, because I don't have any GPUs that are old enough to use the legacy CRTC path in the driver. Still, based on my findings for AtomBIOS GPUs, I believe that the second patch is correct and necessary.
regards,
Ilija
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Ilija Hadzic