On Fri, 15 Jul 2022 14:13:13 -0700, Zhanjun Dong wrote:
>
> -static int wait_for_ct_request_update(struct ct_request *req, u32 *status)
> +static int wait_for_ct_request_update(struct intel_guc_ct *ct, struct ct_request *req, u32 *status)
> {
> int err;
> + bool ct_enabled;
>
> /*
> * Fast commands should complete in less than 10us, so sample quickly
> @@ -481,12 +483,15 @@ static int wait_for_ct_request_update(struct ct_request *req, u32 *status)
> #define GUC_CTB_RESPONSE_TIMEOUT_SHORT_MS 10
> #define GUC_CTB_RESPONSE_TIMEOUT_LONG_MS 1000
> #define done \
> - (FIELD_GET(GUC_HXG_MSG_0_ORIGIN, READ_ONCE(req->status)) == \
> + (!(ct_enabled = intel_guc_ct_enabled(ct)) || \
> + FIELD_GET(GUC_HXG_MSG_0_ORIGIN, READ_ONCE(req->status)) == \
> GUC_HXG_ORIGIN_GUC)
> err = wait_for_us(done, GUC_CTB_RESPONSE_TIMEOUT_SHORT_MS);
> if (err)
> err = wait_for(done, GUC_CTB_RESPONSE_TIMEOUT_LONG_MS);
> #undef done
> + if (!ct_enabled)
> + err = -ENODEV;
Good, -ENODEV seems to be the correct return value in this case.
>
> *status = req->status;
> return err;
> @@ -703,11 +708,18 @@ static int ct_send(struct intel_guc_ct *ct,
>
> intel_guc_notify(ct_to_guc(ct));
>
> - err = wait_for_ct_request_update(&request, status);
> + err = wait_for_ct_request_update(ct, &request, status);
> g2h_release_space(ct, GUC_CTB_HXG_MSG_MAX_LEN);
> if (unlikely(err)) {
> - CT_ERROR(ct, "No response for request %#x (fence %u)\n",
> - action[0], request.fence);
> + if (err == -ENODEV)
> + /* wait_for_ct_request_update returns -ENODEV on reset/suspend in progress.
> + * In this case, output is debug rather than error info
> + */
> + CT_DEBUG(ct, "Request %#x (fence %u) cancelled as CTB is disabled\n",
> + action[0], request.fence);
> + else
> + CT_ERROR(ct, "No response for request %#x (fence %u)\n",
> + action[0], request.fence);
A nit but I would probably prefer to move the CT_DEBUG() inside
wait_for_ct_request_update() (so we only keep 'if (err != -ENODEV)' checks
here) though it would mean adding the action argument also to
wait_for_ct_request_update().
In any case, since we have discussed this patch ad nauseam previously, this
is now:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
