[PATCH] drm/amdgpu: For virtual_display feature, define a variable for vblank count of cpu vsync timer.

[Deng, Emily]

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 at daenzer.net]
> Sent: Tuesday, August 16, 2016 11:12 AM
> To: Deng, Emily <Emily.Deng at amd.com>
> Cc: amd-gfx at lists.freedesktop.org; dri-devel at 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 at 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 at 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