> -----Original Message----- > From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf > Of Ville Syrjälä > Sent: mercredi 30 septembre 2015 16:15 > To: Daniel Vetter > Cc: Thierry Reding; dri-devel@xxxxxxxxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxxx > wrote: > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > 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@xxxxxx> > > > Cc: Thierry Reding <treding@xxxxxxxxxx> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > --- > > > 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@xxxxxxxxxxxxxxxxxxxxx > > > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel