[Intel-gfx] [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT

[Michel Thierry]

On 3/27/2018 11:25 AM, Daniele Ceraolo Spurio wrote:
> 
> 
> On 26/03/18 12:48, 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.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Oscar Mateo <oscar.mateo at intel.com>
>> Reviewed-by: Michel Thierry <michel.thierry at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_guc.c    | 5 +++++
>>   drivers/gpu/drm/i915/intel_guc.h    | 1 +
>>   drivers/gpu/drm/i915/intel_guc_ct.c | 9 +++++++++
>>   3 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c 
>> b/drivers/gpu/drm/i915/intel_guc.c
>> index 118db81..b6d2778 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -416,6 +416,11 @@ 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)
>> +{
>>       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;
> 
> Don't we need to remove the non-enabled messages here? i.e. like we do 
> for the mmio receive:
> 
>      msg = *payload & guc->msg_enabled_mask;
> 
> otherwise I think we'll try to process log interrupts even if the relay 
> is not enabled.

In Daniele's specific example, I still think guc_log_enable_flush_events 
should have been called (and don't do anything inside 
handle_flush_event, the goal was to not serve it).

But it's true that we should filter any unexpected incoming request (& 
guc->msg_enabled_mask), and any new user should be calling 
intel_guc_enable_msg during its setup.

Thanks,
> 
> Daniele
> 
>> +        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;
>>