On 30.07.2014 06:32, Daniel Vetter wrote:
> As usual in both a crtc index and a struct drm_crtc * version.
>
> The function assumes that no one drivers their display below 10Hz, and
> it will complain if the vblank wait takes longer than that.
>
> v2: Also check dev->max_vblank_counter since some drivers register a
> fake get_vblank_counter function.
What does that refer to? Can't find any other reference to
max_vblank_counter in the patch.
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 0de123afdb34..76024fdde452 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -999,6 +999,51 @@ void drm_crtc_vblank_put(struct drm_crtc *crtc)
> EXPORT_SYMBOL(drm_crtc_vblank_put);
>
> /**
> + * drm_vblank_wait - wait for one vblank
> + * @dev: DRM device
> + * @crtc: crtc index
> + *
> + * This waits for one vblank to pass on @crtc, using the irq driver
> interfaces.
> + * It is a failure to call this when the vblank irq for @crtc is disable,
> e.g.
Spelling: 'disabled'
> + * due to lack of driver support or because the crtc is off.
> + */
> +void drm_vblank_wait(struct drm_device *dev, int crtc)
> +{
> + int ret;
> + u32 last;
> +
> + ret = drm_vblank_get(dev, crtc);
> + if (WARN_ON(ret))
> + return;
> +
> + last = drm_vblank_count(dev, crtc);
> +
> +#define C (last != drm_vblank_count(dev, crtc))
> + ret = wait_event_timeout(dev->vblank[crtc].queue,
> + C, msecs_to_jiffies(100));
> +
> + WARN_ON(ret == 0);
> +#undef C
What's the point of the C macro?
> + drm_vblank_put(dev, crtc);
> +}
> +EXPORT_SYMBOL(drm_vblank_wait);
> +
> +/**
> + * drm_crtc_vblank_wait - wait for one vblank
> + * @crtc: DRM crtc
> + *
> + * This waits for one vblank to pass on @crtc, using the irq driver
> interfaces.
> + * It is a failure to call this when the vblank irq for @crtc is disable,
> e.g.
Same typo as above.
> + * due to lack of driver support or because the crtc is off.
> + */
> +void drm_crtc_vblank_wait(struct drm_crtc *crtc)
> +{
> + drm_vblank_wait(crtc->dev, drm_crtc_index(crtc));
> +}
> +EXPORT_SYMBOL(drm_crtc_vblank_wait);
> +
> +/**
Maybe the function names should be *_vblank_wait_next() or something to
clarify the purpose and reduce potential confusion versus drm_wait_vblank().
Looks good to me other than that.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
