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

[Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

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