[PATCH v2 4/8] drm/crtc: Add a generic infrastructure to fake VBLANK events

[Boris Brezillon]

On Mon, 2 Jul 2018 10:40:54 +0200
Daniel Vetter <daniel at ffwll.ch> wrote:

> > >   
> > > > + *	Note that, even when no_blank is set to true, the CRTC driver can still
> > > > + *	steal the &drm_crtc_state.event object and send the event on its own.
> > > > + *	That's usually what happens when a job is queued to the writeback
> > > > + *	connector.    
> > > 
> > > The last sentence is confusing imo. Just drop it?  
> > 
> > Yes, I know, but it's also important to state that the ->no_blank +
> > event == NULL is a valid combination, and just means that the driver
> > decided to generate the event (that happens when a new WB job is
> > queued).  
> 
> Then make it more explicit, as-is I had no idea what you meant exactly.
> What about

Your suggestion is missing :P.

> >   
> > > 
> > > Please use the inline comment style for struct members, and then also
> > > polish the formatting a bit (e.g. paragraph breaks, which are only
> > > possible with the inline style).  
> > 
> > I considered that, but the other fields were already documented in the
> > single block above the struct, so I thought keeping things consistent
> > was better. Should I just add this field doc inline and keep the other
> > ones where they are, or should I add a patch moving all docs inline?  
> 
> There's _lots_ of inline comments already, and all new ones should be
> inline. The only reason I haven't done the conversion to all of them is
> that it would be a nice opportunity to update/clean up the comments
> (often there's a lot more to say than what's captured in the single line),
> which is why it didn't happen yet. Just moving the comments without
> updating them seems less useful imo.
> 
> I'd just do the inline comment for this, that's what everyone else is
> doing too.

Okay, will do.