[PATCH 00/15] drm/i915/guc: Support for Guc responses and requests

[PATCH 00/15] drm/i915/guc: Support for Guc responses and requests

[Michal Wajdeczko]

With this series we will be able to receive more data from the Guc.
New Guc firmware will be required to actually use that feature.

Michal Wajdeczko (13):
  drm/i915/guc: Add support for data reporting in GuC responses
  drm/i915/guc: Prepare send() function to accept bigger response
  drm/i915/guc: Add send_and_receive() helper function
  drm/i915/guc: Implement response handling in send_mmio()
  drm/i915/guc: Move Guc notification handling to separate function
  drm/i915/guc: Update CT message header definition
  drm/i915/guc: Prepare to handle messages from CT RECV buffer
  drm/i915/guc: Use better name for helper wait function
  drm/i915/guc: Implement response handling in send_ct()
  drm/i915/guc: Prepare to process incoming requests from CT
  drm/i915/guc: Handle default action received over CT
  drm/i915/guc: Enable GuC interrupts when using CT
  drm/i915/guc: Trace messages from CT while in debug

Oscar Mateo (2):
  drm/i915/guc: Move flushing the GuC logs outside notification handler
  drm/i915/guc: Create a GuC receive function

 drivers/gpu/drm/i915/i915_guc_submission.c |   2 +-
 drivers/gpu/drm/i915/i915_irq.c            |  33 +--
 drivers/gpu/drm/i915/intel_guc_ct.c        | 359 +++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_guc_ct.h        |   9 +
 drivers/gpu/drm/i915/intel_guc_fwif.h      |  17 +-
 drivers/gpu/drm/i915/intel_guc_log.c       |   8 +
 drivers/gpu/drm/i915/intel_uc.c            |  74 +++++-
 drivers/gpu/drm/i915/intel_uc.h            |  23 +-
 8 files changed, 462 insertions(+), 63 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/15] drm/i915/guc: Add support for data reporting in GuC responses

[Michal Wajdeczko]

GuC may return additional data in the command status response.
Format and meaning of this data is action specific.
We will use this non-negative data as a new success return value.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++++++-------
 drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++++++
 drivers/gpu/drm/i915/intel_uc.c       |  5 ++++-
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index c4cbec1..1249868 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
 	err = wait_for_response(desc, fence, status);
 	if (unlikely(err))
 		return err;
-	if (*status != INTEL_GUC_STATUS_SUCCESS)
+	if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
 		return -EIO;
-	return 0;
+	return INTEL_GUC_RECV_TO_DATA(*status);
 }
 
 /*
@@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
 {
 	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
 	u32 status = ~0; /* undefined */
-	int err;
+	int ret;
 
 	mutex_lock(&guc->send_mutex);
 
-	err = ctch_send(guc, ctch, action, len, &status);
-	if (unlikely(err)) {
+	ret = ctch_send(guc, ctch, action, len, &status);
+	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
-			  action[0], err, status);
+			  action[0], ret, status);
 	}
 
 	mutex_unlock(&guc->send_mutex);
-	return err;
+	return ret;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 5fa2860..98c0560 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -567,10 +567,16 @@ enum intel_guc_action {
  * command in SS0. The response is distinguishable from a command
  * by the fact that all the MASK bits are set. The remaining bits
  * give more detail.
+ * Bits [16:27] are reserved for optional data reporting.
  */
 #define	INTEL_GUC_RECV_MASK	((u32)0xF0000000)
 #define	INTEL_GUC_RECV_IS_RESPONSE(x)	((u32)(x) >= INTEL_GUC_RECV_MASK)
 #define	INTEL_GUC_RECV_STATUS(x)	(INTEL_GUC_RECV_MASK | (x))
+#define INTEL_GUC_RECV_DATA_SHIFT	16
+#define INTEL_GUC_RECV_DATA_MASK	(0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
+#define INTEL_GUC_RECV_TO_STATUS(x)	((x) & ~ INTEL_GUC_RECV_DATA_MASK)
+#define INTEL_GUC_RECV_TO_DATA(x)	(((x) & INTEL_GUC_RECV_DATA_MASK) >> \
+					 INTEL_GUC_RECV_DATA_SHIFT)
 
 /* GUC will return status back to SOFT_SCRATCH_O_REG */
 enum intel_guc_status {
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 27e072c..ff25477 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
 					   INTEL_GUC_RECV_MASK,
 					   INTEL_GUC_RECV_MASK,
 					   10, 10, &status);
-	if (status != INTEL_GUC_STATUS_SUCCESS) {
+	if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
 		/*
 		 * Either the GuC explicitly returned an error (which
 		 * we convert to -EIO here) or no response at all was
@@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
 		DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
 			 " ret=%d status=0x%08X response=0x%08X\n",
 			 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
+	} else {
+		/* Use data encoded in status dword as return value */
+		ret = INTEL_GUC_RECV_TO_DATA(status);
 	}
 
 	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/15] drm/i915/guc: Add support for data reporting in GuC responses

[Michel Thierry]

On 8/4/2017 9:26 AM, Michal Wajdeczko wrote:
> GuC may return additional data in the command status response.
> Format and meaning of this data is action specific.
> We will use this non-negative data as a new success return value.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++++++-------
>   drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++++++
>   drivers/gpu/drm/i915/intel_uc.c       |  5 ++++-
>   3 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index c4cbec1..1249868 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
>   	err = wait_for_response(desc, fence, status);
>   	if (unlikely(err))
>   		return err;
> -	if (*status != INTEL_GUC_STATUS_SUCCESS)
> +	if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
>   		return -EIO;
> -	return 0;
> +	return INTEL_GUC_RECV_TO_DATA(*status);
>   }
>   
>   /*
> @@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
>   {
>   	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
>   	u32 status = ~0; /* undefined */
> -	int err;
> +	int ret;
>   
>   	mutex_lock(&guc->send_mutex);
>   
> -	err = ctch_send(guc, ctch, action, len, &status);
> -	if (unlikely(err)) {
> +	ret = ctch_send(guc, ctch, action, len, &status);
> +	if (unlikely(ret < 0)) {
>   		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
> -			  action[0], err, status);
> +			  action[0], ret, status);
>   	}
>   
>   	mutex_unlock(&guc->send_mutex);
> -	return err;
> +	return ret;
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 5fa2860..98c0560 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -567,10 +567,16 @@ enum intel_guc_action {
>    * command in SS0. The response is distinguishable from a command
>    * by the fact that all the MASK bits are set. The remaining bits
>    * give more detail.
> + * Bits [16:27] are reserved for optional data reporting.
>    */
>   #define	INTEL_GUC_RECV_MASK	((u32)0xF0000000)
>   #define	INTEL_GUC_RECV_IS_RESPONSE(x)	((u32)(x) >= INTEL_GUC_RECV_MASK)
>   #define	INTEL_GUC_RECV_STATUS(x)	(INTEL_GUC_RECV_MASK | (x))
> +#define INTEL_GUC_RECV_DATA_SHIFT	16
> +#define INTEL_GUC_RECV_DATA_MASK	(0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
> +#define INTEL_GUC_RECV_TO_STATUS(x)	((x) & ~ INTEL_GUC_RECV_DATA_MASK)

checkpatch should have complained about the blank space after '~'.

> +#define INTEL_GUC_RECV_TO_DATA(x)	(((x) & INTEL_GUC_RECV_DATA_MASK) >> \
> +					 INTEL_GUC_RECV_DATA_SHIFT)
>   
>   /* GUC will return status back to SOFT_SCRATCH_O_REG */
>   enum intel_guc_status {
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 27e072c..ff25477 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
>   					   INTEL_GUC_RECV_MASK,
>   					   INTEL_GUC_RECV_MASK,
>   					   10, 10, &status);
> -	if (status != INTEL_GUC_STATUS_SUCCESS) {
> +	if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
>   		/*
>   		 * Either the GuC explicitly returned an error (which
>   		 * we convert to -EIO here) or no response at all was
> @@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
>   		DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
>   			 " ret=%d status=0x%08X response=0x%08X\n",
>   			 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
> +	} else {
> +		/* Use data encoded in status dword as return value */
> +		ret = INTEL_GUC_RECV_TO_DATA(status);
>   	}
>   
>   	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
> 

Other than the blank space after that '~', it looks good to me.

Just a note, for other people reading this; there are 3 cases in which 
intel_guc_send return value is only checked for truthiness (i.e. 
__guc_allocate_doorbell, __guc_deallocate_doorbell and 
intel_guc_sample_forcewake callers are checking for 'if (err)').

I know none of the existing H2G commands will return any extra data, so 
intel_guc_send should be returning only negative numbers or zero (for now).

-Michel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/15] drm/i915/guc: Add support for data reporting in GuC responses

[Daniele Ceraolo Spurio]

On 04/08/17 13:40, Michel Thierry wrote:
> On 8/4/2017 9:26 AM, Michal Wajdeczko wrote:
>> GuC may return additional data in the command status response.
>> Format and meaning of this data is action specific.
>> We will use this non-negative data as a new success return value.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++++++-------
>>   drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++++++
>>   drivers/gpu/drm/i915/intel_uc.c       |  5 ++++-
>>   3 files changed, 17 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
>> b/drivers/gpu/drm/i915/intel_guc_ct.c
>> index c4cbec1..1249868 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
>> @@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
>>       err = wait_for_response(desc, fence, status);
>>       if (unlikely(err))
>>           return err;
>> -    if (*status != INTEL_GUC_STATUS_SUCCESS)
>> +    if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
>>           return -EIO;
>> -    return 0;
>> +    return INTEL_GUC_RECV_TO_DATA(*status);
>>   }
>>   /*
>> @@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc 
>> *guc, const u32 *action, u32 len)
>>   {
>>       struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
>>       u32 status = ~0; /* undefined */
>> -    int err;
>> +    int ret;
>>       mutex_lock(&guc->send_mutex);
>> -    err = ctch_send(guc, ctch, action, len, &status);
>> -    if (unlikely(err)) {
>> +    ret = ctch_send(guc, ctch, action, len, &status);
>> +    if (unlikely(ret < 0)) {
>>           DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
>> -              action[0], err, status);
>> +              action[0], ret, status);
>>       }
>>       mutex_unlock(&guc->send_mutex);
>> -    return err;
>> +    return ret;
>>   }
>>   /**
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
>> b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> index 5fa2860..98c0560 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> @@ -567,10 +567,16 @@ enum intel_guc_action {
>>    * command in SS0. The response is distinguishable from a command
>>    * by the fact that all the MASK bits are set. The remaining bits
>>    * give more detail.
>> + * Bits [16:27] are reserved for optional data reporting.

mmm, from the information I can find the optional data reporting bits 
are only [16:22], while [23:27] should be MBZ. Are you extending the 
range to cope with future changes on the GuC side or am I missing 
something? If it is the first case, my personal preference would be to 
stick with whatever is in the GuC API to avoid confusion. Since you're 
adding all the defines below it should be trivial to extend it if we 
ever need to.

>>    */
>>   #define    INTEL_GUC_RECV_MASK    ((u32)0xF0000000)
>>   #define    INTEL_GUC_RECV_IS_RESPONSE(x)    ((u32)(x) >= 
>> INTEL_GUC_RECV_MASK)
>>   #define    INTEL_GUC_RECV_STATUS(x)    (INTEL_GUC_RECV_MASK | (x))
>> +#define INTEL_GUC_RECV_DATA_SHIFT    16
>> +#define INTEL_GUC_RECV_DATA_MASK    (0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
>> +#define INTEL_GUC_RECV_TO_STATUS(x)    ((x) & ~ 
>> INTEL_GUC_RECV_DATA_MASK)
> 
> checkpatch should have complained about the blank space after '~'.
> 
>> +#define INTEL_GUC_RECV_TO_DATA(x)    (((x) & 
>> INTEL_GUC_RECV_DATA_MASK) >> \
>> +                     INTEL_GUC_RECV_DATA_SHIFT)
>>   /* GUC will return status back to SOFT_SCRATCH_O_REG */
>>   enum intel_guc_status {
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 27e072c..ff25477 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, 
>> const u32 *action, u32 len)
>>                          INTEL_GUC_RECV_MASK,
>>                          INTEL_GUC_RECV_MASK,
>>                          10, 10, &status);
>> -    if (status != INTEL_GUC_STATUS_SUCCESS) {
>> +    if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
>>           /*
>>            * Either the GuC explicitly returned an error (which
>>            * we convert to -EIO here) or no response at all was
>> @@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, 
>> const u32 *action, u32 len)
>>           DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
>>                " ret=%d status=0x%08X response=0x%08X\n",
>>                action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
>> +    } else {
>> +        /* Use data encoded in status dword as return value */
>> +        ret = INTEL_GUC_RECV_TO_DATA(status);
>>       }
>>       intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
>>
> 
> Other than the blank space after that '~', it looks good to me.
> 
> Just a note, for other people reading this; there are 3 cases in which 
> intel_guc_send return value is only checked for truthiness (i.e. 
> __guc_allocate_doorbell, __guc_deallocate_doorbell and 
> intel_guc_sample_forcewake callers are checking for 'if (err)').
> 
> I know none of the existing H2G commands will return any extra data, so 
> intel_guc_send should be returning only negative numbers or zero (for now).
> 

I'd add a note in the commit message about the fact that we currently 
don't expect any command to return data in the status dword.

-Daniele

> -Michel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/15] drm/i915/guc: Add support for data reporting in GuC responses

[Michal Wajdeczko]

On Fri, Aug 04, 2017 at 02:29:33PM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 04/08/17 13:40, Michel Thierry wrote:
> > On 8/4/2017 9:26 AM, Michal Wajdeczko wrote:
> > > GuC may return additional data in the command status response.
> > > Format and meaning of this data is action specific.
> > > We will use this non-negative data as a new success return value.
> > > 
> > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > > Cc: Oscar Mateo <oscar.mateo@intel.com>
> > > Cc: Michel Thierry <michel.thierry@intel.com>
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++++++-------
> > >   drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++++++
> > >   drivers/gpu/drm/i915/intel_uc.c       |  5 ++++-
> > >   3 files changed, 17 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c
> > > b/drivers/gpu/drm/i915/intel_guc_ct.c
> > > index c4cbec1..1249868 100644
> > > --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> > > +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> > > @@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
> > >       err = wait_for_response(desc, fence, status);
> > >       if (unlikely(err))
> > >           return err;
> > > -    if (*status != INTEL_GUC_STATUS_SUCCESS)
> > > +    if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
> > >           return -EIO;
> > > -    return 0;
> > > +    return INTEL_GUC_RECV_TO_DATA(*status);
> > >   }
> > >   /*
> > > @@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc
> > > *guc, const u32 *action, u32 len)
> > >   {
> > >       struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
> > >       u32 status = ~0; /* undefined */
> > > -    int err;
> > > +    int ret;
> > >       mutex_lock(&guc->send_mutex);
> > > -    err = ctch_send(guc, ctch, action, len, &status);
> > > -    if (unlikely(err)) {
> > > +    ret = ctch_send(guc, ctch, action, len, &status);
> > > +    if (unlikely(ret < 0)) {
> > >           DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
> > > -              action[0], err, status);
> > > +              action[0], ret, status);
> > >       }
> > >       mutex_unlock(&guc->send_mutex);
> > > -    return err;
> > > +    return ret;
> > >   }
> > >   /**
> > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > index 5fa2860..98c0560 100644
> > > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > @@ -567,10 +567,16 @@ enum intel_guc_action {
> > >    * command in SS0. The response is distinguishable from a command
> > >    * by the fact that all the MASK bits are set. The remaining bits
> > >    * give more detail.
> > > + * Bits [16:27] are reserved for optional data reporting.
> 
> mmm, from the information I can find the optional data reporting bits are
> only [16:22], while [23:27] should be MBZ. Are you extending the range to
> cope with future changes on the GuC side or am I missing something? If it is
> the first case, my personal preference would be to stick with whatever is in
> the GuC API to avoid confusion. Since you're adding all the defines below it
> should be trivial to extend it if we ever need to.

It's the former case. But by looking the same information, only [15:0] bits are
declared for success/failure code, and [27:23] are MBZ for specific action.
So current proposal is still in line with that spec.

Michal

> 
> > >    */
> > >   #define    INTEL_GUC_RECV_MASK    ((u32)0xF0000000)
> > >   #define    INTEL_GUC_RECV_IS_RESPONSE(x)    ((u32)(x) >=
> > > INTEL_GUC_RECV_MASK)
> > >   #define    INTEL_GUC_RECV_STATUS(x)    (INTEL_GUC_RECV_MASK | (x))
> > > +#define INTEL_GUC_RECV_DATA_SHIFT    16
> > > +#define INTEL_GUC_RECV_DATA_MASK    (0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
> > > +#define INTEL_GUC_RECV_TO_STATUS(x)    ((x) & ~
> > > INTEL_GUC_RECV_DATA_MASK)
> > 
> > checkpatch should have complained about the blank space after '~'.
> > 
> > > +#define INTEL_GUC_RECV_TO_DATA(x)    (((x) &
> > > INTEL_GUC_RECV_DATA_MASK) >> \
> > > +                     INTEL_GUC_RECV_DATA_SHIFT)
> > >   /* GUC will return status back to SOFT_SCRATCH_O_REG */
> > >   enum intel_guc_status {
> > > diff --git a/drivers/gpu/drm/i915/intel_uc.c
> > > b/drivers/gpu/drm/i915/intel_uc.c
> > > index 27e072c..ff25477 100644
> > > --- a/drivers/gpu/drm/i915/intel_uc.c
> > > +++ b/drivers/gpu/drm/i915/intel_uc.c
> > > @@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc,
> > > const u32 *action, u32 len)
> > >                          INTEL_GUC_RECV_MASK,
> > >                          INTEL_GUC_RECV_MASK,
> > >                          10, 10, &status);
> > > -    if (status != INTEL_GUC_STATUS_SUCCESS) {
> > > +    if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
> > >           /*
> > >            * Either the GuC explicitly returned an error (which
> > >            * we convert to -EIO here) or no response at all was
> > > @@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc,
> > > const u32 *action, u32 len)
> > >           DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
> > >                " ret=%d status=0x%08X response=0x%08X\n",
> > >                action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
> > > +    } else {
> > > +        /* Use data encoded in status dword as return value */
> > > +        ret = INTEL_GUC_RECV_TO_DATA(status);
> > >       }
> > >       intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
> > > 
> > 
> > Other than the blank space after that '~', it looks good to me.
> > 
> > Just a note, for other people reading this; there are 3 cases in which
> > intel_guc_send return value is only checked for truthiness (i.e.
> > __guc_allocate_doorbell, __guc_deallocate_doorbell and
> > intel_guc_sample_forcewake callers are checking for 'if (err)').
> > 
> > I know none of the existing H2G commands will return any extra data, so
> > intel_guc_send should be returning only negative numbers or zero (for
> > now).
> > 
> 
> I'd add a note in the commit message about the fact that we currently don't
> expect any command to return data in the status dword.
> 
> -Daniele
> 
> > -Michel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/15] drm/i915/guc: Prepare send() function to accept bigger response

[Michal Wajdeczko]

This is a preparation step for the upcoming patches.
We already can return some small data decoded from the command
status, but we will need more in the future.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 7 ++++---
 drivers/gpu/drm/i915/intel_uc.c     | 6 ++++--
 drivers/gpu/drm/i915/intel_uc.h     | 8 ++++----
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 1249868..c17cb42 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -79,7 +79,7 @@ static int guc_action_register_ct_buffer(struct intel_guc *guc,
 	int err;
 
 	/* Can't use generic send(), CT registration must go over MMIO */
-	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
 	if (err)
 		DRM_ERROR("CT: register %s buffer failed; err=%d\n",
 			  guc_ct_buffer_type_to_str(type), err);
@@ -98,7 +98,7 @@ static int guc_action_deregister_ct_buffer(struct intel_guc *guc,
 	int err;
 
 	/* Can't use generic send(), CT deregistration must go over MMIO */
-	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
 	if (err)
 		DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
 			  guc_ct_buffer_type_to_str(type), owner, err);
@@ -395,7 +395,8 @@ static int ctch_send(struct intel_guc *guc,
 /*
  * Command Transport (CT) buffer based GuC send function.
  */
-static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
+static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
+			     u32 *response)
 {
 	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
 	u32 status = ~0; /* undefined */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index ff25477..77da750 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -459,7 +459,8 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 	i915_ggtt_disable_guc(dev_priv);
 }
 
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
+		       u32 *response)
 {
 	WARN(1, "Unexpected send: action=%#x\n", *action);
 	return -ENODEV;
@@ -468,7 +469,8 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
 /*
  * This function implements the MMIO based host to GuC interface.
  */
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
+			u32 *response)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	u32 status;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b..53ea5f1 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -206,7 +206,7 @@ struct intel_guc {
 	struct mutex send_mutex;
 
 	/* GuC's FW specific send function */
-	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
+	int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
 
 	/* GuC's FW specific notify function */
 	void (*notify)(struct intel_guc *guc);
@@ -227,12 +227,12 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
 {
-	return guc->send(guc, action, len);
+	return guc->send(guc, action, len, NULL);
 }
 
 static inline void intel_guc_notify(struct intel_guc *guc)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/15] drm/i915/guc: Prepare send() function to accept bigger response

[Michel Thierry]

On 8/4/2017 9:26 AM, Michal Wajdeczko wrote:
> This is a preparation step for the upcoming patches.
> We already can return some small data decoded from the command
> status, but we will need more in the future.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_ct.c | 7 ++++---
>   drivers/gpu/drm/i915/intel_uc.c     | 6 ++++--
>   drivers/gpu/drm/i915/intel_uc.h     | 8 ++++----
>   3 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index 1249868..c17cb42 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -79,7 +79,7 @@ static int guc_action_register_ct_buffer(struct intel_guc *guc,
>   	int err;
>   
>   	/* Can't use generic send(), CT registration must go over MMIO */
> -	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
> +	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
>   	if (err)

another one that is only checking for diff 0 (re prev patch), I know the 
h2g register-ct cmd shouldn't be returning any positive value though.

>   		DRM_ERROR("CT: register %s buffer failed; err=%d\n",
>   			  guc_ct_buffer_type_to_str(type), err);
> @@ -98,7 +98,7 @@ static int guc_action_deregister_ct_buffer(struct intel_guc *guc,
>   	int err;
>   
>   	/* Can't use generic send(), CT deregistration must go over MMIO */
> -	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
> +	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
>   	if (err)
>   		DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
>   			  guc_ct_buffer_type_to_str(type), owner, err);
> @@ -395,7 +395,8 @@ static int ctch_send(struct intel_guc *guc,
>   /*
>    * Command Transport (CT) buffer based GuC send function.
>    */
> -static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
> +static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
> +			     u32 *response)
>   {
>   	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
>   	u32 status = ~0; /* undefined */
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index ff25477..77da750 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -459,7 +459,8 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
>   	i915_ggtt_disable_guc(dev_priv);
>   }
>   
> -int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
> +		       u32 *response)
>   {
>   	WARN(1, "Unexpected send: action=%#x\n", *action);
>   	return -ENODEV;
> @@ -468,7 +469,8 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
>   /*
>    * This function implements the MMIO based host to GuC interface.
>    */
> -int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> +int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
> +			u32 *response)
>   {
>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>   	u32 status;
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 22ae52b..53ea5f1 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -206,7 +206,7 @@ struct intel_guc {
>   	struct mutex send_mutex;
>   
>   	/* GuC's FW specific send function */
> -	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
> +	int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
>   
>   	/* GuC's FW specific notify function */
>   	void (*notify)(struct intel_guc *guc);
> @@ -227,12 +227,12 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
>   int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>   void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
>   int intel_guc_sample_forcewake(struct intel_guc *guc);
> -int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
> -int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
> +int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
>   
>   static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
>   {
> -	return guc->send(guc, action, len);
> +	return guc->send(guc, action, len, NULL);
>   }
>   
>   static inline void intel_guc_notify(struct intel_guc *guc)
> 

Reviewed-by: Michel Thierry <michel.thierry@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/15] drm/i915/guc: Add send_and_receive() helper function

[Michal Wajdeczko]

In the previous patch we have changed signature of the send function
pointer but we didn't modify signature of the corresponding helper
function to minimize number of required changes. Let's add separate
helper to expose new functionality but still hide underlying details.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 53ea5f1..9353ac3 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -235,6 +235,13 @@ static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 l
 	return guc->send(guc, action, len, NULL);
 }
 
+static inline int intel_guc_send_and_receive(struct intel_guc *guc,
+					     const u32 *action, u32 len,
+					     u32 *response)
+{
+	return guc->send(guc, action, len, response);
+}
+
 static inline void intel_guc_notify(struct intel_guc *guc)
 {
 	guc->notify(guc);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/15] drm/i915/guc: Add send_and_receive() helper function

[Michel Thierry]

On 8/4/2017 9:27 AM, Michal Wajdeczko wrote:
> In the previous patch we have changed signature of the send function
> pointer but we didn't modify signature of the corresponding helper
> function to minimize number of required changes. Let's add separate
> helper to expose new functionality but still hide underlying details.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 53ea5f1..9353ac3 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -235,6 +235,13 @@ static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 l
>   	return guc->send(guc, action, len, NULL);
>   }
>   
> +static inline int intel_guc_send_and_receive(struct intel_guc *guc,
> +					     const u32 *action, u32 len,
> +					     u32 *response)
> +{
> +	return guc->send(guc, action, len, response);
> +}
> +
>   static inline void intel_guc_notify(struct intel_guc *guc)
>   {
>   	guc->notify(guc);
> 

No users for now, anyway

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

I just think this patch should come after #4 ("drm/i915/guc: Implement 
response handling in send_mmio()").
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/15] drm/i915/guc: Implement response handling in send_mmio()

[Michal Wajdeczko]

In addition to already returned small data encoded in the status MMIO,
GuC may write more additional data in remaining MMIO regs. Lets copy
all that regs into optionally provided response buffer.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 77da750..c704968 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -517,6 +517,11 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 			 " ret=%d status=0x%08X response=0x%08X\n",
 			 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
 	} else {
+		if (response) {
+			/* Skip reg[0] with the status/response mask */
+			for (i = 1; i < guc->send_regs.count; i++)
+				response[i] = I915_READ(guc_send_reg(guc, i));
+		}
 		/* Use data encoded in status dword as return value */
 		ret = INTEL_GUC_RECV_TO_DATA(status);
 	}
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/15] drm/i915/guc: Implement response handling in send_mmio()

[Michel Thierry]

On 8/4/2017 9:27 AM, Michal Wajdeczko wrote:
> In addition to already returned small data encoded in the status MMIO,
> GuC may write more additional data in remaining MMIO regs. Lets copy
> all that regs into optionally provided response buffer.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_uc.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 77da750..c704968 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -517,6 +517,11 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
>                           " ret=%d status=0x%08X response=0x%08X\n",
>                           action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
>          } else {
> +               if (response) {
> +                       /* Skip reg[0] with the status/response mask */
> +                       for (i = 1; i < guc->send_regs.count; i++)
> +                               response[i] = I915_READ(guc_send_reg(guc, i));
> +               }

new line here?
>                  /* Use data encoded in status dword as return value */
>                  ret = INTEL_GUC_RECV_TO_DATA(status);
>          }

Reviewed-by: Michel Thierry <michel.thierry@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/15] drm/i915/guc: Move Guc notification handling to separate function

[Michal Wajdeczko]

To allow future code reuse. While here, fix comment style.

Suggested-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 33 ++-------------------------------
 drivers/gpu/drm/i915/intel_uc.c | 37 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h |  1 +
 3 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 196caa4..ac69534 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1675,37 +1675,8 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
 
 static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 gt_iir)
 {
-	if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT) {
-		/* Sample the log buffer flush related bits & clear them out now
-		 * itself from the message identity register to minimize the
-		 * probability of losing a flush interrupt, when there are back
-		 * to back flush interrupts.
-		 * There can be a new flush interrupt, for different log buffer
-		 * type (like for ISR), whilst Host is handling one (for DPC).
-		 * Since same bit is used in message register for ISR & DPC, it
-		 * could happen that GuC sets the bit for 2nd interrupt but Host
-		 * clears out the bit on handling the 1st interrupt.
-		 */
-		u32 msg, flush;
-
-		msg = I915_READ(SOFT_SCRATCH(15));
-		flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
-			       INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER);
-		if (flush) {
-			/* Clear the message bits that are handled */
-			I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
-
-			/* Handle flush interrupt in bottom half */
-			queue_work(dev_priv->guc.log.runtime.flush_wq,
-				   &dev_priv->guc.log.runtime.flush_work);
-
-			dev_priv->guc.log.flush_interrupt_count++;
-		} else {
-			/* Not clearing of unhandled event bits won't result in
-			 * re-triggering of the interrupt.
-			 */
-		}
-	}
+	if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT)
+		intel_guc_notification_handler(&dev_priv->guc);
 }
 
 static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index c704968..1e6390e 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -532,6 +532,43 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 	return ret;
 }
 
+void intel_guc_notification_handler(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	u32 msg, flush;
+
+	/*
+	 * Sample the log buffer flush related bits & clear them out now
+	 * itself from the message identity register to minimize the
+	 * probability of losing a flush interrupt, when there are back
+	 * to back flush interrupts.
+	 * There can be a new flush interrupt, for different log buffer
+	 * type (like for ISR), whilst Host is handling one (for DPC).
+	 * Since same bit is used in message register for ISR & DPC, it
+	 * could happen that GuC sets the bit for 2nd interrupt but Host
+	 * clears out the bit on handling the 1st interrupt.
+	 */
+
+	msg = I915_READ(SOFT_SCRATCH(15));
+	flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
+		       INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER);
+	if (flush) {
+		/* Clear the message bits that are handled */
+		I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
+
+		/* Handle flush interrupt in bottom half */
+		queue_work(dev_priv->guc.log.runtime.flush_wq,
+				&dev_priv->guc.log.runtime.flush_work);
+
+		dev_priv->guc.log.flush_interrupt_count++;
+	} else {
+		/*
+		 * Not clearing of unhandled event bits won't result in
+		 * re-triggering of the interrupt.
+		 */
+	}
+}
+
 int intel_guc_sample_forcewake(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 9353ac3..2789179 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -226,6 +226,7 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
+void intel_guc_notification_handler(struct intel_guc *guc);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/15] drm/i915/guc: Move Guc notification handling to separate function

[Chris Wilson]

Quoting Michal Wajdeczko (2017-08-04 17:27:02)
> To allow future code reuse. While here, fix comment style.

Might as well fix the alignment as well then... 

> +               /* Handle flush interrupt in bottom half */
> +               queue_work(dev_priv->guc.log.runtime.flush_wq,
> +                               &dev_priv->guc.log.runtime.flush_work);
> +
> +               dev_priv->guc.log.flush_interrupt_count++;
> +       } else {
> +               /*
> +                * Not clearing of unhandled event bits won't result in
> +                * re-triggering of the interrupt.
> +                */
> +       }
> +}

Argh, and those are spaces not tabs! Back to the checkpatch dungeon you
go, until you are sorry!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/15] drm/i915/guc: Move Guc notification handling to separate function

[Michal Wajdeczko]

On Fri, Aug 04, 2017 at 07:00:33PM +0100, Chris Wilson wrote:
> Quoting Michal Wajdeczko (2017-08-04 17:27:02)
> > To allow future code reuse. While here, fix comment style.
> 
> Might as well fix the alignment as well then... 
> 
> > +               /* Handle flush interrupt in bottom half */
> > +               queue_work(dev_priv->guc.log.runtime.flush_wq,
> > +                               &dev_priv->guc.log.runtime.flush_work);
> > +
> > +               dev_priv->guc.log.flush_interrupt_count++;
> > +       } else {
> > +               /*
> > +                * Not clearing of unhandled event bits won't result in
> > +                * re-triggering of the interrupt.
> > +                */
> > +       }
> > +}
> 
> Argh, and those are spaces not tabs! Back to the checkpatch dungeon you
> go, until you are sorry!

Hmm, that's weird. On my side there were all tabs.

$ scripts/checkpatch.pl ../RFC/0005-drm-i915-guc-Move-Guc-notification-handling-to-separ.patch
total: 0 errors, 0 warnings, 89 lines checked

../RFC/0005-drm-i915-guc-Move-Guc-notification-handling-to-separ.patch has no obvious style problems and is ready for submission.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/15] drm/i915/guc: Move flushing the GuC logs outside notification handler

[Michal Wajdeczko]

From: Oscar Mateo <oscar.mateo@intel.com>

To allow future code reuse.

Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_log.c | 8 ++++++++
 drivers/gpu/drm/i915/intel_uc.c      | 6 +-----
 drivers/gpu/drm/i915/intel_uc.h      | 1 +
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 16d3b87..acd9a3f 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -520,6 +520,14 @@ static void guc_flush_logs(struct intel_guc *guc)
 	guc_log_capture_logs(guc);
 }
 
+void intel_guc_log_flush(struct intel_guc *guc)
+{
+	/* Handle flush interrupt in bottom half */
+	queue_work(guc->log.runtime.flush_wq, &guc->log.runtime.flush_work);
+
+	guc->log.flush_interrupt_count++;
+}
+
 int intel_guc_log_create(struct intel_guc *guc)
 {
 	struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 1e6390e..a091e83 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -556,11 +556,7 @@ void intel_guc_notification_handler(struct intel_guc *guc)
 		/* Clear the message bits that are handled */
 		I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
 
-		/* Handle flush interrupt in bottom half */
-		queue_work(dev_priv->guc.log.runtime.flush_wq,
-				&dev_priv->guc.log.runtime.flush_work);
-
-		dev_priv->guc.log.flush_interrupt_count++;
+		intel_guc_log_flush(&dev_priv->guc);
 	} else {
 		/*
 		 * Not clearing of unhandled event bits won't result in
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 2789179..4808f47 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -267,6 +267,7 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
 /* intel_guc_log.c */
 int intel_guc_log_create(struct intel_guc *guc);
 void intel_guc_log_destroy(struct intel_guc *guc);
+void intel_guc_log_flush(struct intel_guc *guc);
 int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
 void i915_guc_log_register(struct drm_i915_private *dev_priv);
 void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/15] drm/i915/guc: Create a GuC receive function

[Michal Wajdeczko]

From: Oscar Mateo <oscar.mateo@intel.com>

This function, symmetrical to the send(), will handle Guc2Host message
interrupts (which at the moment still only covers requests to flush
the GuC logs).

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 18 +++++++++++++++++-
 drivers/gpu/drm/i915/intel_uc.h |  5 +++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index a091e83..258e0d0 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -109,6 +109,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 
 	mutex_init(&guc->send_mutex);
 	guc->send = intel_guc_send_nop;
+	guc->recv = intel_guc_receive_nop;
 	guc->notify = guc_write_irq_trigger;
 }
 
@@ -315,6 +316,7 @@ static int guc_enable_communication(struct intel_guc *guc)
 		return intel_guc_enable_ct(guc);
 
 	guc->send = intel_guc_send_mmio;
+	guc->recv = intel_guc_receive_mmio;
 	return 0;
 }
 
@@ -326,6 +328,7 @@ static void guc_disable_communication(struct intel_guc *guc)
 		intel_guc_disable_ct(guc);
 
 	guc->send = intel_guc_send_nop;
+	guc->recv = intel_guc_receive_nop;
 }
 
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
@@ -466,6 +469,11 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
 	return -ENODEV;
 }
 
+void intel_guc_receive_nop(struct intel_guc *guc)
+{
+	WARN(1, "Unexpected receive\n");
+}
+
 /*
  * This function implements the MMIO based host to GuC interface.
  */
@@ -532,7 +540,10 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 	return ret;
 }
 
-void intel_guc_notification_handler(struct intel_guc *guc)
+/*
+ * This function implements the MMIO based GuC to host interface.
+ */
+void intel_guc_receive_mmio(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	u32 msg, flush;
@@ -565,6 +576,11 @@ void intel_guc_notification_handler(struct intel_guc *guc)
 	}
 }
 
+void intel_guc_notification_handler(struct intel_guc *guc)
+{
+	guc->recv(guc);
+}
+
 int intel_guc_sample_forcewake(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 4808f47..6f20e66 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -208,6 +208,9 @@ struct intel_guc {
 	/* GuC's FW specific send function */
 	int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
 
+	/* GuC's FW specific receive function */
+	void (*recv)(struct intel_guc *guc);
+
 	/* GuC's FW specific notify function */
 	void (*notify)(struct intel_guc *guc);
 };
@@ -230,6 +233,8 @@ void intel_guc_notification_handler(struct intel_guc *guc);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
+void intel_guc_receive_nop(struct intel_guc *guc);
+void intel_guc_receive_mmio(struct intel_guc *guc);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
 {
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/15] drm/i915/guc: Create a GuC receive function

[Michel Thierry]

On 8/4/2017 9:27 AM, Michal Wajdeczko wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> This function, symmetrical to the send(), will handle Guc2Host message
> interrupts (which at the moment still only covers requests to flush
> the GuC logs).
> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_uc.c | 18 +++++++++++++++++-
>   drivers/gpu/drm/i915/intel_uc.h |  5 +++++
>   2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index a091e83..258e0d0 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -109,6 +109,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
> 
>          mutex_init(&guc->send_mutex);
>          guc->send = intel_guc_send_nop;
> +       guc->recv = intel_guc_receive_nop;
>          guc->notify = guc_write_irq_trigger;
>   }
> 
> @@ -315,6 +316,7 @@ static int guc_enable_communication(struct intel_guc *guc)
>                  return intel_guc_enable_ct(guc);
> 
>          guc->send = intel_guc_send_mmio;
> +       guc->recv = intel_guc_receive_mmio;
>          return 0;
>   }
> 
> @@ -326,6 +328,7 @@ static void guc_disable_communication(struct intel_guc *guc)
>                  intel_guc_disable_ct(guc);
> 
>          guc->send = intel_guc_send_nop;
> +       guc->recv = intel_guc_receive_nop;
>   }
> 
>   int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> @@ -466,6 +469,11 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
>          return -ENODEV;
>   }
> 
> +void intel_guc_receive_nop(struct intel_guc *guc)
> +{
> +       WARN(1, "Unexpected receive\n");
> +}
> +
>   /*
>    * This function implements the MMIO based host to GuC interface.
>    */
> @@ -532,7 +540,10 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
>          return ret;
>   }
> 
> -void intel_guc_notification_handler(struct intel_guc *guc)
> +/*
> + * This function implements the MMIO based GuC to host interface.
> + */
> +void intel_guc_receive_mmio(struct intel_guc *guc)
>   {
>          struct drm_i915_private *dev_priv = guc_to_i915(guc);
>          u32 msg, flush;
> @@ -565,6 +576,11 @@ void intel_guc_notification_handler(struct intel_guc *guc)
>          }
>   }
> 
> +void intel_guc_notification_handler(struct intel_guc *guc)
> +{
> +       guc->recv(guc);
> +}
> +
>   int intel_guc_sample_forcewake(struct intel_guc *guc)
>   {
>          struct drm_i915_private *dev_priv = guc_to_i915(guc);
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 4808f47..6f20e66 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -208,6 +208,9 @@ struct intel_guc {
>          /* GuC's FW specific send function */
>          int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
> 
> +       /* GuC's FW specific receive function */
> +       void (*recv)(struct intel_guc *guc);
> +

I think you already explained to some of us why returning any error code 
in recv would be pretty much useless (the error would be useful to 
whoever send the data). But feel free to say it yourself ;)

>          /* GuC's FW specific notify function */
>          void (*notify)(struct intel_guc *guc);
>   };
> @@ -230,6 +233,8 @@ void intel_guc_notification_handler(struct intel_guc *guc);
>   int intel_guc_sample_forcewake(struct intel_guc *guc);
>   int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
>   int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
> +void intel_guc_receive_nop(struct intel_guc *guc);
> +void intel_guc_receive_mmio(struct intel_guc *guc);
> 
>   static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
>   {
> --
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/15] drm/i915/guc: Update CT message header definition

[Michal Wajdeczko]

Flags bits are different in G2H message.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Kelvin Gardiner <kelvin.gardiner@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 98c0560..9bc3067 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -358,17 +358,24 @@ struct guc_ct_buffer_desc {
  *
  * bit[4..0]	message len (in dwords)
  * bit[7..5]	reserved
+ * bit[10..8]	flags
+ * bit[15..11]	reserved
+ * bit[31..16]	action code
+ *
+ * H2G flags:
  * bit[8]	write fence to desc
  * bit[9]	write status to H2G buff
  * bit[10]	send status (via G2H)
- * bit[15..11]	reserved
- * bit[31..16]	action code
+ *
+ * G2H flags:
+ * bit[8]	is response
  */
 #define GUC_CT_MSG_LEN_SHIFT			0
 #define GUC_CT_MSG_LEN_MASK			0x1F
 #define GUC_CT_MSG_WRITE_FENCE_TO_DESC		(1 << 8)
 #define GUC_CT_MSG_WRITE_STATUS_TO_BUFF		(1 << 9)
 #define GUC_CT_MSG_SEND_STATUS			(1 << 10)
+#define GUC_CT_MSG_IS_RESPONSE			(1 << 8)
 #define GUC_CT_MSG_ACTION_SHIFT			16
 #define GUC_CT_MSG_ACTION_MASK			0xFFFF
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/15] drm/i915/guc: Prepare to handle messages from CT RECV buffer

[Michal Wajdeczko]

GuC can respond to our commands not only by updating SEND buffer
descriptor, but can send us message over RECV buffer. Additionally
Guc can also send us unsolicited requests over RECV buffer.
Lets start reading those messages and make placeholders for actual
response/request handlers.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 120 ++++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index c17cb42..dd30c83 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -414,6 +414,124 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 	return ret;
 }
 
+static inline unsigned int ct_header_get_len(u32 header)
+{
+	return (header >> GUC_CT_MSG_LEN_SHIFT) & GUC_CT_MSG_LEN_MASK;
+}
+
+static inline unsigned int ct_header_get_action(u32 header)
+{
+	return (header >> GUC_CT_MSG_ACTION_SHIFT) & GUC_CT_MSG_ACTION_MASK;
+}
+
+static inline bool ct_header_is_response(u32 header)
+{
+	return !!(header & GUC_CT_MSG_IS_RESPONSE);
+}
+
+static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
+{
+	struct guc_ct_buffer_desc *desc = ctb->desc;
+	u32 head = desc->head / 4;	/* in dwords */
+	u32 tail = desc->tail / 4;	/* in dwords */
+	u32 size = desc->size / 4;	/* in dwords */
+	u32 *cmds = ctb->cmds;
+	s32 available;			/* in dwords */
+	unsigned int len;
+	unsigned int i;
+
+	GEM_BUG_ON(desc->size % 4);
+	GEM_BUG_ON(desc->head % 4);
+	GEM_BUG_ON(desc->tail % 4);
+	GEM_BUG_ON(tail >= size);
+	GEM_BUG_ON(head >= size);
+
+	/* tail == head condition indicates empty */
+	available = tail - head;
+	if (available == 0)
+		return -ENODATA;
+
+	/* beware of buffer wrap case */
+	if (available < 0)
+		available += size;
+	GEM_BUG_ON(available < 0);
+
+	data[0] = cmds[head];
+	head = (head + 1) % size;
+
+	/* message len with header */
+	len = ct_header_get_len(data[0]) + 1;
+	if (unlikely(len > (u32)available)) {
+		DRM_ERROR("CT: incomplete message %*phn %*phn %*phn\n",
+			  4, data,
+			  4 * (head + available - 1 > size ?
+			       size - head : available - 1), &cmds[head],
+			  4 * (head + available - 1 > size ?
+			       available - 1 - size + head : 0), &cmds[0]);
+		return -EPROTO;
+	}
+
+	for (i = 1; i < len; i++) {
+		data[i] = cmds[head];
+		head = (head + 1) % size;
+	}
+
+	desc->head = head * 4;
+	return 0;
+}
+
+static int guc_handle_response(struct intel_guc *guc, const u32 *data)
+{
+	u32 header = data[0];
+	u32 len = ct_header_get_len(header) + 1; /* total len with header */
+
+	GEM_BUG_ON(!ct_header_is_response(header));
+	/* beyond header, data shall at least include fence and status */
+	if (unlikely(len < 3)) {
+		DRM_ERROR("CT: corrupted response %*phn\n", 4*len, data);
+		return -EPROTO;
+	}
+
+	return 0;
+}
+
+static int guc_handle_request(struct intel_guc *guc, const u32 *data)
+{
+	u32 header = data[0];
+
+	GEM_BUG_ON(ct_header_is_response(header));
+	/* data layout beyond header is request specific */
+
+	return 0;
+}
+
+static void intel_guc_receive_ct(struct intel_guc *guc)
+{
+	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
+	struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_RECV];
+	u32 msg[GUC_CT_MSG_LEN_MASK+1];
+	int err = 0;
+
+	if (!ctch_is_open(ctch))
+		return;
+
+	do {
+		err = ctb_read(ctb, msg);
+		if (err)
+			break;
+
+		if (ct_header_is_response(msg[0]))
+			err = guc_handle_response(guc, msg);
+		else
+			err = guc_handle_request(guc, msg);
+	} while (!err);
+
+	if (GEM_WARN_ON(err == -EPROTO)) {
+		DRM_ERROR("CT: corrupted message detected!\n");
+		ctb->desc->is_in_error = 1;
+	}
+}
+
 /**
  * Enable buffer based command transport
  * Shall only be called for platforms with HAS_GUC_CT.
@@ -435,6 +553,7 @@ int intel_guc_enable_ct(struct intel_guc *guc)
 
 	/* Switch into cmd transport buffer based send() */
 	guc->send = intel_guc_send_ct;
+	guc->recv = intel_guc_receive_ct;
 	DRM_INFO("CT: %s\n", enableddisabled(true));
 	return 0;
 }
@@ -458,5 +577,6 @@ void intel_guc_disable_ct(struct intel_guc *guc)
 
 	/* Disable send */
 	guc->send = intel_guc_send_nop;
+	guc->recv = intel_guc_receive_nop;
 	DRM_INFO("CT: %s\n", enableddisabled(false));
 }
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/15] drm/i915/guc: Use better name for helper wait function

[Michal Wajdeczko]

In next patch we will introduce another way of waiting for the response
that will use RECV buffer. To avoid mismatched names, rename old wait
function to reflect the fact that it is based on descriptor update.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index dd30c83..4fabea17 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -321,16 +321,18 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 	return 0;
 }
 
-/* Wait for the response from the GuC.
+/* Wait for the descriptor update.
+ * Guc will update this descriptor with new fence and status.
+ * @desc:	buffer descriptor
  * @fence:	response fence
  * @status:	placeholder for status
  * return:	0 response received (status is valid)
  *		-ETIMEDOUT no response within hardcoded timeout
  *		-EPROTO no response, ct buffer was in error
  */
-static int wait_for_response(struct guc_ct_buffer_desc *desc,
-			     u32 fence,
-			     u32 *status)
+static int wait_for_desc_update(struct guc_ct_buffer_desc *desc,
+				u32 fence,
+				u32 *status)
 {
 	int err;
 
@@ -384,7 +386,7 @@ static int ctch_send(struct intel_guc *guc,
 
 	intel_guc_notify(guc);
 
-	err = wait_for_response(desc, fence, status);
+	err = wait_for_desc_update(desc, fence, status);
 	if (unlikely(err))
 		return err;
 	if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 11/15] drm/i915/guc: Implement response handling in send_ct()

[Michal Wajdeczko]

GuC may return not only small data encoded in the status dword,
but can also append additional data into the response message.
We will copy this extra data into provided buffer, and use
number of received data dwords as new success return value.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 120 +++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_guc_ct.h |   5 ++
 2 files changed, 115 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 4fabea17..4dfa0b9 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,14 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+struct ct_request {
+	struct list_head link;
+	u32 fence;
+	u32 status;
+	u32 response_len;
+	u32 *response_buf;
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
@@ -32,6 +40,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 {
 	/* we're using static channel owners */
 	ct->host_channel.owner = CTB_OWNER_HOST;
+
+	spin_lock_init(&ct->lock);
+	INIT_LIST_HEAD(&ct->pending_requests);
 }
 
 static inline const char *guc_ct_buffer_type_to_str(u32 type)
@@ -265,7 +276,8 @@ static u32 ctch_get_next_fence(struct intel_guc_ct_channel *ctch)
 static int ctb_write(struct intel_guc_ct_buffer *ctb,
 		     const u32 *action,
 		     u32 len /* in dwords */,
-		     u32 fence)
+		     u32 fence,
+		     bool send_response)
 {
 	struct guc_ct_buffer_desc *desc = ctb->desc;
 	u32 head = desc->head / 4;	/* in dwords */
@@ -301,6 +313,7 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 	 */
 	header = (len << GUC_CT_MSG_LEN_SHIFT) |
 		 (GUC_CT_MSG_WRITE_FENCE_TO_DESC) |
+		 (send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 		 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
 	cmds[tail] = header;
@@ -364,14 +377,48 @@ static int wait_for_desc_update(struct guc_ct_buffer_desc *desc,
 	return err;
 }
 
+/* Wait for the Guc response.
+ * We will update request status from the response message handler.
+ *
+ * @req:	pointer to pending request
+ * @status:	placeholder for status
+ * return:	0 response received (status is valid)
+ *		-ETIMEDOUT no response within hardcoded timeout
+ * \see guc_handle_response()
+ */
+static int wait_for_response_msg(struct ct_request *req, u32 *status)
+{
+	int err;
+
+	/*
+	 * Fast commands should complete in less than 10us, so sample quickly
+	 * up to that length of time, then switch to a slower sleep-wait loop.
+	 * No GuC command should ever take longer than 10ms.
+	 */
+#define done INTEL_GUC_RECV_IS_RESPONSE(READ_ONCE(req->status))
+	err = wait_for_us(done, 10);
+	if (err)
+		err = wait_for(done, 1000);
+#undef done
+
+	if (unlikely(err))
+		DRM_ERROR("CT: fence %u err %d\n", req->fence, err);
+
+	*status = req->status;
+	return err;
+}
+
 static int ctch_send(struct intel_guc *guc,
 		     struct intel_guc_ct_channel *ctch,
 		     const u32 *action,
 		     u32 len,
-		     u32 *status)
+		     u32 *status,
+		     u32 *response)
 {
 	struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_SEND];
 	struct guc_ct_buffer_desc *desc = ctb->desc;
+	struct ct_request request;
+	unsigned long flags;
 	u32 fence;
 	int err;
 
@@ -380,18 +427,48 @@ static int ctch_send(struct intel_guc *guc,
 	GEM_BUG_ON(len & ~GUC_CT_MSG_LEN_MASK);
 
 	fence = ctch_get_next_fence(ctch);
-	err = ctb_write(ctb, action, len, fence);
+	request.fence = fence;
+	request.status = 0;
+	request.response_len = 0;
+	request.response_buf = response;
+
+	spin_lock_irqsave(&guc->ct.lock, flags);
+	list_add_tail(&request.link, &guc->ct.pending_requests);
+	spin_unlock_irqrestore(&guc->ct.lock, flags);
+
+	err = ctb_write(ctb, action, len, fence, !!response);
 	if (unlikely(err))
-		return err;
+		goto unlink;
 
 	intel_guc_notify(guc);
 
-	err = wait_for_desc_update(desc, fence, status);
+	if (response)
+		err = wait_for_response_msg(&request, status);
+	else
+		err = wait_for_desc_update(desc, fence, status);
 	if (unlikely(err))
-		return err;
-	if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
-		return -EIO;
-	return INTEL_GUC_RECV_TO_DATA(*status);
+		goto unlink;
+
+	if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS) {
+		err = -EIO;
+		goto unlink;
+	}
+
+	if (response) {
+		/* If response is expected, there shall be no data in the status */
+		WARN_ON(INTEL_GUC_RECV_TO_DATA(request.status));
+		err = request.response_len;
+	} else {
+		/* If response is not expected, return data from the status */
+		err = INTEL_GUC_RECV_TO_DATA(*status);
+	}
+
+unlink:
+	spin_lock_irqsave(&guc->ct.lock, flags);
+	list_del(&request.link);
+	spin_unlock_irqrestore(&guc->ct.lock, flags);
+
+	return err;
 }
 
 /*
@@ -406,7 +483,7 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 
 	mutex_lock(&guc->send_mutex);
 
-	ret = ctch_send(guc, ctch, action, len, &status);
+	ret = ctch_send(guc, ctch, action, len, &status, response);
 	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
 			  action[0], ret, status);
@@ -485,7 +562,12 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 static int guc_handle_response(struct intel_guc *guc, const u32 *data)
 {
 	u32 header = data[0];
+	u32 fence = data[1];
+	u32 status = data[2];
 	u32 len = ct_header_get_len(header) + 1; /* total len with header */
+	struct ct_request *req;
+	bool found = false;
+	unsigned long flags;
 
 	GEM_BUG_ON(!ct_header_is_response(header));
 	/* beyond header, data shall at least include fence and status */
@@ -494,6 +576,24 @@ static int guc_handle_response(struct intel_guc *guc, const u32 *data)
 		return -EPROTO;
 	}
 
+	spin_lock_irqsave(&guc->ct.lock, flags);
+	list_for_each_entry(req, &guc->ct.pending_requests, link) {
+		if (req->fence != fence) {
+			DRM_DEBUG_DRIVER("CT: request %u awaits response\n",
+					 req->fence);
+			continue;
+		}
+		req->response_len = len - 3;
+		if (req->response_buf)
+			memcpy(req->response_buf, data + 3, 4*req->response_len);
+		WRITE_ONCE(req->status, status);
+		found = true;
+		break;
+	}
+	spin_unlock_irqrestore(&guc->ct.lock, flags);
+
+	if (!found)
+		DRM_ERROR("CT: unsolicited response %*phn\n", 4*len, data);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index 6d97f36..557c1e8 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -71,10 +71,15 @@ struct intel_guc_ct_channel {
 /** Holds all command transport channels.
  *
  * @host_channel: main channel used by the host
+ * @lock: spin lock for pending requests list
+ * @pending_requests: list of pending requests
  */
 struct intel_guc_ct {
 	struct intel_guc_ct_channel host_channel;
 	/* other channels are tbd */
+
+	spinlock_t lock;
+	struct list_head pending_requests;
 };
 
 void intel_guc_ct_init_early(struct intel_guc_ct *ct);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 12/15] drm/i915/guc: Prepare to process incoming requests from CT

[Michal Wajdeczko]

Requests are read from CT in the irq handler, but actual processing
will be done in the work thread. Processing of specific actions will
be added in the upcoming patches.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 72 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc_ct.h |  4 +++
 2 files changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 4dfa0b9..75cd7af 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -32,10 +32,17 @@ struct ct_request {
 	u32 *response_buf;
 };
 
+struct ct_incoming_request {
+	struct list_head link;
+	u32 data[GUC_CT_MSG_LEN_MASK+1];
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
 
+static void ct_worker_func(struct work_struct *w);
+
 void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 {
 	/* we're using static channel owners */
@@ -43,6 +50,8 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 
 	spin_lock_init(&ct->lock);
 	INIT_LIST_HEAD(&ct->pending_requests);
+	INIT_LIST_HEAD(&ct->incoming_requests);
+	INIT_WORK(&ct->worker, ct_worker_func);
 }
 
 static inline const char *guc_ct_buffer_type_to_str(u32 type)
@@ -600,13 +609,76 @@ static int guc_handle_response(struct intel_guc *guc, const u32 *data)
 static int guc_handle_request(struct intel_guc *guc, const u32 *data)
 {
 	u32 header = data[0];
+	u32 len = ct_header_get_len(header) + 1; /* total len with header */
+	struct ct_incoming_request *request;
+	unsigned long flags;
 
 	GEM_BUG_ON(ct_header_is_response(header));
 	/* data layout beyond header is request specific */
 
+	request = kmalloc(sizeof(*request), GFP_ATOMIC);
+	if (unlikely(!request)) {
+		DRM_ERROR("CT: dropping request %*phn\n", 4*len, data);
+		return 0; /* XXX: -ENOMEM ? */
+	}
+
+	GEM_BUG_ON(len > GUC_CT_MSG_LEN_MASK + 1);
+	memcpy(request->data, data, 4*len);
+
+	spin_lock_irqsave(&guc->ct.lock, flags);
+	list_add_tail(&request->link, &guc->ct.incoming_requests);
+	spin_unlock_irqrestore(&guc->ct.lock, flags);
+
+	queue_work(system_unbound_wq, &guc->ct.worker);
 	return 0;
 }
 
+static bool guc_process_incoming_requests(struct intel_guc *guc)
+{
+	unsigned long flags;
+	struct ct_incoming_request *request;
+	bool done;
+	u32 header;
+	u32 action;
+	u32 len;
+
+	spin_lock_irqsave(&guc->ct.lock, flags);
+	request = list_first_entry_or_null(&guc->ct.incoming_requests,
+					   struct ct_incoming_request, link);
+	if (request)
+		list_del(&request->link);
+	done = !!list_empty(&guc->ct.incoming_requests);
+	spin_unlock_irqrestore(&guc->ct.lock, flags);
+
+	if (!request)
+		return true;
+
+	header = request->data[0];
+	action = ct_header_get_action(header);
+	len = ct_header_get_len(header) + 1; /* also count header dw */
+
+	switch (action) {
+	default:
+		DRM_ERROR("CT: unexpected request %*phn\n",
+			  4*len, request->data);
+		break;
+	}
+
+	kfree(request);
+	return done;
+}
+
+static void ct_worker_func(struct work_struct *w)
+{
+	struct intel_guc_ct *ct = container_of(w, struct intel_guc_ct, worker);
+	struct intel_guc *guc = container_of(ct, struct intel_guc, ct);
+	bool done;
+
+	done = guc_process_incoming_requests(guc);
+	if (!done)
+		queue_work(system_unbound_wq, &ct->worker);
+}
+
 static void intel_guc_receive_ct(struct intel_guc *guc)
 {
 	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index 557c1e8..125e004 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -73,6 +73,8 @@ struct intel_guc_ct_channel {
  * @host_channel: main channel used by the host
  * @lock: spin lock for pending requests list
  * @pending_requests: list of pending requests
+ * @incoming_requests: list of incoming requests
+ * @tasklet: tasklet for handling incoming requests
  */
 struct intel_guc_ct {
 	struct intel_guc_ct_channel host_channel;
@@ -80,6 +82,8 @@ struct intel_guc_ct {
 
 	spinlock_t lock;
 	struct list_head pending_requests;
+	struct list_head incoming_requests;
+	struct work_struct worker;
 };
 
 void intel_guc_ct_init_early(struct intel_guc_ct *ct);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/15] drm/i915/guc: Prepare to process incoming requests from CT

[Chris Wilson]

Quoting Michal Wajdeczko (2017-08-04 17:27:09)
>  static inline const char *guc_ct_buffer_type_to_str(u32 type)
> @@ -600,13 +609,76 @@ static int guc_handle_response(struct intel_guc *guc, const u32 *data)
>  static int guc_handle_request(struct intel_guc *guc, const u32 *data)
>  {
>         u32 header = data[0];
> +       u32 len = ct_header_get_len(header) + 1; /* total len with header */
> +       struct ct_incoming_request *request;
> +       unsigned long flags;
>  
>         GEM_BUG_ON(ct_header_is_response(header));
>         /* data layout beyond header is request specific */
>  
> +       request = kmalloc(sizeof(*request), GFP_ATOMIC);
> +       if (unlikely(!request)) {
> +               DRM_ERROR("CT: dropping request %*phn\n", 4*len, data);
> +               return 0; /* XXX: -ENOMEM ? */
> +       }
> +
> +       GEM_BUG_ON(len > GUC_CT_MSG_LEN_MASK + 1);

This is incoming from the guc, if we can validate it, do so. Keep
GEM_BUG_ON() for programming errors and absolute catastrophe.
-Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/15] drm/i915/guc: Prepare to process incoming requests from CT

[Michal Wajdeczko]

On Fri, Aug 04, 2017 at 06:13:42PM +0100, Chris Wilson wrote:
> Quoting Michal Wajdeczko (2017-08-04 17:27:09)
> >  static inline const char *guc_ct_buffer_type_to_str(u32 type)
> > @@ -600,13 +609,76 @@ static int guc_handle_response(struct intel_guc *guc, const u32 *data)
> >  static int guc_handle_request(struct intel_guc *guc, const u32 *data)
> >  {
> >         u32 header = data[0];
> > +       u32 len = ct_header_get_len(header) + 1; /* total len with header */
> > +       struct ct_incoming_request *request;
> > +       unsigned long flags;
> >  
> >         GEM_BUG_ON(ct_header_is_response(header));
> >         /* data layout beyond header is request specific */
> >  
> > +       request = kmalloc(sizeof(*request), GFP_ATOMIC);
> > +       if (unlikely(!request)) {
> > +               DRM_ERROR("CT: dropping request %*phn\n", 4*len, data);
> > +               return 0; /* XXX: -ENOMEM ? */
> > +       }
> > +
> > +       GEM_BUG_ON(len > GUC_CT_MSG_LEN_MASK + 1);
> 
> This is incoming from the guc, if we can validate it, do so. Keep
> GEM_BUG_ON() for programming errors and absolute catastrophe.

Sorry, this check is leftover from earlier design.

Now it will be always satisfied as len can't be encoded beyond given mask ;)
I can change it into more appropriate runtime check against our buffer size:

	if (4*len > sizeof(request->data)) ...

or into compile time check (with assumption then len can't larger than mask)

	BUILD_BUG_ON(sizeof(request->data) < 4*(GUC_MSG_LEN_MASK+1));

or both

-Michal


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 13/15] drm/i915/guc: Handle default action received over CT

[Michal Wajdeczko]

With enabled CT, instead of programming SCRATCH 15 register with the
Guc to host message, Guc will send us CT request. Content of the data[1]
of this message follows format of the data in scratch register.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 3 +++
 drivers/gpu/drm/i915/intel_uc.c     | 7 +++++++
 drivers/gpu/drm/i915/intel_uc.h     | 1 +
 3 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 75cd7af..9f7fc5e 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -658,6 +658,9 @@ static bool guc_process_incoming_requests(struct intel_guc *guc)
 	len = ct_header_get_len(header) + 1; /* also count header dw */
 
 	switch (action) {
+	case INTEL_GUC_ACTION_DEFAULT:
+		intel_guc_process_default_action(guc, request->data[1]);
+		break;
 	default:
 		DRM_ERROR("CT: unexpected request %*phn\n",
 			  4*len, request->data);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 258e0d0..27758ce 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -596,3 +596,10 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
 
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
+
+void intel_guc_process_default_action(struct intel_guc *guc, u32 msg)
+{
+	if (msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
+		   INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER))
+		intel_guc_log_flush(guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 6f20e66..2a8394b 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -230,6 +230,7 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 void intel_guc_notification_handler(struct intel_guc *guc);
+void intel_guc_process_default_action(struct intel_guc *guc, u32 msg);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 14/15] drm/i915/guc: Enable GuC interrupts when using CT

[Michal Wajdeczko]

We will need them in G2H communication to properly handle
responses and requests from the Guc.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
 drivers/gpu/drm/i915/intel_uc.c            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 48a1e93..509497e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1328,7 +1328,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
 	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return 0;
 
-	if (i915.guc_log_level >= 0)
+	if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
 		gen9_enable_guc_interrupts(dev_priv);
 
 	ctx = dev_priv->kernel_context;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 27758ce..0402e32 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -395,7 +395,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
 	intel_guc_auth_huc(dev_priv);
 	if (i915.enable_guc_submission) {
-		if (i915.guc_log_level >= 0)
+		if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
 			gen9_enable_guc_interrupts(dev_priv);
 
 		ret = i915_guc_submission_enable(dev_priv);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 15/15] drm/i915/guc: Trace messages from CT while in debug

[Michal Wajdeczko]

During debug we may want to investigate all communication
from the Guc. Add proper tracing macros in debug config.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 9f7fc5e..71daad3 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,12 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+#ifdef CONFIG_DRM_I915_DEBUG
+#define CT_DEBUG_DRIVER(...)	DRM_DEBUG_DRIVER(__VA_ARGS__)
+#else
+#define CT_DEBUG_DRIVER(...)
+#endif
+
 struct ct_request {
 	struct list_head link;
 	u32 fence;
@@ -325,6 +331,10 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 		 (send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 		 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
+	CT_DEBUG_DRIVER("CT: writing %*phn %*phn %*phn\n",
+			4, &header, 4, &fence,
+			4*(len - 1), &action[1]);
+
 	cmds[tail] = header;
 	tail = (tail + 1) % size;
 
@@ -496,6 +506,9 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
 			  action[0], ret, status);
+	} else if (unlikely(ret)) {
+		CT_DEBUG_DRIVER("CT: send action %#x returned %d (%#x)\n",
+				action[0], ret, ret);
 	}
 
 	mutex_unlock(&guc->send_mutex);
@@ -542,10 +555,12 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 	/* beware of buffer wrap case */
 	if (available < 0)
 		available += size;
+	CT_DEBUG_DRIVER("CT: available %d (%u:%u)\n", available, head, tail);
 	GEM_BUG_ON(available < 0);
 
 	data[0] = cmds[head];
 	head = (head + 1) % size;
+	CT_DEBUG_DRIVER("CT: header %#x\n", data[0]);
 
 	/* message len with header */
 	len = ct_header_get_len(data[0]) + 1;
@@ -563,6 +578,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 		data[i] = cmds[head];
 		head = (head + 1) % size;
 	}
+	CT_DEBUG_DRIVER("CT: received %*phn\n", 4*len, data);
 
 	desc->head = head * 4;
 	return 0;
@@ -584,6 +600,7 @@ static int guc_handle_response(struct intel_guc *guc, const u32 *data)
 		DRM_ERROR("CT: corrupted response %*phn\n", 4*len, data);
 		return -EPROTO;
 	}
+	CT_DEBUG_DRIVER("CT: response fence %u status %#x\n", fence, status);
 
 	spin_lock_irqsave(&guc->ct.lock, flags);
 	list_for_each_entry(req, &guc->ct.pending_requests, link) {
@@ -615,6 +632,7 @@ static int guc_handle_request(struct intel_guc *guc, const u32 *data)
 
 	GEM_BUG_ON(ct_header_is_response(header));
 	/* data layout beyond header is request specific */
+	CT_DEBUG_DRIVER("CT: request %#x\n", ct_header_get_action(header));
 
 	request = kmalloc(sizeof(*request), GFP_ATOMIC);
 	if (unlikely(!request)) {
@@ -656,6 +674,7 @@ static bool guc_process_incoming_requests(struct intel_guc *guc)
 	header = request->data[0];
 	action = ct_header_get_action(header);
 	len = ct_header_get_len(header) + 1; /* also count header dw */
+	CT_DEBUG_DRIVER("CT: processing request %*phn\n", 4*len, request->data);
 
 	switch (action) {
 	case INTEL_GUC_ACTION_DEFAULT:
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 15/15] drm/i915/guc: Trace messages from CT while in debug

[Chris Wilson]

Quoting Michal Wajdeczko (2017-08-04 17:27:12)
> During debug we may want to investigate all communication
> from the Guc. Add proper tracing macros in debug config.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_ct.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index 9f7fc5e..71daad3 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -24,6 +24,12 @@
>  #include "i915_drv.h"
>  #include "intel_guc_ct.h"
>  
> +#ifdef CONFIG_DRM_I915_DEBUG
> +#define CT_DEBUG_DRIVER(...)   DRM_DEBUG_DRIVER(__VA_ARGS__)
> +#else
> +#define CT_DEBUG_DRIVER(...)
> +#endif

+1. If we only use a single Kconfig, or enabled them for CI anyway, we
still run the risk of drowning in the noise.  Flooding CI dmesg has many
other unfortunate repercussions as well.

Oh well, back to the ddebug / trace_debug wishlist. (Being able to dump
the pertinent buffer on demand/error without flooding the test runner.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for drm/i915/guc: Support for Guc responses and requests

[Patchwork]

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests
URL   : https://patchwork.freedesktop.org/series/28393/
State : warning

== Summary ==

Series 28393v1 drm/i915/guc: Support for Guc responses and requests
https://patchwork.freedesktop.org/api/1.0/series/28393/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-pnv-d510) fdo#101597
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> DMESG-WARN (fi-byt-j1900)

fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:437s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:418s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:361s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:501s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:491s
fi-byt-j1900     total:279  pass:253  dwarn:2   dfail:0   fail:0   skip:24  time:529s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:514s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:591s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:431s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:409s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:418s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:511s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:479s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:464s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:566s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:577s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:576s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:446s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:643s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:462s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:425s
fi-skl-x1585l    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:482s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:543s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:407s

2a33b48692c55d9145a60527716b7d066815d377 drm-tip: 2017y-08m-04d-13h-34m-53s UTC integration manifest
dd8c5fe51790 drm/i915/guc: Trace messages from CT while in debug
a487efe9d170 drm/i915/guc: Enable GuC interrupts when using CT
7f3acdf7b8c5 drm/i915/guc: Handle default action received over CT
e310d9d5208d drm/i915/guc: Prepare to process incoming requests from CT
0fa1bd92541e drm/i915/guc: Implement response handling in send_ct()
b6e8db3b5d2f drm/i915/guc: Use better name for helper wait function
8b64a6bbed37 drm/i915/guc: Prepare to handle messages from CT RECV buffer
ff55ebe894d4 drm/i915/guc: Update CT message header definition
a4a815e059ef drm/i915/guc: Create a GuC receive function
936b57e8eba1 drm/i915/guc: Move flushing the GuC logs outside notification handler
e95ea974c745 drm/i915/guc: Move Guc notification handling to separate function
70a495bf2b4a drm/i915/guc: Implement response handling in send_mmio()
53b95a141d1f drm/i915/guc: Add send_and_receive() helper function
0dc31d444f83 drm/i915/guc: Prepare send() function to accept bigger response
e40541ac904e drm/i915/guc: Add support for data reporting in GuC responses

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5329/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 00/16] drm/i915/guc: Support for Guc responses and requests

[Michal Wajdeczko]

With this series we will be able to receive more data from the Guc.
New Guc firmware will be required to actually use that feature.

v2: misc improvements after review + HAX

Michal Wajdeczko (14):
  drm/i915/guc: Add support for data reporting in GuC responses
  drm/i915/guc: Prepare send() function to accept bigger response
  drm/i915/guc: Add send_and_receive() helper function
  drm/i915/guc: Implement response handling in send_mmio()
  drm/i915/guc: Move Guc notification handling to separate function
  drm/i915/guc: Update CT message header definition
  drm/i915/guc: Prepare to handle messages from CT RECV buffer
  drm/i915/guc: Use better name for helper wait function
  drm/i915/guc: Implement response handling in send_ct()
  drm/i915/guc: Prepare to process incoming requests from CT
  drm/i915/guc: Handle default action received over CT
  drm/i915/guc: Enable GuC interrupts when using CT
  drm/i915/guc: Trace messages from CT while in debug
  HAX Enable GuC loading & submission

Oscar Mateo (2):
  drm/i915/guc: Move flushing the GuC logs outside notification handler
  drm/i915/guc: Create a GuC receive function

 drivers/gpu/drm/i915/i915_guc_submission.c |   2 +-
 drivers/gpu/drm/i915/i915_irq.c            |  33 +--
 drivers/gpu/drm/i915/i915_params.c         |   8 +-
 drivers/gpu/drm/i915/intel_guc_ct.c        | 389 ++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_guc_ct.h        |   9 +
 drivers/gpu/drm/i915/intel_guc_fwif.h      |  17 +-
 drivers/gpu/drm/i915/intel_guc_log.c       |   8 +
 drivers/gpu/drm/i915/intel_uc.c            |  75 +++++-
 drivers/gpu/drm/i915/intel_uc.h            |  26 +-
 9 files changed, 488 insertions(+), 79 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 01/16] drm/i915/guc: Add support for data reporting in GuC responses

[Michal Wajdeczko]

GuC may return additional data in the command status response.
Format and meaning of this data is action specific.
We will use this non-negative data as a new success return value.
Currently used actions don't return data that way yet.

v2: fix prohibited space after '~' (Michel)
    update commit message (Daniele)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++++++-------
 drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++++++
 drivers/gpu/drm/i915/intel_uc.c       |  5 ++++-
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index c4cbec1..1249868 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
 	err = wait_for_response(desc, fence, status);
 	if (unlikely(err))
 		return err;
-	if (*status != INTEL_GUC_STATUS_SUCCESS)
+	if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
 		return -EIO;
-	return 0;
+	return INTEL_GUC_RECV_TO_DATA(*status);
 }
 
 /*
@@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
 {
 	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
 	u32 status = ~0; /* undefined */
-	int err;
+	int ret;
 
 	mutex_lock(&guc->send_mutex);
 
-	err = ctch_send(guc, ctch, action, len, &status);
-	if (unlikely(err)) {
+	ret = ctch_send(guc, ctch, action, len, &status);
+	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
-			  action[0], err, status);
+			  action[0], ret, status);
 	}
 
 	mutex_unlock(&guc->send_mutex);
-	return err;
+	return ret;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 5fa2860..367aa65 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -567,10 +567,16 @@ enum intel_guc_action {
  * command in SS0. The response is distinguishable from a command
  * by the fact that all the MASK bits are set. The remaining bits
  * give more detail.
+ * Bits [16:27] are reserved for optional data reporting.
  */
 #define	INTEL_GUC_RECV_MASK	((u32)0xF0000000)
 #define	INTEL_GUC_RECV_IS_RESPONSE(x)	((u32)(x) >= INTEL_GUC_RECV_MASK)
 #define	INTEL_GUC_RECV_STATUS(x)	(INTEL_GUC_RECV_MASK | (x))
+#define INTEL_GUC_RECV_DATA_SHIFT	16
+#define INTEL_GUC_RECV_DATA_MASK	(0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
+#define INTEL_GUC_RECV_TO_STATUS(x)	((x) & ~INTEL_GUC_RECV_DATA_MASK)
+#define INTEL_GUC_RECV_TO_DATA(x)	(((x) & INTEL_GUC_RECV_DATA_MASK) >> \
+					 INTEL_GUC_RECV_DATA_SHIFT)
 
 /* GUC will return status back to SOFT_SCRATCH_O_REG */
 enum intel_guc_status {
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 27e072c..3aa0f44 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
 					   INTEL_GUC_RECV_MASK,
 					   INTEL_GUC_RECV_MASK,
 					   10, 10, &status);
-	if (status != INTEL_GUC_STATUS_SUCCESS) {
+	if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
 		/*
 		 * Either the GuC explicitly returned an error (which
 		 * we convert to -EIO here) or no response at all was
@@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
 		DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
 			 " ret=%d status=0x%08X response=0x%08X\n",
 			 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
+	} else {
+		/* Use data encoded by Guc in status dword as return value */
+		ret = INTEL_GUC_RECV_TO_DATA(status);
 	}
 
 	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 01/16] drm/i915/guc: Add support for data reporting in GuC responses

[Michel Thierry]

On 8/7/2017 9:14 AM, Michal Wajdeczko wrote:
> GuC may return additional data in the command status response.
> Format and meaning of this data is action specific.
> We will use this non-negative data as a new success return value.
> Currently used actions don't return data that way yet.
> 
> v2: fix prohibited space after '~' (Michel)
>      update commit message (Daniele)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++++++-------
>   drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++++++
>   drivers/gpu/drm/i915/intel_uc.c       |  5 ++++-
>   3 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index c4cbec1..1249868 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
>   	err = wait_for_response(desc, fence, status);
>   	if (unlikely(err))
>   		return err;
> -	if (*status != INTEL_GUC_STATUS_SUCCESS)
> +	if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
>   		return -EIO;
> -	return 0;
> +	return INTEL_GUC_RECV_TO_DATA(*status);
>   }
>   
>   /*
> @@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
>   {
>   	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
>   	u32 status = ~0; /* undefined */
> -	int err;
> +	int ret;
>   
>   	mutex_lock(&guc->send_mutex);
>   
> -	err = ctch_send(guc, ctch, action, len, &status);
> -	if (unlikely(err)) {
> +	ret = ctch_send(guc, ctch, action, len, &status);
> +	if (unlikely(ret < 0)) {
>   		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
> -			  action[0], err, status);
> +			  action[0], ret, status);
>   	}
>   
>   	mutex_unlock(&guc->send_mutex);
> -	return err;
> +	return ret;
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 5fa2860..367aa65 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -567,10 +567,16 @@ enum intel_guc_action {
>    * command in SS0. The response is distinguishable from a command
>    * by the fact that all the MASK bits are set. The remaining bits
>    * give more detail.
> + * Bits [16:27] are reserved for optional data reporting.
>    */
>   #define	INTEL_GUC_RECV_MASK	((u32)0xF0000000)
>   #define	INTEL_GUC_RECV_IS_RESPONSE(x)	((u32)(x) >= INTEL_GUC_RECV_MASK)
>   #define	INTEL_GUC_RECV_STATUS(x)	(INTEL_GUC_RECV_MASK | (x))
> +#define INTEL_GUC_RECV_DATA_SHIFT	16
> +#define INTEL_GUC_RECV_DATA_MASK	(0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
> +#define INTEL_GUC_RECV_TO_STATUS(x)	((x) & ~INTEL_GUC_RECV_DATA_MASK)
> +#define INTEL_GUC_RECV_TO_DATA(x)	(((x) & INTEL_GUC_RECV_DATA_MASK) >> \
> +					 INTEL_GUC_RECV_DATA_SHIFT)
>   
>   /* GUC will return status back to SOFT_SCRATCH_O_REG */
>   enum intel_guc_status {
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 27e072c..3aa0f44 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
>   					   INTEL_GUC_RECV_MASK,
>   					   INTEL_GUC_RECV_MASK,
>   					   10, 10, &status);
> -	if (status != INTEL_GUC_STATUS_SUCCESS) {
> +	if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
>   		/*
>   		 * Either the GuC explicitly returned an error (which
>   		 * we convert to -EIO here) or no response at all was
> @@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
>   		DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
>   			 " ret=%d status=0x%08X response=0x%08X\n",
>   			 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
> +	} else {
> +		/* Use data encoded by Guc in status dword as return value */
> +		ret = INTEL_GUC_RECV_TO_DATA(status);
>   	}
>   
>   	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
> 

Reviewed-by: Michel Thierry <michel.thierry@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 02/16] drm/i915/guc: Prepare send() function to accept bigger response

[Michal Wajdeczko]

This is a preparation step for the upcoming patches.
We already can return some small data decoded from the command
status, but we will need more in the future.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 7 ++++---
 drivers/gpu/drm/i915/intel_uc.c     | 6 ++++--
 drivers/gpu/drm/i915/intel_uc.h     | 8 ++++----
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 1249868..c17cb42 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -79,7 +79,7 @@ static int guc_action_register_ct_buffer(struct intel_guc *guc,
 	int err;
 
 	/* Can't use generic send(), CT registration must go over MMIO */
-	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
 	if (err)
 		DRM_ERROR("CT: register %s buffer failed; err=%d\n",
 			  guc_ct_buffer_type_to_str(type), err);
@@ -98,7 +98,7 @@ static int guc_action_deregister_ct_buffer(struct intel_guc *guc,
 	int err;
 
 	/* Can't use generic send(), CT deregistration must go over MMIO */
-	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
 	if (err)
 		DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
 			  guc_ct_buffer_type_to_str(type), owner, err);
@@ -395,7 +395,8 @@ static int ctch_send(struct intel_guc *guc,
 /*
  * Command Transport (CT) buffer based GuC send function.
  */
-static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
+static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
+			     u32 *response)
 {
 	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
 	u32 status = ~0; /* undefined */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 3aa0f44..1bb3bc0 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -459,7 +459,8 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 	i915_ggtt_disable_guc(dev_priv);
 }
 
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
+		       u32 *response)
 {
 	WARN(1, "Unexpected send: action=%#x\n", *action);
 	return -ENODEV;
@@ -468,7 +469,8 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
 /*
  * This function implements the MMIO based host to GuC interface.
  */
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
+			u32 *response)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	u32 status;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b..53ea5f1 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -206,7 +206,7 @@ struct intel_guc {
 	struct mutex send_mutex;
 
 	/* GuC's FW specific send function */
-	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
+	int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
 
 	/* GuC's FW specific notify function */
 	void (*notify)(struct intel_guc *guc);
@@ -227,12 +227,12 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
 {
-	return guc->send(guc, action, len);
+	return guc->send(guc, action, len, NULL);
 }
 
 static inline void intel_guc_notify(struct intel_guc *guc)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 03/16] drm/i915/guc: Add send_and_receive() helper function

[Michal Wajdeczko]

In the previous patch we have changed signature of the send function
pointer but we didn't modify signature of the corresponding helper
function to minimize number of required changes. Let's add separate
helper to expose new functionality but still hide underlying details.

v2: enforce response buffer size check (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 53ea5f1..482dfa5 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -235,6 +235,16 @@ static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 l
 	return guc->send(guc, action, len, NULL);
 }
 
+static inline int intel_guc_send_and_receive(struct intel_guc *guc,
+					     const u32 *action, u32 len,
+					     u32 *response_buf, u32 size)
+{
+	BUILD_BUG_ON(!__builtin_constant_p(size));
+	BUILD_BUG_ON(size < GUC_CT_MSG_LEN_MASK);
+
+	return guc->send(guc, action, len, response_buf);
+}
+
 static inline void intel_guc_notify(struct intel_guc *guc)
 {
 	guc->notify(guc);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 04/16] drm/i915/guc: Implement response handling in send_mmio()

[Michal Wajdeczko]

In addition to already returned small data encoded in the status MMIO,
GuC may write more additional data in remaining MMIO regs. Lets copy
all that regs into optionally provided response buffer.

v2: new line (Michel)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 1bb3bc0..433a6a3 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -517,6 +517,12 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 			 " ret=%d status=0x%08X response=0x%08X\n",
 			 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
 	} else {
+		if (response) {
+			/* Skip reg[0] with the status/response mask */
+			for (i = 1; i < guc->send_regs.count; i++)
+				response[i] = I915_READ(guc_send_reg(guc, i));
+		}
+
 		/* Use data encoded by Guc in status dword as return value */
 		ret = INTEL_GUC_RECV_TO_DATA(status);
 	}
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 05/16] drm/i915/guc: Move Guc notification handling to separate function

[Michal Wajdeczko]

To allow future code reuse. While here, fix comment style.

Suggested-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 33 ++-------------------------------
 drivers/gpu/drm/i915/intel_uc.c | 37 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h |  1 +
 3 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 196caa4..ac69534 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1675,37 +1675,8 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
 
 static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 gt_iir)
 {
-	if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT) {
-		/* Sample the log buffer flush related bits & clear them out now
-		 * itself from the message identity register to minimize the
-		 * probability of losing a flush interrupt, when there are back
-		 * to back flush interrupts.
-		 * There can be a new flush interrupt, for different log buffer
-		 * type (like for ISR), whilst Host is handling one (for DPC).
-		 * Since same bit is used in message register for ISR & DPC, it
-		 * could happen that GuC sets the bit for 2nd interrupt but Host
-		 * clears out the bit on handling the 1st interrupt.
-		 */
-		u32 msg, flush;
-
-		msg = I915_READ(SOFT_SCRATCH(15));
-		flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
-			       INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER);
-		if (flush) {
-			/* Clear the message bits that are handled */
-			I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
-
-			/* Handle flush interrupt in bottom half */
-			queue_work(dev_priv->guc.log.runtime.flush_wq,
-				   &dev_priv->guc.log.runtime.flush_work);
-
-			dev_priv->guc.log.flush_interrupt_count++;
-		} else {
-			/* Not clearing of unhandled event bits won't result in
-			 * re-triggering of the interrupt.
-			 */
-		}
-	}
+	if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT)
+		intel_guc_notification_handler(&dev_priv->guc);
 }
 
 static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 433a6a3..3d997a3 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -533,6 +533,43 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 	return ret;
 }
 
+void intel_guc_notification_handler(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	u32 msg, flush;
+
+	/*
+	 * Sample the log buffer flush related bits & clear them out now
+	 * itself from the message identity register to minimize the
+	 * probability of losing a flush interrupt, when there are back
+	 * to back flush interrupts.
+	 * There can be a new flush interrupt, for different log buffer
+	 * type (like for ISR), whilst Host is handling one (for DPC).
+	 * Since same bit is used in message register for ISR & DPC, it
+	 * could happen that GuC sets the bit for 2nd interrupt but Host
+	 * clears out the bit on handling the 1st interrupt.
+	 */
+
+	msg = I915_READ(SOFT_SCRATCH(15));
+	flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
+		       INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER);
+	if (flush) {
+		/* Clear the message bits that are handled */
+		I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
+
+		/* Handle flush interrupt in bottom half */
+		queue_work(dev_priv->guc.log.runtime.flush_wq,
+				&dev_priv->guc.log.runtime.flush_work);
+
+		dev_priv->guc.log.flush_interrupt_count++;
+	} else {
+		/*
+		 * Not clearing of unhandled event bits won't result in
+		 * re-triggering of the interrupt.
+		 */
+	}
+}
+
 int intel_guc_sample_forcewake(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 482dfa5..14dbe9f 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -226,6 +226,7 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
+void intel_guc_notification_handler(struct intel_guc *guc);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 06/16] drm/i915/guc: Move flushing the GuC logs outside notification handler

[Michal Wajdeczko]

From: Oscar Mateo <oscar.mateo@intel.com>

To allow future code reuse.

Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_log.c | 8 ++++++++
 drivers/gpu/drm/i915/intel_uc.c      | 6 +-----
 drivers/gpu/drm/i915/intel_uc.h      | 1 +
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 16d3b87..acd9a3f 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -520,6 +520,14 @@ static void guc_flush_logs(struct intel_guc *guc)
 	guc_log_capture_logs(guc);
 }
 
+void intel_guc_log_flush(struct intel_guc *guc)
+{
+	/* Handle flush interrupt in bottom half */
+	queue_work(guc->log.runtime.flush_wq, &guc->log.runtime.flush_work);
+
+	guc->log.flush_interrupt_count++;
+}
+
 int intel_guc_log_create(struct intel_guc *guc)
 {
 	struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 3d997a3..f1a6af7 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -557,11 +557,7 @@ void intel_guc_notification_handler(struct intel_guc *guc)
 		/* Clear the message bits that are handled */
 		I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
 
-		/* Handle flush interrupt in bottom half */
-		queue_work(dev_priv->guc.log.runtime.flush_wq,
-				&dev_priv->guc.log.runtime.flush_work);
-
-		dev_priv->guc.log.flush_interrupt_count++;
+		intel_guc_log_flush(&dev_priv->guc);
 	} else {
 		/*
 		 * Not clearing of unhandled event bits won't result in
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 14dbe9f..9979a22 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -270,6 +270,7 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
 /* intel_guc_log.c */
 int intel_guc_log_create(struct intel_guc *guc);
 void intel_guc_log_destroy(struct intel_guc *guc);
+void intel_guc_log_flush(struct intel_guc *guc);
 int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
 void i915_guc_log_register(struct drm_i915_private *dev_priv);
 void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 07/16] drm/i915/guc: Create a GuC receive function

[Michal Wajdeczko]

From: Oscar Mateo <oscar.mateo@intel.com>

This function, symmetrical to the send(), will handle Guc2Host message
interrupts (which at the moment still only covers requests to flush
the GuC logs).

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 18 +++++++++++++++++-
 drivers/gpu/drm/i915/intel_uc.h |  5 +++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index f1a6af7..2590769 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -109,6 +109,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 
 	mutex_init(&guc->send_mutex);
 	guc->send = intel_guc_send_nop;
+	guc->recv = intel_guc_receive_nop;
 	guc->notify = guc_write_irq_trigger;
 }
 
@@ -315,6 +316,7 @@ static int guc_enable_communication(struct intel_guc *guc)
 		return intel_guc_enable_ct(guc);
 
 	guc->send = intel_guc_send_mmio;
+	guc->recv = intel_guc_receive_mmio;
 	return 0;
 }
 
@@ -326,6 +328,7 @@ static void guc_disable_communication(struct intel_guc *guc)
 		intel_guc_disable_ct(guc);
 
 	guc->send = intel_guc_send_nop;
+	guc->recv = intel_guc_receive_nop;
 }
 
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
@@ -466,6 +469,11 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
 	return -ENODEV;
 }
 
+void intel_guc_receive_nop(struct intel_guc *guc)
+{
+	WARN(1, "Unexpected receive\n");
+}
+
 /*
  * This function implements the MMIO based host to GuC interface.
  */
@@ -533,7 +541,10 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 	return ret;
 }
 
-void intel_guc_notification_handler(struct intel_guc *guc)
+/*
+ * This function implements the MMIO based GuC to host interface.
+ */
+void intel_guc_receive_mmio(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	u32 msg, flush;
@@ -566,6 +577,11 @@ void intel_guc_notification_handler(struct intel_guc *guc)
 	}
 }
 
+void intel_guc_notification_handler(struct intel_guc *guc)
+{
+	guc->recv(guc);
+}
+
 int intel_guc_sample_forcewake(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 9979a22..445bca3 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -208,6 +208,9 @@ struct intel_guc {
 	/* GuC's FW specific send function */
 	int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
 
+	/* GuC's FW specific receive function */
+	void (*recv)(struct intel_guc *guc);
+
 	/* GuC's FW specific notify function */
 	void (*notify)(struct intel_guc *guc);
 };
@@ -230,6 +233,8 @@ void intel_guc_notification_handler(struct intel_guc *guc);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
+void intel_guc_receive_nop(struct intel_guc *guc);
+void intel_guc_receive_mmio(struct intel_guc *guc);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
 {
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 08/16] drm/i915/guc: Update CT message header definition

[Michal Wajdeczko]

Flags bits are different in G2H message.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Kelvin Gardiner <kelvin.gardiner@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 367aa65..89781d3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -358,17 +358,24 @@ struct guc_ct_buffer_desc {
  *
  * bit[4..0]	message len (in dwords)
  * bit[7..5]	reserved
+ * bit[10..8]	flags
+ * bit[15..11]	reserved
+ * bit[31..16]	action code
+ *
+ * H2G flags:
  * bit[8]	write fence to desc
  * bit[9]	write status to H2G buff
  * bit[10]	send status (via G2H)
- * bit[15..11]	reserved
- * bit[31..16]	action code
+ *
+ * G2H flags:
+ * bit[8]	is response
  */
 #define GUC_CT_MSG_LEN_SHIFT			0
 #define GUC_CT_MSG_LEN_MASK			0x1F
 #define GUC_CT_MSG_WRITE_FENCE_TO_DESC		(1 << 8)
 #define GUC_CT_MSG_WRITE_STATUS_TO_BUFF		(1 << 9)
 #define GUC_CT_MSG_SEND_STATUS			(1 << 10)
+#define GUC_CT_MSG_IS_RESPONSE			(1 << 8)
 #define GUC_CT_MSG_ACTION_SHIFT			16
 #define GUC_CT_MSG_ACTION_MASK			0xFFFF
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 08/16] drm/i915/guc: Update CT message header definition

[Daniele Ceraolo Spurio]

On 07/08/17 09:14, Michal Wajdeczko wrote:
> Flags bits are different in G2H message.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Kelvin Gardiner <kelvin.gardiner@intel.com>

Matches GuC defines, so:

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

-Daniele

> ---
>   drivers/gpu/drm/i915/intel_guc_fwif.h | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 367aa65..89781d3 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -358,17 +358,24 @@ struct guc_ct_buffer_desc {
>    *
>    * bit[4..0]	message len (in dwords)
>    * bit[7..5]	reserved
> + * bit[10..8]	flags
> + * bit[15..11]	reserved
> + * bit[31..16]	action code
> + *
> + * H2G flags:
>    * bit[8]	write fence to desc
>    * bit[9]	write status to H2G buff
>    * bit[10]	send status (via G2H)
> - * bit[15..11]	reserved
> - * bit[31..16]	action code
> + *
> + * G2H flags:
> + * bit[8]	is response
>    */
>   #define GUC_CT_MSG_LEN_SHIFT			0
>   #define GUC_CT_MSG_LEN_MASK			0x1F
>   #define GUC_CT_MSG_WRITE_FENCE_TO_DESC		(1 << 8)
>   #define GUC_CT_MSG_WRITE_STATUS_TO_BUFF		(1 << 9)
>   #define GUC_CT_MSG_SEND_STATUS			(1 << 10)
> +#define GUC_CT_MSG_IS_RESPONSE			(1 << 8)
>   #define GUC_CT_MSG_ACTION_SHIFT			16
>   #define GUC_CT_MSG_ACTION_MASK			0xFFFF
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 09/16] drm/i915/guc: Prepare to handle messages from CT RECV buffer

[Michal Wajdeczko]

GuC can respond to our commands not only by updating SEND buffer
descriptor, but can send us message over RECV buffer. Additionally
Guc can also send us unsolicited requests over RECV buffer.
Lets start reading those messages and make placeholders for actual
response/request handlers.

v2: misc improvements (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 127 ++++++++++++++++++++++++++++++++++++
 1 file changed, 127 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index c17cb42..b25f478 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -414,6 +414,131 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 	return ret;
 }
 
+static inline unsigned int ct_header_get_len(u32 header)
+{
+	return (header >> GUC_CT_MSG_LEN_SHIFT) & GUC_CT_MSG_LEN_MASK;
+}
+
+static inline unsigned int ct_header_get_action(u32 header)
+{
+	return (header >> GUC_CT_MSG_ACTION_SHIFT) & GUC_CT_MSG_ACTION_MASK;
+}
+
+static inline bool ct_header_is_response(u32 header)
+{
+	return !!(header & GUC_CT_MSG_IS_RESPONSE);
+}
+
+static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
+{
+	struct guc_ct_buffer_desc *desc = ctb->desc;
+	u32 head = desc->head / 4;	/* in dwords */
+	u32 tail = desc->tail / 4;	/* in dwords */
+	u32 size = desc->size / 4;	/* in dwords */
+	u32 *cmds = ctb->cmds;
+	s32 available;			/* in dwords */
+	unsigned int len;
+	unsigned int i;
+
+	GEM_BUG_ON(desc->size % 4);
+	GEM_BUG_ON(desc->head % 4);
+	GEM_BUG_ON(desc->tail % 4);
+	GEM_BUG_ON(tail >= size);
+	GEM_BUG_ON(head >= size);
+
+	/* tail == head condition indicates empty */
+	available = tail - head;
+	if (unlikely(available == 0))
+		return -ENODATA;
+
+	/* beware of buffer wrap case */
+	if (unlikely(available < 0))
+		available += size;
+	GEM_BUG_ON(available < 0);
+
+	data[0] = cmds[head];
+	head = (head + 1) % size;
+
+	/* message len with header */
+	len = ct_header_get_len(data[0]) + 1;
+	if (unlikely(len > (u32)available)) {
+		DRM_ERROR("CT: incomplete message %*phn %*phn %*phn\n",
+			  4, data,
+			  4 * (head + available - 1 > size ?
+			       size - head : available - 1), &cmds[head],
+			  4 * (head + available - 1 > size ?
+			       available - 1 - size + head : 0), &cmds[0]);
+		return -EPROTO;
+	}
+
+	for (i = 1; i < len; i++) {
+		data[i] = cmds[head];
+		head = (head + 1) % size;
+	}
+
+	desc->head = head * 4;
+	return 0;
+}
+
+static int guc_handle_response(struct intel_guc *guc, const u32 *msg)
+{
+	u32 header = msg[0];
+	u32 status = msg[2];
+	u32 len = ct_header_get_len(header) + 1; /* total len with header */
+
+	GEM_BUG_ON(!ct_header_is_response(header));
+	/* Response message shall at least include header, fence and status */
+	if (unlikely(len < 3)) {
+		DRM_ERROR("CT: corrupted response %*phn\n", 4*len, msg);
+		return -EPROTO;
+	}
+	if (unlikely(!INTEL_GUC_RECV_IS_RESPONSE(status))) {
+		DRM_ERROR("CT: corrupted status %*phn\n", 4*len, msg);
+		status = INTEL_GUC_STATUS_GENERIC_FAIL;
+	}
+
+	/* XXX */
+	return 0;
+}
+
+static int guc_handle_request(struct intel_guc *guc, const u32 *msg)
+{
+	u32 header = msg[0];
+
+	GEM_BUG_ON(ct_header_is_response(header));
+	/* Request message layout beyond header is request specific */
+
+	/* XXX */
+	return 0;
+}
+
+static void intel_guc_receive_ct(struct intel_guc *guc)
+{
+	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
+	struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_RECV];
+	u32 msg[GUC_CT_MSG_LEN_MASK+1]; /* one extra dw for the header */
+	int err = 0;
+
+	if (!ctch_is_open(ctch))
+		return;
+
+	do {
+		err = ctb_read(ctb, msg);
+		if (err)
+			break;
+
+		if (ct_header_is_response(msg[0]))
+			err = guc_handle_response(guc, msg);
+		else
+			err = guc_handle_request(guc, msg);
+	} while (!err);
+
+	if (GEM_WARN_ON(err == -EPROTO)) {
+		DRM_ERROR("CT: corrupted message detected!\n");
+		ctb->desc->is_in_error = 1;
+	}
+}
+
 /**
  * Enable buffer based command transport
  * Shall only be called for platforms with HAS_GUC_CT.
@@ -435,6 +560,7 @@ int intel_guc_enable_ct(struct intel_guc *guc)
 
 	/* Switch into cmd transport buffer based send() */
 	guc->send = intel_guc_send_ct;
+	guc->recv = intel_guc_receive_ct;
 	DRM_INFO("CT: %s\n", enableddisabled(true));
 	return 0;
 }
@@ -458,5 +584,6 @@ void intel_guc_disable_ct(struct intel_guc *guc)
 
 	/* Disable send */
 	guc->send = intel_guc_send_nop;
+	guc->recv = intel_guc_receive_nop;
 	DRM_INFO("CT: %s\n", enableddisabled(false));
 }
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 10/16] drm/i915/guc: Use better name for helper wait function

[Michal Wajdeczko]

In next patch we will introduce another way of waiting for the response
that will use RECV buffer. To avoid misleading names, rename old wait
function to reflect the fact that it is based on descriptor update.

v2: fix comment style (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index b25f478..235446e 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -321,16 +321,21 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 	return 0;
 }
 
-/* Wait for the response from the GuC.
+/**
+ * Wait for the descriptor update.
+ * @desc:	buffer descriptor
  * @fence:	response fence
  * @status:	placeholder for status
- * return:	0 response received (status is valid)
- *		-ETIMEDOUT no response within hardcoded timeout
- *		-EPROTO no response, ct buffer was in error
+ *
+ * Guc will update this descriptor with new fence and status.
+ * Returns:
+ *	0 response received (status is valid)
+ *	-ETIMEDOUT no response within hardcoded timeout
+ *	-EPROTO no response, ct buffer is in error
  */
-static int wait_for_response(struct guc_ct_buffer_desc *desc,
-			     u32 fence,
-			     u32 *status)
+static int wait_for_desc_update(struct guc_ct_buffer_desc *desc,
+				u32 fence,
+				u32 *status)
 {
 	int err;
 
@@ -384,7 +389,7 @@ static int ctch_send(struct intel_guc *guc,
 
 	intel_guc_notify(guc);
 
-	err = wait_for_response(desc, fence, status);
+	err = wait_for_desc_update(desc, fence, status);
 	if (unlikely(err))
 		return err;
 	if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 11/16] drm/i915/guc: Implement response handling in send_ct()

[Michal Wajdeczko]

GuC may return not only small data encoded in the status dword,
but can also append additional data into the response message.
We will copy this extra data into provided buffer, and use
number of received data dwords as new success return value.

v2: fix timeout and checkpatch warnings (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 121 ++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_guc_ct.h |   5 ++
 2 files changed, 115 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 235446e..3187bf2 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,14 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+struct ct_request {
+	struct list_head link;
+	u32 fence;
+	u32 status;
+	u32 response_len;
+	u32 *response_buf;
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
@@ -32,6 +40,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 {
 	/* we're using static channel owners */
 	ct->host_channel.owner = CTB_OWNER_HOST;
+
+	spin_lock_init(&ct->lock);
+	INIT_LIST_HEAD(&ct->pending_requests);
 }
 
 static inline const char *guc_ct_buffer_type_to_str(u32 type)
@@ -265,7 +276,8 @@ static u32 ctch_get_next_fence(struct intel_guc_ct_channel *ctch)
 static int ctb_write(struct intel_guc_ct_buffer *ctb,
 		     const u32 *action,
 		     u32 len /* in dwords */,
-		     u32 fence)
+		     u32 fence,
+		     bool send_response)
 {
 	struct guc_ct_buffer_desc *desc = ctb->desc;
 	u32 head = desc->head / 4;	/* in dwords */
@@ -301,6 +313,7 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 	 */
 	header = (len << GUC_CT_MSG_LEN_SHIFT) |
 		 (GUC_CT_MSG_WRITE_FENCE_TO_DESC) |
+		 (send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 		 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
 	cmds[tail] = header;
@@ -367,14 +380,49 @@ static int wait_for_desc_update(struct guc_ct_buffer_desc *desc,
 	return err;
 }
 
+/**
+ * Wait for the Guc response.
+ * @req:	pointer to pending request
+ * @status:	placeholder for status
+ *
+ * We will update request status from the response message handler.
+ * Returns:
+ *	0 response received (status is valid)
+ *	-ETIMEDOUT no response within hardcoded timeout
+ */
+static int wait_for_response_msg(struct ct_request *req, u32 *status)
+{
+	int err;
+
+	/*
+	 * Fast commands should complete in less than 10us, so sample quickly
+	 * up to that length of time, then switch to a slower sleep-wait loop.
+	 * No GuC command should ever take longer than 10ms.
+	 */
+#define done INTEL_GUC_RECV_IS_RESPONSE(READ_ONCE(req->status))
+	err = wait_for_us(done, 10);
+	if (err)
+		err = wait_for(done, 10);
+#undef done
+
+	if (unlikely(err))
+		DRM_ERROR("CT: fence %u err %d\n", req->fence, err);
+
+	*status = req->status;
+	return err;
+}
+
 static int ctch_send(struct intel_guc *guc,
 		     struct intel_guc_ct_channel *ctch,
 		     const u32 *action,
 		     u32 len,
-		     u32 *status)
+		     u32 *status,
+		     u32 *response)
 {
 	struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_SEND];
 	struct guc_ct_buffer_desc *desc = ctb->desc;
+	struct ct_request request;
+	unsigned long flags;
 	u32 fence;
 	int err;
 
@@ -383,18 +431,48 @@ static int ctch_send(struct intel_guc *guc,
 	GEM_BUG_ON(len & ~GUC_CT_MSG_LEN_MASK);
 
 	fence = ctch_get_next_fence(ctch);
-	err = ctb_write(ctb, action, len, fence);
+	request.fence = fence;
+	request.status = 0;
+	request.response_len = 0;
+	request.response_buf = response;
+
+	spin_lock_irqsave(&guc->ct.lock, flags);
+	list_add_tail(&request.link, &guc->ct.pending_requests);
+	spin_unlock_irqrestore(&guc->ct.lock, flags);
+
+	err = ctb_write(ctb, action, len, fence, !!response);
 	if (unlikely(err))
-		return err;
+		goto unlink;
 
 	intel_guc_notify(guc);
 
-	err = wait_for_desc_update(desc, fence, status);
+	if (response)
+		err = wait_for_response_msg(&request, status);
+	else
+		err = wait_for_desc_update(desc, fence, status);
 	if (unlikely(err))
-		return err;
-	if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
-		return -EIO;
-	return INTEL_GUC_RECV_TO_DATA(*status);
+		goto unlink;
+
+	if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS) {
+		err = -EIO;
+		goto unlink;
+	}
+
+	if (response) {
+		/* There shall be no data in the status */
+		WARN_ON(INTEL_GUC_RECV_TO_DATA(request.status));
+		err = request.response_len;
+	} else {
+		/* Return data decoded from the status dword */
+		err = INTEL_GUC_RECV_TO_DATA(*status);
+	}
+
+unlink:
+	spin_lock_irqsave(&guc->ct.lock, flags);
+	list_del(&request.link);
+	spin_unlock_irqrestore(&guc->ct.lock, flags);
+
+	return err;
 }
 
 /*
@@ -409,7 +487,7 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 
 	mutex_lock(&guc->send_mutex);
 
-	ret = ctch_send(guc, ctch, action, len, &status);
+	ret = ctch_send(guc, ctch, action, len, &status, response);
 	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
 			  action[0], ret, status);
@@ -488,8 +566,12 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 static int guc_handle_response(struct intel_guc *guc, const u32 *msg)
 {
 	u32 header = msg[0];
+	u32 fence = msg[1];
 	u32 status = msg[2];
 	u32 len = ct_header_get_len(header) + 1; /* total len with header */
+	struct ct_request *req;
+	bool found = false;
+	unsigned long flags;
 
 	GEM_BUG_ON(!ct_header_is_response(header));
 	/* Response message shall at least include header, fence and status */
@@ -502,7 +584,24 @@ static int guc_handle_response(struct intel_guc *guc, const u32 *msg)
 		status = INTEL_GUC_STATUS_GENERIC_FAIL;
 	}
 
-	/* XXX */
+	spin_lock_irqsave(&guc->ct.lock, flags);
+	list_for_each_entry(req, &guc->ct.pending_requests, link) {
+		if (req->fence != fence) {
+			DRM_DEBUG_DRIVER("CT: request %u awaits response\n",
+					 req->fence);
+			continue;
+		}
+		req->response_len = len - 3;
+		if (req->response_buf)
+			memcpy(req->response_buf, msg + 3, 4*(len - 3));
+		WRITE_ONCE(req->status, status);
+		found = true;
+		break;
+	}
+	spin_unlock_irqrestore(&guc->ct.lock, flags);
+
+	if (!found)
+		DRM_ERROR("CT: unsolicited response %*phn\n", 4*len, msg);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index 6d97f36..557c1e8 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -71,10 +71,15 @@ struct intel_guc_ct_channel {
 /** Holds all command transport channels.
  *
  * @host_channel: main channel used by the host
+ * @lock: spin lock for pending requests list
+ * @pending_requests: list of pending requests
  */
 struct intel_guc_ct {
 	struct intel_guc_ct_channel host_channel;
 	/* other channels are tbd */
+
+	spinlock_t lock;
+	struct list_head pending_requests;
 };
 
 void intel_guc_ct_init_early(struct intel_guc_ct *ct);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 12/16] drm/i915/guc: Prepare to process incoming requests from CT

[Michal Wajdeczko]

Requests are read from CT in the irq handler, but actual processing
will be done in the work thread. Processing of specific actions will
be added in the upcoming patches.

v2: don't use GEM_BUG_ON (Chris)
    don't kmalloc too large buffer (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 71 ++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_guc_ct.h |  4 +++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 3187bf2..abf2bd8 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -32,10 +32,17 @@ struct ct_request {
 	u32 *response_buf;
 };
 
+struct ct_incoming_request {
+	struct list_head link;
+	u32 data[];
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
 
+static void ct_worker_func(struct work_struct *w);
+
 void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 {
 	/* we're using static channel owners */
@@ -43,6 +50,8 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 
 	spin_lock_init(&ct->lock);
 	INIT_LIST_HEAD(&ct->pending_requests);
+	INIT_LIST_HEAD(&ct->incoming_requests);
+	INIT_WORK(&ct->worker, ct_worker_func);
 }
 
 static inline const char *guc_ct_buffer_type_to_str(u32 type)
@@ -608,14 +617,74 @@ static int guc_handle_response(struct intel_guc *guc, const u32 *msg)
 static int guc_handle_request(struct intel_guc *guc, const u32 *msg)
 {
 	u32 header = msg[0];
+	u32 len = ct_header_get_len(header) + 1; /* total len with header */
+	struct ct_incoming_request *request;
+	unsigned long flags;
 
 	GEM_BUG_ON(ct_header_is_response(header));
 	/* Request message layout beyond header is request specific */
 
-	/* XXX */
+	request = kmalloc(sizeof(*request) + 4*len, GFP_ATOMIC);
+	if (unlikely(!request)) {
+		DRM_ERROR("CT: dropping request %*phn\n", 4*len, msg);
+		return 0; /* XXX: -ENOMEM ? */
+	}
+	memcpy(request->data, msg, 4*len);
+
+	spin_lock_irqsave(&guc->ct.lock, flags);
+	list_add_tail(&request->link, &guc->ct.incoming_requests);
+	spin_unlock_irqrestore(&guc->ct.lock, flags);
+
+	queue_work(system_unbound_wq, &guc->ct.worker);
 	return 0;
 }
 
+static bool guc_process_incoming_requests(struct intel_guc *guc)
+{
+	unsigned long flags;
+	struct ct_incoming_request *request;
+	bool done;
+	u32 header;
+	u32 action;
+	u32 len;
+
+	spin_lock_irqsave(&guc->ct.lock, flags);
+	request = list_first_entry_or_null(&guc->ct.incoming_requests,
+					   struct ct_incoming_request, link);
+	if (request)
+		list_del(&request->link);
+	done = !!list_empty(&guc->ct.incoming_requests);
+	spin_unlock_irqrestore(&guc->ct.lock, flags);
+
+	if (!request)
+		return true;
+
+	header = request->data[0];
+	action = ct_header_get_action(header);
+	len = ct_header_get_len(header) + 1; /* also count header dw */
+
+	switch (action) {
+	default:
+		DRM_ERROR("CT: unexpected request %*phn\n",
+			  4*len, request->data);
+		break;
+	}
+
+	kfree(request);
+	return done;
+}
+
+static void ct_worker_func(struct work_struct *w)
+{
+	struct intel_guc_ct *ct = container_of(w, struct intel_guc_ct, worker);
+	struct intel_guc *guc = container_of(ct, struct intel_guc, ct);
+	bool done;
+
+	done = guc_process_incoming_requests(guc);
+	if (!done)
+		queue_work(system_unbound_wq, &ct->worker);
+}
+
 static void intel_guc_receive_ct(struct intel_guc *guc)
 {
 	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index 557c1e8..125e004 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -73,6 +73,8 @@ struct intel_guc_ct_channel {
  * @host_channel: main channel used by the host
  * @lock: spin lock for pending requests list
  * @pending_requests: list of pending requests
+ * @incoming_requests: list of incoming requests
+ * @tasklet: tasklet for handling incoming requests
  */
 struct intel_guc_ct {
 	struct intel_guc_ct_channel host_channel;
@@ -80,6 +82,8 @@ struct intel_guc_ct {
 
 	spinlock_t lock;
 	struct list_head pending_requests;
+	struct list_head incoming_requests;
+	struct work_struct worker;
 };
 
 void intel_guc_ct_init_early(struct intel_guc_ct *ct);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 13/16] drm/i915/guc: Handle default action received over CT

[Michal Wajdeczko]

With enabled CT, instead of programming SCRATCH 15 register with the
Guc to host message, Guc will send us unsolicited CT message.
Content of the first payload dword of this request has the same
format as data expected in the above scratch register.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 3 +++
 drivers/gpu/drm/i915/intel_uc.c     | 7 +++++++
 drivers/gpu/drm/i915/intel_uc.h     | 1 +
 3 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index abf2bd8..e6912be 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -664,6 +664,9 @@ static bool guc_process_incoming_requests(struct intel_guc *guc)
 	len = ct_header_get_len(header) + 1; /* also count header dw */
 
 	switch (action) {
+	case INTEL_GUC_ACTION_DEFAULT:
+		intel_guc_process_default_action(guc, request->data[1]);
+		break;
 	default:
 		DRM_ERROR("CT: unexpected request %*phn\n",
 			  4*len, request->data);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 2590769..774d740 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -597,3 +597,10 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
 
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
+
+void intel_guc_process_default_action(struct intel_guc *guc, u32 msg)
+{
+	if (msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
+		   INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER))
+		intel_guc_log_flush(guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 445bca3..478df86 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -230,6 +230,7 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 void intel_guc_notification_handler(struct intel_guc *guc);
+void intel_guc_process_default_action(struct intel_guc *guc, u32 msg);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 *response);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 14/16] drm/i915/guc: Enable GuC interrupts when using CT

[Michal Wajdeczko]

We will need them in G2H communication to properly handle
responses and requests from the Guc.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
 drivers/gpu/drm/i915/intel_uc.c            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 48a1e93..509497e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1328,7 +1328,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
 	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return 0;
 
-	if (i915.guc_log_level >= 0)
+	if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
 		gen9_enable_guc_interrupts(dev_priv);
 
 	ctx = dev_priv->kernel_context;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 774d740..0209ad0 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -395,7 +395,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
 	intel_guc_auth_huc(dev_priv);
 	if (i915.enable_guc_submission) {
-		if (i915.guc_log_level >= 0)
+		if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
 			gen9_enable_guc_interrupts(dev_priv);
 
 		ret = i915_guc_submission_enable(dev_priv);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 14/16] drm/i915/guc: Enable GuC interrupts when using CT

[Oscar Mateo]

On 08/07/2017 09:14 AM, Michal Wajdeczko wrote:
> We will need them in G2H communication to properly handle
> responses and requests from the Guc.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
>   drivers/gpu/drm/i915/intel_uc.c            | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 48a1e93..509497e 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1328,7 +1328,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
>   	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>   		return 0;
>   
> -	if (i915.guc_log_level >= 0)
> +	if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
>   		gen9_enable_guc_interrupts(dev_priv);
>   
>   	ctx = dev_priv->kernel_context;
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 774d740..0209ad0 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -395,7 +395,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>   
>   	intel_guc_auth_huc(dev_priv);
>   	if (i915.enable_guc_submission) {
> -		if (i915.guc_log_level >= 0)
> +		if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
>   			gen9_enable_guc_interrupts(dev_priv);
>   
>   		ret = i915_guc_submission_enable(dev_priv);

Is this enough? There are also gen9_disable_guc_interrupts that will 
break CTB (e.g. when logs get disabled).
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 15/16] drm/i915/guc: Trace messages from CT while in debug

[Michal Wajdeczko]

During debug we may want to investigate all communication
from the Guc. Add proper tracing macros in debug config.

v2: convert remaining DRM_DEBUG into new CT_DEBUG (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 41 +++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index e6912be..568fbc0 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,12 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+#ifdef CONFIG_DRM_I915_DEBUG
+#define CT_DEBUG_DRIVER(...)	DRM_DEBUG_DRIVER(__VA_ARGS__)
+#else
+#define CT_DEBUG_DRIVER(...)
+#endif
+
 struct ct_request {
 	struct list_head link;
 	u32 fence;
@@ -69,8 +75,8 @@ static inline const char *guc_ct_buffer_type_to_str(u32 type)
 static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
 				    u32 cmds_addr, u32 size, u32 owner)
 {
-	DRM_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
-			 desc, cmds_addr, size, owner);
+	CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
+			desc, cmds_addr, size, owner);
 	memset(desc, 0, sizeof(*desc));
 	desc->addr = cmds_addr;
 	desc->size = size;
@@ -79,8 +85,8 @@ static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
 
 static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)
 {
-	DRM_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
-			 desc, desc->head, desc->tail);
+	CT_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
+			desc, desc->head, desc->tail);
 	desc->head = 0;
 	desc->tail = 0;
 	desc->is_in_error = 0;
@@ -176,7 +182,7 @@ static int ctch_init(struct intel_guc *guc,
 		err = PTR_ERR(blob);
 		goto err_vma;
 	}
-	DRM_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
+	CT_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
 
 	/* store pointers to desc and cmds */
 	for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
@@ -190,8 +196,8 @@ static int ctch_init(struct intel_guc *guc,
 err_vma:
 	i915_vma_unpin_and_release(&ctch->vma);
 err_out:
-	DRM_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
-			 ctch->owner, err);
+	CT_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
+			ctch->owner, err);
 	return err;
 }
 
@@ -211,8 +217,8 @@ static int ctch_open(struct intel_guc *guc,
 	int err;
 	int i;
 
-	DRM_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
-			 ctch->owner, yesno(ctch_is_open(ctch)));
+	CT_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
+			ctch->owner, yesno(ctch_is_open(ctch)));
 
 	if (!ctch->vma) {
 		err = ctch_init(guc, ctch);
@@ -325,6 +331,10 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 		 (send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 		 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
+	CT_DEBUG_DRIVER("CT: writing %*phn %*phn %*phn\n",
+			4, &header, 4, &fence,
+			4*(len - 1), &action[1]);
+
 	cmds[tail] = header;
 	tail = (tail + 1) % size;
 
@@ -500,6 +510,9 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
 			  action[0], ret, status);
+	} else if (unlikely(ret)) {
+		CT_DEBUG_DRIVER("CT: send action %#x returned %d (%#x)\n",
+				action[0], ret, ret);
 	}
 
 	mutex_unlock(&guc->send_mutex);
@@ -546,10 +559,12 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 	/* beware of buffer wrap case */
 	if (unlikely(available < 0))
 		available += size;
+	CT_DEBUG_DRIVER("CT: available %d (%u:%u)\n", available, head, tail);
 	GEM_BUG_ON(available < 0);
 
 	data[0] = cmds[head];
 	head = (head + 1) % size;
+	CT_DEBUG_DRIVER("CT: header %#x\n", data[0]);
 
 	/* message len with header */
 	len = ct_header_get_len(data[0]) + 1;
@@ -567,6 +582,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 		data[i] = cmds[head];
 		head = (head + 1) % size;
 	}
+	CT_DEBUG_DRIVER("CT: received %*phn\n", 4*len, data);
 
 	desc->head = head * 4;
 	return 0;
@@ -592,12 +608,13 @@ static int guc_handle_response(struct intel_guc *guc, const u32 *msg)
 		DRM_ERROR("CT: corrupted status %*phn\n", 4*len, msg);
 		status = INTEL_GUC_STATUS_GENERIC_FAIL;
 	}
+	CT_DEBUG_DRIVER("CT: response fence %u status %#x\n", fence, status);
 
 	spin_lock_irqsave(&guc->ct.lock, flags);
 	list_for_each_entry(req, &guc->ct.pending_requests, link) {
 		if (req->fence != fence) {
-			DRM_DEBUG_DRIVER("CT: request %u awaits response\n",
-					 req->fence);
+			CT_DEBUG_DRIVER("CT: request %u awaits response\n",
+					req->fence);
 			continue;
 		}
 		req->response_len = len - 3;
@@ -623,6 +640,7 @@ static int guc_handle_request(struct intel_guc *guc, const u32 *msg)
 
 	GEM_BUG_ON(ct_header_is_response(header));
 	/* Request message layout beyond header is request specific */
+	CT_DEBUG_DRIVER("CT: request %#x\n", ct_header_get_action(header));
 
 	request = kmalloc(sizeof(*request) + 4*len, GFP_ATOMIC);
 	if (unlikely(!request)) {
@@ -662,6 +680,7 @@ static bool guc_process_incoming_requests(struct intel_guc *guc)
 	header = request->data[0];
 	action = ct_header_get_action(header);
 	len = ct_header_get_len(header) + 1; /* also count header dw */
+	CT_DEBUG_DRIVER("CT: processing request %*phn\n", 4*len, request->data);
 
 	switch (action) {
 	case INTEL_GUC_ACTION_DEFAULT:
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 15/16] drm/i915/guc: Trace messages from CT while in debug

[Daniele Ceraolo Spurio]

On 07/08/17 09:14, Michal Wajdeczko wrote:
> During debug we may want to investigate all communication
> from the Guc. Add proper tracing macros in debug config.
> 
> v2: convert remaining DRM_DEBUG into new CT_DEBUG (Michal)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_ct.c | 41 +++++++++++++++++++++++++++----------
>   1 file changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index e6912be..568fbc0 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -24,6 +24,12 @@
>   #include "i915_drv.h"
>   #include "intel_guc_ct.h"
>   
> +#ifdef CONFIG_DRM_I915_DEBUG

Personally I'd prefer to have a separate kconfig for this (e.g. 
CONFIG_DRM_I915_DEBUG_GUC) as this can be quite verbose in some 
GuC-centric scenarios.

-Daniele

> +#define CT_DEBUG_DRIVER(...)	DRM_DEBUG_DRIVER(__VA_ARGS__)
> +#else
> +#define CT_DEBUG_DRIVER(...)
> +#endif
> +
>   struct ct_request {
>   	struct list_head link;
>   	u32 fence;
> @@ -69,8 +75,8 @@ static inline const char *guc_ct_buffer_type_to_str(u32 type)
>   static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
>   				    u32 cmds_addr, u32 size, u32 owner)
>   {
> -	DRM_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
> -			 desc, cmds_addr, size, owner);
> +	CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
> +			desc, cmds_addr, size, owner);
>   	memset(desc, 0, sizeof(*desc));
>   	desc->addr = cmds_addr;
>   	desc->size = size;
> @@ -79,8 +85,8 @@ static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
>   
>   static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)
>   {
> -	DRM_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
> -			 desc, desc->head, desc->tail);
> +	CT_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
> +			desc, desc->head, desc->tail);
>   	desc->head = 0;
>   	desc->tail = 0;
>   	desc->is_in_error = 0;
> @@ -176,7 +182,7 @@ static int ctch_init(struct intel_guc *guc,
>   		err = PTR_ERR(blob);
>   		goto err_vma;
>   	}
> -	DRM_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
> +	CT_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
>   
>   	/* store pointers to desc and cmds */
>   	for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
> @@ -190,8 +196,8 @@ static int ctch_init(struct intel_guc *guc,
>   err_vma:
>   	i915_vma_unpin_and_release(&ctch->vma);
>   err_out:
> -	DRM_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
> -			 ctch->owner, err);
> +	CT_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
> +			ctch->owner, err);
>   	return err;
>   }
>   
> @@ -211,8 +217,8 @@ static int ctch_open(struct intel_guc *guc,
>   	int err;
>   	int i;
>   
> -	DRM_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
> -			 ctch->owner, yesno(ctch_is_open(ctch)));
> +	CT_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
> +			ctch->owner, yesno(ctch_is_open(ctch)));
>   
>   	if (!ctch->vma) {
>   		err = ctch_init(guc, ctch);
> @@ -325,6 +331,10 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
>   		 (send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
>   		 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
>   
> +	CT_DEBUG_DRIVER("CT: writing %*phn %*phn %*phn\n",
> +			4, &header, 4, &fence,
> +			4*(len - 1), &action[1]);
> +
>   	cmds[tail] = header;
>   	tail = (tail + 1) % size;
>   
> @@ -500,6 +510,9 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
>   	if (unlikely(ret < 0)) {
>   		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
>   			  action[0], ret, status);
> +	} else if (unlikely(ret)) {
> +		CT_DEBUG_DRIVER("CT: send action %#x returned %d (%#x)\n",
> +				action[0], ret, ret);
>   	}
>   
>   	mutex_unlock(&guc->send_mutex);
> @@ -546,10 +559,12 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
>   	/* beware of buffer wrap case */
>   	if (unlikely(available < 0))
>   		available += size;
> +	CT_DEBUG_DRIVER("CT: available %d (%u:%u)\n", available, head, tail);
>   	GEM_BUG_ON(available < 0);
>   
>   	data[0] = cmds[head];
>   	head = (head + 1) % size;
> +	CT_DEBUG_DRIVER("CT: header %#x\n", data[0]);
>   
>   	/* message len with header */
>   	len = ct_header_get_len(data[0]) + 1;
> @@ -567,6 +582,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
>   		data[i] = cmds[head];
>   		head = (head + 1) % size;
>   	}
> +	CT_DEBUG_DRIVER("CT: received %*phn\n", 4*len, data);
>   
>   	desc->head = head * 4;
>   	return 0;
> @@ -592,12 +608,13 @@ static int guc_handle_response(struct intel_guc *guc, const u32 *msg)
>   		DRM_ERROR("CT: corrupted status %*phn\n", 4*len, msg);
>   		status = INTEL_GUC_STATUS_GENERIC_FAIL;
>   	}
> +	CT_DEBUG_DRIVER("CT: response fence %u status %#x\n", fence, status);
>   
>   	spin_lock_irqsave(&guc->ct.lock, flags);
>   	list_for_each_entry(req, &guc->ct.pending_requests, link) {
>   		if (req->fence != fence) {
> -			DRM_DEBUG_DRIVER("CT: request %u awaits response\n",
> -					 req->fence);
> +			CT_DEBUG_DRIVER("CT: request %u awaits response\n",
> +					req->fence);
>   			continue;
>   		}
>   		req->response_len = len - 3;
> @@ -623,6 +640,7 @@ static int guc_handle_request(struct intel_guc *guc, const u32 *msg)
>   
>   	GEM_BUG_ON(ct_header_is_response(header));
>   	/* Request message layout beyond header is request specific */
> +	CT_DEBUG_DRIVER("CT: request %#x\n", ct_header_get_action(header));
>   
>   	request = kmalloc(sizeof(*request) + 4*len, GFP_ATOMIC);
>   	if (unlikely(!request)) {
> @@ -662,6 +680,7 @@ static bool guc_process_incoming_requests(struct intel_guc *guc)
>   	header = request->data[0];
>   	action = ct_header_get_action(header);
>   	len = ct_header_get_len(header) + 1; /* also count header dw */
> +	CT_DEBUG_DRIVER("CT: processing request %*phn\n", 4*len, request->data);
>   
>   	switch (action) {
>   	case INTEL_GUC_ACTION_DEFAULT:
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 16/16] HAX Enable GuC loading & submission

[Michal Wajdeczko]

This is just for CI testing, *** DO NOT MERGE ***

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 14e2c2e..4a4c378 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 1,
+	.enable_guc_submission = 1,
 	.guc_log_level = -1,
 	.guc_firmware_path = NULL,
 	.huc_firmware_path = NULL,
@@ -221,12 +221,12 @@ MODULE_PARM_DESC(edp_vswing,
 module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, int, 0400);
 MODULE_PARM_DESC(enable_guc_loading,
 		"Enable GuC firmware loading "
-		"(-1=auto, 0=never [default], 1=if available, 2=required)");
+		"(-1=auto, 0=never, 1=if available [default], 2=required)");
 
 module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, int, 0400);
 MODULE_PARM_DESC(enable_guc_submission,
 		"Enable GuC submission "
-		"(-1=auto, 0=never [default], 1=if available, 2=required)");
+		"(-1=auto, 0=never, 1=if available [default], 2=required)");
 
 module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
 MODULE_PARM_DESC(guc_log_level,
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915/guc: Support for Guc responses and requests

[Patchwork]

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests
URL   : https://patchwork.freedesktop.org/series/28393/
State : failure

== Summary ==

Series 28393v1 drm/i915/guc: Support for Guc responses and requests
https://patchwork.freedesktop.org/api/1.0/series/28393/revisions/1/mbox/

Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> INCOMPLETE (fi-skl-6260u)
                pass       -> INCOMPLETE (fi-skl-6700k)
                pass       -> INCOMPLETE (fi-skl-6770hq)
                pass       -> INCOMPLETE (fi-skl-gvtdvm)
                pass       -> INCOMPLETE (fi-skl-x1585l)
                pass       -> INCOMPLETE (fi-bxt-j4205)
                pass       -> INCOMPLETE (fi-kbl-7500u)
                pass       -> INCOMPLETE (fi-kbl-7560u)
                pass       -> INCOMPLETE (fi-kbl-r)
Test gem_exec_parallel:
        Subgroup basic:
                fail       -> PASS       (fi-ilk-650) fdo#101735
Test gem_ringfill:
        Subgroup basic-default:
                skip       -> PASS       (fi-bsw-n3050) fdo#101915
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-n2820) fdo#101705
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-glk-2a)
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-glk-2a)
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (fi-glk-2a)

fdo#101735 https://bugs.freedesktop.org/show_bug.cgi?id=101735
fdo#101915 https://bugs.freedesktop.org/show_bug.cgi?id=101915
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:433s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:416s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:363s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:498s
fi-bxt-j4205     total:6    pass:5    dwarn:0   dfail:0   fail:0   skip:0  
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:526s
fi-byt-n2820     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:514s
fi-glk-2a        total:279  pass:257  dwarn:3   dfail:0   fail:0   skip:19  time:588s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:437s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:407s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:426s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:511s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:473s
fi-kbl-7500u     total:6    pass:5    dwarn:0   dfail:0   fail:0   skip:0  
fi-kbl-7560u     total:6    pass:5    dwarn:0   dfail:0   fail:0   skip:0  
fi-kbl-r         total:6    pass:5    dwarn:0   dfail:0   fail:0   skip:0  
fi-pnv-d510      total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  time:579s
fi-skl-6260u     total:6    pass:5    dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-6700k     total:6    pass:5    dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-6770hq    total:6    pass:5    dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-gvtdvm    total:6    pass:5    dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-x1585l    total:6    pass:5    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:552s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:410s

96c5eac5f202920356b34ef2da8a785bb8b4f320 drm-tip: 2017y-08m-07d-10h-55m-52s UTC integration manifest
19a746ca7b28 HAX Enable GuC loading & submission
4d5d579265bd drm/i915/guc: Trace messages from CT while in debug
abccbeafa1ea drm/i915/guc: Enable GuC interrupts when using CT
aff2520a47de drm/i915/guc: Handle default action received over CT
fe52f747c1d8 drm/i915/guc: Prepare to process incoming requests from CT
afafb7904900 drm/i915/guc: Implement response handling in send_ct()
244c0164ba7a drm/i915/guc: Use better name for helper wait function
da67c8b8a7e0 drm/i915/guc: Prepare to handle messages from CT RECV buffer
35b9fa7838f8 drm/i915/guc: Update CT message header definition
96158b0c4fca drm/i915/guc: Create a GuC receive function
e3e4f8f1464c drm/i915/guc: Move flushing the GuC logs outside notification handler
f9b68cdc9346 drm/i915/guc: Move Guc notification handling to separate function
4a294751a3dd drm/i915/guc: Implement response handling in send_mmio()
d76483eeffac drm/i915/guc: Add send_and_receive() helper function
f7fe86291307 drm/i915/guc: Prepare send() function to accept bigger response
d9be5f178d67 drm/i915/guc: Add support for data reporting in GuC responses

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5335/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 15/16] drm/i915/guc: Trace messages from CT while in debug

[Michal Wajdeczko]

During debug we may want to investigate all communication
from the Guc. Add proper tracing macros in debug config.

v2: convert remaining DRM_DEBUG into new CT_DEBUG (Michal)
v3: use dedicated Kconfig (Daniele)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Kconfig.debug  | 12 +++++++++++
 drivers/gpu/drm/i915/intel_guc_ct.c | 41 +++++++++++++++++++++++++++----------
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index 78c5c04..0c20b75 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -73,6 +73,18 @@ config DRM_I915_SW_FENCE_CHECK_DAG
 
           If in doubt, say "N".
 
+config DRM_I915_DEBUG_GUC
+        bool "Enable additional driver debugging for GuC"
+        depends on DRM_I915
+        default n
+        help
+          Choose this option to turn on extra driver debugging that may affect
+          performance but will help resolve GuC related issues.
+
+          Recommended for driver developers only.
+
+          If in doubt, say "N".
+
 config DRM_I915_SELFTEST
 	bool "Enable selftests upon driver load"
 	depends on DRM_I915
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index e6912be..28d7228 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,12 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+#ifdef CONFIG_DRM_I915_DEBUG_GUC
+#define CT_DEBUG_DRIVER(...)	DRM_DEBUG_DRIVER(__VA_ARGS__)
+#else
+#define CT_DEBUG_DRIVER(...)
+#endif
+
 struct ct_request {
 	struct list_head link;
 	u32 fence;
@@ -69,8 +75,8 @@ static inline const char *guc_ct_buffer_type_to_str(u32 type)
 static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
 				    u32 cmds_addr, u32 size, u32 owner)
 {
-	DRM_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
-			 desc, cmds_addr, size, owner);
+	CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
+			desc, cmds_addr, size, owner);
 	memset(desc, 0, sizeof(*desc));
 	desc->addr = cmds_addr;
 	desc->size = size;
@@ -79,8 +85,8 @@ static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
 
 static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)
 {
-	DRM_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
-			 desc, desc->head, desc->tail);
+	CT_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
+			desc, desc->head, desc->tail);
 	desc->head = 0;
 	desc->tail = 0;
 	desc->is_in_error = 0;
@@ -176,7 +182,7 @@ static int ctch_init(struct intel_guc *guc,
 		err = PTR_ERR(blob);
 		goto err_vma;
 	}
-	DRM_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
+	CT_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
 
 	/* store pointers to desc and cmds */
 	for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
@@ -190,8 +196,8 @@ static int ctch_init(struct intel_guc *guc,
 err_vma:
 	i915_vma_unpin_and_release(&ctch->vma);
 err_out:
-	DRM_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
-			 ctch->owner, err);
+	CT_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
+			ctch->owner, err);
 	return err;
 }
 
@@ -211,8 +217,8 @@ static int ctch_open(struct intel_guc *guc,
 	int err;
 	int i;
 
-	DRM_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
-			 ctch->owner, yesno(ctch_is_open(ctch)));
+	CT_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
+			ctch->owner, yesno(ctch_is_open(ctch)));
 
 	if (!ctch->vma) {
 		err = ctch_init(guc, ctch);
@@ -325,6 +331,10 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 		 (send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 		 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
+	CT_DEBUG_DRIVER("CT: writing %*phn %*phn %*phn\n",
+			4, &header, 4, &fence,
+			4*(len - 1), &action[1]);
+
 	cmds[tail] = header;
 	tail = (tail + 1) % size;
 
@@ -500,6 +510,9 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
 			  action[0], ret, status);
+	} else if (unlikely(ret)) {
+		CT_DEBUG_DRIVER("CT: send action %#x returned %d (%#x)\n",
+				action[0], ret, ret);
 	}
 
 	mutex_unlock(&guc->send_mutex);
@@ -546,10 +559,12 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 	/* beware of buffer wrap case */
 	if (unlikely(available < 0))
 		available += size;
+	CT_DEBUG_DRIVER("CT: available %d (%u:%u)\n", available, head, tail);
 	GEM_BUG_ON(available < 0);
 
 	data[0] = cmds[head];
 	head = (head + 1) % size;
+	CT_DEBUG_DRIVER("CT: header %#x\n", data[0]);
 
 	/* message len with header */
 	len = ct_header_get_len(data[0]) + 1;
@@ -567,6 +582,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 		data[i] = cmds[head];
 		head = (head + 1) % size;
 	}
+	CT_DEBUG_DRIVER("CT: received %*phn\n", 4*len, data);
 
 	desc->head = head * 4;
 	return 0;
@@ -592,12 +608,13 @@ static int guc_handle_response(struct intel_guc *guc, const u32 *msg)
 		DRM_ERROR("CT: corrupted status %*phn\n", 4*len, msg);
 		status = INTEL_GUC_STATUS_GENERIC_FAIL;
 	}
+	CT_DEBUG_DRIVER("CT: response fence %u status %#x\n", fence, status);
 
 	spin_lock_irqsave(&guc->ct.lock, flags);
 	list_for_each_entry(req, &guc->ct.pending_requests, link) {
 		if (req->fence != fence) {
-			DRM_DEBUG_DRIVER("CT: request %u awaits response\n",
-					 req->fence);
+			CT_DEBUG_DRIVER("CT: request %u awaits response\n",
+					req->fence);
 			continue;
 		}
 		req->response_len = len - 3;
@@ -623,6 +640,7 @@ static int guc_handle_request(struct intel_guc *guc, const u32 *msg)
 
 	GEM_BUG_ON(ct_header_is_response(header));
 	/* Request message layout beyond header is request specific */
+	CT_DEBUG_DRIVER("CT: request %#x\n", ct_header_get_action(header));
 
 	request = kmalloc(sizeof(*request) + 4*len, GFP_ATOMIC);
 	if (unlikely(!request)) {
@@ -662,6 +680,7 @@ static bool guc_process_incoming_requests(struct intel_guc *guc)
 	header = request->data[0];
 	action = ct_header_get_action(header);
 	len = ct_header_get_len(header) + 1; /* also count header dw */
+	CT_DEBUG_DRIVER("CT: processing request %*phn\n", 4*len, request->data);
 
 	switch (action) {
 	case INTEL_GUC_ACTION_DEFAULT:
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 15/16] drm/i915/guc: Trace messages from CT while in debug

[Daniele Ceraolo Spurio]

On 08/08/17 05:30, Michal Wajdeczko wrote:
> During debug we may want to investigate all communication
> from the Guc. Add proper tracing macros in debug config.
> 
> v2: convert remaining DRM_DEBUG into new CT_DEBUG (Michal)
> v3: use dedicated Kconfig (Daniele)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

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

-Daniele
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 14/16] drm/i915/guc: Enable GuC interrupts when using CT

[Michal Wajdeczko]

We will need them in G2H communication to properly handle
responses and requests from the Guc.

v2: keep irq enabled while disabling GuC logging (Oscar)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c |  2 +-
 drivers/gpu/drm/i915/intel_guc_log.c       | 10 ++++++----
 drivers/gpu/drm/i915/intel_uc.c            |  2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 48a1e93..509497e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1328,7 +1328,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
 	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return 0;
 
-	if (i915.guc_log_level >= 0)
+	if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
 		gen9_enable_guc_interrupts(dev_priv);
 
 	ctx = dev_priv->kernel_context;
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index acd9a3f..64fb879 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -505,8 +505,9 @@ static void guc_flush_logs(struct intel_guc *guc)
 	if (!i915.enable_guc_submission || (i915.guc_log_level < 0))
 		return;
 
-	/* First disable the interrupts, will be renabled afterwards */
-	gen9_disable_guc_interrupts(dev_priv);
+	/* GuC logging maybe the only user of Guc2Host interrupts */
+	if (!HAS_GUC_CT(dev_priv))
+		gen9_disable_guc_interrupts(dev_priv);
 
 	/* Before initiating the forceful flush, wait for any pending/ongoing
 	 * flush to complete otherwise forceful flush may not actually happen.
@@ -663,8 +664,9 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
 		return;
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
-	/* GuC logging is currently the only user of Guc2Host interrupts */
-	gen9_disable_guc_interrupts(dev_priv);
+	/* GuC logging maybe the only user of Guc2Host interrupts */
+	if (!HAS_GUC_CT(dev_priv))
+		gen9_disable_guc_interrupts(dev_priv);
 	guc_log_runtime_destroy(&dev_priv->guc);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 }
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 774d740..0209ad0 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -395,7 +395,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
 	intel_guc_auth_huc(dev_priv);
 	if (i915.enable_guc_submission) {
-		if (i915.guc_log_level >= 0)
+		if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
 			gen9_enable_guc_interrupts(dev_priv);
 
 		ret = i915_guc_submission_enable(dev_priv);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 14/16] drm/i915/guc: Enable GuC interrupts when using CT

[Oscar Mateo]

On 08/09/2017 09:24 AM, Michal Wajdeczko wrote:
> We will need them in G2H communication to properly handle
> responses and requests from the Guc.
>
> v2: keep irq enabled while disabling GuC logging (Oscar)
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c |  2 +-
>   drivers/gpu/drm/i915/intel_guc_log.c       | 10 ++++++----
>   drivers/gpu/drm/i915/intel_uc.c            |  2 +-
>   3 files changed, 8 insertions(+), 6 deletions(-)

Acked-by: Oscar Mateo <oscar.mateo@intel.com>

> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 48a1e93..509497e 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1328,7 +1328,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
>   	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>   		return 0;
>   
> -	if (i915.guc_log_level >= 0)
> +	if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
>   		gen9_enable_guc_interrupts(dev_priv);
>   
>   	ctx = dev_priv->kernel_context;
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index acd9a3f..64fb879 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -505,8 +505,9 @@ static void guc_flush_logs(struct intel_guc *guc)
>   	if (!i915.enable_guc_submission || (i915.guc_log_level < 0))
>   		return;
>   
> -	/* First disable the interrupts, will be renabled afterwards */
> -	gen9_disable_guc_interrupts(dev_priv);
> +	/* GuC logging maybe the only user of Guc2Host interrupts */
> +	if (!HAS_GUC_CT(dev_priv))
> +		gen9_disable_guc_interrupts(dev_priv);
>   
>   	/* Before initiating the forceful flush, wait for any pending/ongoing
>   	 * flush to complete otherwise forceful flush may not actually happen.
> @@ -663,8 +664,9 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
>   		return;
>   
>   	mutex_lock(&dev_priv->drm.struct_mutex);
> -	/* GuC logging is currently the only user of Guc2Host interrupts */
> -	gen9_disable_guc_interrupts(dev_priv);
> +	/* GuC logging maybe the only user of Guc2Host interrupts */
> +	if (!HAS_GUC_CT(dev_priv))
> +		gen9_disable_guc_interrupts(dev_priv);
>   	guc_log_runtime_destroy(&dev_priv->guc);
>   	mutex_unlock(&dev_priv->drm.struct_mutex);
>   }
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 774d740..0209ad0 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -395,7 +395,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>   
>   	intel_guc_auth_huc(dev_priv);
>   	if (i915.enable_guc_submission) {
> -		if (i915.guc_log_level >= 0)
> +		if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
>   			gen9_enable_guc_interrupts(dev_priv);
>   
>   		ret = i915_guc_submission_enable(dev_priv);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx