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
