Re: [PATCH 23/28] drm: Document drm_plane_helper_funcs

Daniel Vetter
2015-12-07T14:43:17Z

On Mon, Dec 07, 2015 at 03:27:59PM +0100, Thierry Reding wrote:
> On Fri, Dec 04, 2015 at 09:46:04AM +0100, Daniel Vetter wrote:
> > + * mus ensure that drm_atomic_helper_check_modeset() has been called
> > + * beforehand.
>
> Perhaps mention that the default drm_atomic_helper_check()
> implementation calls them in the right order?

Good idea, added a sentence with reference to drm_atomic_helper_check().
>
> > + *
> > + * When using drm_atomic_helper_check_planes CRTC's ->atomic_check()
>
> Parentheses after the function name? Also, technically I think it would
> need to be "CRTCs'" since it relates to multiple CRTCs. Perhaps just
> leaving out the 's would work as well.

I went with CRTCs'.

> > void (*atomic_begin)(struct drm_crtc *crtc,
> > struct drm_crtc_state *old_crtc_state);
> > + /**
> > + * @atomic_flush:
> > + *
> > + * Drivers should prepare for an atomic update of multiple planes on
> > + * a CRTC in this hook. Depending upon hardware this might include
>
> The first sentence here is the exact same as for ->atomic_begin(), but
> it clearly has a different purpose. Perhaps:
>
> "Drivers should finalize an atomic update of multiple planes on
> a CRTC in this hook. ..."
>
> ?

Sounds good, simply forgotten to update that part.

All other suggestions applied, thanks a lot for all your careful comments.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation