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

[Michel Thierry <michel.thierry@xxxxxxxxx>]

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@xxxxxxxxx>
> > Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> > Reviewed-by: Michel Thierry <michel.thierry@xxxxxxxxx>
> > ---
> >   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;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx