[1/2] drm: Stop using drm_vblank_count() as the hw frame counter
diff mbox

Message ID 1443620809-28329-1-git-send-email-ville.syrjala@linux.intel.com
State New
Headers show

Commit Message

Ville Syrjälä Sept. 30, 2015, 1:46 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_vblank_count() returns the software counter. We should not pretend
it's the hw counter since we use the hw counter to figuere out what the
software counter value should be. So instead provide a new function
drm_vblank_no_hw_counter() for drivers that don't have a real hw
counter. The new function simply returns 0, which is about the only
thing it can do.

Cc: Vincent Abriou <vincent.abriou@st.com>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/armada/armada_drv.c          |  2 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
 drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
 drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
 drivers/gpu/drm/msm/msm_drv.c                |  2 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c        |  2 +-
 drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c        |  2 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c  |  2 +-
 drivers/gpu/drm/shmobile/shmob_drm_drv.c     |  2 +-
 drivers/gpu/drm/sti/sti_drv.c                |  2 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
 include/drm/drmP.h                           |  1 +
 15 files changed, 31 insertions(+), 13 deletions(-)

Comments

Daniel Vetter Sept. 30, 2015, 2:08 p.m. UTC | #1
On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> drm_vblank_count() returns the software counter. We should not pretend
> it's the hw counter since we use the hw counter to figuere out what the
> software counter value should be. So instead provide a new function
> drm_vblank_no_hw_counter() for drivers that don't have a real hw
> counter. The new function simply returns 0, which is about the only
> thing it can do.

Shouldn't we instead just make the get_vblank_counter hook optional?
-Daniel

> 
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/armada/armada_drv.c          |  2 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
>  drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
>  drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
>  drivers/gpu/drm/msm/msm_drv.c                |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_drm.c        |  2 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c        |  2 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c  |  2 +-
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c     |  2 +-
>  drivers/gpu/drm/sti/sti_drv.c                |  2 +-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
>  include/drm/drmP.h                           |  1 +
>  15 files changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> index 225034b..464a13f 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -300,7 +300,7 @@ static struct drm_driver armada_drm_driver = {
>  	.lastclose		= armada_drm_lastclose,
>  	.unload			= armada_drm_unload,
>  	.set_busid		= drm_platform_set_busid,
> -	.get_vblank_counter	= drm_vblank_count,
> +	.get_vblank_counter	= drm_vblank_no_hw_counter,
>  	.enable_vblank		= armada_drm_enable_vblank,
>  	.disable_vblank		= armada_drm_disable_vblank,
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 8bc62ec..2eb1c66 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -697,7 +697,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>  	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>  	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
>  	.irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
> -	.get_vblank_counter = drm_vblank_count,
> +	.get_vblank_counter = drm_vblank_no_hw_counter,
>  	.enable_vblank = atmel_hlcdc_dc_enable_vblank,
>  	.disable_vblank = atmel_hlcdc_dc_disable_vblank,
>  	.gem_free_object = drm_gem_cma_free_object,
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index ed2394e..7d70b7c 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
>  	return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
>  }
>  EXPORT_SYMBOL(drm_crtc_handle_vblank);
> +
> +/**
> + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
> + * @dev: DRM device
> + * @pipe: CRTC for which to read the counter
> + *
> + * Drivers can plug this into the .get_vblank_counter() function if
> + * there is no useable hardware frame counter available.
> + *
> + * Returns:
> + * 0
> + */
> +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_vblank_no_hw_counter);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index f0a5839..fb9cfc5 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -447,7 +447,7 @@ static struct drm_driver exynos_drm_driver = {
>  	.lastclose		= exynos_drm_lastclose,
>  	.postclose		= exynos_drm_postclose,
>  	.set_busid		= drm_platform_set_busid,
> -	.get_vblank_counter	= drm_vblank_count,
> +	.get_vblank_counter	= drm_vblank_no_hw_counter,
>  	.enable_vblank		= exynos_drm_crtc_enable_vblank,
>  	.disable_vblank		= exynos_drm_crtc_disable_vblank,
>  	.gem_free_object	= exynos_drm_gem_free_object,
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> index 9a8e2da..1f4e8b9 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -192,7 +192,7 @@ static struct drm_driver fsl_dcu_drm_driver = {
>  	.unload			= fsl_dcu_unload,
>  	.preclose		= fsl_dcu_drm_preclose,
>  	.irq_handler		= fsl_dcu_drm_irq,
> -	.get_vblank_counter	= drm_vblank_count,
> +	.get_vblank_counter	= drm_vblank_no_hw_counter,
>  	.enable_vblank		= fsl_dcu_drm_enable_vblank,
>  	.disable_vblank		= fsl_dcu_drm_disable_vblank,
>  	.gem_free_object	= drm_gem_cma_free_object,
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> index 74f505b..18a6642 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -487,7 +487,7 @@ static struct drm_driver imx_drm_driver = {
>  	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
>  	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
>  	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
> -	.get_vblank_counter	= drm_vblank_count,
> +	.get_vblank_counter	= drm_vblank_no_hw_counter,
>  	.enable_vblank		= imx_drm_enable_vblank,
>  	.disable_vblank		= imx_drm_disable_vblank,
>  	.ioctls			= imx_drm_ioctls,
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 0339c5d..3eba23f 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -978,7 +978,7 @@ static struct drm_driver msm_driver = {
>  	.irq_preinstall     = msm_irq_preinstall,
>  	.irq_postinstall    = msm_irq_postinstall,
>  	.irq_uninstall      = msm_irq_uninstall,
> -	.get_vblank_counter = drm_vblank_count,
> +	.get_vblank_counter = drm_vblank_no_hw_counter,
>  	.enable_vblank      = msm_enable_vblank,
>  	.disable_vblank     = msm_disable_vblank,
>  	.gem_free_object    = msm_gem_free_object,
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index ccefb64..2416c7d 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -934,7 +934,7 @@ driver_stub = {
>  	.debugfs_cleanup = nouveau_debugfs_takedown,
>  #endif
>  
> -	.get_vblank_counter = drm_vblank_count,
> +	.get_vblank_counter = drm_vblank_no_hw_counter,
>  	.enable_vblank = nouveau_display_vblank_enable,
>  	.disable_vblank = nouveau_display_vblank_disable,
>  	.get_scanout_position = nouveau_display_scanoutpos,
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index d685e23..4d58934 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -839,7 +839,7 @@ static struct drm_driver omap_drm_driver = {
>  	.preclose = dev_preclose,
>  	.postclose = dev_postclose,
>  	.set_busid = drm_platform_set_busid,
> -	.get_vblank_counter = drm_vblank_count,
> +	.get_vblank_counter = drm_vblank_no_hw_counter,
>  	.enable_vblank = omap_irq_enable_vblank,
>  	.disable_vblank = omap_irq_disable_vblank,
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 780ca11..3608e88 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -259,7 +259,7 @@ static struct drm_driver rcar_du_driver = {
>  	.preclose		= rcar_du_preclose,
>  	.lastclose		= rcar_du_lastclose,
>  	.set_busid		= drm_platform_set_busid,
> -	.get_vblank_counter	= drm_vblank_count,
> +	.get_vblank_counter	= drm_vblank_no_hw_counter,
>  	.enable_vblank		= rcar_du_enable_vblank,
>  	.disable_vblank		= rcar_du_disable_vblank,
>  	.gem_free_object	= drm_gem_cma_free_object,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 9a0c291..b468add 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -277,7 +277,7 @@ static struct drm_driver rockchip_drm_driver = {
>  	.load			= rockchip_drm_load,
>  	.unload			= rockchip_drm_unload,
>  	.lastclose		= rockchip_drm_lastclose,
> -	.get_vblank_counter	= drm_vblank_count,
> +	.get_vblank_counter	= drm_vblank_no_hw_counter,
>  	.enable_vblank		= rockchip_drm_crtc_enable_vblank,
>  	.disable_vblank		= rockchip_drm_crtc_disable_vblank,
>  	.gem_vm_ops		= &rockchip_drm_vm_ops,
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> index 666321d..108a03b 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> @@ -269,7 +269,7 @@ static struct drm_driver shmob_drm_driver = {
>  	.preclose		= shmob_drm_preclose,
>  	.set_busid		= drm_platform_set_busid,
>  	.irq_handler		= shmob_drm_irq,
> -	.get_vblank_counter	= drm_vblank_count,
> +	.get_vblank_counter	= drm_vblank_no_hw_counter,
>  	.enable_vblank		= shmob_drm_enable_vblank,
>  	.disable_vblank		= shmob_drm_disable_vblank,
>  	.gem_free_object	= drm_gem_cma_free_object,
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index 9f85988..f846996 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -201,7 +201,7 @@ static struct drm_driver sti_driver = {
>  	.dumb_destroy = drm_gem_dumb_destroy,
>  	.fops = &sti_driver_fops,
>  
> -	.get_vblank_counter = drm_vblank_count,
> +	.get_vblank_counter = drm_vblank_no_hw_counter,
>  	.enable_vblank = sti_crtc_enable_vblank,
>  	.disable_vblank = sti_crtc_disable_vblank,
>  
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 0f283a3..7e0b0c5 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -563,7 +563,7 @@ static struct drm_driver tilcdc_driver = {
>  	.irq_preinstall     = tilcdc_irq_preinstall,
>  	.irq_postinstall    = tilcdc_irq_postinstall,
>  	.irq_uninstall      = tilcdc_irq_uninstall,
> -	.get_vblank_counter = drm_vblank_count,
> +	.get_vblank_counter = drm_vblank_no_hw_counter,
>  	.enable_vblank      = tilcdc_enable_vblank,
>  	.disable_vblank     = tilcdc_disable_vblank,
>  	.gem_free_object    = drm_gem_cma_free_object,
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index d0251ac..f563333 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -952,6 +952,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
>  extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
>  extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
>  extern void drm_vblank_cleanup(struct drm_device *dev);
> +extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe);
>  
>  extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>  						 unsigned int pipe, int *max_error,
> -- 
> 2.4.6
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Ville Syrjälä Sept. 30, 2015, 2:14 p.m. UTC | #2
On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:
> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > drm_vblank_count() returns the software counter. We should not pretend
> > it's the hw counter since we use the hw counter to figuere out what the
> > software counter value should be. So instead provide a new function
> > drm_vblank_no_hw_counter() for drivers that don't have a real hw
> > counter. The new function simply returns 0, which is about the only
> > thing it can do.
> 
> Shouldn't we instead just make the get_vblank_counter hook optional?

Perhaps. But maybe this way would encourage people to go look for a
hw frame counter in their hardware?

> -Daniel
> 
> > 
> > Cc: Vincent Abriou <vincent.abriou@st.com>
> > Cc: Thierry Reding <treding@nvidia.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/armada/armada_drv.c          |  2 +-
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
> >  drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
> >  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
> >  drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
> >  drivers/gpu/drm/msm/msm_drv.c                |  2 +-
> >  drivers/gpu/drm/nouveau/nouveau_drm.c        |  2 +-
> >  drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c        |  2 +-
> >  drivers/gpu/drm/rockchip/rockchip_drm_drv.c  |  2 +-
> >  drivers/gpu/drm/shmobile/shmob_drm_drv.c     |  2 +-
> >  drivers/gpu/drm/sti/sti_drv.c                |  2 +-
> >  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
> >  include/drm/drmP.h                           |  1 +
> >  15 files changed, 31 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> > index 225034b..464a13f 100644
> > --- a/drivers/gpu/drm/armada/armada_drv.c
> > +++ b/drivers/gpu/drm/armada/armada_drv.c
> > @@ -300,7 +300,7 @@ static struct drm_driver armada_drm_driver = {
> >  	.lastclose		= armada_drm_lastclose,
> >  	.unload			= armada_drm_unload,
> >  	.set_busid		= drm_platform_set_busid,
> > -	.get_vblank_counter	= drm_vblank_count,
> > +	.get_vblank_counter	= drm_vblank_no_hw_counter,
> >  	.enable_vblank		= armada_drm_enable_vblank,
> >  	.disable_vblank		= armada_drm_disable_vblank,
> >  #ifdef CONFIG_DEBUG_FS
> > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > index 8bc62ec..2eb1c66 100644
> > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > @@ -697,7 +697,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
> >  	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
> >  	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
> >  	.irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
> > -	.get_vblank_counter = drm_vblank_count,
> > +	.get_vblank_counter = drm_vblank_no_hw_counter,
> >  	.enable_vblank = atmel_hlcdc_dc_enable_vblank,
> >  	.disable_vblank = atmel_hlcdc_dc_disable_vblank,
> >  	.gem_free_object = drm_gem_cma_free_object,
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index ed2394e..7d70b7c 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
> >  	return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
> >  }
> >  EXPORT_SYMBOL(drm_crtc_handle_vblank);
> > +
> > +/**
> > + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
> > + * @dev: DRM device
> > + * @pipe: CRTC for which to read the counter
> > + *
> > + * Drivers can plug this into the .get_vblank_counter() function if
> > + * there is no useable hardware frame counter available.
> > + *
> > + * Returns:
> > + * 0
> > + */
> > +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
> > +{
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_vblank_no_hw_counter);
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > index f0a5839..fb9cfc5 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > @@ -447,7 +447,7 @@ static struct drm_driver exynos_drm_driver = {
> >  	.lastclose		= exynos_drm_lastclose,
> >  	.postclose		= exynos_drm_postclose,
> >  	.set_busid		= drm_platform_set_busid,
> > -	.get_vblank_counter	= drm_vblank_count,
> > +	.get_vblank_counter	= drm_vblank_no_hw_counter,
> >  	.enable_vblank		= exynos_drm_crtc_enable_vblank,
> >  	.disable_vblank		= exynos_drm_crtc_disable_vblank,
> >  	.gem_free_object	= exynos_drm_gem_free_object,
> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > index 9a8e2da..1f4e8b9 100644
> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > @@ -192,7 +192,7 @@ static struct drm_driver fsl_dcu_drm_driver = {
> >  	.unload			= fsl_dcu_unload,
> >  	.preclose		= fsl_dcu_drm_preclose,
> >  	.irq_handler		= fsl_dcu_drm_irq,
> > -	.get_vblank_counter	= drm_vblank_count,
> > +	.get_vblank_counter	= drm_vblank_no_hw_counter,
> >  	.enable_vblank		= fsl_dcu_drm_enable_vblank,
> >  	.disable_vblank		= fsl_dcu_drm_disable_vblank,
> >  	.gem_free_object	= drm_gem_cma_free_object,
> > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> > index 74f505b..18a6642 100644
> > --- a/drivers/gpu/drm/imx/imx-drm-core.c
> > +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> > @@ -487,7 +487,7 @@ static struct drm_driver imx_drm_driver = {
> >  	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
> >  	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
> >  	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
> > -	.get_vblank_counter	= drm_vblank_count,
> > +	.get_vblank_counter	= drm_vblank_no_hw_counter,
> >  	.enable_vblank		= imx_drm_enable_vblank,
> >  	.disable_vblank		= imx_drm_disable_vblank,
> >  	.ioctls			= imx_drm_ioctls,
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > index 0339c5d..3eba23f 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -978,7 +978,7 @@ static struct drm_driver msm_driver = {
> >  	.irq_preinstall     = msm_irq_preinstall,
> >  	.irq_postinstall    = msm_irq_postinstall,
> >  	.irq_uninstall      = msm_irq_uninstall,
> > -	.get_vblank_counter = drm_vblank_count,
> > +	.get_vblank_counter = drm_vblank_no_hw_counter,
> >  	.enable_vblank      = msm_enable_vblank,
> >  	.disable_vblank     = msm_disable_vblank,
> >  	.gem_free_object    = msm_gem_free_object,
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > index ccefb64..2416c7d 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > @@ -934,7 +934,7 @@ driver_stub = {
> >  	.debugfs_cleanup = nouveau_debugfs_takedown,
> >  #endif
> >  
> > -	.get_vblank_counter = drm_vblank_count,
> > +	.get_vblank_counter = drm_vblank_no_hw_counter,
> >  	.enable_vblank = nouveau_display_vblank_enable,
> >  	.disable_vblank = nouveau_display_vblank_disable,
> >  	.get_scanout_position = nouveau_display_scanoutpos,
> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> > index d685e23..4d58934 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> > @@ -839,7 +839,7 @@ static struct drm_driver omap_drm_driver = {
> >  	.preclose = dev_preclose,
> >  	.postclose = dev_postclose,
> >  	.set_busid = drm_platform_set_busid,
> > -	.get_vblank_counter = drm_vblank_count,
> > +	.get_vblank_counter = drm_vblank_no_hw_counter,
> >  	.enable_vblank = omap_irq_enable_vblank,
> >  	.disable_vblank = omap_irq_disable_vblank,
> >  #ifdef CONFIG_DEBUG_FS
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > index 780ca11..3608e88 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > @@ -259,7 +259,7 @@ static struct drm_driver rcar_du_driver = {
> >  	.preclose		= rcar_du_preclose,
> >  	.lastclose		= rcar_du_lastclose,
> >  	.set_busid		= drm_platform_set_busid,
> > -	.get_vblank_counter	= drm_vblank_count,
> > +	.get_vblank_counter	= drm_vblank_no_hw_counter,
> >  	.enable_vblank		= rcar_du_enable_vblank,
> >  	.disable_vblank		= rcar_du_disable_vblank,
> >  	.gem_free_object	= drm_gem_cma_free_object,
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > index 9a0c291..b468add 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > @@ -277,7 +277,7 @@ static struct drm_driver rockchip_drm_driver = {
> >  	.load			= rockchip_drm_load,
> >  	.unload			= rockchip_drm_unload,
> >  	.lastclose		= rockchip_drm_lastclose,
> > -	.get_vblank_counter	= drm_vblank_count,
> > +	.get_vblank_counter	= drm_vblank_no_hw_counter,
> >  	.enable_vblank		= rockchip_drm_crtc_enable_vblank,
> >  	.disable_vblank		= rockchip_drm_crtc_disable_vblank,
> >  	.gem_vm_ops		= &rockchip_drm_vm_ops,
> > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> > index 666321d..108a03b 100644
> > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> > @@ -269,7 +269,7 @@ static struct drm_driver shmob_drm_driver = {
> >  	.preclose		= shmob_drm_preclose,
> >  	.set_busid		= drm_platform_set_busid,
> >  	.irq_handler		= shmob_drm_irq,
> > -	.get_vblank_counter	= drm_vblank_count,
> > +	.get_vblank_counter	= drm_vblank_no_hw_counter,
> >  	.enable_vblank		= shmob_drm_enable_vblank,
> >  	.disable_vblank		= shmob_drm_disable_vblank,
> >  	.gem_free_object	= drm_gem_cma_free_object,
> > diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> > index 9f85988..f846996 100644
> > --- a/drivers/gpu/drm/sti/sti_drv.c
> > +++ b/drivers/gpu/drm/sti/sti_drv.c
> > @@ -201,7 +201,7 @@ static struct drm_driver sti_driver = {
> >  	.dumb_destroy = drm_gem_dumb_destroy,
> >  	.fops = &sti_driver_fops,
> >  
> > -	.get_vblank_counter = drm_vblank_count,
> > +	.get_vblank_counter = drm_vblank_no_hw_counter,
> >  	.enable_vblank = sti_crtc_enable_vblank,
> >  	.disable_vblank = sti_crtc_disable_vblank,
> >  
> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > index 0f283a3..7e0b0c5 100644
> > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > @@ -563,7 +563,7 @@ static struct drm_driver tilcdc_driver = {
> >  	.irq_preinstall     = tilcdc_irq_preinstall,
> >  	.irq_postinstall    = tilcdc_irq_postinstall,
> >  	.irq_uninstall      = tilcdc_irq_uninstall,
> > -	.get_vblank_counter = drm_vblank_count,
> > +	.get_vblank_counter = drm_vblank_no_hw_counter,
> >  	.enable_vblank      = tilcdc_enable_vblank,
> >  	.disable_vblank     = tilcdc_disable_vblank,
> >  	.gem_free_object    = drm_gem_cma_free_object,
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index d0251ac..f563333 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -952,6 +952,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
> >  extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
> >  extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
> >  extern void drm_vblank_cleanup(struct drm_device *dev);
> > +extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe);
> >  
> >  extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
> >  						 unsigned int pipe, int *max_error,
> > -- 
> > 2.4.6
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Vincent Abriou Sept. 30, 2015, 2:44 p.m. UTC | #3
> -----Original Message-----

> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf

> Of Ville Syrjälä

> Sent: mercredi 30 septembre 2015 16:15

> To: Daniel Vetter

> Cc: Thierry Reding; dri-devel@lists.freedesktop.org

> Subject: Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw

> frame counter

> 

> On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:

> > On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com

> wrote:

> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>

> > >

> > > drm_vblank_count() returns the software counter. We should not

> > > pretend it's the hw counter since we use the hw counter to figuere

> > > out what the software counter value should be. So instead provide a

> > > new function

> > > drm_vblank_no_hw_counter() for drivers that don't have a real hw

> > > counter. The new function simply returns 0, which is about the only

> > > thing it can do.

> >

> > Shouldn't we instead just make the get_vblank_counter hook optional?

> 

> Perhaps. But maybe this way would encourage people to go look for a hw

> frame counter in their hardware?


I agree, I think it is better to have such explicit helpers.
Going into that warning make me check if the STI hardware has such vblank hw counter.

Vincent

> 

> > -Daniel

> >

> > >

> > > Cc: Vincent Abriou <vincent.abriou@st.com>

> > > Cc: Thierry Reding <treding@nvidia.com>

> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> > > ---

> > >  drivers/gpu/drm/armada/armada_drv.c          |  2 +-

> > >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-

> > >  drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++

> > >  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-

> > >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-

> > >  drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-

> > >  drivers/gpu/drm/msm/msm_drv.c                |  2 +-

> > >  drivers/gpu/drm/nouveau/nouveau_drm.c        |  2 +-

> > >  drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-

> > >  drivers/gpu/drm/rcar-du/rcar_du_drv.c        |  2 +-

> > >  drivers/gpu/drm/rockchip/rockchip_drm_drv.c  |  2 +-

> > >  drivers/gpu/drm/shmobile/shmob_drm_drv.c     |  2 +-

> > >  drivers/gpu/drm/sti/sti_drv.c                |  2 +-

> > >  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-

> > >  include/drm/drmP.h                           |  1 +

> > >  15 files changed, 31 insertions(+), 13 deletions(-)

> > >

> > > diff --git a/drivers/gpu/drm/armada/armada_drv.c

> > > b/drivers/gpu/drm/armada/armada_drv.c

> > > index 225034b..464a13f 100644

> > > --- a/drivers/gpu/drm/armada/armada_drv.c

> > > +++ b/drivers/gpu/drm/armada/armada_drv.c

> > > @@ -300,7 +300,7 @@ static struct drm_driver armada_drm_driver = {

> > >  	.lastclose		= armada_drm_lastclose,

> > >  	.unload			= armada_drm_unload,

> > >  	.set_busid		= drm_platform_set_busid,

> > > -	.get_vblank_counter	= drm_vblank_count,

> > > +	.get_vblank_counter	= drm_vblank_no_hw_counter,

> > >  	.enable_vblank		= armada_drm_enable_vblank,

> > >  	.disable_vblank		= armada_drm_disable_vblank,

> > >  #ifdef CONFIG_DEBUG_FS

> > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c

> > > b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c

> > > index 8bc62ec..2eb1c66 100644

> > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c

> > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c

> > > @@ -697,7 +697,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {

> > >  	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,

> > >  	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,

> > >  	.irq_uninstall = atmel_hlcdc_dc_irq_uninstall,

> > > -	.get_vblank_counter = drm_vblank_count,

> > > +	.get_vblank_counter = drm_vblank_no_hw_counter,

> > >  	.enable_vblank = atmel_hlcdc_dc_enable_vblank,

> > >  	.disable_vblank = atmel_hlcdc_dc_disable_vblank,

> > >  	.gem_free_object = drm_gem_cma_free_object, diff --git

> > > a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index

> > > ed2394e..7d70b7c 100644

> > > --- a/drivers/gpu/drm/drm_irq.c

> > > +++ b/drivers/gpu/drm/drm_irq.c

> > > @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc

> *crtc)

> > >  	return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));  }

> > > EXPORT_SYMBOL(drm_crtc_handle_vblank);

> > > +

> > > +/**

> > > + * drm_vblank_no_hw_counter - "No hw counter" implementation of

> > > +.get_vblank_counter()

> > > + * @dev: DRM device

> > > + * @pipe: CRTC for which to read the counter

> > > + *

> > > + * Drivers can plug this into the .get_vblank_counter() function if

> > > + * there is no useable hardware frame counter available.

> > > + *

> > > + * Returns:

> > > + * 0

> > > + */

> > > +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe) {

> > > +	return 0;

> > > +}

> > > +EXPORT_SYMBOL(drm_vblank_no_hw_counter);

> > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c

> > > b/drivers/gpu/drm/exynos/exynos_drm_drv.c

> > > index f0a5839..fb9cfc5 100644

> > > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c

> > > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c

> > > @@ -447,7 +447,7 @@ static struct drm_driver exynos_drm_driver = {

> > >  	.lastclose		= exynos_drm_lastclose,

> > >  	.postclose		= exynos_drm_postclose,

> > >  	.set_busid		= drm_platform_set_busid,

> > > -	.get_vblank_counter	= drm_vblank_count,

> > > +	.get_vblank_counter	= drm_vblank_no_hw_counter,

> > >  	.enable_vblank		= exynos_drm_crtc_enable_vblank,

> > >  	.disable_vblank		= exynos_drm_crtc_disable_vblank,

> > >  	.gem_free_object	= exynos_drm_gem_free_object,

> > > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c

> > > b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c

> > > index 9a8e2da..1f4e8b9 100644

> > > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c

> > > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c

> > > @@ -192,7 +192,7 @@ static struct drm_driver fsl_dcu_drm_driver = {

> > >  	.unload			= fsl_dcu_unload,

> > >  	.preclose		= fsl_dcu_drm_preclose,

> > >  	.irq_handler		= fsl_dcu_drm_irq,

> > > -	.get_vblank_counter	= drm_vblank_count,

> > > +	.get_vblank_counter	= drm_vblank_no_hw_counter,

> > >  	.enable_vblank		= fsl_dcu_drm_enable_vblank,

> > >  	.disable_vblank		= fsl_dcu_drm_disable_vblank,

> > >  	.gem_free_object	= drm_gem_cma_free_object,

> > > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c

> > > b/drivers/gpu/drm/imx/imx-drm-core.c

> > > index 74f505b..18a6642 100644

> > > --- a/drivers/gpu/drm/imx/imx-drm-core.c

> > > +++ b/drivers/gpu/drm/imx/imx-drm-core.c

> > > @@ -487,7 +487,7 @@ static struct drm_driver imx_drm_driver = {

> > >  	.gem_prime_vmap		= drm_gem_cma_prime_vmap,

> > >  	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,

> > >  	.gem_prime_mmap		= drm_gem_cma_prime_mmap,

> > > -	.get_vblank_counter	= drm_vblank_count,

> > > +	.get_vblank_counter	= drm_vblank_no_hw_counter,

> > >  	.enable_vblank		= imx_drm_enable_vblank,

> > >  	.disable_vblank		= imx_drm_disable_vblank,

> > >  	.ioctls			= imx_drm_ioctls,

> > > diff --git a/drivers/gpu/drm/msm/msm_drv.c

> > > b/drivers/gpu/drm/msm/msm_drv.c index 0339c5d..3eba23f 100644

> > > --- a/drivers/gpu/drm/msm/msm_drv.c

> > > +++ b/drivers/gpu/drm/msm/msm_drv.c

> > > @@ -978,7 +978,7 @@ static struct drm_driver msm_driver = {

> > >  	.irq_preinstall     = msm_irq_preinstall,

> > >  	.irq_postinstall    = msm_irq_postinstall,

> > >  	.irq_uninstall      = msm_irq_uninstall,

> > > -	.get_vblank_counter = drm_vblank_count,

> > > +	.get_vblank_counter = drm_vblank_no_hw_counter,

> > >  	.enable_vblank      = msm_enable_vblank,

> > >  	.disable_vblank     = msm_disable_vblank,

> > >  	.gem_free_object    = msm_gem_free_object,

> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c

> > > b/drivers/gpu/drm/nouveau/nouveau_drm.c

> > > index ccefb64..2416c7d 100644

> > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c

> > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c

> > > @@ -934,7 +934,7 @@ driver_stub = {

> > >  	.debugfs_cleanup = nouveau_debugfs_takedown,  #endif

> > >

> > > -	.get_vblank_counter = drm_vblank_count,

> > > +	.get_vblank_counter = drm_vblank_no_hw_counter,

> > >  	.enable_vblank = nouveau_display_vblank_enable,

> > >  	.disable_vblank = nouveau_display_vblank_disable,

> > >  	.get_scanout_position = nouveau_display_scanoutpos, diff --git

> > > a/drivers/gpu/drm/omapdrm/omap_drv.c

> > > b/drivers/gpu/drm/omapdrm/omap_drv.c

> > > index d685e23..4d58934 100644

> > > --- a/drivers/gpu/drm/omapdrm/omap_drv.c

> > > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c

> > > @@ -839,7 +839,7 @@ static struct drm_driver omap_drm_driver = {

> > >  	.preclose = dev_preclose,

> > >  	.postclose = dev_postclose,

> > >  	.set_busid = drm_platform_set_busid,

> > > -	.get_vblank_counter = drm_vblank_count,

> > > +	.get_vblank_counter = drm_vblank_no_hw_counter,

> > >  	.enable_vblank = omap_irq_enable_vblank,

> > >  	.disable_vblank = omap_irq_disable_vblank,  #ifdef

> CONFIG_DEBUG_FS

> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c

> > > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c

> > > index 780ca11..3608e88 100644

> > > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c

> > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c

> > > @@ -259,7 +259,7 @@ static struct drm_driver rcar_du_driver = {

> > >  	.preclose		= rcar_du_preclose,

> > >  	.lastclose		= rcar_du_lastclose,

> > >  	.set_busid		= drm_platform_set_busid,

> > > -	.get_vblank_counter	= drm_vblank_count,

> > > +	.get_vblank_counter	= drm_vblank_no_hw_counter,

> > >  	.enable_vblank		= rcar_du_enable_vblank,

> > >  	.disable_vblank		= rcar_du_disable_vblank,

> > >  	.gem_free_object	= drm_gem_cma_free_object,

> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c

> > > b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c

> > > index 9a0c291..b468add 100644

> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c

> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c

> > > @@ -277,7 +277,7 @@ static struct drm_driver rockchip_drm_driver = {

> > >  	.load			= rockchip_drm_load,

> > >  	.unload			= rockchip_drm_unload,

> > >  	.lastclose		= rockchip_drm_lastclose,

> > > -	.get_vblank_counter	= drm_vblank_count,

> > > +	.get_vblank_counter	= drm_vblank_no_hw_counter,

> > >  	.enable_vblank		= rockchip_drm_crtc_enable_vblank,

> > >  	.disable_vblank		= rockchip_drm_crtc_disable_vblank,

> > >  	.gem_vm_ops		= &rockchip_drm_vm_ops,

> > > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c

> > > b/drivers/gpu/drm/shmobile/shmob_drm_drv.c

> > > index 666321d..108a03b 100644

> > > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c

> > > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c

> > > @@ -269,7 +269,7 @@ static struct drm_driver shmob_drm_driver = {

> > >  	.preclose		= shmob_drm_preclose,

> > >  	.set_busid		= drm_platform_set_busid,

> > >  	.irq_handler		= shmob_drm_irq,

> > > -	.get_vblank_counter	= drm_vblank_count,

> > > +	.get_vblank_counter	= drm_vblank_no_hw_counter,

> > >  	.enable_vblank		= shmob_drm_enable_vblank,

> > >  	.disable_vblank		= shmob_drm_disable_vblank,

> > >  	.gem_free_object	= drm_gem_cma_free_object,

> > > diff --git a/drivers/gpu/drm/sti/sti_drv.c

> > > b/drivers/gpu/drm/sti/sti_drv.c index 9f85988..f846996 100644

> > > --- a/drivers/gpu/drm/sti/sti_drv.c

> > > +++ b/drivers/gpu/drm/sti/sti_drv.c

> > > @@ -201,7 +201,7 @@ static struct drm_driver sti_driver = {

> > >  	.dumb_destroy = drm_gem_dumb_destroy,

> > >  	.fops = &sti_driver_fops,

> > >

> > > -	.get_vblank_counter = drm_vblank_count,

> > > +	.get_vblank_counter = drm_vblank_no_hw_counter,

> > >  	.enable_vblank = sti_crtc_enable_vblank,

> > >  	.disable_vblank = sti_crtc_disable_vblank,

> > >

> > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c

> > > b/drivers/gpu/drm/tilcdc/tilcdc_drv.c

> > > index 0f283a3..7e0b0c5 100644

> > > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c

> > > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c

> > > @@ -563,7 +563,7 @@ static struct drm_driver tilcdc_driver = {

> > >  	.irq_preinstall     = tilcdc_irq_preinstall,

> > >  	.irq_postinstall    = tilcdc_irq_postinstall,

> > >  	.irq_uninstall      = tilcdc_irq_uninstall,

> > > -	.get_vblank_counter = drm_vblank_count,

> > > +	.get_vblank_counter = drm_vblank_no_hw_counter,

> > >  	.enable_vblank      = tilcdc_enable_vblank,

> > >  	.disable_vblank     = tilcdc_disable_vblank,

> > >  	.gem_free_object    = drm_gem_cma_free_object,

> > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h index

> > > d0251ac..f563333 100644

> > > --- a/include/drm/drmP.h

> > > +++ b/include/drm/drmP.h

> > > @@ -952,6 +952,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc

> > > *crtc);  extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);

> > > extern void drm_crtc_vblank_on(struct drm_crtc *crtc);  extern void

> > > drm_vblank_cleanup(struct drm_device *dev);

> > > +extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, int

> > > +pipe);

> > >

> > >  extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device

> *dev,

> > >  						 unsigned int pipe, int

> *max_error,

> > > --

> > > 2.4.6

> > >

> > > _______________________________________________

> > > dri-devel mailing list

> > > dri-devel@lists.freedesktop.org

> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel

> >

> > --

> > Daniel Vetter

> > Software Engineer, Intel Corporation

> > http://blog.ffwll.ch

> 

> --

> Ville Syrjälä

> Intel OTC

> _______________________________________________

> dri-devel mailing list

> dri-devel@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Vincent Abriou Oct. 1, 2015, 9:10 a.m. UTC | #4
On 09/30/2015 04:14 PM, Ville Syrjälä wrote:
> On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:

>> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:

>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>

>>>

>>> drm_vblank_count() returns the software counter. We should not pretend

>>> it's the hw counter since we use the hw counter to figuere out what the

>>> software counter value should be. So instead provide a new function

>>> drm_vblank_no_hw_counter() for drivers that don't have a real hw

>>> counter. The new function simply returns 0, which is about the only

>>> thing it can do.

>>

>> Shouldn't we instead just make the get_vblank_counter hook optional?

>

> Perhaps. But maybe this way would encourage people to go look for a

> hw frame counter in their hardware?

>

>> -Daniel

>>

>>>

>>> Cc: Vincent Abriou <vincent.abriou@st.com>

>>> Cc: Thierry Reding <treding@nvidia.com>

>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>>> ---

>>>   drivers/gpu/drm/armada/armada_drv.c          |  2 +-

>>>   drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-

>>>   drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++

>>>   drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-

>>>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-


The 2 patches have been successfully tested on next-20151001. The 
warning message is gone and hw vblank counter stucks to 0.

Tested-by: Vincent Abriou <vincent.abriou@st.com>


Thanks for the patches

Vincent
Rob Clark Oct. 1, 2015, 3:25 p.m. UTC | #5
On Wed, Sep 30, 2015 at 10:14 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:
>> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > drm_vblank_count() returns the software counter. We should not pretend
>> > it's the hw counter since we use the hw counter to figuere out what the
>> > software counter value should be. So instead provide a new function
>> > drm_vblank_no_hw_counter() for drivers that don't have a real hw
>> > counter. The new function simply returns 0, which is about the only
>> > thing it can do.
>>
>> Shouldn't we instead just make the get_vblank_counter hook optional?
>
> Perhaps. But maybe this way would encourage people to go look for a
> hw frame counter in their hardware?

Well, I guess at this point we have more drm/kms drivers for hw
without hw frame counters..  maybe a helper that guestimates based on
time elapsed since last vbl irq would be useful..

BR,
-R

>> -Daniel
>>
>> >
>> > Cc: Vincent Abriou <vincent.abriou@st.com>
>> > Cc: Thierry Reding <treding@nvidia.com>
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/armada/armada_drv.c          |  2 +-
>> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
>> >  drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
>> >  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
>> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
>> >  drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
>> >  drivers/gpu/drm/msm/msm_drv.c                |  2 +-
>> >  drivers/gpu/drm/nouveau/nouveau_drm.c        |  2 +-
>> >  drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
>> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c        |  2 +-
>> >  drivers/gpu/drm/rockchip/rockchip_drm_drv.c  |  2 +-
>> >  drivers/gpu/drm/shmobile/shmob_drm_drv.c     |  2 +-
>> >  drivers/gpu/drm/sti/sti_drv.c                |  2 +-
>> >  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
>> >  include/drm/drmP.h                           |  1 +
>> >  15 files changed, 31 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
>> > index 225034b..464a13f 100644
>> > --- a/drivers/gpu/drm/armada/armada_drv.c
>> > +++ b/drivers/gpu/drm/armada/armada_drv.c
>> > @@ -300,7 +300,7 @@ static struct drm_driver armada_drm_driver = {
>> >     .lastclose              = armada_drm_lastclose,
>> >     .unload                 = armada_drm_unload,
>> >     .set_busid              = drm_platform_set_busid,
>> > -   .get_vblank_counter     = drm_vblank_count,
>> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> >     .enable_vblank          = armada_drm_enable_vblank,
>> >     .disable_vblank         = armada_drm_disable_vblank,
>> >  #ifdef CONFIG_DEBUG_FS
>> > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> > index 8bc62ec..2eb1c66 100644
>> > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> > @@ -697,7 +697,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>> >     .irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>> >     .irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
>> >     .irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
>> > -   .get_vblank_counter = drm_vblank_count,
>> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> >     .enable_vblank = atmel_hlcdc_dc_enable_vblank,
>> >     .disable_vblank = atmel_hlcdc_dc_disable_vblank,
>> >     .gem_free_object = drm_gem_cma_free_object,
>> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> > index ed2394e..7d70b7c 100644
>> > --- a/drivers/gpu/drm/drm_irq.c
>> > +++ b/drivers/gpu/drm/drm_irq.c
>> > @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
>> >     return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
>> >  }
>> >  EXPORT_SYMBOL(drm_crtc_handle_vblank);
>> > +
>> > +/**
>> > + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
>> > + * @dev: DRM device
>> > + * @pipe: CRTC for which to read the counter
>> > + *
>> > + * Drivers can plug this into the .get_vblank_counter() function if
>> > + * there is no useable hardware frame counter available.
>> > + *
>> > + * Returns:
>> > + * 0
>> > + */
>> > +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
>> > +{
>> > +   return 0;
>> > +}
>> > +EXPORT_SYMBOL(drm_vblank_no_hw_counter);
>> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> > index f0a5839..fb9cfc5 100644
>> > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> > @@ -447,7 +447,7 @@ static struct drm_driver exynos_drm_driver = {
>> >     .lastclose              = exynos_drm_lastclose,
>> >     .postclose              = exynos_drm_postclose,
>> >     .set_busid              = drm_platform_set_busid,
>> > -   .get_vblank_counter     = drm_vblank_count,
>> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> >     .enable_vblank          = exynos_drm_crtc_enable_vblank,
>> >     .disable_vblank         = exynos_drm_crtc_disable_vblank,
>> >     .gem_free_object        = exynos_drm_gem_free_object,
>> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> > index 9a8e2da..1f4e8b9 100644
>> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> > @@ -192,7 +192,7 @@ static struct drm_driver fsl_dcu_drm_driver = {
>> >     .unload                 = fsl_dcu_unload,
>> >     .preclose               = fsl_dcu_drm_preclose,
>> >     .irq_handler            = fsl_dcu_drm_irq,
>> > -   .get_vblank_counter     = drm_vblank_count,
>> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> >     .enable_vblank          = fsl_dcu_drm_enable_vblank,
>> >     .disable_vblank         = fsl_dcu_drm_disable_vblank,
>> >     .gem_free_object        = drm_gem_cma_free_object,
>> > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
>> > index 74f505b..18a6642 100644
>> > --- a/drivers/gpu/drm/imx/imx-drm-core.c
>> > +++ b/drivers/gpu/drm/imx/imx-drm-core.c
>> > @@ -487,7 +487,7 @@ static struct drm_driver imx_drm_driver = {
>> >     .gem_prime_vmap         = drm_gem_cma_prime_vmap,
>> >     .gem_prime_vunmap       = drm_gem_cma_prime_vunmap,
>> >     .gem_prime_mmap         = drm_gem_cma_prime_mmap,
>> > -   .get_vblank_counter     = drm_vblank_count,
>> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> >     .enable_vblank          = imx_drm_enable_vblank,
>> >     .disable_vblank         = imx_drm_disable_vblank,
>> >     .ioctls                 = imx_drm_ioctls,
>> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
>> > index 0339c5d..3eba23f 100644
>> > --- a/drivers/gpu/drm/msm/msm_drv.c
>> > +++ b/drivers/gpu/drm/msm/msm_drv.c
>> > @@ -978,7 +978,7 @@ static struct drm_driver msm_driver = {
>> >     .irq_preinstall     = msm_irq_preinstall,
>> >     .irq_postinstall    = msm_irq_postinstall,
>> >     .irq_uninstall      = msm_irq_uninstall,
>> > -   .get_vblank_counter = drm_vblank_count,
>> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> >     .enable_vblank      = msm_enable_vblank,
>> >     .disable_vblank     = msm_disable_vblank,
>> >     .gem_free_object    = msm_gem_free_object,
>> > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> > index ccefb64..2416c7d 100644
>> > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
>> > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> > @@ -934,7 +934,7 @@ driver_stub = {
>> >     .debugfs_cleanup = nouveau_debugfs_takedown,
>> >  #endif
>> >
>> > -   .get_vblank_counter = drm_vblank_count,
>> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> >     .enable_vblank = nouveau_display_vblank_enable,
>> >     .disable_vblank = nouveau_display_vblank_disable,
>> >     .get_scanout_position = nouveau_display_scanoutpos,
>> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
>> > index d685e23..4d58934 100644
>> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
>> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
>> > @@ -839,7 +839,7 @@ static struct drm_driver omap_drm_driver = {
>> >     .preclose = dev_preclose,
>> >     .postclose = dev_postclose,
>> >     .set_busid = drm_platform_set_busid,
>> > -   .get_vblank_counter = drm_vblank_count,
>> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> >     .enable_vblank = omap_irq_enable_vblank,
>> >     .disable_vblank = omap_irq_disable_vblank,
>> >  #ifdef CONFIG_DEBUG_FS
>> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> > index 780ca11..3608e88 100644
>> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> > @@ -259,7 +259,7 @@ static struct drm_driver rcar_du_driver = {
>> >     .preclose               = rcar_du_preclose,
>> >     .lastclose              = rcar_du_lastclose,
>> >     .set_busid              = drm_platform_set_busid,
>> > -   .get_vblank_counter     = drm_vblank_count,
>> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> >     .enable_vblank          = rcar_du_enable_vblank,
>> >     .disable_vblank         = rcar_du_disable_vblank,
>> >     .gem_free_object        = drm_gem_cma_free_object,
>> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> > index 9a0c291..b468add 100644
>> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> > @@ -277,7 +277,7 @@ static struct drm_driver rockchip_drm_driver = {
>> >     .load                   = rockchip_drm_load,
>> >     .unload                 = rockchip_drm_unload,
>> >     .lastclose              = rockchip_drm_lastclose,
>> > -   .get_vblank_counter     = drm_vblank_count,
>> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> >     .enable_vblank          = rockchip_drm_crtc_enable_vblank,
>> >     .disable_vblank         = rockchip_drm_crtc_disable_vblank,
>> >     .gem_vm_ops             = &rockchip_drm_vm_ops,
>> > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
>> > index 666321d..108a03b 100644
>> > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
>> > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
>> > @@ -269,7 +269,7 @@ static struct drm_driver shmob_drm_driver = {
>> >     .preclose               = shmob_drm_preclose,
>> >     .set_busid              = drm_platform_set_busid,
>> >     .irq_handler            = shmob_drm_irq,
>> > -   .get_vblank_counter     = drm_vblank_count,
>> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> >     .enable_vblank          = shmob_drm_enable_vblank,
>> >     .disable_vblank         = shmob_drm_disable_vblank,
>> >     .gem_free_object        = drm_gem_cma_free_object,
>> > diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
>> > index 9f85988..f846996 100644
>> > --- a/drivers/gpu/drm/sti/sti_drv.c
>> > +++ b/drivers/gpu/drm/sti/sti_drv.c
>> > @@ -201,7 +201,7 @@ static struct drm_driver sti_driver = {
>> >     .dumb_destroy = drm_gem_dumb_destroy,
>> >     .fops = &sti_driver_fops,
>> >
>> > -   .get_vblank_counter = drm_vblank_count,
>> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> >     .enable_vblank = sti_crtc_enable_vblank,
>> >     .disable_vblank = sti_crtc_disable_vblank,
>> >
>> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> > index 0f283a3..7e0b0c5 100644
>> > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> > @@ -563,7 +563,7 @@ static struct drm_driver tilcdc_driver = {
>> >     .irq_preinstall     = tilcdc_irq_preinstall,
>> >     .irq_postinstall    = tilcdc_irq_postinstall,
>> >     .irq_uninstall      = tilcdc_irq_uninstall,
>> > -   .get_vblank_counter = drm_vblank_count,
>> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> >     .enable_vblank      = tilcdc_enable_vblank,
>> >     .disable_vblank     = tilcdc_disable_vblank,
>> >     .gem_free_object    = drm_gem_cma_free_object,
>> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
>> > index d0251ac..f563333 100644
>> > --- a/include/drm/drmP.h
>> > +++ b/include/drm/drmP.h
>> > @@ -952,6 +952,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
>> >  extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
>> >  extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
>> >  extern void drm_vblank_cleanup(struct drm_device *dev);
>> > +extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe);
>> >
>> >  extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>> >                                              unsigned int pipe, int *max_error,
>> > --
>> > 2.4.6
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Ville Syrjälä Oct. 1, 2015, 3:44 p.m. UTC | #6
On Thu, Oct 01, 2015 at 11:25:36AM -0400, Rob Clark wrote:
> On Wed, Sep 30, 2015 at 10:14 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:
> >> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >
> >> > drm_vblank_count() returns the software counter. We should not pretend
> >> > it's the hw counter since we use the hw counter to figuere out what the
> >> > software counter value should be. So instead provide a new function
> >> > drm_vblank_no_hw_counter() for drivers that don't have a real hw
> >> > counter. The new function simply returns 0, which is about the only
> >> > thing it can do.
> >>
> >> Shouldn't we instead just make the get_vblank_counter hook optional?
> >
> > Perhaps. But maybe this way would encourage people to go look for a
> > hw frame counter in their hardware?
> 
> Well, I guess at this point we have more drm/kms drivers for hw
> without hw frame counters..  maybe a helper that guestimates based on
> time elapsed since last vbl irq would be useful..

That's already being done for the sw counter.

> 
> BR,
> -R
> 
> >> -Daniel
> >>
> >> >
> >> > Cc: Vincent Abriou <vincent.abriou@st.com>
> >> > Cc: Thierry Reding <treding@nvidia.com>
> >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > ---
> >> >  drivers/gpu/drm/armada/armada_drv.c          |  2 +-
> >> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
> >> >  drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
> >> >  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
> >> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
> >> >  drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
> >> >  drivers/gpu/drm/msm/msm_drv.c                |  2 +-
> >> >  drivers/gpu/drm/nouveau/nouveau_drm.c        |  2 +-
> >> >  drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
> >> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c        |  2 +-
> >> >  drivers/gpu/drm/rockchip/rockchip_drm_drv.c  |  2 +-
> >> >  drivers/gpu/drm/shmobile/shmob_drm_drv.c     |  2 +-
> >> >  drivers/gpu/drm/sti/sti_drv.c                |  2 +-
> >> >  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
> >> >  include/drm/drmP.h                           |  1 +
> >> >  15 files changed, 31 insertions(+), 13 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> >> > index 225034b..464a13f 100644
> >> > --- a/drivers/gpu/drm/armada/armada_drv.c
> >> > +++ b/drivers/gpu/drm/armada/armada_drv.c
> >> > @@ -300,7 +300,7 @@ static struct drm_driver armada_drm_driver = {
> >> >     .lastclose              = armada_drm_lastclose,
> >> >     .unload                 = armada_drm_unload,
> >> >     .set_busid              = drm_platform_set_busid,
> >> > -   .get_vblank_counter     = drm_vblank_count,
> >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> >> >     .enable_vblank          = armada_drm_enable_vblank,
> >> >     .disable_vblank         = armada_drm_disable_vblank,
> >> >  #ifdef CONFIG_DEBUG_FS
> >> > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> >> > index 8bc62ec..2eb1c66 100644
> >> > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> >> > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> >> > @@ -697,7 +697,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
> >> >     .irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
> >> >     .irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
> >> >     .irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
> >> > -   .get_vblank_counter = drm_vblank_count,
> >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> >> >     .enable_vblank = atmel_hlcdc_dc_enable_vblank,
> >> >     .disable_vblank = atmel_hlcdc_dc_disable_vblank,
> >> >     .gem_free_object = drm_gem_cma_free_object,
> >> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> >> > index ed2394e..7d70b7c 100644
> >> > --- a/drivers/gpu/drm/drm_irq.c
> >> > +++ b/drivers/gpu/drm/drm_irq.c
> >> > @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
> >> >     return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
> >> >  }
> >> >  EXPORT_SYMBOL(drm_crtc_handle_vblank);
> >> > +
> >> > +/**
> >> > + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
> >> > + * @dev: DRM device
> >> > + * @pipe: CRTC for which to read the counter
> >> > + *
> >> > + * Drivers can plug this into the .get_vblank_counter() function if
> >> > + * there is no useable hardware frame counter available.
> >> > + *
> >> > + * Returns:
> >> > + * 0
> >> > + */
> >> > +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
> >> > +{
> >> > +   return 0;
> >> > +}
> >> > +EXPORT_SYMBOL(drm_vblank_no_hw_counter);
> >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >> > index f0a5839..fb9cfc5 100644
> >> > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >> > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >> > @@ -447,7 +447,7 @@ static struct drm_driver exynos_drm_driver = {
> >> >     .lastclose              = exynos_drm_lastclose,
> >> >     .postclose              = exynos_drm_postclose,
> >> >     .set_busid              = drm_platform_set_busid,
> >> > -   .get_vblank_counter     = drm_vblank_count,
> >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> >> >     .enable_vblank          = exynos_drm_crtc_enable_vblank,
> >> >     .disable_vblank         = exynos_drm_crtc_disable_vblank,
> >> >     .gem_free_object        = exynos_drm_gem_free_object,
> >> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> >> > index 9a8e2da..1f4e8b9 100644
> >> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> >> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> >> > @@ -192,7 +192,7 @@ static struct drm_driver fsl_dcu_drm_driver = {
> >> >     .unload                 = fsl_dcu_unload,
> >> >     .preclose               = fsl_dcu_drm_preclose,
> >> >     .irq_handler            = fsl_dcu_drm_irq,
> >> > -   .get_vblank_counter     = drm_vblank_count,
> >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> >> >     .enable_vblank          = fsl_dcu_drm_enable_vblank,
> >> >     .disable_vblank         = fsl_dcu_drm_disable_vblank,
> >> >     .gem_free_object        = drm_gem_cma_free_object,
> >> > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> >> > index 74f505b..18a6642 100644
> >> > --- a/drivers/gpu/drm/imx/imx-drm-core.c
> >> > +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> >> > @@ -487,7 +487,7 @@ static struct drm_driver imx_drm_driver = {
> >> >     .gem_prime_vmap         = drm_gem_cma_prime_vmap,
> >> >     .gem_prime_vunmap       = drm_gem_cma_prime_vunmap,
> >> >     .gem_prime_mmap         = drm_gem_cma_prime_mmap,
> >> > -   .get_vblank_counter     = drm_vblank_count,
> >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> >> >     .enable_vblank          = imx_drm_enable_vblank,
> >> >     .disable_vblank         = imx_drm_disable_vblank,
> >> >     .ioctls                 = imx_drm_ioctls,
> >> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> >> > index 0339c5d..3eba23f 100644
> >> > --- a/drivers/gpu/drm/msm/msm_drv.c
> >> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> >> > @@ -978,7 +978,7 @@ static struct drm_driver msm_driver = {
> >> >     .irq_preinstall     = msm_irq_preinstall,
> >> >     .irq_postinstall    = msm_irq_postinstall,
> >> >     .irq_uninstall      = msm_irq_uninstall,
> >> > -   .get_vblank_counter = drm_vblank_count,
> >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> >> >     .enable_vblank      = msm_enable_vblank,
> >> >     .disable_vblank     = msm_disable_vblank,
> >> >     .gem_free_object    = msm_gem_free_object,
> >> > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> >> > index ccefb64..2416c7d 100644
> >> > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> >> > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> >> > @@ -934,7 +934,7 @@ driver_stub = {
> >> >     .debugfs_cleanup = nouveau_debugfs_takedown,
> >> >  #endif
> >> >
> >> > -   .get_vblank_counter = drm_vblank_count,
> >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> >> >     .enable_vblank = nouveau_display_vblank_enable,
> >> >     .disable_vblank = nouveau_display_vblank_disable,
> >> >     .get_scanout_position = nouveau_display_scanoutpos,
> >> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> >> > index d685e23..4d58934 100644
> >> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> >> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> >> > @@ -839,7 +839,7 @@ static struct drm_driver omap_drm_driver = {
> >> >     .preclose = dev_preclose,
> >> >     .postclose = dev_postclose,
> >> >     .set_busid = drm_platform_set_busid,
> >> > -   .get_vblank_counter = drm_vblank_count,
> >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> >> >     .enable_vblank = omap_irq_enable_vblank,
> >> >     .disable_vblank = omap_irq_disable_vblank,
> >> >  #ifdef CONFIG_DEBUG_FS
> >> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> >> > index 780ca11..3608e88 100644
> >> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> >> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> >> > @@ -259,7 +259,7 @@ static struct drm_driver rcar_du_driver = {
> >> >     .preclose               = rcar_du_preclose,
> >> >     .lastclose              = rcar_du_lastclose,
> >> >     .set_busid              = drm_platform_set_busid,
> >> > -   .get_vblank_counter     = drm_vblank_count,
> >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> >> >     .enable_vblank          = rcar_du_enable_vblank,
> >> >     .disable_vblank         = rcar_du_disable_vblank,
> >> >     .gem_free_object        = drm_gem_cma_free_object,
> >> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> >> > index 9a0c291..b468add 100644
> >> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> >> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> >> > @@ -277,7 +277,7 @@ static struct drm_driver rockchip_drm_driver = {
> >> >     .load                   = rockchip_drm_load,
> >> >     .unload                 = rockchip_drm_unload,
> >> >     .lastclose              = rockchip_drm_lastclose,
> >> > -   .get_vblank_counter     = drm_vblank_count,
> >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> >> >     .enable_vblank          = rockchip_drm_crtc_enable_vblank,
> >> >     .disable_vblank         = rockchip_drm_crtc_disable_vblank,
> >> >     .gem_vm_ops             = &rockchip_drm_vm_ops,
> >> > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> >> > index 666321d..108a03b 100644
> >> > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> >> > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> >> > @@ -269,7 +269,7 @@ static struct drm_driver shmob_drm_driver = {
> >> >     .preclose               = shmob_drm_preclose,
> >> >     .set_busid              = drm_platform_set_busid,
> >> >     .irq_handler            = shmob_drm_irq,
> >> > -   .get_vblank_counter     = drm_vblank_count,
> >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> >> >     .enable_vblank          = shmob_drm_enable_vblank,
> >> >     .disable_vblank         = shmob_drm_disable_vblank,
> >> >     .gem_free_object        = drm_gem_cma_free_object,
> >> > diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> >> > index 9f85988..f846996 100644
> >> > --- a/drivers/gpu/drm/sti/sti_drv.c
> >> > +++ b/drivers/gpu/drm/sti/sti_drv.c
> >> > @@ -201,7 +201,7 @@ static struct drm_driver sti_driver = {
> >> >     .dumb_destroy = drm_gem_dumb_destroy,
> >> >     .fops = &sti_driver_fops,
> >> >
> >> > -   .get_vblank_counter = drm_vblank_count,
> >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> >> >     .enable_vblank = sti_crtc_enable_vblank,
> >> >     .disable_vblank = sti_crtc_disable_vblank,
> >> >
> >> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> >> > index 0f283a3..7e0b0c5 100644
> >> > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> >> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> >> > @@ -563,7 +563,7 @@ static struct drm_driver tilcdc_driver = {
> >> >     .irq_preinstall     = tilcdc_irq_preinstall,
> >> >     .irq_postinstall    = tilcdc_irq_postinstall,
> >> >     .irq_uninstall      = tilcdc_irq_uninstall,
> >> > -   .get_vblank_counter = drm_vblank_count,
> >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> >> >     .enable_vblank      = tilcdc_enable_vblank,
> >> >     .disable_vblank     = tilcdc_disable_vblank,
> >> >     .gem_free_object    = drm_gem_cma_free_object,
> >> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> >> > index d0251ac..f563333 100644
> >> > --- a/include/drm/drmP.h
> >> > +++ b/include/drm/drmP.h
> >> > @@ -952,6 +952,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
> >> >  extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
> >> >  extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
> >> >  extern void drm_vblank_cleanup(struct drm_device *dev);
> >> > +extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe);
> >> >
> >> >  extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
> >> >                                              unsigned int pipe, int *max_error,
> >> > --
> >> > 2.4.6
> >> >
> >> > _______________________________________________
> >> > dri-devel mailing list
> >> > dri-devel@lists.freedesktop.org
> >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>
> >> --
> >> Daniel Vetter
> >> Software Engineer, Intel Corporation
> >> http://blog.ffwll.ch
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Ville Syrjälä Oct. 1, 2015, 3:47 p.m. UTC | #7
On Thu, Oct 01, 2015 at 06:44:22PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 01, 2015 at 11:25:36AM -0400, Rob Clark wrote:
> > On Wed, Sep 30, 2015 at 10:14 AM, Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > > On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:
> > >> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:
> > >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> >
> > >> > drm_vblank_count() returns the software counter. We should not pretend
> > >> > it's the hw counter since we use the hw counter to figuere out what the
> > >> > software counter value should be. So instead provide a new function
> > >> > drm_vblank_no_hw_counter() for drivers that don't have a real hw
> > >> > counter. The new function simply returns 0, which is about the only
> > >> > thing it can do.
> > >>
> > >> Shouldn't we instead just make the get_vblank_counter hook optional?
> > >
> > > Perhaps. But maybe this way would encourage people to go look for a
> > > hw frame counter in their hardware?
> > 
> > Well, I guess at this point we have more drm/kms drivers for hw
> > without hw frame counters..  maybe a helper that guestimates based on
> > time elapsed since last vbl irq would be useful..
> 
> That's already being done for the sw counter.

... assuming you have accurate vbl timestamps that is.

> 
> > 
> > BR,
> > -R
> > 
> > >> -Daniel
> > >>
> > >> >
> > >> > Cc: Vincent Abriou <vincent.abriou@st.com>
> > >> > Cc: Thierry Reding <treding@nvidia.com>
> > >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> > ---
> > >> >  drivers/gpu/drm/armada/armada_drv.c          |  2 +-
> > >> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
> > >> >  drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
> > >> >  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
> > >> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
> > >> >  drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
> > >> >  drivers/gpu/drm/msm/msm_drv.c                |  2 +-
> > >> >  drivers/gpu/drm/nouveau/nouveau_drm.c        |  2 +-
> > >> >  drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
> > >> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c        |  2 +-
> > >> >  drivers/gpu/drm/rockchip/rockchip_drm_drv.c  |  2 +-
> > >> >  drivers/gpu/drm/shmobile/shmob_drm_drv.c     |  2 +-
> > >> >  drivers/gpu/drm/sti/sti_drv.c                |  2 +-
> > >> >  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
> > >> >  include/drm/drmP.h                           |  1 +
> > >> >  15 files changed, 31 insertions(+), 13 deletions(-)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> > >> > index 225034b..464a13f 100644
> > >> > --- a/drivers/gpu/drm/armada/armada_drv.c
> > >> > +++ b/drivers/gpu/drm/armada/armada_drv.c
> > >> > @@ -300,7 +300,7 @@ static struct drm_driver armada_drm_driver = {
> > >> >     .lastclose              = armada_drm_lastclose,
> > >> >     .unload                 = armada_drm_unload,
> > >> >     .set_busid              = drm_platform_set_busid,
> > >> > -   .get_vblank_counter     = drm_vblank_count,
> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank          = armada_drm_enable_vblank,
> > >> >     .disable_vblank         = armada_drm_disable_vblank,
> > >> >  #ifdef CONFIG_DEBUG_FS
> > >> > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > >> > index 8bc62ec..2eb1c66 100644
> > >> > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > >> > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > >> > @@ -697,7 +697,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
> > >> >     .irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
> > >> >     .irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
> > >> >     .irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
> > >> > -   .get_vblank_counter = drm_vblank_count,
> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank = atmel_hlcdc_dc_enable_vblank,
> > >> >     .disable_vblank = atmel_hlcdc_dc_disable_vblank,
> > >> >     .gem_free_object = drm_gem_cma_free_object,
> > >> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > >> > index ed2394e..7d70b7c 100644
> > >> > --- a/drivers/gpu/drm/drm_irq.c
> > >> > +++ b/drivers/gpu/drm/drm_irq.c
> > >> > @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
> > >> >     return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
> > >> >  }
> > >> >  EXPORT_SYMBOL(drm_crtc_handle_vblank);
> > >> > +
> > >> > +/**
> > >> > + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
> > >> > + * @dev: DRM device
> > >> > + * @pipe: CRTC for which to read the counter
> > >> > + *
> > >> > + * Drivers can plug this into the .get_vblank_counter() function if
> > >> > + * there is no useable hardware frame counter available.
> > >> > + *
> > >> > + * Returns:
> > >> > + * 0
> > >> > + */
> > >> > +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
> > >> > +{
> > >> > +   return 0;
> > >> > +}
> > >> > +EXPORT_SYMBOL(drm_vblank_no_hw_counter);
> > >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > >> > index f0a5839..fb9cfc5 100644
> > >> > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > >> > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > >> > @@ -447,7 +447,7 @@ static struct drm_driver exynos_drm_driver = {
> > >> >     .lastclose              = exynos_drm_lastclose,
> > >> >     .postclose              = exynos_drm_postclose,
> > >> >     .set_busid              = drm_platform_set_busid,
> > >> > -   .get_vblank_counter     = drm_vblank_count,
> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank          = exynos_drm_crtc_enable_vblank,
> > >> >     .disable_vblank         = exynos_drm_crtc_disable_vblank,
> > >> >     .gem_free_object        = exynos_drm_gem_free_object,
> > >> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > >> > index 9a8e2da..1f4e8b9 100644
> > >> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > >> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > >> > @@ -192,7 +192,7 @@ static struct drm_driver fsl_dcu_drm_driver = {
> > >> >     .unload                 = fsl_dcu_unload,
> > >> >     .preclose               = fsl_dcu_drm_preclose,
> > >> >     .irq_handler            = fsl_dcu_drm_irq,
> > >> > -   .get_vblank_counter     = drm_vblank_count,
> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank          = fsl_dcu_drm_enable_vblank,
> > >> >     .disable_vblank         = fsl_dcu_drm_disable_vblank,
> > >> >     .gem_free_object        = drm_gem_cma_free_object,
> > >> > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> > >> > index 74f505b..18a6642 100644
> > >> > --- a/drivers/gpu/drm/imx/imx-drm-core.c
> > >> > +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> > >> > @@ -487,7 +487,7 @@ static struct drm_driver imx_drm_driver = {
> > >> >     .gem_prime_vmap         = drm_gem_cma_prime_vmap,
> > >> >     .gem_prime_vunmap       = drm_gem_cma_prime_vunmap,
> > >> >     .gem_prime_mmap         = drm_gem_cma_prime_mmap,
> > >> > -   .get_vblank_counter     = drm_vblank_count,
> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank          = imx_drm_enable_vblank,
> > >> >     .disable_vblank         = imx_drm_disable_vblank,
> > >> >     .ioctls                 = imx_drm_ioctls,
> > >> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > >> > index 0339c5d..3eba23f 100644
> > >> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > >> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > >> > @@ -978,7 +978,7 @@ static struct drm_driver msm_driver = {
> > >> >     .irq_preinstall     = msm_irq_preinstall,
> > >> >     .irq_postinstall    = msm_irq_postinstall,
> > >> >     .irq_uninstall      = msm_irq_uninstall,
> > >> > -   .get_vblank_counter = drm_vblank_count,
> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank      = msm_enable_vblank,
> > >> >     .disable_vblank     = msm_disable_vblank,
> > >> >     .gem_free_object    = msm_gem_free_object,
> > >> > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > >> > index ccefb64..2416c7d 100644
> > >> > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> > >> > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > >> > @@ -934,7 +934,7 @@ driver_stub = {
> > >> >     .debugfs_cleanup = nouveau_debugfs_takedown,
> > >> >  #endif
> > >> >
> > >> > -   .get_vblank_counter = drm_vblank_count,
> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank = nouveau_display_vblank_enable,
> > >> >     .disable_vblank = nouveau_display_vblank_disable,
> > >> >     .get_scanout_position = nouveau_display_scanoutpos,
> > >> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> > >> > index d685e23..4d58934 100644
> > >> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> > >> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> > >> > @@ -839,7 +839,7 @@ static struct drm_driver omap_drm_driver = {
> > >> >     .preclose = dev_preclose,
> > >> >     .postclose = dev_postclose,
> > >> >     .set_busid = drm_platform_set_busid,
> > >> > -   .get_vblank_counter = drm_vblank_count,
> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank = omap_irq_enable_vblank,
> > >> >     .disable_vblank = omap_irq_disable_vblank,
> > >> >  #ifdef CONFIG_DEBUG_FS
> > >> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > >> > index 780ca11..3608e88 100644
> > >> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > >> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > >> > @@ -259,7 +259,7 @@ static struct drm_driver rcar_du_driver = {
> > >> >     .preclose               = rcar_du_preclose,
> > >> >     .lastclose              = rcar_du_lastclose,
> > >> >     .set_busid              = drm_platform_set_busid,
> > >> > -   .get_vblank_counter     = drm_vblank_count,
> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank          = rcar_du_enable_vblank,
> > >> >     .disable_vblank         = rcar_du_disable_vblank,
> > >> >     .gem_free_object        = drm_gem_cma_free_object,
> > >> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > >> > index 9a0c291..b468add 100644
> > >> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > >> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > >> > @@ -277,7 +277,7 @@ static struct drm_driver rockchip_drm_driver = {
> > >> >     .load                   = rockchip_drm_load,
> > >> >     .unload                 = rockchip_drm_unload,
> > >> >     .lastclose              = rockchip_drm_lastclose,
> > >> > -   .get_vblank_counter     = drm_vblank_count,
> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank          = rockchip_drm_crtc_enable_vblank,
> > >> >     .disable_vblank         = rockchip_drm_crtc_disable_vblank,
> > >> >     .gem_vm_ops             = &rockchip_drm_vm_ops,
> > >> > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> > >> > index 666321d..108a03b 100644
> > >> > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> > >> > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> > >> > @@ -269,7 +269,7 @@ static struct drm_driver shmob_drm_driver = {
> > >> >     .preclose               = shmob_drm_preclose,
> > >> >     .set_busid              = drm_platform_set_busid,
> > >> >     .irq_handler            = shmob_drm_irq,
> > >> > -   .get_vblank_counter     = drm_vblank_count,
> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank          = shmob_drm_enable_vblank,
> > >> >     .disable_vblank         = shmob_drm_disable_vblank,
> > >> >     .gem_free_object        = drm_gem_cma_free_object,
> > >> > diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> > >> > index 9f85988..f846996 100644
> > >> > --- a/drivers/gpu/drm/sti/sti_drv.c
> > >> > +++ b/drivers/gpu/drm/sti/sti_drv.c
> > >> > @@ -201,7 +201,7 @@ static struct drm_driver sti_driver = {
> > >> >     .dumb_destroy = drm_gem_dumb_destroy,
> > >> >     .fops = &sti_driver_fops,
> > >> >
> > >> > -   .get_vblank_counter = drm_vblank_count,
> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank = sti_crtc_enable_vblank,
> > >> >     .disable_vblank = sti_crtc_disable_vblank,
> > >> >
> > >> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > >> > index 0f283a3..7e0b0c5 100644
> > >> > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > >> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > >> > @@ -563,7 +563,7 @@ static struct drm_driver tilcdc_driver = {
> > >> >     .irq_preinstall     = tilcdc_irq_preinstall,
> > >> >     .irq_postinstall    = tilcdc_irq_postinstall,
> > >> >     .irq_uninstall      = tilcdc_irq_uninstall,
> > >> > -   .get_vblank_counter = drm_vblank_count,
> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
> > >> >     .enable_vblank      = tilcdc_enable_vblank,
> > >> >     .disable_vblank     = tilcdc_disable_vblank,
> > >> >     .gem_free_object    = drm_gem_cma_free_object,
> > >> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > >> > index d0251ac..f563333 100644
> > >> > --- a/include/drm/drmP.h
> > >> > +++ b/include/drm/drmP.h
> > >> > @@ -952,6 +952,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
> > >> >  extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
> > >> >  extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
> > >> >  extern void drm_vblank_cleanup(struct drm_device *dev);
> > >> > +extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe);
> > >> >
> > >> >  extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
> > >> >                                              unsigned int pipe, int *max_error,
> > >> > --
> > >> > 2.4.6
> > >> >
> > >> > _______________________________________________
> > >> > dri-devel mailing list
> > >> > dri-devel@lists.freedesktop.org
> > >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >>
> > >> --
> > >> Daniel Vetter
> > >> Software Engineer, Intel Corporation
> > >> http://blog.ffwll.ch
> > >
> > > --
> > > Ville Syrjälä
> > > Intel OTC
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Rob Clark Oct. 1, 2015, 3:56 p.m. UTC | #8
On Thu, Oct 1, 2015 at 11:47 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Oct 01, 2015 at 06:44:22PM +0300, Ville Syrjälä wrote:
>> On Thu, Oct 01, 2015 at 11:25:36AM -0400, Rob Clark wrote:
>> > On Wed, Sep 30, 2015 at 10:14 AM, Ville Syrjälä
>> > <ville.syrjala@linux.intel.com> wrote:
>> > > On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:
>> > >> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:
>> > >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > >> >
>> > >> > drm_vblank_count() returns the software counter. We should not pretend
>> > >> > it's the hw counter since we use the hw counter to figuere out what the
>> > >> > software counter value should be. So instead provide a new function
>> > >> > drm_vblank_no_hw_counter() for drivers that don't have a real hw
>> > >> > counter. The new function simply returns 0, which is about the only
>> > >> > thing it can do.
>> > >>
>> > >> Shouldn't we instead just make the get_vblank_counter hook optional?
>> > >
>> > > Perhaps. But maybe this way would encourage people to go look for a
>> > > hw frame counter in their hardware?
>> >
>> > Well, I guess at this point we have more drm/kms drivers for hw
>> > without hw frame counters..  maybe a helper that guestimates based on
>> > time elapsed since last vbl irq would be useful..
>>
>> That's already being done for the sw counter.
>
> ... assuming you have accurate vbl timestamps that is.

yeah.. the set of drivers supporting drv->get_vblank_timestamp() is
even smaller than the ones supporting drv->get_vblank_counter() ;-)

That all said, I think msm/mdp5 could actually support
->get_vblank_counter(), and I think even ->get_scanout_position() and
therefore ->get_vblank_timestamp()..   but afaict mdp4 does not have
any line and frame count registers.  And iirc, omap did not.. and I'd
guess a lot of the other small embedded and mobile display controller
blocks out there do not..

BR,
-R

>> >
>> > >> -Daniel
>> > >>
>> > >> >
>> > >> > Cc: Vincent Abriou <vincent.abriou@st.com>
>> > >> > Cc: Thierry Reding <treding@nvidia.com>
>> > >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > >> > ---
>> > >> >  drivers/gpu/drm/armada/armada_drv.c          |  2 +-
>> > >> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
>> > >> >  drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
>> > >> >  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
>> > >> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
>> > >> >  drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
>> > >> >  drivers/gpu/drm/msm/msm_drv.c                |  2 +-
>> > >> >  drivers/gpu/drm/nouveau/nouveau_drm.c        |  2 +-
>> > >> >  drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
>> > >> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c        |  2 +-
>> > >> >  drivers/gpu/drm/rockchip/rockchip_drm_drv.c  |  2 +-
>> > >> >  drivers/gpu/drm/shmobile/shmob_drm_drv.c     |  2 +-
>> > >> >  drivers/gpu/drm/sti/sti_drv.c                |  2 +-
>> > >> >  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
>> > >> >  include/drm/drmP.h                           |  1 +
>> > >> >  15 files changed, 31 insertions(+), 13 deletions(-)
>> > >> >
>> > >> > diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
>> > >> > index 225034b..464a13f 100644
>> > >> > --- a/drivers/gpu/drm/armada/armada_drv.c
>> > >> > +++ b/drivers/gpu/drm/armada/armada_drv.c
>> > >> > @@ -300,7 +300,7 @@ static struct drm_driver armada_drm_driver = {
>> > >> >     .lastclose              = armada_drm_lastclose,
>> > >> >     .unload                 = armada_drm_unload,
>> > >> >     .set_busid              = drm_platform_set_busid,
>> > >> > -   .get_vblank_counter     = drm_vblank_count,
>> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank          = armada_drm_enable_vblank,
>> > >> >     .disable_vblank         = armada_drm_disable_vblank,
>> > >> >  #ifdef CONFIG_DEBUG_FS
>> > >> > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> > >> > index 8bc62ec..2eb1c66 100644
>> > >> > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> > >> > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> > >> > @@ -697,7 +697,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>> > >> >     .irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>> > >> >     .irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
>> > >> >     .irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
>> > >> > -   .get_vblank_counter = drm_vblank_count,
>> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank = atmel_hlcdc_dc_enable_vblank,
>> > >> >     .disable_vblank = atmel_hlcdc_dc_disable_vblank,
>> > >> >     .gem_free_object = drm_gem_cma_free_object,
>> > >> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> > >> > index ed2394e..7d70b7c 100644
>> > >> > --- a/drivers/gpu/drm/drm_irq.c
>> > >> > +++ b/drivers/gpu/drm/drm_irq.c
>> > >> > @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
>> > >> >     return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
>> > >> >  }
>> > >> >  EXPORT_SYMBOL(drm_crtc_handle_vblank);
>> > >> > +
>> > >> > +/**
>> > >> > + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
>> > >> > + * @dev: DRM device
>> > >> > + * @pipe: CRTC for which to read the counter
>> > >> > + *
>> > >> > + * Drivers can plug this into the .get_vblank_counter() function if
>> > >> > + * there is no useable hardware frame counter available.
>> > >> > + *
>> > >> > + * Returns:
>> > >> > + * 0
>> > >> > + */
>> > >> > +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
>> > >> > +{
>> > >> > +   return 0;
>> > >> > +}
>> > >> > +EXPORT_SYMBOL(drm_vblank_no_hw_counter);
>> > >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> > >> > index f0a5839..fb9cfc5 100644
>> > >> > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> > >> > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> > >> > @@ -447,7 +447,7 @@ static struct drm_driver exynos_drm_driver = {
>> > >> >     .lastclose              = exynos_drm_lastclose,
>> > >> >     .postclose              = exynos_drm_postclose,
>> > >> >     .set_busid              = drm_platform_set_busid,
>> > >> > -   .get_vblank_counter     = drm_vblank_count,
>> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank          = exynos_drm_crtc_enable_vblank,
>> > >> >     .disable_vblank         = exynos_drm_crtc_disable_vblank,
>> > >> >     .gem_free_object        = exynos_drm_gem_free_object,
>> > >> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> > >> > index 9a8e2da..1f4e8b9 100644
>> > >> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> > >> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> > >> > @@ -192,7 +192,7 @@ static struct drm_driver fsl_dcu_drm_driver = {
>> > >> >     .unload                 = fsl_dcu_unload,
>> > >> >     .preclose               = fsl_dcu_drm_preclose,
>> > >> >     .irq_handler            = fsl_dcu_drm_irq,
>> > >> > -   .get_vblank_counter     = drm_vblank_count,
>> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank          = fsl_dcu_drm_enable_vblank,
>> > >> >     .disable_vblank         = fsl_dcu_drm_disable_vblank,
>> > >> >     .gem_free_object        = drm_gem_cma_free_object,
>> > >> > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
>> > >> > index 74f505b..18a6642 100644
>> > >> > --- a/drivers/gpu/drm/imx/imx-drm-core.c
>> > >> > +++ b/drivers/gpu/drm/imx/imx-drm-core.c
>> > >> > @@ -487,7 +487,7 @@ static struct drm_driver imx_drm_driver = {
>> > >> >     .gem_prime_vmap         = drm_gem_cma_prime_vmap,
>> > >> >     .gem_prime_vunmap       = drm_gem_cma_prime_vunmap,
>> > >> >     .gem_prime_mmap         = drm_gem_cma_prime_mmap,
>> > >> > -   .get_vblank_counter     = drm_vblank_count,
>> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank          = imx_drm_enable_vblank,
>> > >> >     .disable_vblank         = imx_drm_disable_vblank,
>> > >> >     .ioctls                 = imx_drm_ioctls,
>> > >> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
>> > >> > index 0339c5d..3eba23f 100644
>> > >> > --- a/drivers/gpu/drm/msm/msm_drv.c
>> > >> > +++ b/drivers/gpu/drm/msm/msm_drv.c
>> > >> > @@ -978,7 +978,7 @@ static struct drm_driver msm_driver = {
>> > >> >     .irq_preinstall     = msm_irq_preinstall,
>> > >> >     .irq_postinstall    = msm_irq_postinstall,
>> > >> >     .irq_uninstall      = msm_irq_uninstall,
>> > >> > -   .get_vblank_counter = drm_vblank_count,
>> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank      = msm_enable_vblank,
>> > >> >     .disable_vblank     = msm_disable_vblank,
>> > >> >     .gem_free_object    = msm_gem_free_object,
>> > >> > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> > >> > index ccefb64..2416c7d 100644
>> > >> > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
>> > >> > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> > >> > @@ -934,7 +934,7 @@ driver_stub = {
>> > >> >     .debugfs_cleanup = nouveau_debugfs_takedown,
>> > >> >  #endif
>> > >> >
>> > >> > -   .get_vblank_counter = drm_vblank_count,
>> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank = nouveau_display_vblank_enable,
>> > >> >     .disable_vblank = nouveau_display_vblank_disable,
>> > >> >     .get_scanout_position = nouveau_display_scanoutpos,
>> > >> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
>> > >> > index d685e23..4d58934 100644
>> > >> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
>> > >> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
>> > >> > @@ -839,7 +839,7 @@ static struct drm_driver omap_drm_driver = {
>> > >> >     .preclose = dev_preclose,
>> > >> >     .postclose = dev_postclose,
>> > >> >     .set_busid = drm_platform_set_busid,
>> > >> > -   .get_vblank_counter = drm_vblank_count,
>> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank = omap_irq_enable_vblank,
>> > >> >     .disable_vblank = omap_irq_disable_vblank,
>> > >> >  #ifdef CONFIG_DEBUG_FS
>> > >> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> > >> > index 780ca11..3608e88 100644
>> > >> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> > >> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> > >> > @@ -259,7 +259,7 @@ static struct drm_driver rcar_du_driver = {
>> > >> >     .preclose               = rcar_du_preclose,
>> > >> >     .lastclose              = rcar_du_lastclose,
>> > >> >     .set_busid              = drm_platform_set_busid,
>> > >> > -   .get_vblank_counter     = drm_vblank_count,
>> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank          = rcar_du_enable_vblank,
>> > >> >     .disable_vblank         = rcar_du_disable_vblank,
>> > >> >     .gem_free_object        = drm_gem_cma_free_object,
>> > >> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> > >> > index 9a0c291..b468add 100644
>> > >> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> > >> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> > >> > @@ -277,7 +277,7 @@ static struct drm_driver rockchip_drm_driver = {
>> > >> >     .load                   = rockchip_drm_load,
>> > >> >     .unload                 = rockchip_drm_unload,
>> > >> >     .lastclose              = rockchip_drm_lastclose,
>> > >> > -   .get_vblank_counter     = drm_vblank_count,
>> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank          = rockchip_drm_crtc_enable_vblank,
>> > >> >     .disable_vblank         = rockchip_drm_crtc_disable_vblank,
>> > >> >     .gem_vm_ops             = &rockchip_drm_vm_ops,
>> > >> > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
>> > >> > index 666321d..108a03b 100644
>> > >> > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
>> > >> > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
>> > >> > @@ -269,7 +269,7 @@ static struct drm_driver shmob_drm_driver = {
>> > >> >     .preclose               = shmob_drm_preclose,
>> > >> >     .set_busid              = drm_platform_set_busid,
>> > >> >     .irq_handler            = shmob_drm_irq,
>> > >> > -   .get_vblank_counter     = drm_vblank_count,
>> > >> > +   .get_vblank_counter     = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank          = shmob_drm_enable_vblank,
>> > >> >     .disable_vblank         = shmob_drm_disable_vblank,
>> > >> >     .gem_free_object        = drm_gem_cma_free_object,
>> > >> > diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
>> > >> > index 9f85988..f846996 100644
>> > >> > --- a/drivers/gpu/drm/sti/sti_drv.c
>> > >> > +++ b/drivers/gpu/drm/sti/sti_drv.c
>> > >> > @@ -201,7 +201,7 @@ static struct drm_driver sti_driver = {
>> > >> >     .dumb_destroy = drm_gem_dumb_destroy,
>> > >> >     .fops = &sti_driver_fops,
>> > >> >
>> > >> > -   .get_vblank_counter = drm_vblank_count,
>> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank = sti_crtc_enable_vblank,
>> > >> >     .disable_vblank = sti_crtc_disable_vblank,
>> > >> >
>> > >> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> > >> > index 0f283a3..7e0b0c5 100644
>> > >> > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> > >> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> > >> > @@ -563,7 +563,7 @@ static struct drm_driver tilcdc_driver = {
>> > >> >     .irq_preinstall     = tilcdc_irq_preinstall,
>> > >> >     .irq_postinstall    = tilcdc_irq_postinstall,
>> > >> >     .irq_uninstall      = tilcdc_irq_uninstall,
>> > >> > -   .get_vblank_counter = drm_vblank_count,
>> > >> > +   .get_vblank_counter = drm_vblank_no_hw_counter,
>> > >> >     .enable_vblank      = tilcdc_enable_vblank,
>> > >> >     .disable_vblank     = tilcdc_disable_vblank,
>> > >> >     .gem_free_object    = drm_gem_cma_free_object,
>> > >> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
>> > >> > index d0251ac..f563333 100644
>> > >> > --- a/include/drm/drmP.h
>> > >> > +++ b/include/drm/drmP.h
>> > >> > @@ -952,6 +952,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
>> > >> >  extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
>> > >> >  extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
>> > >> >  extern void drm_vblank_cleanup(struct drm_device *dev);
>> > >> > +extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe);
>> > >> >
>> > >> >  extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>> > >> >                                              unsigned int pipe, int *max_error,
>> > >> > --
>> > >> > 2.4.6
>> > >> >
>> > >> > _______________________________________________
>> > >> > dri-devel mailing list
>> > >> > dri-devel@lists.freedesktop.org
>> > >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> > >>
>> > >> --
>> > >> Daniel Vetter
>> > >> Software Engineer, Intel Corporation
>> > >> http://blog.ffwll.ch
>> > >
>> > > --
>> > > Ville Syrjälä
>> > > Intel OTC
>> > > _______________________________________________
>> > > dri-devel mailing list
>> > > dri-devel@lists.freedesktop.org
>> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> --
>> Ville Syrjälä
>> Intel OTC
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
Vincent Abriou Oct. 2, 2015, 8:25 a.m. UTC | #9
On 09/30/2015 04:14 PM, Ville Syrjälä wrote:
> On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:

>> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:

>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>

>>>

>>> drm_vblank_count() returns the software counter. We should not pretend

>>> it's the hw counter since we use the hw counter to figuere out what the

>>> software counter value should be. So instead provide a new function

>>> drm_vblank_no_hw_counter() for drivers that don't have a real hw

>>> counter. The new function simply returns 0, which is about the only

>>> thing it can do.

>>

>> Shouldn't we instead just make the get_vblank_counter hook optional?

>

> Perhaps. But maybe this way would encourage people to go look for a

> hw frame counter in their hardware?

>

>> -Daniel

>>

>>>

>>> Cc: Vincent Abriou <vincent.abriou@st.com>

>>> Cc: Thierry Reding <treding@nvidia.com>

>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>>> ---

>>>   drivers/gpu/drm/armada/armada_drv.c          |  2 +-

>>>   drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-

>>>   drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++

>>>   drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-

>>>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-

>>>   drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-


[..]

>>> --- a/drivers/gpu/drm/drm_irq.c

>>> +++ b/drivers/gpu/drm/drm_irq.c

>>> @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)

>>>      return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));

>>>   }

>>>   EXPORT_SYMBOL(drm_crtc_handle_vblank);

>>> +

>>> +/**

>>> + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()

>>> + * @dev: DRM device

>>> + * @pipe: CRTC for which to read the counter

>>> + *

>>> + * Drivers can plug this into the .get_vblank_counter() function if

>>> + * there is no useable hardware frame counter available.

>>> + *

>>> + * Returns:

>>> + * 0

>>> + */

>>> +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)


warning when building the kernel:
int pipe => unsigned int pipe

BR
Vincent
Ville Syrjälä Oct. 2, 2015, 12:48 p.m. UTC | #10
On Fri, Oct 02, 2015 at 10:25:27AM +0200, Vincent ABRIOU wrote:
> 
> 
> On 09/30/2015 04:14 PM, Ville Syrjälä wrote:
> > On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:
> >> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> drm_vblank_count() returns the software counter. We should not pretend
> >>> it's the hw counter since we use the hw counter to figuere out what the
> >>> software counter value should be. So instead provide a new function
> >>> drm_vblank_no_hw_counter() for drivers that don't have a real hw
> >>> counter. The new function simply returns 0, which is about the only
> >>> thing it can do.
> >>
> >> Shouldn't we instead just make the get_vblank_counter hook optional?
> >
> > Perhaps. But maybe this way would encourage people to go look for a
> > hw frame counter in their hardware?
> >
> >> -Daniel
> >>
> >>>
> >>> Cc: Vincent Abriou <vincent.abriou@st.com>
> >>> Cc: Thierry Reding <treding@nvidia.com>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>   drivers/gpu/drm/armada/armada_drv.c          |  2 +-
> >>>   drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
> >>>   drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
> >>>   drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
> >>>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
> >>>   drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
> 
> [..]
> 
> >>> --- a/drivers/gpu/drm/drm_irq.c
> >>> +++ b/drivers/gpu/drm/drm_irq.c
> >>> @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
> >>>      return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
> >>>   }
> >>>   EXPORT_SYMBOL(drm_crtc_handle_vblank);
> >>> +
> >>> +/**
> >>> + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
> >>> + * @dev: DRM device
> >>> + * @pipe: CRTC for which to read the counter
> >>> + *
> >>> + * Drivers can plug this into the .get_vblank_counter() function if
> >>> + * there is no useable hardware frame counter available.
> >>> + *
> >>> + * Returns:
> >>> + * 0
> >>> + */
> >>> +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
> 
> warning when building the kernel:
> int pipe => unsigned int pipe

Where exactly? The function pointer signature still has a signed int
here, and I was too lazy to go on a rampage to change all the remaining
signed ints to unsigned for the vblank driver hooks.
Vincent Abriou Oct. 2, 2015, 1:03 p.m. UTC | #11
On 10/02/2015 02:48 PM, Ville Syrjälä wrote:
> On Fri, Oct 02, 2015 at 10:25:27AM +0200, Vincent ABRIOU wrote:
>>
>>
>> On 09/30/2015 04:14 PM, Ville Syrjälä wrote:
>>> On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:
>>>> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:
>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>
>>>>> drm_vblank_count() returns the software counter. We should not pretend
>>>>> it's the hw counter since we use the hw counter to figuere out what the
>>>>> software counter value should be. So instead provide a new function
>>>>> drm_vblank_no_hw_counter() for drivers that don't have a real hw
>>>>> counter. The new function simply returns 0, which is about the only
>>>>> thing it can do.
>>>>
>>>> Shouldn't we instead just make the get_vblank_counter hook optional?
>>>
>>> Perhaps. But maybe this way would encourage people to go look for a
>>> hw frame counter in their hardware?
>>>
>>>> -Daniel
>>>>
>>>>>
>>>>> Cc: Vincent Abriou <vincent.abriou@st.com>
>>>>> Cc: Thierry Reding <treding@nvidia.com>
>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/armada/armada_drv.c          |  2 +-
>>>>>    drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
>>>>>    drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
>>>>>    drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
>>>>>    drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
>>>>>    drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
>>
>> [..]
>>
>>>>> --- a/drivers/gpu/drm/drm_irq.c
>>>>> +++ b/drivers/gpu/drm/drm_irq.c
>>>>> @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
>>>>>       return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
>>>>>    }
>>>>>    EXPORT_SYMBOL(drm_crtc_handle_vblank);
>>>>> +
>>>>> +/**
>>>>> + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
>>>>> + * @dev: DRM device
>>>>> + * @pipe: CRTC for which to read the counter
>>>>> + *
>>>>> + * Drivers can plug this into the .get_vblank_counter() function if
>>>>> + * there is no useable hardware frame counter available.
>>>>> + *
>>>>> + * Returns:
>>>>> + * 0
>>>>> + */
>>>>> +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
>>
>> warning when building the kernel:
>> int pipe => unsigned int pipe
>
> Where exactly? The function pointer signature still has a signed int
> here, and I was too lazy to go on a rampage to change all the remaining
> signed ints to unsigned for the vblank driver hooks.
>

I made the comment on the wrong patch. Sorry for that. It concern 
"[PATCH 2/2] drm: s/int pipe/unsigned int pipe/"

Vincent
Vincent Abriou Oct. 7, 2015, 12:54 p.m. UTC | #12
Reviewed-by: Vincent Abriou <vincent.abriou@st.com>

On 10/02/2015 02:48 PM, Ville Syrjälä wrote:
> On Fri, Oct 02, 2015 at 10:25:27AM +0200, Vincent ABRIOU wrote:
>>
>>
>> On 09/30/2015 04:14 PM, Ville Syrjälä wrote:
>>> On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:
>>>> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:
>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>
>>>>> drm_vblank_count() returns the software counter. We should not pretend
>>>>> it's the hw counter since we use the hw counter to figuere out what the
>>>>> software counter value should be. So instead provide a new function
>>>>> drm_vblank_no_hw_counter() for drivers that don't have a real hw
>>>>> counter. The new function simply returns 0, which is about the only
>>>>> thing it can do.
>>>>
>>>> Shouldn't we instead just make the get_vblank_counter hook optional?
>>>
>>> Perhaps. But maybe this way would encourage people to go look for a
>>> hw frame counter in their hardware?
>>>
>>>> -Daniel
>>>>
>>>>>
>>>>> Cc: Vincent Abriou <vincent.abriou@st.com>
>>>>> Cc: Thierry Reding <treding@nvidia.com>
>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/armada/armada_drv.c          |  2 +-
>>>>>    drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
>>>>>    drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
>>>>>    drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
>>>>>    drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
>>>>>    drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
>>
>> [..]
>>
>>>>> --- a/drivers/gpu/drm/drm_irq.c
>>>>> +++ b/drivers/gpu/drm/drm_irq.c
>>>>> @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
>>>>>       return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
>>>>>    }
>>>>>    EXPORT_SYMBOL(drm_crtc_handle_vblank);
>>>>> +
>>>>> +/**
>>>>> + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
>>>>> + * @dev: DRM device
>>>>> + * @pipe: CRTC for which to read the counter
>>>>> + *
>>>>> + * Drivers can plug this into the .get_vblank_counter() function if
>>>>> + * there is no useable hardware frame counter available.
>>>>> + *
>>>>> + * Returns:
>>>>> + * 0
>>>>> + */
>>>>> +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
>>
>> warning when building the kernel:
>> int pipe => unsigned int pipe
>
> Where exactly? The function pointer signature still has a signed int
> here, and I was too lazy to go on a rampage to change all the remaining
> signed ints to unsigned for the vblank driver hooks.
>
Daniel Vetter Oct. 7, 2015, 2:14 p.m. UTC | #13
Since I already applied Thierry's interface change I've added the unsigned
here as a fixup and dropped patch 2 (since I have that already).

Thanks, Daniel

On Wed, Oct 07, 2015 at 02:54:24PM +0200, Vincent ABRIOU wrote:
> Reviewed-by: Vincent Abriou <vincent.abriou@st.com>
> 
> On 10/02/2015 02:48 PM, Ville Syrjälä wrote:
> > On Fri, Oct 02, 2015 at 10:25:27AM +0200, Vincent ABRIOU wrote:
> >>
> >>
> >> On 09/30/2015 04:14 PM, Ville Syrjälä wrote:
> >>> On Wed, Sep 30, 2015 at 04:08:02PM +0200, Daniel Vetter wrote:
> >>>> On Wed, Sep 30, 2015 at 04:46:48PM +0300, ville.syrjala@linux.intel.com wrote:
> >>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>
> >>>>> drm_vblank_count() returns the software counter. We should not pretend
> >>>>> it's the hw counter since we use the hw counter to figuere out what the
> >>>>> software counter value should be. So instead provide a new function
> >>>>> drm_vblank_no_hw_counter() for drivers that don't have a real hw
> >>>>> counter. The new function simply returns 0, which is about the only
> >>>>> thing it can do.
> >>>>
> >>>> Shouldn't we instead just make the get_vblank_counter hook optional?
> >>>
> >>> Perhaps. But maybe this way would encourage people to go look for a
> >>> hw frame counter in their hardware?
> >>>
> >>>> -Daniel
> >>>>
> >>>>>
> >>>>> Cc: Vincent Abriou <vincent.abriou@st.com>
> >>>>> Cc: Thierry Reding <treding@nvidia.com>
> >>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>> ---
> >>>>>    drivers/gpu/drm/armada/armada_drv.c          |  2 +-
> >>>>>    drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
> >>>>>    drivers/gpu/drm/drm_irq.c                    | 17 +++++++++++++++++
> >>>>>    drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
> >>>>>    drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    |  2 +-
> >>>>>    drivers/gpu/drm/imx/imx-drm-core.c           |  2 +-
> >>
> >> [..]
> >>
> >>>>> --- a/drivers/gpu/drm/drm_irq.c
> >>>>> +++ b/drivers/gpu/drm/drm_irq.c
> >>>>> @@ -1797,3 +1797,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
> >>>>>       return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
> >>>>>    }
> >>>>>    EXPORT_SYMBOL(drm_crtc_handle_vblank);
> >>>>> +
> >>>>> +/**
> >>>>> + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
> >>>>> + * @dev: DRM device
> >>>>> + * @pipe: CRTC for which to read the counter
> >>>>> + *
> >>>>> + * Drivers can plug this into the .get_vblank_counter() function if
> >>>>> + * there is no useable hardware frame counter available.
> >>>>> + *
> >>>>> + * Returns:
> >>>>> + * 0
> >>>>> + */
> >>>>> +u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
> >>
> >> warning when building the kernel:
> >> int pipe => unsigned int pipe
> >
> > Where exactly? The function pointer signature still has a signed int
> > here, and I was too lazy to go on a rampage to change all the remaining
> > signed ints to unsigned for the vblank driver hooks.
> >

Patch
diff mbox

diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index 225034b..464a13f 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -300,7 +300,7 @@  static struct drm_driver armada_drm_driver = {
 	.lastclose		= armada_drm_lastclose,
 	.unload			= armada_drm_unload,
 	.set_busid		= drm_platform_set_busid,
-	.get_vblank_counter	= drm_vblank_count,
+	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= armada_drm_enable_vblank,
 	.disable_vblank		= armada_drm_disable_vblank,
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 8bc62ec..2eb1c66 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -697,7 +697,7 @@  static struct drm_driver atmel_hlcdc_dc_driver = {
 	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
 	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
 	.irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
-	.get_vblank_counter = drm_vblank_count,
+	.get_vblank_counter = drm_vblank_no_hw_counter,
 	.enable_vblank = atmel_hlcdc_dc_enable_vblank,
 	.disable_vblank = atmel_hlcdc_dc_disable_vblank,
 	.gem_free_object = drm_gem_cma_free_object,
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index ed2394e..7d70b7c 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1797,3 +1797,20 @@  bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
 	return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
 }
 EXPORT_SYMBOL(drm_crtc_handle_vblank);
+
+/**
+ * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter()
+ * @dev: DRM device
+ * @pipe: CRTC for which to read the counter
+ *
+ * Drivers can plug this into the .get_vblank_counter() function if
+ * there is no useable hardware frame counter available.
+ *
+ * Returns:
+ * 0
+ */
+u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe)
+{
+	return 0;
+}
+EXPORT_SYMBOL(drm_vblank_no_hw_counter);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index f0a5839..fb9cfc5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -447,7 +447,7 @@  static struct drm_driver exynos_drm_driver = {
 	.lastclose		= exynos_drm_lastclose,
 	.postclose		= exynos_drm_postclose,
 	.set_busid		= drm_platform_set_busid,
-	.get_vblank_counter	= drm_vblank_count,
+	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= exynos_drm_crtc_enable_vblank,
 	.disable_vblank		= exynos_drm_crtc_disable_vblank,
 	.gem_free_object	= exynos_drm_gem_free_object,
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 9a8e2da..1f4e8b9 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -192,7 +192,7 @@  static struct drm_driver fsl_dcu_drm_driver = {
 	.unload			= fsl_dcu_unload,
 	.preclose		= fsl_dcu_drm_preclose,
 	.irq_handler		= fsl_dcu_drm_irq,
-	.get_vblank_counter	= drm_vblank_count,
+	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= fsl_dcu_drm_enable_vblank,
 	.disable_vblank		= fsl_dcu_drm_disable_vblank,
 	.gem_free_object	= drm_gem_cma_free_object,
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 74f505b..18a6642 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -487,7 +487,7 @@  static struct drm_driver imx_drm_driver = {
 	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
 	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
 	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
-	.get_vblank_counter	= drm_vblank_count,
+	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= imx_drm_enable_vblank,
 	.disable_vblank		= imx_drm_disable_vblank,
 	.ioctls			= imx_drm_ioctls,
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 0339c5d..3eba23f 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -978,7 +978,7 @@  static struct drm_driver msm_driver = {
 	.irq_preinstall     = msm_irq_preinstall,
 	.irq_postinstall    = msm_irq_postinstall,
 	.irq_uninstall      = msm_irq_uninstall,
-	.get_vblank_counter = drm_vblank_count,
+	.get_vblank_counter = drm_vblank_no_hw_counter,
 	.enable_vblank      = msm_enable_vblank,
 	.disable_vblank     = msm_disable_vblank,
 	.gem_free_object    = msm_gem_free_object,
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index ccefb64..2416c7d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -934,7 +934,7 @@  driver_stub = {
 	.debugfs_cleanup = nouveau_debugfs_takedown,
 #endif
 
-	.get_vblank_counter = drm_vblank_count,
+	.get_vblank_counter = drm_vblank_no_hw_counter,
 	.enable_vblank = nouveau_display_vblank_enable,
 	.disable_vblank = nouveau_display_vblank_disable,
 	.get_scanout_position = nouveau_display_scanoutpos,
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index d685e23..4d58934 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -839,7 +839,7 @@  static struct drm_driver omap_drm_driver = {
 	.preclose = dev_preclose,
 	.postclose = dev_postclose,
 	.set_busid = drm_platform_set_busid,
-	.get_vblank_counter = drm_vblank_count,
+	.get_vblank_counter = drm_vblank_no_hw_counter,
 	.enable_vblank = omap_irq_enable_vblank,
 	.disable_vblank = omap_irq_disable_vblank,
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 780ca11..3608e88 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -259,7 +259,7 @@  static struct drm_driver rcar_du_driver = {
 	.preclose		= rcar_du_preclose,
 	.lastclose		= rcar_du_lastclose,
 	.set_busid		= drm_platform_set_busid,
-	.get_vblank_counter	= drm_vblank_count,
+	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= rcar_du_enable_vblank,
 	.disable_vblank		= rcar_du_disable_vblank,
 	.gem_free_object	= drm_gem_cma_free_object,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 9a0c291..b468add 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -277,7 +277,7 @@  static struct drm_driver rockchip_drm_driver = {
 	.load			= rockchip_drm_load,
 	.unload			= rockchip_drm_unload,
 	.lastclose		= rockchip_drm_lastclose,
-	.get_vblank_counter	= drm_vblank_count,
+	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= rockchip_drm_crtc_enable_vblank,
 	.disable_vblank		= rockchip_drm_crtc_disable_vblank,
 	.gem_vm_ops		= &rockchip_drm_vm_ops,
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index 666321d..108a03b 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -269,7 +269,7 @@  static struct drm_driver shmob_drm_driver = {
 	.preclose		= shmob_drm_preclose,
 	.set_busid		= drm_platform_set_busid,
 	.irq_handler		= shmob_drm_irq,
-	.get_vblank_counter	= drm_vblank_count,
+	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= shmob_drm_enable_vblank,
 	.disable_vblank		= shmob_drm_disable_vblank,
 	.gem_free_object	= drm_gem_cma_free_object,
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index 9f85988..f846996 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -201,7 +201,7 @@  static struct drm_driver sti_driver = {
 	.dumb_destroy = drm_gem_dumb_destroy,
 	.fops = &sti_driver_fops,
 
-	.get_vblank_counter = drm_vblank_count,
+	.get_vblank_counter = drm_vblank_no_hw_counter,
 	.enable_vblank = sti_crtc_enable_vblank,
 	.disable_vblank = sti_crtc_disable_vblank,
 
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 0f283a3..7e0b0c5 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -563,7 +563,7 @@  static struct drm_driver tilcdc_driver = {
 	.irq_preinstall     = tilcdc_irq_preinstall,
 	.irq_postinstall    = tilcdc_irq_postinstall,
 	.irq_uninstall      = tilcdc_irq_uninstall,
-	.get_vblank_counter = drm_vblank_count,
+	.get_vblank_counter = drm_vblank_no_hw_counter,
 	.enable_vblank      = tilcdc_enable_vblank,
 	.disable_vblank     = tilcdc_disable_vblank,
 	.gem_free_object    = drm_gem_cma_free_object,
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d0251ac..f563333 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -952,6 +952,7 @@  extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
 extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
 extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
 extern void drm_vblank_cleanup(struct drm_device *dev);
+extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, int pipe);
 
 extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 						 unsigned int pipe, int *max_error,