On 10/02/16 20:31, Yu Dai wrote:
On 02/10/2016 09:30 AM, Tvrtko Ursulin wrote:
Hi,
On 10/02/16 00:05, yu.dai@xxxxxxxxx wrote:
> From: Alex Dai <yu.dai@xxxxxxxxx>
>
> The cached work queue header pointer is set to last byte of work
> queue buffer. It will make sure the whole work queue buffer is
> available after coming back from reset or init.
[snip]
> @@ -730,6 +728,8 @@ static struct i915_guc_client
*guc_client_alloc(struct drm_device *dev,
> client->client_obj = obj;
> client->wq_offset = GUC_DB_SIZE;
> client->wq_size = GUC_WQ_SIZE;
> + client->wq_head = GUC_WQ_SIZE - 1;
> + client->wq_tail = 0;
>
> client->doorbell_offset = select_doorbell_cacheline(guc);
This one I can't really figure out without I suppose knowing more about
the code design. How come it was OK when it was zero (apart after reset)?
The value is otherwise only updated from the GuC shared page and a
driver does not appear to modify it. Perhaps just a better commit
message to explain things?
The way this kernel CIRC_xx works is it leaves one byte free and treat
head == tail case as empty. So, there won't be a problem if this head
happens to be 0. If it comes with some random number between [1,
sizeof(WQ item)], there will be a SW dead looping in driver.
Actually, I think it works like this:
if both wq_tail and wq_head (which is a CACHE of the GuC's WQ tail) are
initialised to 0, the driver thinks the WQ is empty, which is correct at
startup but not necessarily after a GPU reset.
By initialising them as above, the driver will at first think the WQ is
FULL, and will then refresh the actual value from the GuC's process
descriptor. This should ensure that it doesn't assume more WQ space than
the GuC thinks there is.
It's very much the same sort of issue as (re)initialising the CSB
pointers after a reset, because the CS h/w (or in this case the GuC
firmware) has internally set them to a new value and the driver needs to
get in sync.
Anyway all of this is superseded by Alex's later patch that leaves the
client permanently kmapped, 'cos that gets rid of wq_head entirely :)
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
