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

From: Daniel Vetter <daniel@ffwll.ch>
To: Vincent ABRIOU <vincent.abriou@st.com>
Cc: "dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Thierry Reding <treding@nvidia.com>
Subject: Re: [PATCH 1/2] drm: Stop using drm_vblank_count() as the hw frame counter
Date: Wed, 7 Oct 2015 16:14:45 +0200	[thread overview]
Message-ID: <20151007141445.GI3383@phenom.ffwll.local> (raw)
In-Reply-To: <56151600.3070903@st.com>

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