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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux