diff mbox series

[v10,3/6] drm/i915/gt: Drop invalidate_csb_entries

Message ID 20220210183636.1187973-4-michael.cheng@intel.com (mailing list archive)
State New, archived
Headers show
Series Use drm_clflush* instead of clflush | expand

Commit Message

Michael Cheng Feb. 10, 2022, 6:36 p.m. UTC
Drop invalidate_csb_entries and directly call drm_clflush_virt_range.
This allows for one less function call, and prevent complier errors when
building for non-x86 architectures.

v2(Michael Cheng): Drop invalidate_csb_entries function and directly
		   invoke drm_clflush_virt_range. Thanks to Tvrtko for the
		   sugguestion.

v3(Michael Cheng): Use correct parameters for drm_clflush_virt_range.
		   Thanks to Tvrtko for pointing this out.

Signed-off-by: Michael Cheng <michael.cheng@intel.com>
---
 .../gpu/drm/i915/gt/intel_execlists_submission.c    | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

Comments

Matt Roper Feb. 22, 2022, 10:31 p.m. UTC | #1
On Thu, Feb 10, 2022 at 10:36:33AM -0800, Michael Cheng wrote:
> Drop invalidate_csb_entries and directly call drm_clflush_virt_range.
> This allows for one less function call, and prevent complier errors when
> building for non-x86 architectures.
> 
> v2(Michael Cheng): Drop invalidate_csb_entries function and directly
> 		   invoke drm_clflush_virt_range. Thanks to Tvrtko for the
> 		   sugguestion.
> 
> v3(Michael Cheng): Use correct parameters for drm_clflush_virt_range.
> 		   Thanks to Tvrtko for pointing this out.
> 
> Signed-off-by: Michael Cheng <michael.cheng@intel.com>
> ---
>  .../gpu/drm/i915/gt/intel_execlists_submission.c    | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> index 9bb7c863172f..6186a5e4b191 100644
> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> @@ -1646,12 +1646,6 @@ cancel_port_requests(struct intel_engine_execlists * const execlists,
>  	return inactive;
>  }
>  
> -static void invalidate_csb_entries(const u64 *first, const u64 *last)
> -{
> -	clflush((void *)first);
> -	clflush((void *)last);
> -}
> -
>  /*
>   * Starting with Gen12, the status has a new format:
>   *
> @@ -1999,7 +1993,7 @@ process_csb(struct intel_engine_cs *engine, struct i915_request **inactive)
>  	 * the wash as hardware, working or not, will need to do the
>  	 * invalidation before.
>  	 */
> -	invalidate_csb_entries(&buf[0], &buf[num_entries - 1]);
> +	drm_clflush_virt_range(&buf[0], num_entries * sizeof(buf[0]));
>  
>  	/*
>  	 * We assume that any event reflects a change in context flow
> @@ -2783,8 +2777,9 @@ static void reset_csb_pointers(struct intel_engine_cs *engine)
>  
>  	/* Check that the GPU does indeed update the CSB entries! */
>  	memset(execlists->csb_status, -1, (reset_value + 1) * sizeof(u64));
> -	invalidate_csb_entries(&execlists->csb_status[0],
> -			       &execlists->csb_status[reset_value]);
> +	drm_clflush_virt_range(&execlists->csb_status[0],

I think you could simplify the parameter slightly by just writing it as
'execlists->csb_status'

> +			       execlists->csb_size *
> +			       sizeof(execlists->csb_status[0]));

The existing code only issues a clflush for the first and last entries
rather than the range from 0..reset_value, but since there are only a
maximum of 12 u64 entries, which fits into two cachelines, the end
result should be the same either way.

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>


>  
>  	/* Once more for luck and our trusty paranoia */
>  	ENGINE_WRITE(engine, RING_CONTEXT_STATUS_PTR,
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 9bb7c863172f..6186a5e4b191 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -1646,12 +1646,6 @@  cancel_port_requests(struct intel_engine_execlists * const execlists,
 	return inactive;
 }
 
-static void invalidate_csb_entries(const u64 *first, const u64 *last)
-{
-	clflush((void *)first);
-	clflush((void *)last);
-}
-
 /*
  * Starting with Gen12, the status has a new format:
  *
@@ -1999,7 +1993,7 @@  process_csb(struct intel_engine_cs *engine, struct i915_request **inactive)
 	 * the wash as hardware, working or not, will need to do the
 	 * invalidation before.
 	 */
-	invalidate_csb_entries(&buf[0], &buf[num_entries - 1]);
+	drm_clflush_virt_range(&buf[0], num_entries * sizeof(buf[0]));
 
 	/*
 	 * We assume that any event reflects a change in context flow
@@ -2783,8 +2777,9 @@  static void reset_csb_pointers(struct intel_engine_cs *engine)
 
 	/* Check that the GPU does indeed update the CSB entries! */
 	memset(execlists->csb_status, -1, (reset_value + 1) * sizeof(u64));
-	invalidate_csb_entries(&execlists->csb_status[0],
-			       &execlists->csb_status[reset_value]);
+	drm_clflush_virt_range(&execlists->csb_status[0],
+			       execlists->csb_size *
+			       sizeof(execlists->csb_status[0]));
 
 	/* Once more for luck and our trusty paranoia */
 	ENGINE_WRITE(engine, RING_CONTEXT_STATUS_PTR,