* [PATCH] drm/i915/guc: Unify parameters of public CT functions
@ 2018-03-19 15:28 Michal Wajdeczko
2018-03-19 17:06 ` ✗ Fi.CI.BAT: failure for " Patchwork
2018-03-20 7:24 ` [PATCH] " Sagar Arun Kamble
0 siblings, 2 replies; 6+ messages in thread
From: Michal Wajdeczko @ 2018-03-19 15:28 UTC (permalink / raw)
To: intel-gfx
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@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_guc_ct.c | 34 ++++++++++++++++++++++++----------
drivers/gpu/drm/i915/intel_guc_ct.h | 6 ++----
drivers/gpu/drm/i915/intel_uc.c | 4 ++--
3 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 0a0d3d5..7dd7de0 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -28,12 +28,21 @@
enum { CTB_OWNER_HOST = 0 };
+/**
+ * intel_guc_ct_init_early - Initialize CT state without requiring device access
+ * @ct: pointer to CT struct
+ */
void intel_guc_ct_init_early(struct intel_guc_ct *ct)
{
/* we're using static channel owners */
ct->host_channel.owner = CTB_OWNER_HOST;
}
+static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
+{
+ return container_of(ct, struct intel_guc, ct);
+}
+
static inline const char *guc_ct_buffer_type_to_str(u32 type)
{
switch (type) {
@@ -416,16 +425,19 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
}
/**
- * 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.
*/
-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);
- struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
+ struct intel_guc_ct_channel *ctch = &ct->host_channel;
int err;
GEM_BUG_ON(!HAS_GUC_CT(dev_priv));
@@ -441,14 +453,16 @@ int intel_guc_enable_ct(struct intel_guc *guc)
}
/**
- * Disable buffer based command transport.
+ * intel_guc_ct_disable - Disable buffer based command transport.
+ * @ct: pointer to CT struct
+ *
* Shall only be called for platforms with HAS_GUC_CT.
- * @guc: the guc
*/
-void intel_guc_disable_ct(struct intel_guc *guc)
+void intel_guc_ct_disable(struct intel_guc_ct *ct)
{
+ struct intel_guc *guc = ct_to_guc(ct);
struct drm_i915_private *dev_priv = guc_to_i915(guc);
- struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
+ struct intel_guc_ct_channel *ctch = &ct->host_channel;
GEM_BUG_ON(!HAS_GUC_CT(dev_priv));
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index 6d97f36..595c8ad 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -78,9 +78,7 @@ struct intel_guc_ct {
};
void intel_guc_ct_init_early(struct intel_guc_ct *ct);
-
-/* XXX: move to intel_uc.h ? don't fit there either */
-int intel_guc_enable_ct(struct intel_guc *guc);
-void intel_guc_disable_ct(struct intel_guc *guc);
+int intel_guc_ct_enable(struct intel_guc_ct *ct);
+void intel_guc_ct_disable(struct intel_guc_ct *ct);
#endif /* _INTEL_GUC_CT_H_ */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 34e847d..a45c2ce 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -232,7 +232,7 @@ static int guc_enable_communication(struct intel_guc *guc)
gen9_enable_guc_interrupts(dev_priv);
if (HAS_GUC_CT(dev_priv))
- return intel_guc_enable_ct(guc);
+ return intel_guc_ct_enable(&guc->ct);
guc->send = intel_guc_send_mmio;
return 0;
@@ -243,7 +243,7 @@ static void guc_disable_communication(struct intel_guc *guc)
struct drm_i915_private *dev_priv = guc_to_i915(guc);
if (HAS_GUC_CT(dev_priv))
- intel_guc_disable_ct(guc);
+ intel_guc_ct_disable(&guc->ct);
gen9_disable_guc_interrupts(dev_priv);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/guc: Unify parameters of public CT functions
2018-03-19 15:28 [PATCH] drm/i915/guc: Unify parameters of public CT functions Michal Wajdeczko
@ 2018-03-19 17:06 ` Patchwork
2018-03-20 7:24 ` [PATCH] " Sagar Arun Kamble
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-03-19 17:06 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/guc: Unify parameters of public CT functions
URL : https://patchwork.freedesktop.org/series/40197/
State : failure
== Summary ==
Series 40197v1 drm/i915/guc: Unify parameters of public CT functions
https://patchwork.freedesktop.org/api/1.0/series/40197/revisions/1/mbox/
---- Possible new issues:
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> INCOMPLETE (fi-bxt-dsi)
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:438s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:441s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:380s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:536s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:298s
fi-bxt-dsi total:217 pass:194 dwarn:0 dfail:0 fail:0 skip:22
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:513s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:513s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:503s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:407s
fi-cfl-s2 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:579s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:510s
fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:539s
fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:419s
fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:319s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:533s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:403s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:429s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:474s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:427s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:473s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:462s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:511s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:653s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:445s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:533s
fi-skl-6700hq total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:541s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:505s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:493s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:425s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:445s
fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:578s
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:398s
9b79492f331e36cf2e08d65ab3221abb66f67844 drm-tip: 2018y-03m-19d-14h-18m-12s UTC integration manifest
b6545a236eb7 drm/i915/guc: Unify parameters of public CT functions
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8396/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/guc: Unify parameters of public CT functions
2018-03-19 15:28 [PATCH] drm/i915/guc: Unify parameters of public CT functions Michal Wajdeczko
2018-03-19 17:06 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2018-03-20 7:24 ` Sagar Arun Kamble
2018-03-20 13:00 ` Michal Wajdeczko
1 sibling, 1 reply; 6+ messages in thread
From: Sagar Arun Kamble @ 2018-03-20 7:24 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
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@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_guc_ct.c | 34 ++++++++++++++++++++++++----------
> drivers/gpu/drm/i915/intel_guc_ct.h | 6 ++----
> drivers/gpu/drm/i915/intel_uc.c | 4 ++--
> 3 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index 0a0d3d5..7dd7de0 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -28,12 +28,21 @@
>
> enum { CTB_OWNER_HOST = 0 };
>
> +/**
> + * intel_guc_ct_init_early - Initialize CT state without requiring device access
> + * @ct: pointer to CT struct
> + */
> void intel_guc_ct_init_early(struct intel_guc_ct *ct)
> {
> /* we're using static channel owners */
> ct->host_channel.owner = CTB_OWNER_HOST;
> }
>
> +static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
> +{
> + return container_of(ct, struct intel_guc, ct);
> +}
> +
> static inline const char *guc_ct_buffer_type_to_str(u32 type)
> {
> switch (type) {
> @@ -416,16 +425,19 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
> }
>
> /**
> - * 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 ...
> */
> -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.
Otherwise LGTM
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> - struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
> + struct intel_guc_ct_channel *ctch = &ct->host_channel;
> int err;
>
> GEM_BUG_ON(!HAS_GUC_CT(dev_priv));
> @@ -441,14 +453,16 @@ int intel_guc_enable_ct(struct intel_guc *guc)
> }
>
> /**
> - * Disable buffer based command transport.
> + * intel_guc_ct_disable - Disable buffer based command transport.
> + * @ct: pointer to CT struct
> + *
> * Shall only be called for platforms with HAS_GUC_CT.
> - * @guc: the guc
> */
> -void intel_guc_disable_ct(struct intel_guc *guc)
> +void intel_guc_ct_disable(struct intel_guc_ct *ct)
> {
> + struct intel_guc *guc = ct_to_guc(ct);
> struct drm_i915_private *dev_priv = guc_to_i915(guc);
> - struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
> + struct intel_guc_ct_channel *ctch = &ct->host_channel;
>
> GEM_BUG_ON(!HAS_GUC_CT(dev_priv));
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
> index 6d97f36..595c8ad 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.h
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.h
> @@ -78,9 +78,7 @@ struct intel_guc_ct {
> };
>
> void intel_guc_ct_init_early(struct intel_guc_ct *ct);
> -
> -/* XXX: move to intel_uc.h ? don't fit there either */
> -int intel_guc_enable_ct(struct intel_guc *guc);
> -void intel_guc_disable_ct(struct intel_guc *guc);
> +int intel_guc_ct_enable(struct intel_guc_ct *ct);
> +void intel_guc_ct_disable(struct intel_guc_ct *ct);
>
> #endif /* _INTEL_GUC_CT_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 34e847d..a45c2ce 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -232,7 +232,7 @@ static int guc_enable_communication(struct intel_guc *guc)
> gen9_enable_guc_interrupts(dev_priv);
>
> if (HAS_GUC_CT(dev_priv))
> - return intel_guc_enable_ct(guc);
> + return intel_guc_ct_enable(&guc->ct);
>
> guc->send = intel_guc_send_mmio;
> return 0;
> @@ -243,7 +243,7 @@ static void guc_disable_communication(struct intel_guc *guc)
> struct drm_i915_private *dev_priv = guc_to_i915(guc);
>
> if (HAS_GUC_CT(dev_priv))
> - intel_guc_disable_ct(guc);
> + intel_guc_ct_disable(&guc->ct);
>
> gen9_disable_guc_interrupts(dev_priv);
>
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/guc: Unify parameters of public CT functions
2018-03-20 7:24 ` [PATCH] " Sagar Arun Kamble
@ 2018-03-20 13:00 ` Michal Wajdeczko
2018-03-20 15:08 ` Sagar Arun Kamble
0 siblings, 1 reply; 6+ messages in thread
From: Michal Wajdeczko @ 2018-03-20 13:00 UTC (permalink / raw)
To: intel-gfx, Sagar Arun Kamble
On Tue, 20 Mar 2018 08:24:14 +0100, Sagar Arun Kamble
<sagar.a.kamble@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@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>> drivers/gpu/drm/i915/intel_guc_ct.c | 34
>> ++++++++++++++++++++++++----------
>> drivers/gpu/drm/i915/intel_guc_ct.h | 6 ++----
>> drivers/gpu/drm/i915/intel_uc.c | 4 ++--
>> 3 files changed, 28 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c
>> b/drivers/gpu/drm/i915/intel_guc_ct.c
>> index 0a0d3d5..7dd7de0 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
>> @@ -28,12 +28,21 @@
>> enum { CTB_OWNER_HOST = 0 };
>> +/**
>> + * intel_guc_ct_init_early - Initialize CT state without requiring
>> device access
>> + * @ct: pointer to CT struct
>> + */
>> void intel_guc_ct_init_early(struct intel_guc_ct *ct)
>> {
>> /* we're using static channel owners */
>> ct->host_channel.owner = CTB_OWNER_HOST;
>> }
>> +static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
>> +{
>> + return container_of(ct, struct intel_guc, ct);
>> +}
>> +
>> static inline const char *guc_ct_buffer_type_to_str(u32 type)
>> {
>> switch (type) {
>> @@ -416,16 +425,19 @@ static int intel_guc_send_ct(struct intel_guc
>> *guc, const u32 *action, u32 len)
>> }
>> /**
>> - * 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
>> */
>> -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@intel.com>
thanks
/m
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/guc: Unify parameters of public CT functions
2018-03-20 13:00 ` Michal Wajdeczko
@ 2018-03-20 15:08 ` Sagar Arun Kamble
2018-03-27 10:23 ` Jani Nikula
0 siblings, 1 reply; 6+ messages in thread
From: Sagar Arun Kamble @ 2018-03-20 15:08 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
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@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@intel.com>
>>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> Cc: Chris Wilson <chris@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@intel.com>
>
> thanks
> /m
>
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/guc: Unify parameters of public CT functions
2018-03-20 15:08 ` Sagar Arun Kamble
@ 2018-03-27 10:23 ` Jani Nikula
0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2018-03-27 10:23 UTC (permalink / raw)
To: Sagar Arun Kamble, Michal Wajdeczko, intel-gfx
On Tue, 20 Mar 2018, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote:
> 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@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@intel.com>
>>>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>>> Cc: Chris Wilson <chris@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:".
Correct. For legacy reasons, a bunch of variants are recognized and
canonicalized to "Return" in the output. I also recommend documenting
the return values immediately following "Return: ", without \n, similar
to the parameter documentation.
BR,
Jani.
>
> 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@intel.com>
>>
>> thanks
>> /m
>>
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-27 10:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-19 15:28 [PATCH] drm/i915/guc: Unify parameters of public CT functions Michal Wajdeczko
2018-03-19 17:06 ` ✗ Fi.CI.BAT: failure for " Patchwork
2018-03-20 7:24 ` [PATCH] " Sagar Arun Kamble
2018-03-20 13:00 ` Michal Wajdeczko
2018-03-20 15:08 ` Sagar Arun Kamble
2018-03-27 10:23 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).