Re: [Mesa-dev] [PATCH v5] etnaviv: fix resource usage tracking across different pipe_context's
On 3/17/19 10:48 AM, Christian Gmeiner wrote: > Am Sa., 16. März 2019 um 20:55 Uhr schrieb Marek Vasut : >> >> On 2/22/19 10:30 AM, Boris Brezillon wrote: >>> On Thu, 21 Feb 2019 23:29:53 +0100 >>> Boris Brezillon wrote: >>> Christian, Marek, On Wed, 30 Jan 2019 05:28:14 +0100 Marek Vasut wrote: > From: Christian Gmeiner > > A pipe_resource can be shared by all the pipe_context's hanging off the > same pipe_screen. We seem to be impacted by the problem you're fixing here, but, while this patch definitely make things much better, the problem does not seem to be entirely fixed (at least in our case). A bit more context: we have Qt App using QtWebEngine to render html content. When we call QtWebEngine::initialize(), which as for effect to allow shared GL contexts, we sometimes notice that part of the web page is mis-rendered. That does not happen when we omit the QtWebEngine::initialize() call. As said above, this patch make those rendering issues less likely to happen, but we still have the problem from time to time. So I thought I'd share my guesses about what could cause these issues before debugging it further. First thing I noticed: I couldn't reproduce the problem with [1] applied (+ a tiny change forcing CPU-based tiling no matter the size of the region to tile/untile). So, my guess is that it's related to how we handle GPU-based tiling/untiling. Also noticed that we're testing the rsc->status here [2] without the screen->lock held, and there might be a race with another thread calling resource_used(). We might also lack a resource_read(ctx, >base) here [3]. But even after fixing those problems, the rendering issues are not gone. >>> >>> I tested again with the following diff applied on top of your patch, and >>> the remaining rendering issues we had seem to be gone (don't know what I >>> messed up in my previous tests :-/). >>> >>> --->8--- >>> diff --git a/src/gallium/drivers/etnaviv/etnaviv_rs.c >>> b/src/gallium/drivers/etnaviv/etnaviv_rs.c >>> index fc4f65dbeee1..b8c8b96a6f72 100644 >>> --- a/src/gallium/drivers/etnaviv/etnaviv_rs.c >>> +++ b/src/gallium/drivers/etnaviv/etnaviv_rs.c >>> @@ -729,6 +729,7 @@ etna_try_rs_blit(struct pipe_context *pctx, >>> >>> etna_submit_rs_state(ctx, _to_screen); >>> resource_written(ctx, >base); >>> + resource_read(ctx, >base); >>> dst->seqno++; >>> dst->levels[blit_info->dst.level].ts_valid = false; >>> ctx->dirty |= ETNA_DIRTY_DERIVE_TS; >> While the V6 of this patch [1] is now in mesa upstream, this hunk is >> missing from the V6 . Any special reason for that or is this just an >> omission ? >> > > This and an other change was done in separates commits - see: > > https://cgit.freedesktop.org/mesa/mesa/commit/?id=7244e768040859126a5b74023f587beaa8bef81c > https://cgit.freedesktop.org/mesa/mesa/commit/?id=c56e73449698c31fe72b99a95b7e4ecdb2985b73 > > So everything went in. Ah, thanks :) Do you plan to backport those to mesa 18.3.5 too ? -- Best regards, Marek Vasut ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v5] etnaviv: fix resource usage tracking across different pipe_context's
Am Sa., 16. März 2019 um 20:55 Uhr schrieb Marek Vasut : > > On 2/22/19 10:30 AM, Boris Brezillon wrote: > > On Thu, 21 Feb 2019 23:29:53 +0100 > > Boris Brezillon wrote: > > > >> Christian, Marek, > >> > >> On Wed, 30 Jan 2019 05:28:14 +0100 > >> Marek Vasut wrote: > >> > >>> From: Christian Gmeiner > >>> > >>> A pipe_resource can be shared by all the pipe_context's hanging off the > >>> same pipe_screen. > >> > >> We seem to be impacted by the problem you're fixing here, but, while > >> this patch definitely make things much better, the problem does not > >> seem to be entirely fixed (at least in our case). > >> > >> A bit more context: we have Qt App using QtWebEngine to render html > >> content. When we call QtWebEngine::initialize(), which as for effect > >> to allow shared GL contexts, we sometimes notice that part of the web > >> page is mis-rendered. That does not happen when we omit the > >> QtWebEngine::initialize() call. > >> As said above, this patch make those rendering issues less likely to > >> happen, but we still have the problem from time to time. So I thought > >> I'd share my guesses about what could cause these issues before > >> debugging it further. > >> > >> First thing I noticed: I couldn't reproduce the problem with [1] > >> applied (+ a tiny change forcing CPU-based tiling no matter the size of > >> the region to tile/untile). So, my guess is that it's related to how we > >> handle GPU-based tiling/untiling. > >> Also noticed that we're testing the rsc->status here [2] without the > >> screen->lock held, and there might be a race with another thread calling > >> resource_used(). We might also lack a resource_read(ctx, >base) > >> here [3]. But even after fixing those problems, the rendering issues > >> are not gone. > > > > I tested again with the following diff applied on top of your patch, and > > the remaining rendering issues we had seem to be gone (don't know what I > > messed up in my previous tests :-/). > > > > --->8--- > > diff --git a/src/gallium/drivers/etnaviv/etnaviv_rs.c > > b/src/gallium/drivers/etnaviv/etnaviv_rs.c > > index fc4f65dbeee1..b8c8b96a6f72 100644 > > --- a/src/gallium/drivers/etnaviv/etnaviv_rs.c > > +++ b/src/gallium/drivers/etnaviv/etnaviv_rs.c > > @@ -729,6 +729,7 @@ etna_try_rs_blit(struct pipe_context *pctx, > > > > etna_submit_rs_state(ctx, _to_screen); > > resource_written(ctx, >base); > > + resource_read(ctx, >base); > > dst->seqno++; > > dst->levels[blit_info->dst.level].ts_valid = false; > > ctx->dirty |= ETNA_DIRTY_DERIVE_TS; > While the V6 of this patch [1] is now in mesa upstream, this hunk is > missing from the V6 . Any special reason for that or is this just an > omission ? > This and an other change was done in separates commits - see: https://cgit.freedesktop.org/mesa/mesa/commit/?id=7244e768040859126a5b74023f587beaa8bef81c https://cgit.freedesktop.org/mesa/mesa/commit/?id=c56e73449698c31fe72b99a95b7e4ecdb2985b73 So everything went in. -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v5] etnaviv: fix resource usage tracking across different pipe_context's
On 2/22/19 10:30 AM, Boris Brezillon wrote: > On Thu, 21 Feb 2019 23:29:53 +0100 > Boris Brezillon wrote: > >> Christian, Marek, >> >> On Wed, 30 Jan 2019 05:28:14 +0100 >> Marek Vasut wrote: >> >>> From: Christian Gmeiner >>> >>> A pipe_resource can be shared by all the pipe_context's hanging off the >>> same pipe_screen. >> >> We seem to be impacted by the problem you're fixing here, but, while >> this patch definitely make things much better, the problem does not >> seem to be entirely fixed (at least in our case). >> >> A bit more context: we have Qt App using QtWebEngine to render html >> content. When we call QtWebEngine::initialize(), which as for effect >> to allow shared GL contexts, we sometimes notice that part of the web >> page is mis-rendered. That does not happen when we omit the >> QtWebEngine::initialize() call. >> As said above, this patch make those rendering issues less likely to >> happen, but we still have the problem from time to time. So I thought >> I'd share my guesses about what could cause these issues before >> debugging it further. >> >> First thing I noticed: I couldn't reproduce the problem with [1] >> applied (+ a tiny change forcing CPU-based tiling no matter the size of >> the region to tile/untile). So, my guess is that it's related to how we >> handle GPU-based tiling/untiling. >> Also noticed that we're testing the rsc->status here [2] without the >> screen->lock held, and there might be a race with another thread calling >> resource_used(). We might also lack a resource_read(ctx, >base) >> here [3]. But even after fixing those problems, the rendering issues >> are not gone. > > I tested again with the following diff applied on top of your patch, and > the remaining rendering issues we had seem to be gone (don't know what I > messed up in my previous tests :-/). > > --->8--- > diff --git a/src/gallium/drivers/etnaviv/etnaviv_rs.c > b/src/gallium/drivers/etnaviv/etnaviv_rs.c > index fc4f65dbeee1..b8c8b96a6f72 100644 > --- a/src/gallium/drivers/etnaviv/etnaviv_rs.c > +++ b/src/gallium/drivers/etnaviv/etnaviv_rs.c > @@ -729,6 +729,7 @@ etna_try_rs_blit(struct pipe_context *pctx, > > etna_submit_rs_state(ctx, _to_screen); > resource_written(ctx, >base); > + resource_read(ctx, >base); > dst->seqno++; > dst->levels[blit_info->dst.level].ts_valid = false; > ctx->dirty |= ETNA_DIRTY_DERIVE_TS; While the V6 of this patch [1] is now in mesa upstream, this hunk is missing from the V6 . Any special reason for that or is this just an omission ? [1] https://patchwork.freedesktop.org/patch/287603/ -- Best regards, Marek Vasut ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v5] etnaviv: fix resource usage tracking across different pipe_context's
On 2/22/19 10:30 AM, Boris Brezillon wrote:
> On Thu, 21 Feb 2019 23:29:53 +0100
> Boris Brezillon wrote:
>
>> Christian, Marek,
>>
>> On Wed, 30 Jan 2019 05:28:14 +0100
>> Marek Vasut wrote:
>>
>>> From: Christian Gmeiner
>>>
>>> A pipe_resource can be shared by all the pipe_context's hanging off the
>>> same pipe_screen.
>>
>> We seem to be impacted by the problem you're fixing here, but, while
>> this patch definitely make things much better, the problem does not
>> seem to be entirely fixed (at least in our case).
>>
>> A bit more context: we have Qt App using QtWebEngine to render html
>> content. When we call QtWebEngine::initialize(), which as for effect
>> to allow shared GL contexts, we sometimes notice that part of the web
>> page is mis-rendered. That does not happen when we omit the
>> QtWebEngine::initialize() call.
>> As said above, this patch make those rendering issues less likely to
>> happen, but we still have the problem from time to time. So I thought
>> I'd share my guesses about what could cause these issues before
>> debugging it further.
>>
>> First thing I noticed: I couldn't reproduce the problem with [1]
>> applied (+ a tiny change forcing CPU-based tiling no matter the size of
>> the region to tile/untile). So, my guess is that it's related to how we
>> handle GPU-based tiling/untiling.
>> Also noticed that we're testing the rsc->status here [2] without the
>> screen->lock held, and there might be a race with another thread calling
>> resource_used(). We might also lack a resource_read(ctx, >base)
>> here [3]. But even after fixing those problems, the rendering issues
>> are not gone.
>
> I tested again with the following diff applied on top of your patch, and
> the remaining rendering issues we had seem to be gone (don't know what I
> messed up in my previous tests :-/).
>
> --->8---
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_rs.c
> b/src/gallium/drivers/etnaviv/etnaviv_rs.c
> index fc4f65dbeee1..b8c8b96a6f72 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_rs.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_rs.c
> @@ -729,6 +729,7 @@ etna_try_rs_blit(struct pipe_context *pctx,
>
> etna_submit_rs_state(ctx, _to_screen);
> resource_written(ctx, >base);
> + resource_read(ctx, >base);
> dst->seqno++;
> dst->levels[blit_info->dst.level].ts_valid = false;
> ctx->dirty |= ETNA_DIRTY_DERIVE_TS;
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> index a3013e624ead..e4b2ac605e63 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> @@ -356,6 +356,7 @@ etna_transfer_map(struct pipe_context *pctx, struct
> pipe_resource *prsc,
> * transfers without a temporary resource.
> */
> if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
> + struct etna_screen *screen = ctx->screen;
>uint32_t prep_flags = 0;
>
>/*
> @@ -364,11 +365,13 @@ etna_transfer_map(struct pipe_context *pctx, struct
> pipe_resource *prsc,
> * current GPU usage (reads must wait for GPU writes, writes must have
> * exclusive access to the buffer).
> */
> + mtx_lock(>lock);
>if ((trans->rsc && (etna_resource(trans->rsc)->status &
> ETNA_PENDING_WRITE)) ||
>(!trans->rsc &&
> (((usage & PIPE_TRANSFER_READ) && (rsc->status &
> ETNA_PENDING_WRITE)) ||
> ((usage & PIPE_TRANSFER_WRITE) && rsc->status
> pctx->flush(pctx, NULL, 0);
> + mtx_unlock(>lock);
>
>if (usage & PIPE_TRANSFER_READ)
> prep_flags |= DRM_ETNA_PREP_READ;
>
On iMX6Q
Tested-by: Marek Vasut
with multiple custom applications. I don't see any breakage.
--
Best regards,
Marek Vasut
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v5] etnaviv: fix resource usage tracking across different pipe_context's
On Thu, 21 Feb 2019 23:29:53 +0100
Boris Brezillon wrote:
> Christian, Marek,
>
> On Wed, 30 Jan 2019 05:28:14 +0100
> Marek Vasut wrote:
>
> > From: Christian Gmeiner
> >
> > A pipe_resource can be shared by all the pipe_context's hanging off the
> > same pipe_screen.
>
> We seem to be impacted by the problem you're fixing here, but, while
> this patch definitely make things much better, the problem does not
> seem to be entirely fixed (at least in our case).
>
> A bit more context: we have Qt App using QtWebEngine to render html
> content. When we call QtWebEngine::initialize(), which as for effect
> to allow shared GL contexts, we sometimes notice that part of the web
> page is mis-rendered. That does not happen when we omit the
> QtWebEngine::initialize() call.
> As said above, this patch make those rendering issues less likely to
> happen, but we still have the problem from time to time. So I thought
> I'd share my guesses about what could cause these issues before
> debugging it further.
>
> First thing I noticed: I couldn't reproduce the problem with [1]
> applied (+ a tiny change forcing CPU-based tiling no matter the size of
> the region to tile/untile). So, my guess is that it's related to how we
> handle GPU-based tiling/untiling.
> Also noticed that we're testing the rsc->status here [2] without the
> screen->lock held, and there might be a race with another thread calling
> resource_used(). We might also lack a resource_read(ctx, >base)
> here [3]. But even after fixing those problems, the rendering issues
> are not gone.
I tested again with the following diff applied on top of your patch, and
the remaining rendering issues we had seem to be gone (don't know what I
messed up in my previous tests :-/).
--->8---
diff --git a/src/gallium/drivers/etnaviv/etnaviv_rs.c
b/src/gallium/drivers/etnaviv/etnaviv_rs.c
index fc4f65dbeee1..b8c8b96a6f72 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_rs.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_rs.c
@@ -729,6 +729,7 @@ etna_try_rs_blit(struct pipe_context *pctx,
etna_submit_rs_state(ctx, _to_screen);
resource_written(ctx, >base);
+ resource_read(ctx, >base);
dst->seqno++;
dst->levels[blit_info->dst.level].ts_valid = false;
ctx->dirty |= ETNA_DIRTY_DERIVE_TS;
diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
index a3013e624ead..e4b2ac605e63 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
@@ -356,6 +356,7 @@ etna_transfer_map(struct pipe_context *pctx, struct
pipe_resource *prsc,
* transfers without a temporary resource.
*/
if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
+ struct etna_screen *screen = ctx->screen;
uint32_t prep_flags = 0;
/*
@@ -364,11 +365,13 @@ etna_transfer_map(struct pipe_context *pctx, struct
pipe_resource *prsc,
* current GPU usage (reads must wait for GPU writes, writes must have
* exclusive access to the buffer).
*/
+ mtx_lock(>lock);
if ((trans->rsc && (etna_resource(trans->rsc)->status &
ETNA_PENDING_WRITE)) ||
(!trans->rsc &&
(((usage & PIPE_TRANSFER_READ) && (rsc->status &
ETNA_PENDING_WRITE)) ||
((usage & PIPE_TRANSFER_WRITE) && rsc->status
pctx->flush(pctx, NULL, 0);
+ mtx_unlock(>lock);
if (usage & PIPE_TRANSFER_READ)
prep_flags |= DRM_ETNA_PREP_READ;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v5] etnaviv: fix resource usage tracking across different pipe_context's
Hi Boris,
Am Fr., 22. Feb. 2019 um 10:30 Uhr schrieb Boris Brezillon
:
>
> On Thu, 21 Feb 2019 23:29:53 +0100
> Boris Brezillon wrote:
>
> > Christian, Marek,
> >
> > On Wed, 30 Jan 2019 05:28:14 +0100
> > Marek Vasut wrote:
> >
> > > From: Christian Gmeiner
> > >
> > > A pipe_resource can be shared by all the pipe_context's hanging off the
> > > same pipe_screen.
> >
> > We seem to be impacted by the problem you're fixing here, but, while
> > this patch definitely make things much better, the problem does not
> > seem to be entirely fixed (at least in our case).
> >
I also got some (private) reports about problems with QtWebEngine but had no
time yet to look more deeply into the issue.
>
> --->8---
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_rs.c
> b/src/gallium/drivers/etnaviv/etnaviv_rs.c
> index fc4f65dbeee1..b8c8b96a6f72 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_rs.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_rs.c
> @@ -729,6 +729,7 @@ etna_try_rs_blit(struct pipe_context *pctx,
>
> etna_submit_rs_state(ctx, _to_screen);
> resource_written(ctx, >base);
> + resource_read(ctx, >base);
> dst->seqno++;
> dst->levels[blit_info->dst.level].ts_valid = false;
> ctx->dirty |= ETNA_DIRTY_DERIVE_TS;
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> index a3013e624ead..e4b2ac605e63 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> @@ -356,6 +356,7 @@ etna_transfer_map(struct pipe_context *pctx, struct
> pipe_resource *prsc,
> * transfers without a temporary resource.
> */
> if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
> + struct etna_screen *screen = ctx->screen;
>uint32_t prep_flags = 0;
>
>/*
> @@ -364,11 +365,13 @@ etna_transfer_map(struct pipe_context *pctx, struct
> pipe_resource *prsc,
> * current GPU usage (reads must wait for GPU writes, writes must have
> * exclusive access to the buffer).
> */
> + mtx_lock(>lock);
>if ((trans->rsc && (etna_resource(trans->rsc)->status &
> ETNA_PENDING_WRITE)) ||
>(!trans->rsc &&
> (((usage & PIPE_TRANSFER_READ) && (rsc->status &
> ETNA_PENDING_WRITE)) ||
> ((usage & PIPE_TRANSFER_WRITE) && rsc->status
> pctx->flush(pctx, NULL, 0);
> + mtx_unlock(>lock);
>
>if (usage & PIPE_TRANSFER_READ)
> prep_flags |= DRM_ETNA_PREP_READ;
>
Looks good to me - will prepare a v6 of this patch with your changes
incorporated.
Thanks a lot!
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v5] etnaviv: fix resource usage tracking across different pipe_context's
Christian, Marek,
On Wed, 30 Jan 2019 05:28:14 +0100
Marek Vasut wrote:
> From: Christian Gmeiner
>
> A pipe_resource can be shared by all the pipe_context's hanging off the
> same pipe_screen.
We seem to be impacted by the problem you're fixing here, but, while
this patch definitely make things much better, the problem does not
seem to be entirely fixed (at least in our case).
A bit more context: we have Qt App using QtWebEngine to render html
content. When we call QtWebEngine::initialize(), which as for effect
to allow shared GL contexts, we sometimes notice that part of the web
page is mis-rendered. That does not happen when we omit the
QtWebEngine::initialize() call.
As said above, this patch make those rendering issues less likely to
happen, but we still have the problem from time to time. So I thought
I'd share my guesses about what could cause these issues before
debugging it further.
First thing I noticed: I couldn't reproduce the problem with [1]
applied (+ a tiny change forcing CPU-based tiling no matter the size of
the region to tile/untile). So, my guess is that it's related to how we
handle GPU-based tiling/untiling.
Also noticed that we're testing the rsc->status here [2] without the
screen->lock held, and there might be a race with another thread calling
resource_used(). We might also lack a resource_read(ctx, >base)
here [3]. But even after fixing those problems, the rendering issues
are not gone. So I'm wondering is the problem is not coming from the
GPU-based tiling/untiling logic we have in etnaviv_transfer.c.
Indeed, RS-based tiling require the surface we tile/untile to be
aligned on at least 16 pixels (and even more in case of multi/super
tiles). In order to handle those constraints when the region the user
wants to map/update is not properly aligned, etna_transfer_map() first
untiles an RS-aligned region into a temporary linear buffer which will
then be updated according to the caller's needs and tiled back to the
actual texture when etna_transfer_unmap() is called. But what happens
if, between the untile operation in etna_transfer_map() and the tile
operation in etna_transfer_unmap(), another rendering request is done
on region that overlaps the RS-aligned region we untiled in our
temporary buffer?
Can't we end up in a situation where some a the data we are about to
copy back to the actual texture are actually outdated?
I'm completely new to etnaviv and mesa, so I wouldn't be surprised if
my analysis of the problem was wrong, but I thought it was worth an
email before digging further.
Please let me know if you have other ideas or need more details about
our use case.
Thanks,
Boris
[1]https://lists.freedesktop.org/archives/mesa-dev/2019-February/215597.html
[2]https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/etnaviv/etnaviv_transfer.c#n316
[3]https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/etnaviv/etnaviv_rs.c#n731
>
> Signed-off-by: Christian Gmeiner
> Signed-off-by: Marek Vasut
> To: mesa-dev@lists.freedesktop.org
> Cc: etna...@lists.freedesktop.org
> ---
> Changes from v1 -> v2:
> - to remove the resource from the used_resources set when it is destroyed
> Changes from v2 -> v3:
> - add locking with mtx_*() to resource and screen (Marek)
> Changes from v3 -> v4:
> - drop rsc->lock, just use screen->lock for the entire serialization (Marek)
> - simplify etna_resource_used() flush condition, which also prevents
>potentially flushing resources twice (Marek)
> - don't remove resouces from screen->used_resources in
>etna_cmd_stream_reset_notify(), they may still be used in other
>contexts and may need flushing there later on (Marek)
> Changes from v4 -> v5:
> - Fix coding style issues reported by Guido
> ---
> src/gallium/drivers/etnaviv/etnaviv_context.c | 26 +-
> src/gallium/drivers/etnaviv/etnaviv_context.h | 3 --
> .../drivers/etnaviv/etnaviv_resource.c| 52 +++
> .../drivers/etnaviv/etnaviv_resource.h| 8 +--
> src/gallium/drivers/etnaviv/etnaviv_screen.c | 12 +
> src/gallium/drivers/etnaviv/etnaviv_screen.h | 6 +++
> 6 files changed, 78 insertions(+), 29 deletions(-)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.c
> b/src/gallium/drivers/etnaviv/etnaviv_context.c
> index 3038d21..2f8cae8 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_context.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_context.c
> @@ -36,6 +36,7 @@
> #include "etnaviv_query.h"
> #include "etnaviv_query_hw.h"
> #include "etnaviv_rasterizer.h"
> +#include "etnaviv_resource.h"
> #include "etnaviv_screen.h"
> #include "etnaviv_shader.h"
> #include "etnaviv_state.h"
> @@ -329,7 +330,8 @@ static void
> etna_cmd_stream_reset_notify(struct etna_cmd_stream *stream, void *priv)
> {
> struct etna_context *ctx = priv;
> - struct etna_resource *rsc, *rsc_tmp;
> + struct etna_screen *screen = ctx->screen;
> + struct set_entry *entry;
>
> etna_set_state(stream,
Re: [Mesa-dev] [PATCH v5] etnaviv: fix resource usage tracking across different pipe_context's
Hi,
On Wed, Jan 30, 2019 at 05:28:14AM +0100, Marek Vasut wrote:
> From: Christian Gmeiner
>
> A pipe_resource can be shared by all the pipe_context's hanging off the
> same pipe_screen.
>
> Signed-off-by: Christian Gmeiner
> Signed-off-by: Marek Vasut
> To: mesa-dev@lists.freedesktop.org
> Cc: etna...@lists.freedesktop.org
Tested-By: Guido Günther
I'm not bold enough to add a
Reviewed-By: Guido Günther
Cheers,
-- Guido
> ---
> Changes from v1 -> v2:
> - to remove the resource from the used_resources set when it is destroyed
> Changes from v2 -> v3:
> - add locking with mtx_*() to resource and screen (Marek)
> Changes from v3 -> v4:
> - drop rsc->lock, just use screen->lock for the entire serialization (Marek)
> - simplify etna_resource_used() flush condition, which also prevents
>potentially flushing resources twice (Marek)
> - don't remove resouces from screen->used_resources in
>etna_cmd_stream_reset_notify(), they may still be used in other
>contexts and may need flushing there later on (Marek)
> Changes from v4 -> v5:
> - Fix coding style issues reported by Guido
> ---
> src/gallium/drivers/etnaviv/etnaviv_context.c | 26 +-
> src/gallium/drivers/etnaviv/etnaviv_context.h | 3 --
> .../drivers/etnaviv/etnaviv_resource.c| 52 +++
> .../drivers/etnaviv/etnaviv_resource.h| 8 +--
> src/gallium/drivers/etnaviv/etnaviv_screen.c | 12 +
> src/gallium/drivers/etnaviv/etnaviv_screen.h | 6 +++
> 6 files changed, 78 insertions(+), 29 deletions(-)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.c
> b/src/gallium/drivers/etnaviv/etnaviv_context.c
> index 3038d21..2f8cae8 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_context.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_context.c
> @@ -36,6 +36,7 @@
> #include "etnaviv_query.h"
> #include "etnaviv_query_hw.h"
> #include "etnaviv_rasterizer.h"
> +#include "etnaviv_resource.h"
> #include "etnaviv_screen.h"
> #include "etnaviv_shader.h"
> #include "etnaviv_state.h"
> @@ -329,7 +330,8 @@ static void
> etna_cmd_stream_reset_notify(struct etna_cmd_stream *stream, void *priv)
> {
> struct etna_context *ctx = priv;
> - struct etna_resource *rsc, *rsc_tmp;
> + struct etna_screen *screen = ctx->screen;
> + struct set_entry *entry;
>
> etna_set_state(stream, VIVS_GL_API_MODE, VIVS_GL_API_MODE_OPENGL);
> etna_set_state(stream, VIVS_GL_VERTEX_ELEMENT_CONFIG, 0x0001);
> @@ -384,16 +386,18 @@ etna_cmd_stream_reset_notify(struct etna_cmd_stream
> *stream, void *priv)
> ctx->dirty = ~0L;
> ctx->dirty_sampler_views = ~0L;
>
> - /* go through all the used resources and clear their status flag */
> - LIST_FOR_EACH_ENTRY_SAFE(rsc, rsc_tmp, >used_resources, list)
> - {
> - debug_assert(rsc->status != 0);
> - rsc->status = 0;
> - rsc->pending_ctx = NULL;
> - list_delinit(>list);
> - }
> + /*
> +* Go through all _resources_ associated with this _screen_, pending
> +* in this _context_ and mark them as not pending in this _context_
> +* anymore, since they were just flushed.
> +*/
> + mtx_lock(>lock);
> + set_foreach(screen->used_resources, entry) {
> + struct etna_resource *rsc = (struct etna_resource *)entry->key;
>
> - assert(LIST_IS_EMPTY(>used_resources));
> + _mesa_set_remove_key(rsc->pending_ctx, ctx);
> + }
> + mtx_unlock(>lock);
> }
>
> static void
> @@ -437,8 +441,6 @@ etna_context_create(struct pipe_screen *pscreen, void
> *priv, unsigned flags)
> /* need some sane default in case state tracker doesn't set some state: */
> ctx->sample_mask = 0x;
>
> - list_inithead(>used_resources);
> -
> /* Set sensible defaults for state */
> etna_cmd_stream_reset_notify(ctx->stream, ctx);
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.h
> b/src/gallium/drivers/etnaviv/etnaviv_context.h
> index 584caa7..eff0a23 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_context.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_context.h
> @@ -136,9 +136,6 @@ struct etna_context {
> uint32_t prim_hwsupport;
> struct primconvert_context *primconvert;
>
> - /* list of resources used by currently-unsubmitted renders */
> - struct list_head used_resources;
> -
> struct slab_child_pool transfer_pool;
> struct blitter_context *blitter;
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.c
> b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> index 3808c29..46ab849 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> @@ -33,6 +33,7 @@
> #include "etnaviv_screen.h"
> #include "etnaviv_translate.h"
>
> +#include "util/hash_table.h"
> #include "util/u_inlines.h"
> #include "util/u_memory.h"
>
> @@ -275,7 +276,6 @@ etna_resource_alloc(struct pipe_screen *pscreen, unsigned
> layout,
> rsc->halign = halign;
>
>
