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

[Michel Dänzer]

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 vbl.request.sequence will be 2, but the 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