diff mbox

[v7,10/12] drm/i915/guc: Handle default action received over CT

Message ID 20180327214124.70680-1-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Wajdeczko March 27, 2018, 9:41 p.m. UTC
When running on platform with CTB based GuC communication enabled,
GuC to Host event data will be delivered as CT request message.
However, content of the data[1] of this CT message follows format
of the scratch register used in MMIO based communication, so some
code reuse is still possible.

v2:  filter disabled messages (Daniele)

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

Comments

Michel Thierry March 27, 2018, 10:49 p.m. UTC | #1
On 3/27/2018 2:41 PM, Michal Wajdeczko wrote:
> When running on platform with CTB based GuC communication enabled,
> GuC to Host event data will be delivered as CT request message.
> However, content of the data[1] of this CT message follows format
> of the scratch register used in MMIO based communication, so some
> code reuse is still possible.
> 
> v2:  filter disabled messages (Daniele)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Reviewed-by: Michel Thierry <michel.thierry@intel.com> #1
   ^ still applies for v2, but I would wait for Daniele's blessing

> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc.c    | 8 ++++++++
>   drivers/gpu/drm/i915/intel_guc.h    | 1 +
>   drivers/gpu/drm/i915/intel_guc_ct.c | 9 +++++++++
>   3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 118db81..d77dde7 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -416,6 +416,14 @@ void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc)
>   	I915_WRITE(SOFT_SCRATCH(15), val & ~msg);
>   	spin_unlock(&guc->irq_lock);
>   
> +	intel_guc_to_host_process_recv_msg(guc, msg);
> +}
> +
> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg)
> +{

(as I said before, this will need to change later on to receive more 
than jut data[1]/payload anyway)

> +	/* Make sure to handle only enabled messages */
> +	msg &= guc->msg_enabled_mask;
> +
>   	if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
>   		   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
>   		intel_guc_log_handle_flush_event(&guc->log);
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 6dc109a..f1265e1 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -163,6 +163,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
>   void intel_guc_to_host_event_handler(struct intel_guc *guc);
>   void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
>   void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc);
> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg);
>   int intel_guc_sample_forcewake(struct intel_guc *guc);
>   int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
>   int intel_guc_suspend(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index aa810ad..e837084 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -694,8 +694,17 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
>   static void ct_process_request(struct intel_guc_ct *ct,
>   			       u32 action, u32 len, const u32 *payload)
>   {
> +	struct intel_guc *guc = ct_to_guc(ct);
> +
>   	switch (action) {
> +	case INTEL_GUC_ACTION_DEFAULT:
> +		if (unlikely(len < 1))
> +			goto fail_unexpected;
> +		intel_guc_to_host_process_recv_msg(guc, *payload);
> +		break;
> +
>   	default:
> +fail_unexpected:
>   		DRM_ERROR("CT: unexpected request %x %*phn\n",
>   			  action, 4 * len, payload);
>   		break;
>
Daniele Ceraolo Spurio March 28, 2018, 4:05 p.m. UTC | #2
On 3/27/2018 3:49 PM, Michel Thierry wrote:
> On 3/27/2018 2:41 PM, Michal Wajdeczko wrote:
>> When running on platform with CTB based GuC communication enabled,
>> GuC to Host event data will be delivered as CT request message.
>> However, content of the data[1] of this CT message follows format
>> of the scratch register used in MMIO based communication, so some
>> code reuse is still possible.
>>
>> v2:  filter disabled messages (Daniele)
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Reviewed-by: Michel Thierry <michel.thierry@intel.com> #1
>    ^ still applies for v2, but I would wait for Daniele's blessing
> 

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

A minor comment below.

Daniele

<snip>

>> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
>> @@ -694,8 +694,17 @@ static int ct_handle_response(struct intel_guc_ct 
>> *ct, const u32 *msg)
>>   static void ct_process_request(struct intel_guc_ct *ct,
>>                      u32 action, u32 len, const u32 *payload)
>>   {
>> +    struct intel_guc *guc = ct_to_guc(ct);
>> +
>>       switch (action) {
>> +    case INTEL_GUC_ACTION_DEFAULT:
>> +        if (unlikely(len < 1))
>> +            goto fail_unexpected;
>> +        intel_guc_to_host_process_recv_msg(guc, *payload);
>> +        break;
>> +
>>       default:
>> +fail_unexpected:
>>           DRM_ERROR("CT: unexpected request %x %*phn\n",
>>                 action, 4 * len, payload);

if we end up here from the goto with len == 0 this print will probably 
not output a fully clear message (error with a correct action number and 
no data). Not a blocker because we can still infer it was a length issue.

>>           break;
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 118db81..d77dde7 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -416,6 +416,14 @@  void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc)
 	I915_WRITE(SOFT_SCRATCH(15), val & ~msg);
 	spin_unlock(&guc->irq_lock);
 
+	intel_guc_to_host_process_recv_msg(guc, msg);
+}
+
+void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg)
+{
+	/* Make sure to handle only enabled messages */
+	msg &= guc->msg_enabled_mask;
+
 	if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
 		   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
 		intel_guc_log_handle_flush_event(&guc->log);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 6dc109a..f1265e1 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -163,6 +163,7 @@  int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 void intel_guc_to_host_event_handler(struct intel_guc *guc);
 void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
 void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc);
+void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 int intel_guc_suspend(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index aa810ad..e837084 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -694,8 +694,17 @@  static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 static void ct_process_request(struct intel_guc_ct *ct,
 			       u32 action, u32 len, const u32 *payload)
 {
+	struct intel_guc *guc = ct_to_guc(ct);
+
 	switch (action) {
+	case INTEL_GUC_ACTION_DEFAULT:
+		if (unlikely(len < 1))
+			goto fail_unexpected;
+		intel_guc_to_host_process_recv_msg(guc, *payload);
+		break;
+
 	default:
+fail_unexpected:
 		DRM_ERROR("CT: unexpected request %x %*phn\n",
 			  action, 4 * len, payload);
 		break;