[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.

[Deng, Emily]

> -----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.

Best Wishes,
Emily Deng
> 
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer