[Intel-gfx] [PATCH 1/3] drm/i915: Add kerneldoc for intel_pipe_update_{start, end}

Ander Conselvan de Oliveira ander.conselvan.de.oliveira at intel.com
Mon Nov 3 13:41:57 CET 2014


On 11/03/2014 02:33 PM, Daniel Vetter wrote:
> On Tue, Oct 28, 2014 at 03:10:12PM +0200, Ander Conselvan de Oliveira wrote:
>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_sprite.c | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>> index 8b80d68..f9ddedc 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -46,6 +46,22 @@ static int usecs_to_scanlines(const struct drm_display_mode *mode, int usecs)
>>   	return DIV_ROUND_UP(usecs * mode->crtc_clock, 1000 * mode->crtc_htotal);
>>   }
>>
>> +/**
>> + * intel_pipe_update_start() - start update of a set of display registers
>> + * @crtc: the crtc of which the registers are going to be updated
>> + * @start_vbl_count: vblank counter return pointer used for error checking
>> + *
>> + * Mark the start of an update to pipe registers that should be updated
>> + * atomically regarding vblank. If the next vblank will happens within
>> + * the next 100 us, this function waits until the vblank passes.
>> + *
>> + * After a successful call to this function, interrupts will be disabled
>> + * until a subsequent call to intel_pipe_update_end(). That is done to
>> + * avoid random delays. The value written to @start_vbl_count should be
>> + * supplied to intel_pipe_update_end() for error checking.
>> + *
>> + * Return: true if the call was successful
>> + */
>
> It's nice that people now go overboard with kerneldoc, but I think we need
> to strike a good balance. And in general I think documenting static inline
> functions isn't worth it - they really should be self-explanatory as-is.
>
> Documentation is imo only really useful for the bigger stuff, which
> usually means it's used in a few places all over. So non-static functions.
>
> This one here is a bit a corner-case since it's more of a generic piece of
> infrastructure. But otoh I think in the end we'll have exactly one caller
> of these functions for all atomic plane updates we'll need. Everyone else
> (legacy code, modeset code, ...) will just call that higher-level
> function. Not sure what to do here.

s/**/*/ ?

Cheers,
Ander
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.




More information about the Intel-gfx mailing list