* RE: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-09-30 14:14 ` Ville Syrjälä
@ 2015-09-30 14:44 ` Vincent ABRIOU
2015-10-01 9:10 ` Vincent ABRIOU
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Vincent ABRIOU @ 2015-09-30 14:44 UTC (permalink / raw)
To: Ville Syrjälä, Daniel Vetter
Cc: Thierry Reding, dri-devel@lists.freedesktop.org
> -----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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-09-30 14:14 ` Ville Syrjälä
2015-09-30 14:44 ` Vincent ABRIOU
@ 2015-10-01 9:10 ` Vincent ABRIOU
2015-10-01 15:25 ` Rob Clark
2015-10-02 8:25 ` Vincent ABRIOU
3 siblings, 0 replies; 21+ messages in thread
From: Vincent ABRIOU @ 2015-10-01 9:10 UTC (permalink / raw)
To: Ville Syrjälä, Daniel Vetter
Cc: Thierry Reding, dri-devel@lists.freedesktop.org
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-09-30 14:14 ` Ville Syrjälä
2015-09-30 14:44 ` Vincent ABRIOU
2015-10-01 9:10 ` Vincent ABRIOU
@ 2015-10-01 15:25 ` Rob Clark
2015-10-01 15:44 ` Ville Syrjälä
2015-10-02 8:25 ` Vincent ABRIOU
3 siblings, 1 reply; 21+ messages in thread
From: Rob Clark @ 2015-10-01 15:25 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Thierry Reding, dri-devel@lists.freedesktop.org
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-10-01 15:25 ` Rob Clark
@ 2015-10-01 15:44 ` Ville Syrjälä
2015-10-01 15:47 ` Ville Syrjälä
0 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2015-10-01 15:44 UTC (permalink / raw)
To: Rob Clark; +Cc: Thierry Reding, dri-devel@lists.freedesktop.org
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ä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-10-01 15:44 ` Ville Syrjälä
@ 2015-10-01 15:47 ` Ville Syrjälä
2015-10-01 15:56 ` Rob Clark
0 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2015-10-01 15:47 UTC (permalink / raw)
To: Rob Clark; +Cc: Thierry Reding, dri-devel@lists.freedesktop.org
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
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-10-01 15:47 ` Ville Syrjälä
@ 2015-10-01 15:56 ` Rob Clark
0 siblings, 0 replies; 21+ messages in thread
From: Rob Clark @ 2015-10-01 15:56 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Thierry Reding, dri-devel@lists.freedesktop.org
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-09-30 14:14 ` Ville Syrjälä
` (2 preceding siblings ...)
2015-10-01 15:25 ` Rob Clark
@ 2015-10-02 8:25 ` Vincent ABRIOU
2015-10-02 12:48 ` Ville Syrjälä
3 siblings, 1 reply; 21+ messages in thread
From: Vincent ABRIOU @ 2015-10-02 8:25 UTC (permalink / raw)
To: Ville Syrjälä, Daniel Vetter
Cc: Thierry Reding, dri-devel@lists.freedesktop.org
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-10-02 8:25 ` Vincent ABRIOU
@ 2015-10-02 12:48 ` Ville Syrjälä
2015-10-02 13:03 ` Vincent ABRIOU
2015-10-07 12:54 ` Vincent ABRIOU
0 siblings, 2 replies; 21+ messages in thread
From: Ville Syrjälä @ 2015-10-02 12:48 UTC (permalink / raw)
To: Vincent ABRIOU; +Cc: Thierry Reding, dri-devel@lists.freedesktop.org
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.
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-10-02 12:48 ` Ville Syrjälä
@ 2015-10-02 13:03 ` Vincent ABRIOU
2015-10-07 12:54 ` Vincent ABRIOU
1 sibling, 0 replies; 21+ messages in thread
From: Vincent ABRIOU @ 2015-10-02 13:03 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Thierry Reding, dri-devel@lists.freedesktop.org
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-10-02 12:48 ` Ville Syrjälä
2015-10-02 13:03 ` Vincent ABRIOU
@ 2015-10-07 12:54 ` Vincent ABRIOU
2015-10-07 14:14 ` Daniel Vetter
1 sibling, 1 reply; 21+ messages in thread
From: Vincent ABRIOU @ 2015-10-07 12:54 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Thierry Reding, dri-devel@lists.freedesktop.org
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.
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
2015-10-07 12:54 ` Vincent ABRIOU
@ 2015-10-07 14:14 ` Daniel Vetter
0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2015-10-07 14:14 UTC (permalink / raw)
To: Vincent ABRIOU; +Cc: dri-devel@lists.freedesktop.org, Thierry Reding
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.
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread