Hi Emil
Am 27.01.20 um 19:12 schrieb Emil Velikov:
> 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.
No problem on being more precise here. I'll update the docs accordingly.
>
> - s/no_vblank/fake_vblank/g or s/no_vblank/no_hw_vblank/g
> Simple and concise. With slight inclination towards the former wording :-)
I'd prefer to not change the field's name. The current name 'no_vblank'
indicates state and lets helpers decide what to do with it. The name
'fake_vblank' indicates an instruction to the helpers, telling them what
to do. It does neither seem to fit into drm_crtc_state, nor into the
overall concept.
Best regards
Thomas
>
> If you and Daniel agree with the rename, then the first sentence of
> the description should probably be tweaked.
>
> HTH
> Emil
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
