[Mesa-dev] [PATCH 01/10] winsys/radeon: remove cs_write_reloc, add simpler cs_get_reloc

Marek Olšák maraeo at gmail.com
Mon Apr 14 16:21:46 PDT 2014


Actually, I take it back. There are several reasons for having an explicit flag.

1) Flushes from the state tracker are always synchronous. It's because
the CS ioctl must be finished before libGL sends the SwapBuffers
request to the X server.

2) Flushes before calling buffer_map where synchronization is expected
are also synchronous. The buffer_map and buffer_wait functions don't
call cs_sync_flush, instead, they loop and call sched_yield(). This is
probably fixable now that there is a global CS queue (originally, each
CS instance had its own thread).

3) I guess there is some overhead in switching threads, and doing
command submission in the main thread is probably easier for the CPU.

Marek


On Mon, Apr 14, 2014 at 2:41 PM, Marek Olšák <maraeo at gmail.com> wrote:
> Yes, I will remove the flag in the next patch series.
>
> Marek
>
> On Sun, Apr 13, 2014 at 10:45 AM, Christian König
> <deathsimple at vodafone.de> wrote:
>> For this series: Reviewed-by: Christian König <christian.koenig at amd.com>
>>
>> BTW: Can we get rid of RADEON_FLUSH_ASYNC and always flush asynchronously?
>> Just calling cs_sync_flush directly after the flush should have the same
>> effect as synchronous flushing.
>>
>> Christian.
>>
>> Am 12.04.2014 18:34, schrieb Marek Olšák:
>>
>>> From: Marek Olšák <marek.olsak at amd.com>
>>>
>>> The only difference is that it doesn't write to the CS and only returns
>>> the index.
>>> ---
>>>   src/gallium/drivers/r300/r300_cs.h            |  3 ++-
>>>   src/gallium/drivers/r300/r300_emit.c          |  4 ++--
>>>   src/gallium/winsys/radeon/drm/radeon_drm_cs.c | 26
>>> +++++++++-----------------
>>>   src/gallium/winsys/radeon/drm/radeon_winsys.h | 19 ++++++++++---------
>>>   4 files changed, 23 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/r300/r300_cs.h
>>> b/src/gallium/drivers/r300/r300_cs.h
>>> index 744e19e..748d6ea 100644
>>> --- a/src/gallium/drivers/r300/r300_cs.h
>>> +++ b/src/gallium/drivers/r300/r300_cs.h
>>> @@ -109,7 +109,8 @@
>>>   #define OUT_CS_RELOC(r) do { \
>>>       assert((r)); \
>>>       assert((r)->cs_buf); \
>>> -    cs_winsys->cs_write_reloc(cs_copy, (r)->cs_buf); \
>>> +    OUT_CS(0xc0001000); /* PKT3_NOP */ \
>>> +    OUT_CS(cs_winsys->cs_get_reloc(cs_copy, (r)->cs_buf) * 4); \
>>>       CS_USED_DW(2); \
>>>   } while (0)
>>>   diff --git a/src/gallium/drivers/r300/r300_emit.c
>>> b/src/gallium/drivers/r300/r300_emit.c
>>> index d99b919..9f16413 100644
>>> --- a/src/gallium/drivers/r300/r300_emit.c
>>> +++ b/src/gallium/drivers/r300/r300_emit.c
>>> @@ -1043,8 +1043,8 @@ void r300_emit_vertex_arrays_swtcl(struct
>>> r300_context *r300, boolean indexed)
>>>       OUT_CS(0);
>>>         assert(r300->vbo_cs);
>>> -    cs_winsys->cs_write_reloc(cs_copy, r300->vbo_cs);
>>> -    CS_USED_DW(2);
>>> +    OUT_CS(0xc0001000); /* PKT3_NOP */
>>> +    OUT_CS(r300->rws->cs_get_reloc(r300->cs, r300->vbo_cs) * 4);
>>>       END_CS;
>>>   }
>>>   diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>>> b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>>> index b55eb80..4ce1717 100644
>>> --- a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>>> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>>> @@ -313,6 +313,14 @@ static unsigned radeon_drm_cs_add_reloc(struct
>>> radeon_winsys_cs *rcs,
>>>       return index;
>>>   }
>>>   +static int radeon_drm_cs_get_reloc(struct radeon_winsys_cs *rcs,
>>> +                                   struct radeon_winsys_cs_handle *buf)
>>> +{
>>> +    struct radeon_drm_cs *cs = radeon_drm_cs(rcs);
>>> +
>>> +    return radeon_get_reloc(cs->csc, (struct radeon_bo*)buf, NULL);
>>> +}
>>> +
>>>   static boolean radeon_drm_cs_validate(struct radeon_winsys_cs *rcs)
>>>   {
>>>       struct radeon_drm_cs *cs = radeon_drm_cs(rcs);
>>> @@ -359,22 +367,6 @@ static boolean
>>> radeon_drm_cs_memory_below_limit(struct radeon_winsys_cs *rcs, ui
>>>       return status;
>>>   }
>>>   -static void radeon_drm_cs_write_reloc(struct radeon_winsys_cs *rcs,
>>> -                                      struct radeon_winsys_cs_handle
>>> *buf)
>>> -{
>>> -    struct radeon_drm_cs *cs = radeon_drm_cs(rcs);
>>> -    struct radeon_bo *bo = (struct radeon_bo*)buf;
>>> -    unsigned index = radeon_get_reloc(cs->csc, bo, NULL);
>>> -
>>> -    if (index == -1) {
>>> -        fprintf(stderr, "radeon: Cannot get a relocation in %s.\n",
>>> __func__);
>>> -        return;
>>> -    }
>>> -
>>> -    OUT_CS(&cs->base, 0xc0001000);
>>> -    OUT_CS(&cs->base, index * RELOC_DWORDS);
>>> -}
>>> -
>>>   void radeon_drm_cs_emit_ioctl_oneshot(struct radeon_drm_cs *cs, struct
>>> radeon_cs_context *csc)
>>>   {
>>>       unsigned i;
>>> @@ -650,9 +642,9 @@ void radeon_drm_cs_init_functions(struct
>>> radeon_drm_winsys *ws)
>>>       ws->base.cs_create = radeon_drm_cs_create;
>>>       ws->base.cs_destroy = radeon_drm_cs_destroy;
>>>       ws->base.cs_add_reloc = radeon_drm_cs_add_reloc;
>>> +    ws->base.cs_get_reloc = radeon_drm_cs_get_reloc;
>>>       ws->base.cs_validate = radeon_drm_cs_validate;
>>>       ws->base.cs_memory_below_limit = radeon_drm_cs_memory_below_limit;
>>> -    ws->base.cs_write_reloc = radeon_drm_cs_write_reloc;
>>>       ws->base.cs_flush = radeon_drm_cs_flush;
>>>       ws->base.cs_set_flush_callback = radeon_drm_cs_set_flush;
>>>       ws->base.cs_is_buffer_referenced = radeon_bo_is_referenced;
>>> diff --git a/src/gallium/winsys/radeon/drm/radeon_winsys.h
>>> b/src/gallium/winsys/radeon/drm/radeon_winsys.h
>>> index 485e925..320989c 100644
>>> --- a/src/gallium/winsys/radeon/drm/radeon_winsys.h
>>> +++ b/src/gallium/winsys/radeon/drm/radeon_winsys.h
>>> @@ -450,6 +450,16 @@ struct radeon_winsys {
>>>                                enum radeon_bo_priority priority);
>>>         /**
>>> +     * Return the index of an already-added buffer.
>>> +     *
>>> +     * \param cs        Command stream
>>> +     * \param buf       Buffer
>>> +     * \return          The buffer index, or -1 if the buffer has not
>>> been added.
>>> +     */
>>> +    int (*cs_get_reloc)(struct radeon_winsys_cs *cs,
>>> +                        struct radeon_winsys_cs_handle *buf);
>>> +
>>> +    /**
>>>        * Return TRUE if there is enough memory in VRAM and GTT for the
>>> relocs
>>>        * added so far. If the validation fails, all the relocations which
>>> have
>>>        * been added since the last call of cs_validate will be removed and
>>> @@ -470,15 +480,6 @@ struct radeon_winsys {
>>>       boolean (*cs_memory_below_limit)(struct radeon_winsys_cs *cs,
>>> uint64_t vram, uint64_t gtt);
>>>         /**
>>> -     * Write a relocated dword to a command buffer.
>>> -     *
>>> -     * \param cs        A command stream the relocation is written to.
>>> -     * \param buf       A winsys buffer to write the relocation for.
>>> -     */
>>> -    void (*cs_write_reloc)(struct radeon_winsys_cs *cs,
>>> -                           struct radeon_winsys_cs_handle *buf);
>>> -
>>> -    /**
>>>        * Flush a command stream.
>>>        *
>>>        * \param cs          A command stream to flush.
>>
>>


More information about the mesa-dev mailing list