Re: [PATCH 1/2] drm/i915/guc: Simplify code by keeping kmap of guc_client object

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 02/15/2016 06:39 AM, Dave Gordon wrote:
On 12/02/16 13:03, Tvrtko Ursulin wrote:
>
> On 11/02/16 23:09, yu.dai@xxxxxxxxx wrote:
>> From: Alex Dai <yu.dai@xxxxxxxxx>
>>
>> GuC client object is always pinned during its life cycle. We cache
>> the kmap of its first page, which includes guc_process_desc and
>> doorbell. By doing so, we can simplify the code where we read from
>> this page to get where GuC is progressing on work queue; and the
>> code where driver program doorbell to send work queue item to GuC.

[snip]

>>
>> -    /* Finally, update the cached copy of the GuC's WQ head */
>> -    gc->wq_head = desc->head;
>
> Did you mean to remove the above?

I wondered that too at first, but the answer is "yes" -- see below.

>>
>> +    client->client_base = kmap(i915_gem_object_get_dirty_page(obj, 0));
>
> Was this another bug, that the page/object wasn't dirtied before?

It wouldn't have made any difference; the object is pinned in the GTT
forever, so it can't be swapped out or reclaimed.

>> -    uint32_t wq_head;
>
> Hm ok I don't get why kmap caching means removing this as well?

'wq_head' was an optimisation so that we could check whether there was
known to be space in the workqueue without kmapping and reading the
process descriptor. Now that the client (which includes the process
descriptor) is permanently mapped, there's no advantage to caching the
head; we might just as well read the current value from 'desc->head'
each time.

> Btw I don't see patch 2/2 ?
>

My bad, there is no 2/2. Thanks Dave for answering the questions. I have no more comments. :-)

Thanks,
Alex
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux