[Intel-gfx] [PATCH] drm/i915/guc: Unify parameters of public CT functions

Sagar Arun Kamble sagar.a.kamble at intel.com
Tue Mar 20 15:08:09 UTC 2018



On 3/20/2018 6:30 PM, Michal Wajdeczko wrote:
> On Tue, 20 Mar 2018 08:24:14 +0100, Sagar Arun Kamble 
> <sagar.a.kamble at intel.com> wrote:
>
>>
>>
>> On 3/19/2018 8:58 PM, Michal Wajdeczko wrote:
>>> There is no need to mix parameter types in public CT functions
>>> as we can always accept intel_guc_ct.
>>>
>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
<snip>
>>>     /**
>>> - * Enable buffer based command transport
>>> + * intel_guc_ct_enable - Enable buffer based command transport.
>>> + * @ct: pointer to CT struct
>>> + *
>>>    * Shall only be called for platforms with HAS_GUC_CT.
>>> - * @guc:    the guc
>>> - * return:    0 on success
>>> - *        non-zero on failure
>>> + *
>>> + * Returns:
>>> + * 0 on success, a negative errno code on failure.
>> Should be
>>       * Return: 0 on sucess ...
>
> hmm, I'm not so sure:
>
> $ grep -r "\* Return: .*" drivers/gpu/drm/* | wc -l
> 153
>
> $ grep -r "\* Returns:$" drivers/gpu/drm/* | wc -l
> 344
>
Hi Michal,

kernel-doc rules recommend "Return:".

Thanks,
Sagar
>>>    */
>>> -int intel_guc_enable_ct(struct intel_guc *guc)
>>> +int intel_guc_ct_enable(struct intel_guc_ct *ct)
>>>   {
>>> +    struct intel_guc *guc = ct_to_guc(ct);
>>>       struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> change to *i915 as part of this patch itself? :) similar for disable.
>
> sure
>
>> Otherwise LGTM
>> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>
> thanks
> /m
>

-- 
Thanks,
Sagar



More information about the Intel-gfx mailing list