<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text --><style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
</head>
<body>
<meta content="text/html; charset=UTF-8">
<style type="text/css" style="">
<!--
p
        {margin-top:0;
        margin-bottom:0}
-->
</style>
<div dir="ltr">
<div id="x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Arial,Helvetica,sans-serif">
<p>if ring_write_multiple is organized in a separate patch, doesn't it introduces an function that no client using it ??</p>
<p><br>
</p>
<p>fine by me although ...<br>
</p>
<p><br>
</p>
<p>BR Monk<br>
</p>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>发件人:</b> Christian König <deathsimple@vodafone.de><br>
<b>发送时间:</b> 2017年1月12日 20:27:48<br>
<b>收件人:</b> Liu, Monk; amd-gfx@lists.freedesktop.org<br>
<b>主题:</b> Re: [PATCH 2/3] drm/amdgpu:implement META-DATA write routines</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt;">
<div class="PlainText">Am 12.01.2017 um 08:41 schrieb Monk Liu:<br>
> Change-Id: I66007a7f7e4e27fb129121f36143dce3cfb43738<br>
> Signed-off-by: Monk Liu <Monk.Liu@amd.com><br>
> ---<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 31 ++++++++++++++++++<br>
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 61 +++++++++++++++++++++++++++++++++++<br>
>   2 files changed, 92 insertions(+)<br>
><br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br>
> index e9983fb..2039da7 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br>
> @@ -1599,6 +1599,37 @@ static inline void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v)<br>
>        ring->count_dw--;<br>
>   }<br>
>   <br>
> +static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring, void *src, int count_dw)<br>
> +{<br>
> +     unsigned occupied, chunk1, chunk2;<br>
> +     void *dst;<br>
> +<br>
> +     if (ring->count_dw < count_dw)<br>
> +             DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");<br>
> +     else {<br>
<br>
Coding style says when either the "if" or the "else" branch uses "{" and <br>
"}" both should use them.<br>
<br>
I think even better would be to use a return statement in the "if", <br>
cause this is just checking the prerequisites for errors.<br>
<br>
Additional to that adding this function should be a separate patch.<br>
<br>
Christian.<br>
<br>
> +             occupied = ring->wptr & ring->ptr_mask;<br>
> +             dst = (void *)&ring->ring[occupied];<br>
> +             chunk1 = ring->ptr_mask + 1 - occupied;<br>
> +             chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;<br>
> +             chunk2 = count_dw - chunk1;<br>
> +             chunk1 <<= 2;<br>
> +             chunk2 <<= 2;<br>
> +             if (chunk1) {<br>
> +                     memcpy(dst, src, chunk1);<br>
> +             }<br>
> +<br>
> +             if (chunk2) {<br>
> +                     src += chunk1;<br>
> +                     dst = (void *)ring->ring;<br>
> +                     memcpy(dst, src, chunk2);<br>
> +             }<br>
> +<br>
> +             ring->wptr += count_dw;<br>
> +             ring->wptr &= ring->ptr_mask;<br>
> +             ring->count_dw -= count_dw;<br>
> +     }<br>
> +}<br>
> +<br>
>   static inline struct amdgpu_sdma_instance *<br>
>   amdgpu_get_sdma_instance(struct amdgpu_ring *ring)<br>
>   {<br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c<br>
> index 375784d..3e8cff3 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c<br>
> @@ -657,6 +657,8 @@ static void gfx_v8_0_set_gds_init(struct amdgpu_device *adev);<br>
>   static void gfx_v8_0_set_rlc_funcs(struct amdgpu_device *adev);<br>
>   static u32 gfx_v8_0_get_csb_size(struct amdgpu_device *adev);<br>
>   static void gfx_v8_0_get_cu_info(struct amdgpu_device *adev);<br>
> +static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring *ring, uint64_t addr);<br>
> +static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring *ring, uint64_t addr);<br>
>   <br>
>   static void gfx_v8_0_init_golden_registers(struct amdgpu_device *adev)<br>
>   {<br>
> @@ -7212,3 +7214,62 @@ const struct amdgpu_ip_block_version gfx_v8_1_ip_block =<br>
>        .rev = 0,<br>
>        .funcs = &gfx_v8_0_ip_funcs,<br>
>   };<br>
> +<br>
> +static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring *ring, uint64_t csa_addr)<br>
> +{<br>
> +     uint64_t ce_payload_addr;<br>
> +     int cnt_ce;<br>
> +     static union {<br>
> +             struct amdgpu_ce_ib_state regular;<br>
> +             struct amdgpu_ce_ib_state_chained_ib chained;<br>
> +     } ce_payload = {0};<br>
> +<br>
> +     if (ring->adev->virt.chained_ib_support) {<br>
> +             ce_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data_chained_ib, ce_payload);<br>
> +             cnt_ce = (sizeof(ce_payload.chained) >> 2) + 4 - 2;<br>
> +     } else {<br>
> +             ce_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data, ce_payload);<br>
> +             cnt_ce = (sizeof(ce_payload.regular) >> 2) + 4 - 2;<br>
> +     }<br>
> +<br>
> +     amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_ce));<br>
> +     amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(2) |<br>
> +                             WRITE_DATA_DST_SEL(8) |<br>
> +                             WR_CONFIRM) |<br>
> +                             WRITE_DATA_CACHE_POLICY(0));<br>
> +     amdgpu_ring_write(ring, lower_32_bits(ce_payload_addr));<br>
> +     amdgpu_ring_write(ring, upper_32_bits(ce_payload_addr));<br>
> +     amdgpu_ring_write_multiple(ring, (void *)&ce_payload, cnt_ce - 2);<br>
> +}<br>
> +<br>
> +static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring *ring, uint64_t csa_addr)<br>
> +{<br>
> +     uint64_t de_payload_addr, gds_addr;<br>
> +     int cnt_de;<br>
> +     static union {<br>
> +             struct amdgpu_de_ib_state regular;<br>
> +             struct amdgpu_de_ib_state_chained_ib chained;<br>
> +     } de_payload = {0};<br>
> +<br>
> +     gds_addr = csa_addr + 4096;<br>
> +     if (ring->adev->virt.chained_ib_support) {<br>
> +             de_payload.chained.gds_backup_addrlo = lower_32_bits(gds_addr);<br>
> +             de_payload.chained.gds_backup_addrhi = upper_32_bits(gds_addr);<br>
> +             de_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data_chained_ib, de_payload);<br>
> +             cnt_de = (sizeof(de_payload.chained) >> 2) + 4 - 2;<br>
> +     } else {<br>
> +             de_payload.regular.gds_backup_addrlo = lower_32_bits(gds_addr);<br>
> +             de_payload.regular.gds_backup_addrhi = upper_32_bits(gds_addr);<br>
> +             de_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data, de_payload);<br>
> +             cnt_de = (sizeof(de_payload.regular) >> 2) + 4 - 2;<br>
> +     }<br>
> +<br>
> +     amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_de));<br>
> +     amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(1) |<br>
> +                             WRITE_DATA_DST_SEL(8) |<br>
> +                             WR_CONFIRM) |<br>
> +                             WRITE_DATA_CACHE_POLICY(0));<br>
> +     amdgpu_ring_write(ring, lower_32_bits(de_payload_addr));<br>
> +     amdgpu_ring_write(ring, upper_32_bits(de_payload_addr));<br>
> +     amdgpu_ring_write_multiple(ring, (void *)&de_payload, cnt_de - 2);<br>
> +}<br>
<br>
<br>
</div>
</span></font>
</body>
</html>