[2/5] i915/drm/guc: Don't pass CTB while writing
diff mbox series

Message ID 20200115140822.55756-3-michal.wajdeczko@intel.com
State New
Headers show
Series
  • Misc GuC CT improvements - part II
Related show

Commit Message

Michal Wajdeczko Jan. 15, 2020, 2:08 p.m. UTC
Since we only have one SEND buffer we don't need to explicitly pass
it to the write function.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Daniele Ceraolo Spurio Jan. 16, 2020, 6:53 p.m. UTC | #1
On 1/15/20 6:08 AM, Michal Wajdeczko wrote:
> Since we only have one SEND buffer we don't need to explicitly pass
> it to the write function.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> index 0d3556a820a3..dedbf3b8ab01 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> @@ -311,12 +311,13 @@ static u32 ct_get_next_fence(struct intel_guc_ct *ct)
>    *                   ^-----------------len-------------------^
>    */
>   
> -static int ctb_write(struct intel_guc_ct_buffer *ctb,
> -		     const u32 *action,
> -		     u32 len /* in dwords */,
> -		     u32 fence,
> -		     bool want_response)
> +static int ct_write(struct intel_guc_ct *ct,
> +		    const u32 *action,
> +		    u32 len /* in dwords */,
> +		    u32 fence,
> +		    bool want_response)
>   {
> +	struct intel_guc_ct_buffer *ctb = &ct->ctbs[CTB_SEND];
>   	struct guc_ct_buffer_desc *desc = ctb->desc;
>   	u32 head = desc->head / 4;	/* in dwords */
>   	u32 tail = desc->tail / 4;	/* in dwords */
> @@ -492,7 +493,7 @@ static int ct_send(struct intel_guc_ct *ct,
>   	list_add_tail(&request.link, &ct->requests.pending);
>   	spin_unlock_irqrestore(&ct->requests.lock, flags);
>   
> -	err = ctb_write(ctb, action, len, fence, !!response_buf);
> +	err = ct_write(ct, action, len, fence, !!response_buf);

I'd update wait_for_ctb_desc_update() to work on struct intel_guc_ct as 
well, so we can hide the ctb desc access in the lower level functions 
and drop the ctb variable from ct_send(). With or without that:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

>   	if (unlikely(err))
>   		goto unlink;
>   
>
Michal Wajdeczko Jan. 16, 2020, 7:15 p.m. UTC | #2
On Thu, 16 Jan 2020 19:53:08 +0100, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

>
>
> On 1/15/20 6:08 AM, Michal Wajdeczko wrote:
>> Since we only have one SEND buffer we don't need to explicitly pass
>> it to the write function.
>>  Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>  diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c  
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> index 0d3556a820a3..dedbf3b8ab01 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> @@ -311,12 +311,13 @@ static u32 ct_get_next_fence(struct intel_guc_ct  
>> *ct)
>>    *                   ^-----------------len-------------------^
>>    */
>>   -static int ctb_write(struct intel_guc_ct_buffer *ctb,
>> -		     const u32 *action,
>> -		     u32 len /* in dwords */,
>> -		     u32 fence,
>> -		     bool want_response)
>> +static int ct_write(struct intel_guc_ct *ct,
>> +		    const u32 *action,
>> +		    u32 len /* in dwords */,
>> +		    u32 fence,
>> +		    bool want_response)
>>   {
>> +	struct intel_guc_ct_buffer *ctb = &ct->ctbs[CTB_SEND];
>>   	struct guc_ct_buffer_desc *desc = ctb->desc;
>>   	u32 head = desc->head / 4;	/* in dwords */
>>   	u32 tail = desc->tail / 4;	/* in dwords */
>> @@ -492,7 +493,7 @@ static int ct_send(struct intel_guc_ct *ct,
>>   	list_add_tail(&request.link, &ct->requests.pending);
>>   	spin_unlock_irqrestore(&ct->requests.lock, flags);
>>   -	err = ctb_write(ctb, action, len, fence, !!response_buf);
>> +	err = ct_write(ct, action, len, fence, !!response_buf);
>
> I'd update wait_for_ctb_desc_update() to work on struct intel_guc_ct as  
> well, so we can hide the ctb desc access in the lower level functions  
> and drop the ctb variable from ct_send(). With or without that:

in next series I was planning to remove wait_for_ctb_desc_update()
that's why I didn't update it now

>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> Daniele
>
>>   	if (unlikely(err))
>>   		goto unlink;

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index 0d3556a820a3..dedbf3b8ab01 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -311,12 +311,13 @@  static u32 ct_get_next_fence(struct intel_guc_ct *ct)
  *                   ^-----------------len-------------------^
  */
 
-static int ctb_write(struct intel_guc_ct_buffer *ctb,
-		     const u32 *action,
-		     u32 len /* in dwords */,
-		     u32 fence,
-		     bool want_response)
+static int ct_write(struct intel_guc_ct *ct,
+		    const u32 *action,
+		    u32 len /* in dwords */,
+		    u32 fence,
+		    bool want_response)
 {
+	struct intel_guc_ct_buffer *ctb = &ct->ctbs[CTB_SEND];
 	struct guc_ct_buffer_desc *desc = ctb->desc;
 	u32 head = desc->head / 4;	/* in dwords */
 	u32 tail = desc->tail / 4;	/* in dwords */
@@ -492,7 +493,7 @@  static int ct_send(struct intel_guc_ct *ct,
 	list_add_tail(&request.link, &ct->requests.pending);
 	spin_unlock_irqrestore(&ct->requests.lock, flags);
 
-	err = ctb_write(ctb, action, len, fence, !!response_buf);
+	err = ct_write(ct, action, len, fence, !!response_buf);
 	if (unlikely(err))
 		goto unlink;