diff mbox series

[2/3] drm/i915/guc: Clean up of register capture search

Message ID 20230203011043.3427096-3-John.C.Harrison@Intel.com (mailing list archive)
State New, archived
Headers show
Series More error capture improvements | expand

Commit Message

John Harrison Feb. 3, 2023, 1:10 a.m. UTC 
From: John Harrison <John.C.Harrison@Intel.com>

The comparison in the search for a matching register capture node was
not the most readable. So remove two redundant terms and re-format to
keep each term on a single line, and only one term per line.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Teres Alexis, Alan Previn Feb. 4, 2023, 7:29 a.m. UTC | #1 
I see you are inferring that a guc-id of zero can be valid.
I am guessing that might have contributed to some lost captures?
Thanks for catching this.

Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

On Thu, 2023-02-02 at 17:10 -0800, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> The comparison in the search for a matching register capture node was
> not the most readable. So remove two redundant terms and re-format to
> keep each term on a single line, and only one term per line.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> index 710999d7189ee..87b080dd6bead 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> @@ -1627,9 +1627,8 @@ void intel_guc_capture_get_matching_node(struct intel_gt *gt,
>         list_for_each_entry_safe(n, ntmp, &guc->capture->outlist, link) {
>                 if (n->eng_inst == GUC_ID_TO_ENGINE_INSTANCE(ee->engine->guc_id) &&
>                     n->eng_class == GUC_ID_TO_ENGINE_CLASS(ee->engine->guc_id) &&
> -                   n->guc_id && n->guc_id == ce->guc_id.id &&
> -                   (n->lrca & CTX_GTT_ADDRESS_MASK) && (n->lrca & CTX_GTT_ADDRESS_MASK) ==
> -                   (ce->lrc.lrca & CTX_GTT_ADDRESS_MASK)) {
> +                   n->guc_id == ce->guc_id.id &&
> +                   (n->lrca & CTX_GTT_ADDRESS_MASK) == (ce->lrc.lrca & CTX_GTT_ADDRESS_MASK)) {
>                         list_del(&n->link);
>                         ee->guc_capture_node = n;
>                         ee->guc_capture = guc->capture;
John Harrison Feb. 7, 2023, 3:18 a.m. UTC | #2 
On 2/3/2023 23:29, Teres Alexis, Alan Previn wrote:
> I see you are inferring that a guc-id of zero can be valid.
> I am guessing that might have contributed to some lost captures?
> Thanks for catching this.
I'm not inferring anything. I might be implying something, though. The 
patch description probably should have mentioned that change. I'll add 
something in.

There is nothing special about id zero. The lower X many ids are 
reserved for multi-LRC use. So you won't see zero being allocated 
normally. But run a multi-LRC app/test and the first context allocated 
should be id zero.

John.

>
> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
>
> On Thu, 2023-02-02 at 17:10 -0800, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> The comparison in the search for a matching register capture node was
>> not the most readable. So remove two redundant terms and re-format to
>> keep each term on a single line, and only one term per line.
>>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
>> index 710999d7189ee..87b080dd6bead 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
>> @@ -1627,9 +1627,8 @@ void intel_guc_capture_get_matching_node(struct intel_gt *gt,
>>          list_for_each_entry_safe(n, ntmp, &guc->capture->outlist, link) {
>>                  if (n->eng_inst == GUC_ID_TO_ENGINE_INSTANCE(ee->engine->guc_id) &&
>>                      n->eng_class == GUC_ID_TO_ENGINE_CLASS(ee->engine->guc_id) &&
>> -                   n->guc_id && n->guc_id == ce->guc_id.id &&
>> -                   (n->lrca & CTX_GTT_ADDRESS_MASK) && (n->lrca & CTX_GTT_ADDRESS_MASK) ==
>> -                   (ce->lrc.lrca & CTX_GTT_ADDRESS_MASK)) {
>> +                   n->guc_id == ce->guc_id.id &&
>> +                   (n->lrca & CTX_GTT_ADDRESS_MASK) == (ce->lrc.lrca & CTX_GTT_ADDRESS_MASK)) {
>>                          list_del(&n->link);
>>                          ee->guc_capture_node = n;
>>                          ee->guc_capture = guc->capture;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index 710999d7189ee..87b080dd6bead 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -1627,9 +1627,8 @@  void intel_guc_capture_get_matching_node(struct intel_gt *gt,
 	list_for_each_entry_safe(n, ntmp, &guc->capture->outlist, link) {
 		if (n->eng_inst == GUC_ID_TO_ENGINE_INSTANCE(ee->engine->guc_id) &&
 		    n->eng_class == GUC_ID_TO_ENGINE_CLASS(ee->engine->guc_id) &&
-		    n->guc_id && n->guc_id == ce->guc_id.id &&
-		    (n->lrca & CTX_GTT_ADDRESS_MASK) && (n->lrca & CTX_GTT_ADDRESS_MASK) ==
-		    (ce->lrc.lrca & CTX_GTT_ADDRESS_MASK)) {
+		    n->guc_id == ce->guc_id.id &&
+		    (n->lrca & CTX_GTT_ADDRESS_MASK) == (ce->lrc.lrca & CTX_GTT_ADDRESS_MASK)) {
 			list_del(&n->link);
 			ee->guc_capture_node = n;
 			ee->guc_capture = guc->capture;