Re: [PATCH] drm/qxl: Use drm_vblank_count()

[Mario Kleiner <mario.kleiner.de@gmail.com>]

On 12/15/2014 04:56 PM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The QXL driver duplicates part of the core's drm_vblank_count(), so it
> might as well use the core's variant for the extra goodies.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>   drivers/gpu/drm/qxl/qxl_drv.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 1d9b80c91a15..497024461a3c 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -196,11 +196,6 @@ static int qxl_pm_restore(struct device *dev)
>   	return qxl_drm_resume(drm_dev, false);
>   }
>   
> -static u32 qxl_noop_get_vblank_counter(struct drm_device *dev, int crtc)
> -{
> -	return dev->vblank[crtc].count.counter;
> -}
> -
>   static int qxl_noop_enable_vblank(struct drm_device *dev, int crtc)
>   {
>   	return 0;
> @@ -231,7 +226,7 @@ static struct drm_driver qxl_driver = {
>   			   DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED,
>   	.load = qxl_driver_load,
>   	.unload = qxl_driver_unload,
> -	.get_vblank_counter = qxl_noop_get_vblank_counter,
> +	.get_vblank_counter = drm_vblank_count,
>   	.enable_vblank = qxl_noop_enable_vblank,
>   	.disable_vblank = qxl_noop_disable_vblank,
>   

Hi

That doesn't really help, although it doesn't hurt either. Just wanted 
to point out that both the old and new method implement a no-op. The 
get_vblank_counter() driver function is meant to implement a hardware 
vblank counter query. It's only use case atm. is to reinitialize the 
dev->vblank[crtc].count.counter counter returned by drm_vblank_count().

The most honest implementation if there isn't any way to get a hw vblank 
count would be to just "return 0;" - Same net effect, but at least a 
marker in the code that there is future work to do.

I think a better solution would be if we wouldn't require 
.get_vblank_counter to be non-NULL, don't fake implement it in 
kms-drivers which can't do it, and make the drm core deal with lack of 
hw counter queries, e.g., by not disabling vblank irqs.

-mario

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel