Re: [Xen-devel] [PATCH v4 01/15] drm: Initialize struct drm_crtc_state.no_vblank from device settings

[Emil Velikov <emil.l.velikov@xxxxxxxxx>]

Hi Thomas,

On Thu, 23 Jan 2020 at 09:21, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:

> @@ -174,12 +174,22 @@ struct drm_crtc_state {
>          * @no_vblank:
>          *
>          * Reflects the ability of a CRTC to send VBLANK events. This state
> -        * usually depends on the pipeline configuration, and the main usuage
> -        * is CRTCs feeding a writeback connector operating in oneshot mode.
> -        * In this case the VBLANK event is only generated when a job is 
> queued
> -        * to the writeback connector, and we want the core to fake VBLANK
> -        * events when this part of the pipeline hasn't changed but others had
> -        * or when the CRTC and connectors are being disabled.
> +        * usually depends on the pipeline configuration. If set to true, DRM
> +        * atomic helpers will sendout a fake VBLANK event during display
> +        * updates.
> +        *
> +        * One usage is for drivers and/or hardware without support for VBLANK
> +        * interrupts. Such drivers typically do not initialize vblanking
> +        * (i.e., call drm_vblank_init() wit the number of CRTCs). For CRTCs
> +        * without initialized vblanking, the field is initialized to true and
> +        * a VBLANK event will be send out on each update of the display
> +        * pipeline.
> +        *
> +        * Another usage is CRTCs feeding a writeback connector operating in
> +        * oneshot mode. In this case the VBLANK event is only generated when
> +        * a job is queued to the writeback connector, and we want the core
> +        * to fake VBLANK events when this part of the pipeline hasn't changed
> +        * but others had or when the CRTC and connectors are being disabled.
>          *

Perhaps it's just me, yet the following ideas would make the topic
significantly easier and clearer.

 - adding explicit "fake" when talking about drm/atomic _helpers_
generating/sending a VBLANK event.
For example, in 15/15 the commit message says "fake", while inline
comment omits it... Leading to the confusion pointed out.

- s/no_vblank/fake_vblank/g or s/no_vblank/no_hw_vblank/g
Simple and concise. With slight inclination towards the former wording :-)

If you and Daniel agree with the rename, then the first sentence of
the description should probably be tweaked.

HTH
Emil

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel