[PATCH 0/3] GFX8's meta-data write feature for SRIOV

[PATCH 0/3] GFX8's meta-data write feature for SRIOV

[Monk Liu]

*** BLURB HERE ***
to enable GFX SRIOV we need insert some new packag in gfx ring
which called META-DATA write command, and meta-data is composed
by two parts: CE and DE.
CE's meta-data write need prior to CNTX_CNTL package while
DE's should following that.

meta-data will notify CP about the CSA address for usage when
preeption occured.

Monk Liu (3):
  drm/amdgpu:add META_DATA struct for CSA/SRIOV
  drm/amdgpu:implement META-DATA write routines
  drm/amdgpu:invoke meta-data write around cntx_cntl

 drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  32 +++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h |   1 +
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    |  77 +++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/vi.h          | 112 +++++++++++++++++++++++++++++++
 5 files changed, 225 insertions(+)

-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/3] drm/amdgpu:add META_DATA struct for CSA/SRIOV

[Monk Liu]

META-DATA is used in GFX cmd submit, we have two
types of META-DATA now, one is legacy and the other
is for chained-ib-preemption, which is used in vulkan
UMD.

Change-Id: I17324905b4822a93fd3ffb3e8f86e544871b89ab
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h |   1 +
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    |   8 +++
 drivers/gpu/drm/amd/amdgpu/vi.h          | 112 +++++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
index fa1d569..cf1c94b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
@@ -34,6 +34,7 @@ struct amdgpu_virt {
 	uint32_t caps;
 	struct amdgpu_bo *csa_obj;
 	uint64_t csa_vmid0_addr;
+	bool chained_ib_support;
 };
 
 #define AMDGPU_CSA_SIZE    (8 * 1024)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index a9087f0..375784d 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -941,6 +941,14 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device *adev)
 		goto out;
 	cp_hdr = (const struct gfx_firmware_header_v1_0 *)adev->gfx.me_fw->data;
 	adev->gfx.me_fw_version = le32_to_cpu(cp_hdr->header.ucode_version);
+
+	/* chain ib ucode isn't formal released, use fake number 999 should be enough
+	 * we need to change 999 to a correct value after chain ib preemption landed in
+	 * formal ucode release
+	 */
+	if (adev->gfx.me_fw_version > 999) {
+		adev->virt.chained_ib_support = true;
+	}
 	adev->gfx.me_feature_version = le32_to_cpu(cp_hdr->ucode_feature_version);
 
 	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ce.bin", chip_name);
diff --git a/drivers/gpu/drm/amd/amdgpu/vi.h b/drivers/gpu/drm/amd/amdgpu/vi.h
index 575d7ae..719587b 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.h
+++ b/drivers/gpu/drm/amd/amdgpu/vi.h
@@ -28,4 +28,116 @@ void vi_srbm_select(struct amdgpu_device *adev,
 		    u32 me, u32 pipe, u32 queue, u32 vmid);
 int vi_set_ip_blocks(struct amdgpu_device *adev);
 
+struct amdgpu_ce_ib_state
+{
+	uint32_t    ce_ib_completion_status;
+	uint32_t    ce_constegnine_count;
+	uint32_t    ce_ibOffset_ib1;
+	uint32_t    ce_ibOffset_ib2;
+}; /* Total of 4 DWORD */
+
+struct amdgpu_de_ib_state
+{
+	uint32_t    ib_completion_status;
+	uint32_t    de_constEngine_count;
+	uint32_t    ib_offset_ib1;
+	uint32_t    ib_offset_ib2;
+	uint32_t    preamble_begin_ib1;
+	uint32_t    preamble_begin_ib2;
+	uint32_t    preamble_end_ib1;
+	uint32_t    preamble_end_ib2;
+	uint32_t    draw_indirect_baseLo;
+	uint32_t    draw_indirect_baseHi;
+	uint32_t    disp_indirect_baseLo;
+	uint32_t    disp_indirect_baseHi;
+	uint32_t    gds_backup_addrlo;
+	uint32_t    gds_backup_addrhi;
+	uint32_t    index_base_addrlo;
+	uint32_t    index_base_addrhi;
+	uint32_t    sample_cntl;
+}; /* Total of 17 DWORD */
+
+struct amdgpu_ce_ib_state_chained_ib
+{
+	/* section of non chained ib part */
+	uint32_t    ce_ib_completion_status;
+	uint32_t    ce_constegnine_count;
+	uint32_t    ce_ibOffset_ib1;
+	uint32_t    ce_ibOffset_ib2;
+
+	/* section of chained ib */
+	uint32_t    ce_chainib_addrlo_ib1;
+	uint32_t    ce_chainib_addrlo_ib2;
+	uint32_t    ce_chainib_addrhi_ib1;
+	uint32_t    ce_chainib_addrhi_ib2;
+	uint32_t    ce_chainib_size_ib1;
+	uint32_t    ce_chainib_size_ib2;
+}; /* total 10 DWORD */
+
+struct amdgpu_de_ib_state_chained_ib
+{
+	/* section of non chained ib part */
+	uint32_t    ib_completion_status;
+	uint32_t    de_constEngine_count;
+	uint32_t    ib_offset_ib1;
+	uint32_t    ib_offset_ib2;
+
+	/* section of chained ib */
+	uint32_t    chain_ib_addrlo_ib1;
+	uint32_t    chain_ib_addrlo_ib2;
+	uint32_t    chain_ib_addrhi_ib1;
+	uint32_t    chain_ib_addrhi_ib2;
+	uint32_t    chain_ib_size_ib1;
+	uint32_t    chain_ib_size_ib2;
+
+	/* section of non chained ib part */
+	uint32_t    preamble_begin_ib1;
+	uint32_t    preamble_begin_ib2;
+	uint32_t    preamble_end_ib1;
+	uint32_t    preamble_end_ib2;
+
+	/* section of chained ib */
+	uint32_t    chain_ib_pream_addrlo_ib1;
+	uint32_t    chain_ib_pream_addrlo_ib2;
+	uint32_t    chain_ib_pream_addrhi_ib1;
+	uint32_t    chain_ib_pream_addrhi_ib2;
+
+	/* section of non chained ib part */
+	uint32_t    draw_indirect_baseLo;
+	uint32_t    draw_indirect_baseHi;
+	uint32_t    disp_indirect_baseLo;
+	uint32_t    disp_indirect_baseHi;
+	uint32_t    gds_backup_addrlo;
+	uint32_t    gds_backup_addrhi;
+	uint32_t    index_base_addrlo;
+	uint32_t    index_base_addrhi;
+	uint32_t    sample_cntl;
+}; /* Total of 27 DWORD */
+
+struct amdgpu_gfx_meta_data
+{
+	/* 4 DWORD, address must be 4KB aligned */
+	struct amdgpu_ce_ib_state    ce_payload;
+	uint32_t                     reserved1[60];
+	/* 17 DWORD, address must be 64B aligned */
+	struct amdgpu_de_ib_state    de_payload;
+	/* PFP IB base address which get pre-empted */
+	uint32_t                     DeIbBaseAddrLo;
+	uint32_t                     DeIbBaseAddrHi;
+	uint32_t                     reserved2[941];
+}; /* Total of 4K Bytes */
+
+struct amdgpu_gfx_meta_data_chained_ib
+{
+	/* 10 DWORD, address must be 4KB aligned */
+	struct amdgpu_ce_ib_state_chained_ib   ce_payload;
+	uint32_t                               reserved1[54];
+	/* 27 DWORD, address must be 64B aligned */
+	struct amdgpu_de_ib_state_chained_ib   de_payload;
+	/* PFP IB base address which get pre-empted */
+	uint32_t                               DeIbBaseAddrLo;
+	uint32_t                               DeIbBaseAddrHi;
+	uint32_t                               reserved2[931];
+}; /* Total of 4K Bytes */
+
 #endif
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/3] drm/amdgpu:implement META-DATA write routines

[Monk Liu]

Change-Id: I66007a7f7e4e27fb129121f36143dce3cfb43738
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 31 ++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 61 +++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e9983fb..2039da7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1599,6 +1599,37 @@ static inline void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v)
 	ring->count_dw--;
 }
 
+static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring, void *src, int count_dw)
+{
+	unsigned occupied, chunk1, chunk2;
+	void *dst;
+
+	if (ring->count_dw < count_dw)
+		DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
+	else {
+		occupied = ring->wptr & ring->ptr_mask;
+		dst = (void *)&ring->ring[occupied];
+		chunk1 = ring->ptr_mask + 1 - occupied;
+		chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
+		chunk2 = count_dw - chunk1;
+		chunk1 <<= 2;
+		chunk2 <<= 2;
+		if (chunk1) {
+			memcpy(dst, src, chunk1);
+		}
+
+		if (chunk2) {
+			src += chunk1;
+			dst = (void *)ring->ring;
+			memcpy(dst, src, chunk2);
+		}
+
+		ring->wptr += count_dw;
+		ring->wptr &= ring->ptr_mask;
+		ring->count_dw -= count_dw;
+	}
+}
+
 static inline struct amdgpu_sdma_instance *
 amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
 {
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 375784d..3e8cff3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -657,6 +657,8 @@ static void gfx_v8_0_set_gds_init(struct amdgpu_device *adev);
 static void gfx_v8_0_set_rlc_funcs(struct amdgpu_device *adev);
 static u32 gfx_v8_0_get_csb_size(struct amdgpu_device *adev);
 static void gfx_v8_0_get_cu_info(struct amdgpu_device *adev);
+static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring *ring, uint64_t addr);
+static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring *ring, uint64_t addr);
 
 static void gfx_v8_0_init_golden_registers(struct amdgpu_device *adev)
 {
@@ -7212,3 +7214,62 @@ const struct amdgpu_ip_block_version gfx_v8_1_ip_block =
 	.rev = 0,
 	.funcs = &gfx_v8_0_ip_funcs,
 };
+
+static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring *ring, uint64_t csa_addr)
+{
+	uint64_t ce_payload_addr;
+	int cnt_ce;
+	static union {
+		struct amdgpu_ce_ib_state regular;
+		struct amdgpu_ce_ib_state_chained_ib chained;
+	} ce_payload = {0};
+
+	if (ring->adev->virt.chained_ib_support) {
+		ce_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data_chained_ib, ce_payload);
+		cnt_ce = (sizeof(ce_payload.chained) >> 2) + 4 - 2;
+	} else {
+		ce_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data, ce_payload);
+		cnt_ce = (sizeof(ce_payload.regular) >> 2) + 4 - 2;
+	}
+
+	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_ce));
+	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(2) |
+				WRITE_DATA_DST_SEL(8) |
+				WR_CONFIRM) |
+				WRITE_DATA_CACHE_POLICY(0));
+	amdgpu_ring_write(ring, lower_32_bits(ce_payload_addr));
+	amdgpu_ring_write(ring, upper_32_bits(ce_payload_addr));
+	amdgpu_ring_write_multiple(ring, (void *)&ce_payload, cnt_ce - 2);
+}
+
+static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring *ring, uint64_t csa_addr)
+{
+	uint64_t de_payload_addr, gds_addr;
+	int cnt_de;
+	static union {
+		struct amdgpu_de_ib_state regular;
+		struct amdgpu_de_ib_state_chained_ib chained;
+	} de_payload = {0};
+
+	gds_addr = csa_addr + 4096;
+	if (ring->adev->virt.chained_ib_support) {
+		de_payload.chained.gds_backup_addrlo = lower_32_bits(gds_addr);
+		de_payload.chained.gds_backup_addrhi = upper_32_bits(gds_addr);
+		de_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data_chained_ib, de_payload);
+		cnt_de = (sizeof(de_payload.chained) >> 2) + 4 - 2;
+	} else {
+		de_payload.regular.gds_backup_addrlo = lower_32_bits(gds_addr);
+		de_payload.regular.gds_backup_addrhi = upper_32_bits(gds_addr);
+		de_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data, de_payload);
+		cnt_de = (sizeof(de_payload.regular) >> 2) + 4 - 2;
+	}
+
+	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_de));
+	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(1) |
+				WRITE_DATA_DST_SEL(8) |
+				WR_CONFIRM) |
+				WRITE_DATA_CACHE_POLICY(0));
+	amdgpu_ring_write(ring, lower_32_bits(de_payload_addr));
+	amdgpu_ring_write(ring, upper_32_bits(de_payload_addr));
+	amdgpu_ring_write_multiple(ring, (void *)&de_payload, cnt_de - 2);
+}
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/3] drm/amdgpu:invoke meta-data write around cntx_cntl

[Monk Liu]

ce's META-DATA write need prior to cntx_cntrl package
and de's should following.

Change-Id: I2bf436d8a16bae38ce484f846f0b399e16cfe80a
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 3 +++
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 8 ++++++++
 3 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 2039da7..b7f7679 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -953,6 +953,7 @@ struct amdgpu_cs_parser {
 #define AMDGPU_PREAMBLE_IB_PRESENT          (1 << 0) /* bit set means command submit involves a preamble IB */
 #define AMDGPU_PREAMBLE_IB_PRESENT_FIRST    (1 << 1) /* bit set means preamble IB is first presented in belonging context */
 #define AMDGPU_HAVE_CTX_SWITCH              (1 << 2) /* bit set means context switch occured */
+#define AMDGPU_VM_DOMAIN                    (1 << 3) /* bit set means in virtual memory context */
 
 struct amdgpu_job {
 	struct amd_sched_job    base;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 16308eb..6939822 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -184,6 +184,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 		if (need_ctx_switch)
 			status |= AMDGPU_HAVE_CTX_SWITCH;
 		status |= job->preamble_status;
+
+		if (vm)
+			status |= AMDGPU_VM_DOMAIN;
 		amdgpu_ring_emit_cntxcntl(ring, status);
 	}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 3e8cff3..8a5c818 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6704,6 +6704,10 @@ static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
 {
 	uint32_t dw2 = 0;
 
+	if (amdgpu_sriov_vf(ring->adev))
+		gfx_v8_0_ring_emit_ce_meta_init(ring,
+			(flags & AMDGPU_VM_DOMAIN) ? AMDGPU_CSA_VADDR : ring->adev->virt.csa_vmid0_addr);
+
 	dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
 	if (flags & AMDGPU_HAVE_CTX_SWITCH) {
 		gfx_v8_0_ring_emit_vgt_flush(ring);
@@ -6728,6 +6732,10 @@ static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
 	amdgpu_ring_write(ring, PACKET3(PACKET3_CONTEXT_CONTROL, 1));
 	amdgpu_ring_write(ring, dw2);
 	amdgpu_ring_write(ring, 0);
+
+	if (amdgpu_sriov_vf(ring->adev))
+		gfx_v8_0_ring_emit_de_meta_init(ring,
+			(flags & AMDGPU_VM_DOMAIN) ? AMDGPU_CSA_VADDR : ring->adev->virt.csa_vmid0_addr);
 }
 
 static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/amdgpu:implement META-DATA write routines

[Christian König]

Am 12.01.2017 um 08:41 schrieb Monk Liu:
> Change-Id: I66007a7f7e4e27fb129121f36143dce3cfb43738
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 31 ++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 61 +++++++++++++++++++++++++++++++++++
>   2 files changed, 92 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e9983fb..2039da7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1599,6 +1599,37 @@ static inline void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v)
>   	ring->count_dw--;
>   }
>   
> +static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring, void *src, int count_dw)
> +{
> +	unsigned occupied, chunk1, chunk2;
> +	void *dst;
> +
> +	if (ring->count_dw < count_dw)
> +		DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
> +	else {

Coding style says when either the "if" or the "else" branch uses "{" and 
"}" both should use them.

I think even better would be to use a return statement in the "if", 
cause this is just checking the prerequisites for errors.

Additional to that adding this function should be a separate patch.

Christian.

> +		occupied = ring->wptr & ring->ptr_mask;
> +		dst = (void *)&ring->ring[occupied];
> +		chunk1 = ring->ptr_mask + 1 - occupied;
> +		chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
> +		chunk2 = count_dw - chunk1;
> +		chunk1 <<= 2;
> +		chunk2 <<= 2;
> +		if (chunk1) {
> +			memcpy(dst, src, chunk1);
> +		}
> +
> +		if (chunk2) {
> +			src += chunk1;
> +			dst = (void *)ring->ring;
> +			memcpy(dst, src, chunk2);
> +		}
> +
> +		ring->wptr += count_dw;
> +		ring->wptr &= ring->ptr_mask;
> +		ring->count_dw -= count_dw;
> +	}
> +}
> +
>   static inline struct amdgpu_sdma_instance *
>   amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
>   {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 375784d..3e8cff3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -657,6 +657,8 @@ static void gfx_v8_0_set_gds_init(struct amdgpu_device *adev);
>   static void gfx_v8_0_set_rlc_funcs(struct amdgpu_device *adev);
>   static u32 gfx_v8_0_get_csb_size(struct amdgpu_device *adev);
>   static void gfx_v8_0_get_cu_info(struct amdgpu_device *adev);
> +static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring *ring, uint64_t addr);
> +static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring *ring, uint64_t addr);
>   
>   static void gfx_v8_0_init_golden_registers(struct amdgpu_device *adev)
>   {
> @@ -7212,3 +7214,62 @@ const struct amdgpu_ip_block_version gfx_v8_1_ip_block =
>   	.rev = 0,
>   	.funcs = &gfx_v8_0_ip_funcs,
>   };
> +
> +static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring *ring, uint64_t csa_addr)
> +{
> +	uint64_t ce_payload_addr;
> +	int cnt_ce;
> +	static union {
> +		struct amdgpu_ce_ib_state regular;
> +		struct amdgpu_ce_ib_state_chained_ib chained;
> +	} ce_payload = {0};
> +
> +	if (ring->adev->virt.chained_ib_support) {
> +		ce_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data_chained_ib, ce_payload);
> +		cnt_ce = (sizeof(ce_payload.chained) >> 2) + 4 - 2;
> +	} else {
> +		ce_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data, ce_payload);
> +		cnt_ce = (sizeof(ce_payload.regular) >> 2) + 4 - 2;
> +	}
> +
> +	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_ce));
> +	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(2) |
> +				WRITE_DATA_DST_SEL(8) |
> +				WR_CONFIRM) |
> +				WRITE_DATA_CACHE_POLICY(0));
> +	amdgpu_ring_write(ring, lower_32_bits(ce_payload_addr));
> +	amdgpu_ring_write(ring, upper_32_bits(ce_payload_addr));
> +	amdgpu_ring_write_multiple(ring, (void *)&ce_payload, cnt_ce - 2);
> +}
> +
> +static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring *ring, uint64_t csa_addr)
> +{
> +	uint64_t de_payload_addr, gds_addr;
> +	int cnt_de;
> +	static union {
> +		struct amdgpu_de_ib_state regular;
> +		struct amdgpu_de_ib_state_chained_ib chained;
> +	} de_payload = {0};
> +
> +	gds_addr = csa_addr + 4096;
> +	if (ring->adev->virt.chained_ib_support) {
> +		de_payload.chained.gds_backup_addrlo = lower_32_bits(gds_addr);
> +		de_payload.chained.gds_backup_addrhi = upper_32_bits(gds_addr);
> +		de_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data_chained_ib, de_payload);
> +		cnt_de = (sizeof(de_payload.chained) >> 2) + 4 - 2;
> +	} else {
> +		de_payload.regular.gds_backup_addrlo = lower_32_bits(gds_addr);
> +		de_payload.regular.gds_backup_addrhi = upper_32_bits(gds_addr);
> +		de_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data, de_payload);
> +		cnt_de = (sizeof(de_payload.regular) >> 2) + 4 - 2;
> +	}
> +
> +	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_de));
> +	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(1) |
> +				WRITE_DATA_DST_SEL(8) |
> +				WR_CONFIRM) |
> +				WRITE_DATA_CACHE_POLICY(0));
> +	amdgpu_ring_write(ring, lower_32_bits(de_payload_addr));
> +	amdgpu_ring_write(ring, upper_32_bits(de_payload_addr));
> +	amdgpu_ring_write_multiple(ring, (void *)&de_payload, cnt_de - 2);
> +}


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/amdgpu:implement META-DATA write routines

[Alex Deucher]

On Thu, Jan 12, 2017 at 2:41 AM, Monk Liu <Monk.Liu@amd.com> wrote:
> Change-Id: I66007a7f7e4e27fb129121f36143dce3cfb43738
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 31 ++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 61 +++++++++++++++++++++++++++++++++++
>  2 files changed, 92 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e9983fb..2039da7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1599,6 +1599,37 @@ static inline void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v)
>         ring->count_dw--;
>  }
>
> +static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring, void *src, int count_dw)
> +{
> +       unsigned occupied, chunk1, chunk2;
> +       void *dst;
> +
> +       if (ring->count_dw < count_dw)
> +               DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
> +       else {

Parens coding style as noted by Christian.  Also, this behavior
differs from amdgpu_ring_write() which completes the writes anyway.

> +               occupied = ring->wptr & ring->ptr_mask;
> +               dst = (void *)&ring->ring[occupied];
> +               chunk1 = ring->ptr_mask + 1 - occupied;
> +               chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
> +               chunk2 = count_dw - chunk1;
> +               chunk1 <<= 2;
> +               chunk2 <<= 2;
> +               if (chunk1) {
> +                       memcpy(dst, src, chunk1);
> +               }
> +
> +               if (chunk2) {
> +                       src += chunk1;
> +                       dst = (void *)ring->ring;
> +                       memcpy(dst, src, chunk2);
> +               }
> +
> +               ring->wptr += count_dw;
> +               ring->wptr &= ring->ptr_mask;
> +               ring->count_dw -= count_dw;
> +       }
> +}
> +

This hunk should be a separate patch.


>  static inline struct amdgpu_sdma_instance *
>  amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
>  {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 375784d..3e8cff3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -657,6 +657,8 @@ static void gfx_v8_0_set_gds_init(struct amdgpu_device *adev);
>  static void gfx_v8_0_set_rlc_funcs(struct amdgpu_device *adev);
>  static u32 gfx_v8_0_get_csb_size(struct amdgpu_device *adev);
>  static void gfx_v8_0_get_cu_info(struct amdgpu_device *adev);
> +static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring *ring, uint64_t addr);
> +static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring *ring, uint64_t addr);
>
>  static void gfx_v8_0_init_golden_registers(struct amdgpu_device *adev)
>  {
> @@ -7212,3 +7214,62 @@ const struct amdgpu_ip_block_version gfx_v8_1_ip_block =
>         .rev = 0,
>         .funcs = &gfx_v8_0_ip_funcs,
>  };
> +
> +static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring *ring, uint64_t csa_addr)
> +{
> +       uint64_t ce_payload_addr;
> +       int cnt_ce;
> +       static union {
> +               struct amdgpu_ce_ib_state regular;
> +               struct amdgpu_ce_ib_state_chained_ib chained;
> +       } ce_payload = {0};
> +
> +       if (ring->adev->virt.chained_ib_support) {
> +               ce_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data_chained_ib, ce_payload);
> +               cnt_ce = (sizeof(ce_payload.chained) >> 2) + 4 - 2;
> +       } else {
> +               ce_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data, ce_payload);
> +               cnt_ce = (sizeof(ce_payload.regular) >> 2) + 4 - 2;
> +       }
> +
> +       amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_ce));
> +       amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(2) |
> +                               WRITE_DATA_DST_SEL(8) |
> +                               WR_CONFIRM) |
> +                               WRITE_DATA_CACHE_POLICY(0));
> +       amdgpu_ring_write(ring, lower_32_bits(ce_payload_addr));
> +       amdgpu_ring_write(ring, upper_32_bits(ce_payload_addr));
> +       amdgpu_ring_write_multiple(ring, (void *)&ce_payload, cnt_ce - 2);
> +}
> +
> +static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring *ring, uint64_t csa_addr)
> +{
> +       uint64_t de_payload_addr, gds_addr;
> +       int cnt_de;
> +       static union {
> +               struct amdgpu_de_ib_state regular;
> +               struct amdgpu_de_ib_state_chained_ib chained;
> +       } de_payload = {0};
> +
> +       gds_addr = csa_addr + 4096;
> +       if (ring->adev->virt.chained_ib_support) {
> +               de_payload.chained.gds_backup_addrlo = lower_32_bits(gds_addr);
> +               de_payload.chained.gds_backup_addrhi = upper_32_bits(gds_addr);
> +               de_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data_chained_ib, de_payload);
> +               cnt_de = (sizeof(de_payload.chained) >> 2) + 4 - 2;
> +       } else {
> +               de_payload.regular.gds_backup_addrlo = lower_32_bits(gds_addr);
> +               de_payload.regular.gds_backup_addrhi = upper_32_bits(gds_addr);
> +               de_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data, de_payload);
> +               cnt_de = (sizeof(de_payload.regular) >> 2) + 4 - 2;
> +       }
> +
> +       amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_de));
> +       amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(1) |
> +                               WRITE_DATA_DST_SEL(8) |
> +                               WR_CONFIRM) |
> +                               WRITE_DATA_CACHE_POLICY(0));
> +       amdgpu_ring_write(ring, lower_32_bits(de_payload_addr));
> +       amdgpu_ring_write(ring, upper_32_bits(de_payload_addr));
> +       amdgpu_ring_write_multiple(ring, (void *)&de_payload, cnt_de - 2);
> +}
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/amdgpu:invoke meta-data write around cntx_cntl

[Alex Deucher]

On Thu, Jan 12, 2017 at 2:41 AM, Monk Liu <Monk.Liu@amd.com> wrote:
> ce's META-DATA write need prior to cntx_cntrl package
> and de's should following.
>
> Change-Id: I2bf436d8a16bae38ce484f846f0b399e16cfe80a
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h    | 1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 3 +++
>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 8 ++++++++
>  3 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 2039da7..b7f7679 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -953,6 +953,7 @@ struct amdgpu_cs_parser {
>  #define AMDGPU_PREAMBLE_IB_PRESENT          (1 << 0) /* bit set means command submit involves a preamble IB */
>  #define AMDGPU_PREAMBLE_IB_PRESENT_FIRST    (1 << 1) /* bit set means preamble IB is first presented in belonging context */
>  #define AMDGPU_HAVE_CTX_SWITCH              (1 << 2) /* bit set means context switch occured */
> +#define AMDGPU_VM_DOMAIN                    (1 << 3) /* bit set means in virtual memory context */
>
>  struct amdgpu_job {
>         struct amd_sched_job    base;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 16308eb..6939822 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -184,6 +184,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>                 if (need_ctx_switch)
>                         status |= AMDGPU_HAVE_CTX_SWITCH;
>                 status |= job->preamble_status;
> +
> +               if (vm)
> +                       status |= AMDGPU_VM_DOMAIN;
>                 amdgpu_ring_emit_cntxcntl(ring, status);
>         }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 3e8cff3..8a5c818 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6704,6 +6704,10 @@ static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>  {
>         uint32_t dw2 = 0;
>
> +       if (amdgpu_sriov_vf(ring->adev))
> +               gfx_v8_0_ring_emit_ce_meta_init(ring,
> +                       (flags & AMDGPU_VM_DOMAIN) ? AMDGPU_CSA_VADDR : ring->adev->virt.csa_vmid0_addr);
> +
>         dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
>         if (flags & AMDGPU_HAVE_CTX_SWITCH) {
>                 gfx_v8_0_ring_emit_vgt_flush(ring);
> @@ -6728,6 +6732,10 @@ static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>         amdgpu_ring_write(ring, PACKET3(PACKET3_CONTEXT_CONTROL, 1));
>         amdgpu_ring_write(ring, dw2);
>         amdgpu_ring_write(ring, 0);
> +
> +       if (amdgpu_sriov_vf(ring->adev))
> +               gfx_v8_0_ring_emit_de_meta_init(ring,
> +                       (flags & AMDGPU_VM_DOMAIN) ? AMDGPU_CSA_VADDR : ring->adev->virt.csa_vmid0_addr);
>  }
>
>  static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/amdgpu:invoke meta-data write around cntx_cntl

[Alex Deucher]

On Fri, Jan 13, 2017 at 11:28 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, Jan 12, 2017 at 2:41 AM, Monk Liu <Monk.Liu@amd.com> wrote:
>> ce's META-DATA write need prior to cntx_cntrl package
>> and de's should following.
>>
>> Change-Id: I2bf436d8a16bae38ce484f846f0b399e16cfe80a
>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Actually, I take that back.  A few comments below.

>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu.h    | 1 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 3 +++
>>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 8 ++++++++
>>  3 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 2039da7..b7f7679 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -953,6 +953,7 @@ struct amdgpu_cs_parser {
>>  #define AMDGPU_PREAMBLE_IB_PRESENT          (1 << 0) /* bit set means command submit involves a preamble IB */
>>  #define AMDGPU_PREAMBLE_IB_PRESENT_FIRST    (1 << 1) /* bit set means preamble IB is first presented in belonging context */
>>  #define AMDGPU_HAVE_CTX_SWITCH              (1 << 2) /* bit set means context switch occured */
>> +#define AMDGPU_VM_DOMAIN                    (1 << 3) /* bit set means in virtual memory context */
>>
>>  struct amdgpu_job {
>>         struct amd_sched_job    base;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 16308eb..6939822 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -184,6 +184,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>>                 if (need_ctx_switch)
>>                         status |= AMDGPU_HAVE_CTX_SWITCH;
>>                 status |= job->preamble_status;
>> +
>> +               if (vm)
>> +                       status |= AMDGPU_VM_DOMAIN;
>>                 amdgpu_ring_emit_cntxcntl(ring, status);
>>         }
>>

This top chunk should be a separate patch.


>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> index 3e8cff3..8a5c818 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> @@ -6704,6 +6704,10 @@ static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>>  {
>>         uint32_t dw2 = 0;
>>
>> +       if (amdgpu_sriov_vf(ring->adev))
>> +               gfx_v8_0_ring_emit_ce_meta_init(ring,
>> +                       (flags & AMDGPU_VM_DOMAIN) ? AMDGPU_CSA_VADDR : ring->adev->virt.csa_vmid0_addr);
>> +
>>         dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
>>         if (flags & AMDGPU_HAVE_CTX_SWITCH) {
>>                 gfx_v8_0_ring_emit_vgt_flush(ring);
>> @@ -6728,6 +6732,10 @@ static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>>         amdgpu_ring_write(ring, PACKET3(PACKET3_CONTEXT_CONTROL, 1));
>>         amdgpu_ring_write(ring, dw2);
>>         amdgpu_ring_write(ring, 0);
>> +
>> +       if (amdgpu_sriov_vf(ring->adev))
>> +               gfx_v8_0_ring_emit_de_meta_init(ring,
>> +                       (flags & AMDGPU_VM_DOMAIN) ? AMDGPU_CSA_VADDR : ring->adev->virt.csa_vmid0_addr);
>>  }
>>

I think you need to update the emit_frame_size to reflect the larger frame.

>>  static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

答复: [PATCH 2/3] drm/amdgpu:implement META-DATA write routines

[Liu, Monk]

[-- Attachment #1.1: Type: text/plain, Size: 6516 bytes --]

if ring_write_multiple is organized in a separate patch, doesn't it introduces an function that no client using it ??


fine by me although ...


BR Monk

________________________________
发件人: Christian König <deathsimple@vodafone.de>
发送时间: 2017年1月12日 20:27:48
收件人: Liu, Monk; amd-gfx@lists.freedesktop.org
主题: Re: [PATCH 2/3] drm/amdgpu:implement META-DATA write routines

Am 12.01.2017 um 08:41 schrieb Monk Liu:
> Change-Id: I66007a7f7e4e27fb129121f36143dce3cfb43738
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 31 ++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 61 +++++++++++++++++++++++++++++++++++
>   2 files changed, 92 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e9983fb..2039da7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1599,6 +1599,37 @@ static inline void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v)
>        ring->count_dw--;
>   }
>
> +static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring, void *src, int count_dw)
> +{
> +     unsigned occupied, chunk1, chunk2;
> +     void *dst;
> +
> +     if (ring->count_dw < count_dw)
> +             DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
> +     else {

Coding style says when either the "if" or the "else" branch uses "{" and
"}" both should use them.

I think even better would be to use a return statement in the "if",
cause this is just checking the prerequisites for errors.

Additional to that adding this function should be a separate patch.

Christian.

> +             occupied = ring->wptr & ring->ptr_mask;
> +             dst = (void *)&ring->ring[occupied];
> +             chunk1 = ring->ptr_mask + 1 - occupied;
> +             chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
> +             chunk2 = count_dw - chunk1;
> +             chunk1 <<= 2;
> +             chunk2 <<= 2;
> +             if (chunk1) {
> +                     memcpy(dst, src, chunk1);
> +             }
> +
> +             if (chunk2) {
> +                     src += chunk1;
> +                     dst = (void *)ring->ring;
> +                     memcpy(dst, src, chunk2);
> +             }
> +
> +             ring->wptr += count_dw;
> +             ring->wptr &= ring->ptr_mask;
> +             ring->count_dw -= count_dw;
> +     }
> +}
> +
>   static inline struct amdgpu_sdma_instance *
>   amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
>   {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 375784d..3e8cff3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -657,6 +657,8 @@ static void gfx_v8_0_set_gds_init(struct amdgpu_device *adev);
>   static void gfx_v8_0_set_rlc_funcs(struct amdgpu_device *adev);
>   static u32 gfx_v8_0_get_csb_size(struct amdgpu_device *adev);
>   static void gfx_v8_0_get_cu_info(struct amdgpu_device *adev);
> +static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring *ring, uint64_t addr);
> +static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring *ring, uint64_t addr);
>
>   static void gfx_v8_0_init_golden_registers(struct amdgpu_device *adev)
>   {
> @@ -7212,3 +7214,62 @@ const struct amdgpu_ip_block_version gfx_v8_1_ip_block =
>        .rev = 0,
>        .funcs = &gfx_v8_0_ip_funcs,
>   };
> +
> +static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring *ring, uint64_t csa_addr)
> +{
> +     uint64_t ce_payload_addr;
> +     int cnt_ce;
> +     static union {
> +             struct amdgpu_ce_ib_state regular;
> +             struct amdgpu_ce_ib_state_chained_ib chained;
> +     } ce_payload = {0};
> +
> +     if (ring->adev->virt.chained_ib_support) {
> +             ce_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data_chained_ib, ce_payload);
> +             cnt_ce = (sizeof(ce_payload.chained) >> 2) + 4 - 2;
> +     } else {
> +             ce_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data, ce_payload);
> +             cnt_ce = (sizeof(ce_payload.regular) >> 2) + 4 - 2;
> +     }
> +
> +     amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_ce));
> +     amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(2) |
> +                             WRITE_DATA_DST_SEL(8) |
> +                             WR_CONFIRM) |
> +                             WRITE_DATA_CACHE_POLICY(0));
> +     amdgpu_ring_write(ring, lower_32_bits(ce_payload_addr));
> +     amdgpu_ring_write(ring, upper_32_bits(ce_payload_addr));
> +     amdgpu_ring_write_multiple(ring, (void *)&ce_payload, cnt_ce - 2);
> +}
> +
> +static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring *ring, uint64_t csa_addr)
> +{
> +     uint64_t de_payload_addr, gds_addr;
> +     int cnt_de;
> +     static union {
> +             struct amdgpu_de_ib_state regular;
> +             struct amdgpu_de_ib_state_chained_ib chained;
> +     } de_payload = {0};
> +
> +     gds_addr = csa_addr + 4096;
> +     if (ring->adev->virt.chained_ib_support) {
> +             de_payload.chained.gds_backup_addrlo = lower_32_bits(gds_addr);
> +             de_payload.chained.gds_backup_addrhi = upper_32_bits(gds_addr);
> +             de_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data_chained_ib, de_payload);
> +             cnt_de = (sizeof(de_payload.chained) >> 2) + 4 - 2;
> +     } else {
> +             de_payload.regular.gds_backup_addrlo = lower_32_bits(gds_addr);
> +             de_payload.regular.gds_backup_addrhi = upper_32_bits(gds_addr);
> +             de_payload_addr = csa_addr + offsetof(struct amdgpu_gfx_meta_data, de_payload);
> +             cnt_de = (sizeof(de_payload.regular) >> 2) + 4 - 2;
> +     }
> +
> +     amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_de));
> +     amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(1) |
> +                             WRITE_DATA_DST_SEL(8) |
> +                             WR_CONFIRM) |
> +                             WRITE_DATA_CACHE_POLICY(0));
> +     amdgpu_ring_write(ring, lower_32_bits(de_payload_addr));
> +     amdgpu_ring_write(ring, upper_32_bits(de_payload_addr));
> +     amdgpu_ring_write_multiple(ring, (void *)&de_payload, cnt_de - 2);
> +}



[-- Attachment #1.2: Type: text/html, Size: 13244 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

答复: [PATCH 3/3] drm/amdgpu:invoke meta-data write around cntx_cntl

[Liu, Monk]

[-- Attachment #1.1: Type: text/plain, Size: 4035 bytes --]

okay, thanks

________________________________
发件人: Alex Deucher <alexdeucher@gmail.com>
发送时间: 2017年1月14日 0:35:45
收件人: Liu, Monk
抄送: amd-gfx list
主题: Re: [PATCH 3/3] drm/amdgpu:invoke meta-data write around cntx_cntl

On Fri, Jan 13, 2017 at 11:28 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, Jan 12, 2017 at 2:41 AM, Monk Liu <Monk.Liu@amd.com> wrote:
>> ce's META-DATA write need prior to cntx_cntrl package
>> and de's should following.
>>
>> Change-Id: I2bf436d8a16bae38ce484f846f0b399e16cfe80a
>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Actually, I take that back.  A few comments below.

>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu.h    | 1 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 3 +++
>>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 8 ++++++++
>>  3 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 2039da7..b7f7679 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -953,6 +953,7 @@ struct amdgpu_cs_parser {
>>  #define AMDGPU_PREAMBLE_IB_PRESENT          (1 << 0) /* bit set means command submit involves a preamble IB */
>>  #define AMDGPU_PREAMBLE_IB_PRESENT_FIRST    (1 << 1) /* bit set means preamble IB is first presented in belonging context */
>>  #define AMDGPU_HAVE_CTX_SWITCH              (1 << 2) /* bit set means context switch occured */
>> +#define AMDGPU_VM_DOMAIN                    (1 << 3) /* bit set means in virtual memory context */
>>
>>  struct amdgpu_job {
>>         struct amd_sched_job    base;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 16308eb..6939822 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -184,6 +184,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>>                 if (need_ctx_switch)
>>                         status |= AMDGPU_HAVE_CTX_SWITCH;
>>                 status |= job->preamble_status;
>> +
>> +               if (vm)
>> +                       status |= AMDGPU_VM_DOMAIN;
>>                 amdgpu_ring_emit_cntxcntl(ring, status);
>>         }
>>

This top chunk should be a separate patch.


>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> index 3e8cff3..8a5c818 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> @@ -6704,6 +6704,10 @@ static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>>  {
>>         uint32_t dw2 = 0;
>>
>> +       if (amdgpu_sriov_vf(ring->adev))
>> +               gfx_v8_0_ring_emit_ce_meta_init(ring,
>> +                       (flags & AMDGPU_VM_DOMAIN) ? AMDGPU_CSA_VADDR : ring->adev->virt.csa_vmid0_addr);
>> +
>>         dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
>>         if (flags & AMDGPU_HAVE_CTX_SWITCH) {
>>                 gfx_v8_0_ring_emit_vgt_flush(ring);
>> @@ -6728,6 +6732,10 @@ static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>>         amdgpu_ring_write(ring, PACKET3(PACKET3_CONTEXT_CONTROL, 1));
>>         amdgpu_ring_write(ring, dw2);
>>         amdgpu_ring_write(ring, 0);
>> +
>> +       if (amdgpu_sriov_vf(ring->adev))
>> +               gfx_v8_0_ring_emit_de_meta_init(ring,
>> +                       (flags & AMDGPU_VM_DOMAIN) ? AMDGPU_CSA_VADDR : ring->adev->virt.csa_vmid0_addr);
>>  }
>>

I think you need to update the emit_frame_size to reflect the larger frame.

>>  static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 7881 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: 答复: [PATCH 2/3] drm/amdgpu:implement META-DATA write routines

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 7077 bytes --]

That is perfectly fine as long as you add the code using it in the same 
set of patches.

What we should avoid is adding a lot of code and then not using for for 
quite some time, that get certainly removed sooner or later.

Regards,
Christian.

Am 16.01.2017 um 09:20 schrieb Liu, Monk:
>
> if ring_write_multiple is organized in a separate patch, doesn't it 
> introduces an function that no client using it ??
>
>
> fine by me although ...
>
>
> BR Monk
>
> ------------------------------------------------------------------------
> *发件人:* Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
> *发送时间:* 2017年1月12日 20:27:48
> *收件人:* Liu, Monk; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *主题:* Re: [PATCH 2/3] drm/amdgpu:implement META-DATA write routines
> Am 12.01.2017 um 08:41 schrieb Monk Liu:
> > Change-Id: I66007a7f7e4e27fb129121f36143dce3cfb43738
> > Signed-off-by: Monk Liu <Monk.Liu-5C7GfCeVMHo@public.gmane.org>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 31 ++++++++++++++++++
> >   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 61 
> +++++++++++++++++++++++++++++++++++
> >   2 files changed, 92 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index e9983fb..2039da7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -1599,6 +1599,37 @@ static inline void amdgpu_ring_write(struct 
> amdgpu_ring *ring, uint32_t v)
> >        ring->count_dw--;
> >   }
> >
> > +static inline void amdgpu_ring_write_multiple(struct amdgpu_ring 
> *ring, void *src, int count_dw)
> > +{
> > +     unsigned occupied, chunk1, chunk2;
> > +     void *dst;
> > +
> > +     if (ring->count_dw < count_dw)
> > +             DRM_ERROR("amdgpu: writing more dwords to the ring 
> than expected!\n");
> > +     else {
>
> Coding style says when either the "if" or the "else" branch uses "{" and
> "}" both should use them.
>
> I think even better would be to use a return statement in the "if",
> cause this is just checking the prerequisites for errors.
>
> Additional to that adding this function should be a separate patch.
>
> Christian.
>
> > +             occupied = ring->wptr & ring->ptr_mask;
> > +             dst = (void *)&ring->ring[occupied];
> > +             chunk1 = ring->ptr_mask + 1 - occupied;
> > +             chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
> > +             chunk2 = count_dw - chunk1;
> > +             chunk1 <<= 2;
> > +             chunk2 <<= 2;
> > +             if (chunk1) {
> > +                     memcpy(dst, src, chunk1);
> > +             }
> > +
> > +             if (chunk2) {
> > +                     src += chunk1;
> > +                     dst = (void *)ring->ring;
> > +                     memcpy(dst, src, chunk2);
> > +             }
> > +
> > +             ring->wptr += count_dw;
> > +             ring->wptr &= ring->ptr_mask;
> > +             ring->count_dw -= count_dw;
> > +     }
> > +}
> > +
> >   static inline struct amdgpu_sdma_instance *
> >   amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
> >   {
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 375784d..3e8cff3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -657,6 +657,8 @@ static void gfx_v8_0_set_gds_init(struct 
> amdgpu_device *adev);
> >   static void gfx_v8_0_set_rlc_funcs(struct amdgpu_device *adev);
> >   static u32 gfx_v8_0_get_csb_size(struct amdgpu_device *adev);
> >   static void gfx_v8_0_get_cu_info(struct amdgpu_device *adev);
> > +static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring 
> *ring, uint64_t addr);
> > +static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring 
> *ring, uint64_t addr);
> >
> >   static void gfx_v8_0_init_golden_registers(struct amdgpu_device *adev)
> >   {
> > @@ -7212,3 +7214,62 @@ const struct amdgpu_ip_block_version 
> gfx_v8_1_ip_block =
> >        .rev = 0,
> >        .funcs = &gfx_v8_0_ip_funcs,
> >   };
> > +
> > +static void gfx_v8_0_ring_emit_ce_meta_init(struct amdgpu_ring 
> *ring, uint64_t csa_addr)
> > +{
> > +     uint64_t ce_payload_addr;
> > +     int cnt_ce;
> > +     static union {
> > +             struct amdgpu_ce_ib_state regular;
> > +             struct amdgpu_ce_ib_state_chained_ib chained;
> > +     } ce_payload = {0};
> > +
> > +     if (ring->adev->virt.chained_ib_support) {
> > +             ce_payload_addr = csa_addr + offsetof(struct 
> amdgpu_gfx_meta_data_chained_ib, ce_payload);
> > +             cnt_ce = (sizeof(ce_payload.chained) >> 2) + 4 - 2;
> > +     } else {
> > +             ce_payload_addr = csa_addr + offsetof(struct 
> amdgpu_gfx_meta_data, ce_payload);
> > +             cnt_ce = (sizeof(ce_payload.regular) >> 2) + 4 - 2;
> > +     }
> > +
> > +     amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_ce));
> > +     amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(2) |
> > +                             WRITE_DATA_DST_SEL(8) |
> > +                             WR_CONFIRM) |
> > + WRITE_DATA_CACHE_POLICY(0));
> > +     amdgpu_ring_write(ring, lower_32_bits(ce_payload_addr));
> > +     amdgpu_ring_write(ring, upper_32_bits(ce_payload_addr));
> > +     amdgpu_ring_write_multiple(ring, (void *)&ce_payload, cnt_ce - 2);
> > +}
> > +
> > +static void gfx_v8_0_ring_emit_de_meta_init(struct amdgpu_ring 
> *ring, uint64_t csa_addr)
> > +{
> > +     uint64_t de_payload_addr, gds_addr;
> > +     int cnt_de;
> > +     static union {
> > +             struct amdgpu_de_ib_state regular;
> > +             struct amdgpu_de_ib_state_chained_ib chained;
> > +     } de_payload = {0};
> > +
> > +     gds_addr = csa_addr + 4096;
> > +     if (ring->adev->virt.chained_ib_support) {
> > +             de_payload.chained.gds_backup_addrlo = 
> lower_32_bits(gds_addr);
> > +             de_payload.chained.gds_backup_addrhi = 
> upper_32_bits(gds_addr);
> > +             de_payload_addr = csa_addr + offsetof(struct 
> amdgpu_gfx_meta_data_chained_ib, de_payload);
> > +             cnt_de = (sizeof(de_payload.chained) >> 2) + 4 - 2;
> > +     } else {
> > +             de_payload.regular.gds_backup_addrlo = 
> lower_32_bits(gds_addr);
> > +             de_payload.regular.gds_backup_addrhi = 
> upper_32_bits(gds_addr);
> > +             de_payload_addr = csa_addr + offsetof(struct 
> amdgpu_gfx_meta_data, de_payload);
> > +             cnt_de = (sizeof(de_payload.regular) >> 2) + 4 - 2;
> > +     }
> > +
> > +     amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt_de));
> > +     amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(1) |
> > +                             WRITE_DATA_DST_SEL(8) |
> > +                             WR_CONFIRM) |
> > + WRITE_DATA_CACHE_POLICY(0));
> > +     amdgpu_ring_write(ring, lower_32_bits(de_payload_addr));
> > +     amdgpu_ring_write(ring, upper_32_bits(de_payload_addr));
> > +     amdgpu_ring_write_multiple(ring, (void *)&de_payload, cnt_de - 2);
> > +}
>
>


[-- Attachment #1.2: Type: text/html, Size: 12904 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx