On 25.05.2021 20:15, Matthew Brost wrote:
> On Thu, May 06, 2021 at 12:13:32PM -0700, Matthew Brost wrote:
>> From: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
>>
>> In irq handler try to receive just single G2H message, let other
>> messages to be received from tasklet.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
>> Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 67 ++++++++++++++++-------
>> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 3 +
>> 2 files changed, 50 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> index cb58fa7f970c..d630ec32decf 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> @@ -81,6 +81,7 @@ enum { CTB_SEND = 0, CTB_RECV = 1 };
>>
>> enum { CTB_OWNER_HOST = 0 };
>>
>> +static void ct_receive_tasklet_func(unsigned long data);
>> static void ct_incoming_request_worker_func(struct work_struct *w);
>>
>> /**
>> @@ -95,6 +96,7 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
>> INIT_LIST_HEAD(&ct->requests.pending);
>> INIT_LIST_HEAD(&ct->requests.incoming);
>> INIT_WORK(&ct->requests.worker, ct_incoming_request_worker_func);
>> + tasklet_init(&ct->receive_tasklet, ct_receive_tasklet_func, (unsigned long)ct);
>
> This function is deprecated. tasklet_setup should be used.
> I can fix this up when I post the CTB patches.
I didn't notice that, but yes, passing struct to callback will be
cleaner, so please fix it
Thanks,
Michal
>
>> }
>>
>> static inline const char *guc_ct_buffer_type_to_str(u32 type)
>> @@ -244,6 +246,7 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct)
>> {
>> GEM_BUG_ON(ct->enabled);
>>
>> + tasklet_kill(&ct->receive_tasklet);
>> i915_vma_unpin_and_release(&ct->vma, I915_VMA_RELEASE_MAP);
>> memset(ct, 0, sizeof(*ct));
>> }
>> @@ -629,7 +632,7 @@ static int ct_read(struct intel_guc_ct *ct, u32 *data)
>> CT_DEBUG(ct, "received %*ph\n", 4 * len, data);
>>
>> desc->head = head * 4;
>> - return 0;
>> + return available - len;
>>
>> corrupted:
>> CT_ERROR(ct, "Corrupted descriptor addr=%#x head=%u tail=%u size=%u\n",
>> @@ -665,10 +668,10 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
>> u32 status;
>> u32 datalen;
>> struct ct_request *req;
>> + unsigned long flags;
>> bool found = false;
>>
>> GEM_BUG_ON(!ct_header_is_response(header));
>> - GEM_BUG_ON(!in_irq());
>>
>> /* Response payload shall at least include fence and status */
>> if (unlikely(len < 2)) {
>> @@ -688,7 +691,7 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
>>
>> CT_DEBUG(ct, "response fence %u status %#x\n", fence, status);
>>
>> - spin_lock(&ct->requests.lock);
>> + spin_lock_irqsave(&ct->requests.lock, flags);
>> list_for_each_entry(req, &ct->requests.pending, link) {
>> if (unlikely(fence != req->fence)) {
>> CT_DEBUG(ct, "request %u awaits response\n",
>> @@ -707,7 +710,7 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
>> found = true;
>> break;
>> }
>> - spin_unlock(&ct->requests.lock);
>> + spin_unlock_irqrestore(&ct->requests.lock, flags);
>>
>> if (!found)
>> CT_ERROR(ct, "Unsolicited response %*ph\n", msgsize, msg);
>> @@ -821,31 +824,55 @@ static int ct_handle_request(struct intel_guc_ct *ct, const u32 *msg)
>> return 0;
>> }
>>
>> +static int ct_receive(struct intel_guc_ct *ct)
>> +{
>> + u32 msg[GUC_CT_MSG_LEN_MASK + 1]; /* one extra dw for the header */
>> + unsigned long flags;
>> + int ret;
>> +
>> + spin_lock_irqsave(&ct->ctbs.recv.lock, flags);
>> + ret = ct_read(ct, msg);
>> + spin_unlock_irqrestore(&ct->ctbs.recv.lock, flags);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (ct_header_is_response(msg[0]))
>> + ct_handle_response(ct, msg);
>> + else
>> + ct_handle_request(ct, msg);
>> +
>> + return ret;
>> +}
>> +
>> +static void ct_try_receive_message(struct intel_guc_ct *ct)
>> +{
>> + int ret;
>> +
>> + if (GEM_WARN_ON(!ct->enabled))
>> + return;
>> +
>> + ret = ct_receive(ct);
>> + if (ret > 0)
>> + tasklet_hi_schedule(&ct->receive_tasklet);
>> +}
>> +
>> +static void ct_receive_tasklet_func(unsigned long data)
>> +{
>
> As mentioned above tasklet_init is deprecated. The callback now accepts
> the tasklet structure and ct can be looked up via container_of macro.
>
> Everything else looks good.
>
> With that and changing to the new tasklet API:
> Reviewed-by: Matthew Brost <matthew.brost@xxxxxxxxx>
>
>> + struct intel_guc_ct *ct = (struct intel_guc_ct *)data;
>> +
>> + ct_try_receive_message(ct);
>> +}
>> +
>> /*
>> * When we're communicating with the GuC over CT, GuC uses events
>> * to notify us about new messages being posted on the RECV buffer.
>> */
>> void intel_guc_ct_event_handler(struct intel_guc_ct *ct)
>> {
>> - u32 msg[GUC_CT_MSG_LEN_MASK + 1]; /* one extra dw for the header */
>> - unsigned long flags;
>> - int err = 0;
>> -
>> if (unlikely(!ct->enabled)) {
>> WARN(1, "Unexpected GuC event received while CT disabled!\n");
>> return;
>> }
>>
>> - do {
>> - spin_lock_irqsave(&ct->ctbs.recv.lock, flags);
>> - err = ct_read(ct, msg);
>> - spin_unlock_irqrestore(&ct->ctbs.recv.lock, flags);
>> - if (err)
>> - break;
>> -
>> - if (ct_header_is_response(msg[0]))
>> - err = ct_handle_response(ct, msg);
>> - else
>> - err = ct_handle_request(ct, msg);
>> - } while (!err);
>> + ct_try_receive_message(ct);
>> }
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> index bc52dc479a14..cb222f202301 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> @@ -6,6 +6,7 @@
>> #ifndef _INTEL_GUC_CT_H_
>> #define _INTEL_GUC_CT_H_
>>
>> +#include <linux/interrupt.h>
>> #include <linux/spinlock.h>
>> #include <linux/workqueue.h>
>>
>> @@ -55,6 +56,8 @@ struct intel_guc_ct {
>> struct intel_guc_ct_buffer recv;
>> } ctbs;
>>
>> + struct tasklet_struct receive_tasklet;
>> +
>> struct {
>> u32 last_fence; /* last fence used to send request */
>>
>> --
>> 2.28.0
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
