Hi Michel, Thanks, I still couldn't see the issue that use a variable to calculate the vblank count when vsync timer interrupt occurs. I just think it only emulates the hardware behavior. And the code change will only in virtual display files which won't touch drm layer. I incline to not modify the code in drm layer or amdgpu other codes, because it may lead to other issues .
Best Wishes, Emily Deng
-----Original Message----- From: Michel Dänzer [mailto:michel@daenzer.net] Sent: Tuesday, August 16, 2016 11:12 AM To: Deng, Emily Emily.Deng@amd.com Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: For virtual_display feature, define a variable for vblank count of cpu vsync timer.
On 15/08/16 03:45 PM, Deng, Emily wrote:
From: Michel Dänzer [mailto:michel@daenzer.net] Sent: Monday, August 15, 2016 9:46 AM On 11/08/16 12:46 PM, Emily Deng wrote:
The adev->ddev->vblank[crtc].count couldn't be used here, so define another variable to compute the vblank count.
Signed-off-by: Emily Deng Emily.Deng@amd.com
[...]
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c index 2ce5f90..d616ab9 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c @@ -58,7 +58,7 @@ static u32 dce_virtual_vblank_get_counter(struct
amdgpu_device *adev, int crtc)
if (crtc >= adev->mode_info.num_crtc) return 0; else
return adev->ddev->vblank[crtc].count;
return adev->mode_info.timer_vblank_count;}
AFAIK the vblank_get_counter hook is supposed to always return 0 when there's no hardware frame counter which can be used. That's what drm_vblank_no_hw_counter was created for.
[[EmilyD]] Sorry, I don't know much about drm_vblank_no_hw_counter, can it support vsync when return to 0?
That's its purpose. BTW, I realized in the meantime that we can't use drm_vblank_no_hw_counter directly, because there's a single struct drm_driver used by all amdgpu driver instances.
You mentioned internally that you ran into trouble when trying this though. Please provide more information about that, e.g.: Which base kernel version did you test it with? What values did the DRM_IOCTL_WAIT_VBLANK ioctl return to userspace? ...
[[EmilyD]] I run the driver on kernel version 4.6, and run glxgears with vsync enabled. It is hard to detail the issue, I will try my best to
description the issue I found.
After I double checked, it is not segment fault in libGL.so when run glxgears with vsync, but the glxgears will be stucked in waiting for the before swap buffers to complete. This is because when enable vsync, every time swap buffers, the DDX driver will call DRM_IOCTL_WAIT_VBLANK to queue the vblank event, and the
vbl.request.sequence will be set to current_vblank_count + swap_interval. Then in kernel driver, when timer interrupt occurs, it will call drm_handle_vblank_events, it will call drm_vblank_count_and_time to get current seq, and only seq >= vbl.request.sequence, then will call send_vblank_event. So it will never call send_vblank_event.
For example, the DDX driver call DRM_IOCTL_WAIT_VBLANK , then kernel driver will call drm_queue_vblank_event, and current vblank_count is 1 (As we only return 0 in vblank_get_counter, so the vblank_count will never change except calling drm_reset_vblank_timestamp which will make adev->ddev->vblank[crtc].count++), and swap_interval is 1, then adev->ddev->vbl.request.sequence will be 2, but the adev->ddev->drm_vblank_count_and_time will always return 1 except calling drm_reset_vblank_timestamp(The function drm_reset_vblank_timestamp will only be called in
drm_vblank_post_modeset and drm_vblank_on ), so the send_vblank_event will never be called, and swap buffers won't complete, so the glxgears will be stucked.
Looking at the drm_update_vblank_count() code, you also need to do the following in the virtual DCE case:
- Set dev->max_vblank_count = 0
- Make amdgpu_get_vblank_timestamp_kms either return values based on when the virtual vblank interrupt timer last fired, or just return a negative error code immediately, instead of calling drm_calc_vbltimestamp_from_scanoutpos
- Make amdgpu_get_vblank_counter_kms take the else path (or just return 0 immediately), not run any of the scanout position related code
-- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer