[PATCH] drm/amdgpu: For virtual_display feature, the vblank_get_counter hook is always return 0 when there's no hardware frame counter which can be used.

[Michel Dänzer]

On 17/08/16 02:47 PM, Deng, Emily wrote:
>> -----Original Message-----
>> From: Michel Dänzer [mailto:michel at daenzer.net]
>> Sent: Wednesday, August 17, 2016 11:50 AM
>> To: Deng, Emily <Emily.Deng at amd.com>
>> Cc: amd-gfx at lists.freedesktop.org
>> Subject: Re: [PATCH] drm/amdgpu: For virtual_display feature, the
>> vblank_get_counter hook is always return 0 when there's no hardware frame
>> counter which can be used.
>>
>> On 16/08/16 07:15 PM, Emily Deng wrote:
>>> Signed-off-by: Emily Deng <Emily.Deng at amd.com>
>>
>> Please change the shortlog to be no longer than ~72 characters. Maybe
>> something like this for the commit log:
>>
>> drm/amdgpu: Hardcode virtual DCE vblank / scanout position return values
>>
>> By hardcoding 0 for the vblank counter and -EINVAL for the scanout position
>> return value, we signal to the core DRM code that there are no hardware
>> counters we can use for these.
>>
> [[EmilyD]] Thanks, will modify this.
>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>>> b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>>> index 2ce5f90..85f14a6 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>>> @@ -55,10 +55,7 @@ static void dce_virtual_vblank_wait(struct
>>> amdgpu_device *adev, int crtc)
>>>
>>>  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 0;
>>>  }
>>>
>>>  static void dce_virtual_page_flip(struct amdgpu_device *adev, @@
>>> -70,13 +67,10 @@ static void dce_virtual_page_flip(struct
>>> amdgpu_device *adev,  static int dce_virtual_crtc_get_scanoutpos(struct
>> amdgpu_device *adev, int crtc,
>>>  					u32 *vbl, u32 *position)
>>>  {
>>> -	if ((crtc < 0) || (crtc >= adev->mode_info.num_crtc))
>>> -		return -EINVAL;
>>> -
>>>  	*vbl = 0;
>>>  	*position = 0;
>>>
>>> -	return 0;
>>> +	return -EINVAL;
>>>  }
>>
>> Would it be possible to add short-circuits for the virtual display case in
>> amdgpu_get_crtc_scanoutpos and amdgpu_get_vblank_counter_kms, so they
>> don't do any unnecessary work, and remove dce_virtual_vblank_get_counter
>> and dce_virtual_crtc_get_scanoutpos?
>>
> [[EmilyD]] Hi Michel, I am inclined not to add virtual display relevant code in amdgpu other place except that
> must do case. And don't want to break the amdgpu code level.

Okay. With the commit log fixed, the change is

Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer